You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by pp...@apache.org on 2007/09/08 01:08:03 UTC

svn commit: r573750 - in /openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/meta/ openjpa-persistence/src/main/java/org/apache/openjpa/persistence/

Author: ppoddar
Date: Fri Sep  7 16:08:02 2007
New Revision: 573750

URL: http://svn.apache.org/viewvc?rev=573750&view=rev
Log:
Fix for FetchGroup inclusion and recursion depth calculation.

Modified:
    openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
    openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java
    openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java

Modified: openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
URL: http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java?rev=573750&r1=573749&r2=573750&view=diff
==============================================================================
--- openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java (original)
+++ openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java Fri Sep  7 16:08:02 2007
@@ -604,6 +604,8 @@
         int cur;
         for (int i = 0; max != FetchGroup.DEPTH_INFINITE 
             && i < groups.length; i++) {
+            // ignore custom groups that are inactive in this configuration
+            if (!this.hasFetchGroup(groups[i])) continue;
             cur = meta.getFetchGroup(groups[i]).getRecursionDepth(fm);
             if (cur == FetchGroup.DEPTH_INFINITE || cur > max) 
                 max = cur;
@@ -625,7 +627,7 @@
             return avail;
         return Math.min(max, avail);
     }
- 
+
     /**
      * Return the relation type of the given field.
      */

Modified: openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java
URL: http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java?rev=573750&r1=573749&r2=573750&view=diff
==============================================================================
--- openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java (original)
+++ openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java Fri Sep  7 16:08:02 2007
@@ -21,9 +21,11 @@
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.ObjectUtils;
@@ -74,6 +76,7 @@
     private final ClassMetaData _meta;
     private final boolean _readOnly;
     private List _includes;
+    private Set  _containedBy;
     private Map _depths;
     private Boolean _postLoad;
 
@@ -171,6 +174,32 @@
             }
         }
         return false;
+    }
+    
+    /**
+     * Sets this receiver as one of the included fetch groups of the given
+     * parent. 
+     * The parent fecth grop must include this receiver before this call.
+     * 
+     * @see #includes(String, boolean)
+     * @see #addDeclaredInclude(String) 
+     */
+    public boolean setContainedBy(FetchGroup parent) {
+    	parent.addDeclaredInclude(this.getName());
+    	if (_containedBy==null)
+    		_containedBy = new HashSet();
+    	return _containedBy.add(parent.getName());
+    }
+    
+    /**
+     * Gets the name of the fetch groups in which this receiver has been
+     * included.
+     * 
+     * @see #setContainedBy(FetchGroup)
+     */
+    public String[] getContainedBy() {
+    	return (_containedBy == null) ? new String[0] 
+            : (String[]) _containedBy.toArray(new String[_containedBy.size()]);
     }
 
     /**

Modified: openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
URL: http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java?rev=573750&r1=573749&r2=573750&view=diff
==============================================================================
--- openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java (original)
+++ openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java Fri Sep  7 16:08:02 2007
@@ -875,6 +875,14 @@
 
     /**
      * Create fetch groups.
+     * If FetchGroup A includes FetchGroup B, then a bi-link is set between
+     * A and B. Both A and B must be declared in the same Class. 
+     * <br>
+     * Call {@link #parseFetchAttribute(ClassMetaData, 
+     * org.apache.openjpa.meta.FetchGroup, FetchAttribute) only after the
+     * bi-links have been established, because a field f will not only add the
+     * fetch group A which explictly includes f to its custom fetch groups but 
+     * also will also add any fetch group B that includes A.  
      */
     private void parseFetchGroups(ClassMetaData meta, FetchGroup... groups) {
         org.apache.openjpa.meta.FetchGroup fg;
@@ -885,12 +893,25 @@
             fg = meta.addDeclaredFetchGroup(group.name());
             if (group.postLoad())
                 fg.setPostLoad(true); 
-            for (String s : group.fetchGroups())
+            for (String s : group.fetchGroups()) {
                 fg.addDeclaredInclude(s);
+            }
+        }
+        
+        for (FetchGroup group:groups) {
+        	fg = meta.getFetchGroup(group.name());
+        	String[] includedFetchGropNames = fg.getDeclaredIncludes();
+        	for (String includedFectchGroupName:includedFetchGropNames)
+        	    meta.getFetchGroup(includedFectchGroupName).setContainedBy(fg);
+        }
+        
+        for (FetchGroup group : groups) {
+            fg = meta.getFetchGroup(group.name());
             for (FetchAttribute attr : group.attributes())
                 parseFetchAttribute(meta, fg, attr);
         }
     }
+    
 
     /**
      * Set a field's fetch group.
@@ -904,6 +925,9 @@
                 meta, attr.name()));
 
         field.setInFetchGroup(fg.getName(), true);
+        String[] parentFetchGroups = fg.getContainedBy();
+        for (String parentFetchGroup:parentFetchGroups)
+        	field.setInFetchGroup(parentFetchGroup, true);
         if (attr.recursionDepth() != Integer.MIN_VALUE)
             fg.setRecursionDepth(field, attr.recursionDepth());
     }



Re: svn commit: r573750 - in /openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/meta/ openjpa-persistence/src/main/java/org/apache/openjpa/persistence/

Posted by Patrick Linskey <pl...@gmail.com>.
>   Please suggest/discuss what is the best way to keep SVN commit and
> JIRA issues linked together.

In your svn commit message, include a string identifying all the
issues that the commit is related to. I usually put just the issue
number(s) in the commit message, and put the description of what
happened in the JIRA issue. When I want to add more details to the
commit, I put the issue numbers at the beginning.

So, in this case, if I had been checking it in, the commit message
would have been "OPENJPA-357, OPENJPA-358".

When I check in a patch from someone else, I say something like
"OPENJPA-357. Applied foo's patch."

This data is parsed by JIRA, and is visible if you hit the 'All' or
'Subversion Commits' link above the comments area.

You can see an example of this at
https://issues.apache.org/jira/browse/OPENJPA-293?page=com.atlassian.jira.plugin.ext.subversion:subversion-commits-tabpanel

In that URL, I made a number of changes that resolved parts of the
various issues (some of which are still open). So, I put additional
content in the commit messages describing what each one does. When I
do a commit that resolves an issue in its entirety, I just list the
issue numbers.

-Patrick

On 9/8/07, Pinaki Poddar <pp...@bea.com> wrote:
> Kevin,
>   This is the first attempt to fix 357 & 358. I linked 357 to 358.
> Also added a comment on 357 to link it to SVN revision 573750.
>
>   Please suggest/discuss what is the best way to keep SVN commit and
> JIRA issues
> linked together.
>
> Pinaki Poddar
> 972.834.2865
>
>
> >-----Original Message-----
> >From: Kevin Sutter [mailto:kwsutter@gmail.com]
> >Sent: Saturday, September 08, 2007 12:41 PM
> >To: dev@openjpa.apache.org
> >Subject: Re: svn commit: r573750 - in /openjpa/trunk:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/
> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/
> >openjpa-persistence/src/main/java/org/apache/openjpa/persistence/
> >
> >Was this commit for OPENJPA-358 or OPENJPA-357, or both?  :-)
> >We need to remember to include the tag in the commit message
> >so that we get the SVN changes in the JIRA Issues.  Thanks!
> >
> >Kevin
> >
> >On 9/7/07, ppoddar@apache.org <pp...@apache.org> wrote:
> >>
> >> Author: ppoddar
> >> Date: Fri Sep  7 16:08:02 2007
> >> New Revision: 573750
> >>
> >> URL: http://svn.apache.org/viewvc?rev=573750&view=rev
> >> Log:
> >> Fix for FetchGroup inclusion and recursion depth calculation.
> >>
> >> Modified:
> >>
> >>
> >>
> >openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/F
> >> etchConfigurationImpl.java
> >>
> >>
> >>
> >openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/Fet
> >> chGroup.java
> >>
> >>
> >>
> >openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/per
> >> sistence/AnnotationPersistenceMetaDataParser.java
> >>
> >> Modified:
> >>
> >openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/F
> >> etchConfigurationImpl.java
> >> URL:
> >>
> >http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/jav
> >>
> >a/org/apache/openjpa/kernel/FetchConfigurationImpl.java?rev=573750&r1=
> >> 573749&r2=573750&view=diff
> >>
> >>
> >======================================================================
> >> ========
> >> ---
> >>
> >openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/F
> >> etchConfigurationImpl.java
> >> (original)
> >> +++
> >>
> >openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/F
> >> etchConfigurationImpl.java
> >> Fri Sep  7 16:08:02 2007
> >> @@ -604,6 +604,8 @@
> >>          int cur;
> >>          for (int i = 0; max != FetchGroup.DEPTH_INFINITE
> >>              && i < groups.length; i++) {
> >> +            // ignore custom groups that are inactive in this
> >> configuration
> >> +            if (!this.hasFetchGroup(groups[i])) continue;
> >>              cur =
> >meta.getFetchGroup(groups[i]).getRecursionDepth(fm);
> >>              if (cur == FetchGroup.DEPTH_INFINITE || cur > max)
> >>                  max = cur;
> >> @@ -625,7 +627,7 @@
> >>              return avail;
> >>          return Math.min(max, avail);
> >>      }
> >> -
> >> +
> >>      /**
> >>       * Return the relation type of the given field.
> >>       */
> >>
> >> Modified:
> >>
> >openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/Fet
> >> chGroup.java
> >> URL:
> >>
> >http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/jav
> >>
> >a/org/apache/openjpa/meta/FetchGroup.java?rev=573750&r1=573749&r2=5737
> >> 50&view=diff
> >>
> >>
> >======================================================================
> >> ========
> >> ---
> >>
> >openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/Fet
> >> chGroup.java
> >> (original)
> >> +++
> >>
> >openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/Fet
> >> chGroup.java
> >> Fri Sep  7 16:08:02 2007
> >> @@ -21,9 +21,11 @@
> >> import java.io.Serializable;
> >> import java.util.ArrayList;
> >> import java.util.HashMap;
> >> +import java.util.HashSet;
> >> import java.util.Iterator;
> >> import java.util.List;
> >> import java.util.Map;
> >> +import java.util.Set;
> >>
> >> import org.apache.commons.lang.StringUtils;
> >> import org.apache.commons.lang.ObjectUtils;
> >> @@ -74,6 +76,7 @@
> >>      private final ClassMetaData _meta;
> >>      private final boolean _readOnly;
> >>      private List _includes;
> >> +    private Set  _containedBy;
> >>      private Map _depths;
> >>      private Boolean _postLoad;
> >>
> >> @@ -171,6 +174,32 @@
> >>              }
> >>          }
> >>          return false;
> >> +    }
> >> +
> >> +    /**
> >> +     * Sets this receiver as one of the included fetch groups of the
> >> given
> >> +     * parent.
> >> +     * The parent fecth grop must include this receiver
> >before this call.
> >> +     *
> >> +     * @see #includes(String, boolean)
> >> +     * @see #addDeclaredInclude(String)
> >> +     */
> >> +    public boolean setContainedBy(FetchGroup parent) {
> >> +       parent.addDeclaredInclude(this.getName());
> >> +       if (_containedBy==null)
> >> +               _containedBy = new HashSet();
> >> +       return _containedBy.add(parent.getName());
> >> +    }
> >> +
> >> +    /**
> >> +     * Gets the name of the fetch groups in which this
> >receiver has been
> >> +     * included.
> >> +     *
> >> +     * @see #setContainedBy(FetchGroup)
> >> +     */
> >> +    public String[] getContainedBy() {
> >> +       return (_containedBy == null) ? new String[0]
> >> +            : (String[]) _containedBy.toArray(new
> >> String[_containedBy.size()]);
> >>      }
> >>
> >>      /**
> >>
> >> Modified:
> >>
> >openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/per
> >> sistence/AnnotationPersistenceMetaDataParser.java
> >> URL:
> >>
> >http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence/src/mai
> >>
> >n/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataPar
> >> ser.java?rev=573750&r1=573749&r2=573750&view=diff
> >>
> >>
> >======================================================================
> >> ========
> >> ---
> >>
> >openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/per
> >> sistence/AnnotationPersistenceMetaDataParser.java
> >> (original)
> >> +++
> >>
> >openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/per
> >> sistence/AnnotationPersistenceMetaDataParser.java
> >> Fri Sep  7 16:08:02 2007
> >> @@ -875,6 +875,14 @@
> >>
> >>      /**
> >>       * Create fetch groups.
> >> +     * If FetchGroup A includes FetchGroup B, then a bi-link is set
> >> between
> >> +     * A and B. Both A and B must be declared in the same Class.
> >> +     * <br>
> >> +     * Call {@link #parseFetchAttribute(ClassMetaData,
> >> +     * org.apache.openjpa.meta.FetchGroup, FetchAttribute)
> >only after the
> >> +     * bi-links have been established, because a field f will not
> >> + only
> >> add the
> >> +     * fetch group A which explictly includes f to its custom fetch
> >> groups but
> >> +     * also will also add any fetch group B that includes A.
> >>       */
> >>      private void parseFetchGroups(ClassMetaData meta, FetchGroup...
> >> groups) {
> >>          org.apache.openjpa.meta.FetchGroup fg; @@ -885,12 +893,25 @@
> >>              fg = meta.addDeclaredFetchGroup(group.name());
> >>              if (group.postLoad())
> >>                  fg.setPostLoad(true);
> >> -            for (String s : group.fetchGroups())
> >> +            for (String s : group.fetchGroups()) {
> >>                  fg.addDeclaredInclude(s);
> >> +            }
> >> +        }
> >> +
> >> +        for (FetchGroup group:groups) {
> >> +               fg = meta.getFetchGroup(group.name());
> >> +               String[] includedFetchGropNames =
> >> + fg.getDeclaredIncludes
> >> ();
> >> +               for (String
> >> includedFectchGroupName:includedFetchGropNames)
> >> +                   meta.getFetchGroup
> >> (includedFectchGroupName).setContainedBy(fg);
> >> +        }
> >> +
> >> +        for (FetchGroup group : groups) {
> >> +            fg = meta.getFetchGroup(group.name());
> >>              for (FetchAttribute attr : group.attributes())
> >>                  parseFetchAttribute(meta, fg, attr);
> >>          }
> >>      }
> >> +
> >>
> >>      /**
> >>       * Set a field's fetch group.
> >> @@ -904,6 +925,9 @@
> >>                  meta, attr.name()));
> >>
> >>          field.setInFetchGroup(fg.getName(), true);
> >> +        String[] parentFetchGroups = fg.getContainedBy();
> >> +        for (String parentFetchGroup:parentFetchGroups)
> >> +               field.setInFetchGroup(parentFetchGroup, true);
> >>          if (attr.recursionDepth() != Integer.MIN_VALUE)
> >>              fg.setRecursionDepth(field, attr.recursionDepth());
> >>      }
> >>
> >>
> >>
> >
>
> Notice:  This email message, together with any attachments, may contain information  of  BEA Systems,  Inc.,  its subsidiaries  and  affiliated entities,  that may be confidential,  proprietary,  copyrighted  and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
>


-- 
Patrick Linskey
202 669 5907

RE: svn commit: r573750 - in /openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/meta/ openjpa-persistence/src/main/java/org/apache/openjpa/persistence/

Posted by Pinaki Poddar <pp...@bea.com>.
Kevin,
  This is the first attempt to fix 357 & 358. I linked 357 to 358. 
Also added a comment on 357 to link it to SVN revision 573750.

  Please suggest/discuss what is the best way to keep SVN commit and
JIRA issues
linked together.

Pinaki Poddar
972.834.2865
 

>-----Original Message-----
>From: Kevin Sutter [mailto:kwsutter@gmail.com] 
>Sent: Saturday, September 08, 2007 12:41 PM
>To: dev@openjpa.apache.org
>Subject: Re: svn commit: r573750 - in /openjpa/trunk: 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ 
>openjpa-kernel/src/main/java/org/apache/openjpa/meta/ 
>openjpa-persistence/src/main/java/org/apache/openjpa/persistence/
>
>Was this commit for OPENJPA-358 or OPENJPA-357, or both?  :-)  
>We need to remember to include the tag in the commit message 
>so that we get the SVN changes in the JIRA Issues.  Thanks!
>
>Kevin
>
>On 9/7/07, ppoddar@apache.org <pp...@apache.org> wrote:
>>
>> Author: ppoddar
>> Date: Fri Sep  7 16:08:02 2007
>> New Revision: 573750
>>
>> URL: http://svn.apache.org/viewvc?rev=573750&view=rev
>> Log:
>> Fix for FetchGroup inclusion and recursion depth calculation.
>>
>> Modified:
>>
>>     
>> 
>openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/F
>> etchConfigurationImpl.java
>>
>>     
>> 
>openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/Fet
>> chGroup.java
>>
>>     
>> 
>openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/per
>> sistence/AnnotationPersistenceMetaDataParser.java
>>
>> Modified:
>> 
>openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/F
>> etchConfigurationImpl.java
>> URL:
>> 
>http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/jav
>> 
>a/org/apache/openjpa/kernel/FetchConfigurationImpl.java?rev=573750&r1=
>> 573749&r2=573750&view=diff
>>
>> 
>======================================================================
>> ========
>> ---
>> 
>openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/F
>> etchConfigurationImpl.java
>> (original)
>> +++
>> 
>openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/F
>> etchConfigurationImpl.java
>> Fri Sep  7 16:08:02 2007
>> @@ -604,6 +604,8 @@
>>          int cur;
>>          for (int i = 0; max != FetchGroup.DEPTH_INFINITE
>>              && i < groups.length; i++) {
>> +            // ignore custom groups that are inactive in this
>> configuration
>> +            if (!this.hasFetchGroup(groups[i])) continue;
>>              cur = 
>meta.getFetchGroup(groups[i]).getRecursionDepth(fm);
>>              if (cur == FetchGroup.DEPTH_INFINITE || cur > max)
>>                  max = cur;
>> @@ -625,7 +627,7 @@
>>              return avail;
>>          return Math.min(max, avail);
>>      }
>> -
>> +
>>      /**
>>       * Return the relation type of the given field.
>>       */
>>
>> Modified:
>> 
>openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/Fet
>> chGroup.java
>> URL:
>> 
>http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/jav
>> 
>a/org/apache/openjpa/meta/FetchGroup.java?rev=573750&r1=573749&r2=5737
>> 50&view=diff
>>
>> 
>======================================================================
>> ========
>> ---
>> 
>openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/Fet
>> chGroup.java
>> (original)
>> +++
>> 
>openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/Fet
>> chGroup.java
>> Fri Sep  7 16:08:02 2007
>> @@ -21,9 +21,11 @@
>> import java.io.Serializable;
>> import java.util.ArrayList;
>> import java.util.HashMap;
>> +import java.util.HashSet;
>> import java.util.Iterator;
>> import java.util.List;
>> import java.util.Map;
>> +import java.util.Set;
>>
>> import org.apache.commons.lang.StringUtils;
>> import org.apache.commons.lang.ObjectUtils;
>> @@ -74,6 +76,7 @@
>>      private final ClassMetaData _meta;
>>      private final boolean _readOnly;
>>      private List _includes;
>> +    private Set  _containedBy;
>>      private Map _depths;
>>      private Boolean _postLoad;
>>
>> @@ -171,6 +174,32 @@
>>              }
>>          }
>>          return false;
>> +    }
>> +
>> +    /**
>> +     * Sets this receiver as one of the included fetch groups of the
>> given
>> +     * parent.
>> +     * The parent fecth grop must include this receiver 
>before this call.
>> +     *
>> +     * @see #includes(String, boolean)
>> +     * @see #addDeclaredInclude(String)
>> +     */
>> +    public boolean setContainedBy(FetchGroup parent) {
>> +       parent.addDeclaredInclude(this.getName());
>> +       if (_containedBy==null)
>> +               _containedBy = new HashSet();
>> +       return _containedBy.add(parent.getName());
>> +    }
>> +
>> +    /**
>> +     * Gets the name of the fetch groups in which this 
>receiver has been
>> +     * included.
>> +     *
>> +     * @see #setContainedBy(FetchGroup)
>> +     */
>> +    public String[] getContainedBy() {
>> +       return (_containedBy == null) ? new String[0]
>> +            : (String[]) _containedBy.toArray(new
>> String[_containedBy.size()]);
>>      }
>>
>>      /**
>>
>> Modified:
>> 
>openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/per
>> sistence/AnnotationPersistenceMetaDataParser.java
>> URL:
>> 
>http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence/src/mai
>> 
>n/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataPar
>> ser.java?rev=573750&r1=573749&r2=573750&view=diff
>>
>> 
>======================================================================
>> ========
>> ---
>> 
>openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/per
>> sistence/AnnotationPersistenceMetaDataParser.java
>> (original)
>> +++
>> 
>openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/per
>> sistence/AnnotationPersistenceMetaDataParser.java
>> Fri Sep  7 16:08:02 2007
>> @@ -875,6 +875,14 @@
>>
>>      /**
>>       * Create fetch groups.
>> +     * If FetchGroup A includes FetchGroup B, then a bi-link is set
>> between
>> +     * A and B. Both A and B must be declared in the same Class.
>> +     * <br>
>> +     * Call {@link #parseFetchAttribute(ClassMetaData,
>> +     * org.apache.openjpa.meta.FetchGroup, FetchAttribute) 
>only after the
>> +     * bi-links have been established, because a field f will not 
>> + only
>> add the
>> +     * fetch group A which explictly includes f to its custom fetch
>> groups but
>> +     * also will also add any fetch group B that includes A.
>>       */
>>      private void parseFetchGroups(ClassMetaData meta, FetchGroup...
>> groups) {
>>          org.apache.openjpa.meta.FetchGroup fg; @@ -885,12 +893,25 @@
>>              fg = meta.addDeclaredFetchGroup(group.name());
>>              if (group.postLoad())
>>                  fg.setPostLoad(true);
>> -            for (String s : group.fetchGroups())
>> +            for (String s : group.fetchGroups()) {
>>                  fg.addDeclaredInclude(s);
>> +            }
>> +        }
>> +
>> +        for (FetchGroup group:groups) {
>> +               fg = meta.getFetchGroup(group.name());
>> +               String[] includedFetchGropNames = 
>> + fg.getDeclaredIncludes
>> ();
>> +               for (String
>> includedFectchGroupName:includedFetchGropNames)
>> +                   meta.getFetchGroup
>> (includedFectchGroupName).setContainedBy(fg);
>> +        }
>> +
>> +        for (FetchGroup group : groups) {
>> +            fg = meta.getFetchGroup(group.name());
>>              for (FetchAttribute attr : group.attributes())
>>                  parseFetchAttribute(meta, fg, attr);
>>          }
>>      }
>> +
>>
>>      /**
>>       * Set a field's fetch group.
>> @@ -904,6 +925,9 @@
>>                  meta, attr.name()));
>>
>>          field.setInFetchGroup(fg.getName(), true);
>> +        String[] parentFetchGroups = fg.getContainedBy();
>> +        for (String parentFetchGroup:parentFetchGroups)
>> +               field.setInFetchGroup(parentFetchGroup, true);
>>          if (attr.recursionDepth() != Integer.MIN_VALUE)
>>              fg.setRecursionDepth(field, attr.recursionDepth());
>>      }
>>
>>
>>
>

Notice:  This email message, together with any attachments, may contain information  of  BEA Systems,  Inc.,  its subsidiaries  and  affiliated entities,  that may be confidential,  proprietary,  copyrighted  and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.

Re: svn commit: r573750 - in /openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/meta/ openjpa-persistence/src/main/java/org/apache/openjpa/persistence/

Posted by Kevin Sutter <kw...@gmail.com>.
Was this commit for OPENJPA-358 or OPENJPA-357, or both?  :-)  We need to
remember to include the tag in the commit message so that we get the SVN
changes in the JIRA Issues.  Thanks!

Kevin

On 9/7/07, ppoddar@apache.org <pp...@apache.org> wrote:
>
> Author: ppoddar
> Date: Fri Sep  7 16:08:02 2007
> New Revision: 573750
>
> URL: http://svn.apache.org/viewvc?rev=573750&view=rev
> Log:
> Fix for FetchGroup inclusion and recursion depth calculation.
>
> Modified:
>
>     openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
>
>     openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java
>
>     openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
>
> Modified:
> openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
> URL:
> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java?rev=573750&r1=573749&r2=573750&view=diff
>
> ==============================================================================
> ---
> openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
> (original)
> +++
> openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
> Fri Sep  7 16:08:02 2007
> @@ -604,6 +604,8 @@
>          int cur;
>          for (int i = 0; max != FetchGroup.DEPTH_INFINITE
>              && i < groups.length; i++) {
> +            // ignore custom groups that are inactive in this
> configuration
> +            if (!this.hasFetchGroup(groups[i])) continue;
>              cur = meta.getFetchGroup(groups[i]).getRecursionDepth(fm);
>              if (cur == FetchGroup.DEPTH_INFINITE || cur > max)
>                  max = cur;
> @@ -625,7 +627,7 @@
>              return avail;
>          return Math.min(max, avail);
>      }
> -
> +
>      /**
>       * Return the relation type of the given field.
>       */
>
> Modified:
> openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java
> URL:
> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java?rev=573750&r1=573749&r2=573750&view=diff
>
> ==============================================================================
> ---
> openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java
> (original)
> +++
> openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java
> Fri Sep  7 16:08:02 2007
> @@ -21,9 +21,11 @@
> import java.io.Serializable;
> import java.util.ArrayList;
> import java.util.HashMap;
> +import java.util.HashSet;
> import java.util.Iterator;
> import java.util.List;
> import java.util.Map;
> +import java.util.Set;
>
> import org.apache.commons.lang.StringUtils;
> import org.apache.commons.lang.ObjectUtils;
> @@ -74,6 +76,7 @@
>      private final ClassMetaData _meta;
>      private final boolean _readOnly;
>      private List _includes;
> +    private Set  _containedBy;
>      private Map _depths;
>      private Boolean _postLoad;
>
> @@ -171,6 +174,32 @@
>              }
>          }
>          return false;
> +    }
> +
> +    /**
> +     * Sets this receiver as one of the included fetch groups of the
> given
> +     * parent.
> +     * The parent fecth grop must include this receiver before this call.
> +     *
> +     * @see #includes(String, boolean)
> +     * @see #addDeclaredInclude(String)
> +     */
> +    public boolean setContainedBy(FetchGroup parent) {
> +       parent.addDeclaredInclude(this.getName());
> +       if (_containedBy==null)
> +               _containedBy = new HashSet();
> +       return _containedBy.add(parent.getName());
> +    }
> +
> +    /**
> +     * Gets the name of the fetch groups in which this receiver has been
> +     * included.
> +     *
> +     * @see #setContainedBy(FetchGroup)
> +     */
> +    public String[] getContainedBy() {
> +       return (_containedBy == null) ? new String[0]
> +            : (String[]) _containedBy.toArray(new
> String[_containedBy.size()]);
>      }
>
>      /**
>
> Modified:
> openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
> URL:
> http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java?rev=573750&r1=573749&r2=573750&view=diff
>
> ==============================================================================
> ---
> openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
> (original)
> +++
> openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
> Fri Sep  7 16:08:02 2007
> @@ -875,6 +875,14 @@
>
>      /**
>       * Create fetch groups.
> +     * If FetchGroup A includes FetchGroup B, then a bi-link is set
> between
> +     * A and B. Both A and B must be declared in the same Class.
> +     * <br>
> +     * Call {@link #parseFetchAttribute(ClassMetaData,
> +     * org.apache.openjpa.meta.FetchGroup, FetchAttribute) only after the
> +     * bi-links have been established, because a field f will not only
> add the
> +     * fetch group A which explictly includes f to its custom fetch
> groups but
> +     * also will also add any fetch group B that includes A.
>       */
>      private void parseFetchGroups(ClassMetaData meta, FetchGroup...
> groups) {
>          org.apache.openjpa.meta.FetchGroup fg;
> @@ -885,12 +893,25 @@
>              fg = meta.addDeclaredFetchGroup(group.name());
>              if (group.postLoad())
>                  fg.setPostLoad(true);
> -            for (String s : group.fetchGroups())
> +            for (String s : group.fetchGroups()) {
>                  fg.addDeclaredInclude(s);
> +            }
> +        }
> +
> +        for (FetchGroup group:groups) {
> +               fg = meta.getFetchGroup(group.name());
> +               String[] includedFetchGropNames = fg.getDeclaredIncludes
> ();
> +               for (String
> includedFectchGroupName:includedFetchGropNames)
> +                   meta.getFetchGroup
> (includedFectchGroupName).setContainedBy(fg);
> +        }
> +
> +        for (FetchGroup group : groups) {
> +            fg = meta.getFetchGroup(group.name());
>              for (FetchAttribute attr : group.attributes())
>                  parseFetchAttribute(meta, fg, attr);
>          }
>      }
> +
>
>      /**
>       * Set a field's fetch group.
> @@ -904,6 +925,9 @@
>                  meta, attr.name()));
>
>          field.setInFetchGroup(fg.getName(), true);
> +        String[] parentFetchGroups = fg.getContainedBy();
> +        for (String parentFetchGroup:parentFetchGroups)
> +               field.setInFetchGroup(parentFetchGroup, true);
>          if (attr.recursionDepth() != Integer.MIN_VALUE)
>              fg.setRecursionDepth(field, attr.recursionDepth());
>      }
>
>
>

Re: svn commit: r573750 - in /openjpa/trunk: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ openjpa-kernel/src/main/java/org/apache/openjpa/meta/ openjpa-persistence/src/main/java/org/apache/openjpa/persistence/

Posted by Patrick Linskey <pl...@gmail.com>.
Some comments about the commit:

> +    public boolean setContainedBy(FetchGroup parent) {
> +    public String[] getContainedBy() {

- Personally, I like to add @since tags to any new public methods. I
think I add them to protected methods sometimes too. I don't think
that we have a hard-and-fast policy about this, especially for
internally-facing packages, but I'm hoping to sway as many people as
possible to my way of thinking.

- Why does setContainedBy() return a boolean? It doesn't look like we
use the return value; seems simpler to just leave it out.
Additionally, if we do need it, we should document what it means.

- Why does getContainedBy() return an array? Creating an array is
slow, and they're more difficult to work with than lists usually. If
you're just trying to protect the internal representation, it'd be
better to return a Collection, and make the body just do 'return
Collections.unmodifiableSet(...)' instead of doing the arraycopy.

-Patrick

> +    public boolean setContainedBy(FetchGroup parent) {
> +       parent.addDeclaredInclude(this.getName());
> +       if (_containedBy==null)
> +               _containedBy = new HashSet();
> +       return _containedBy.add(parent.getName());
> +    }
> +
> +    /**
> +     * Gets the name of the fetch groups in which this receiver has been
> +     * included.
> +     *
> +     * @see #setContainedBy(FetchGroup)
> +     */
> +    public String[] getContainedBy() {


On 9/7/07, ppoddar@apache.org <pp...@apache.org> wrote:
> Author: ppoddar
> Date: Fri Sep  7 16:08:02 2007
> New Revision: 573750
>
> URL: http://svn.apache.org/viewvc?rev=573750&view=rev
> Log:
> Fix for FetchGroup inclusion and recursion depth calculation.
>
> Modified:
>     openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
>     openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java
>     openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
>
> Modified: openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
> URL: http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java?rev=573750&r1=573749&r2=573750&view=diff
> ==============================================================================
> --- openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java (original)
> +++ openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java Fri Sep  7 16:08:02 2007
> @@ -604,6 +604,8 @@
>          int cur;
>          for (int i = 0; max != FetchGroup.DEPTH_INFINITE
>              && i < groups.length; i++) {
> +            // ignore custom groups that are inactive in this configuration
> +            if (!this.hasFetchGroup(groups[i])) continue;
>              cur = meta.getFetchGroup(groups[i]).getRecursionDepth(fm);
>              if (cur == FetchGroup.DEPTH_INFINITE || cur > max)
>                  max = cur;
> @@ -625,7 +627,7 @@
>              return avail;
>          return Math.min(max, avail);
>      }
> -
> +
>      /**
>       * Return the relation type of the given field.
>       */
>
> Modified: openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java
> URL: http://svn.apache.org/viewvc/openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java?rev=573750&r1=573749&r2=573750&view=diff
> ==============================================================================
> --- openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java (original)
> +++ openjpa/trunk/openjpa-kernel/src/main/java/org/apache/openjpa/meta/FetchGroup.java Fri Sep  7 16:08:02 2007
> @@ -21,9 +21,11 @@
>  import java.io.Serializable;
>  import java.util.ArrayList;
>  import java.util.HashMap;
> +import java.util.HashSet;
>  import java.util.Iterator;
>  import java.util.List;
>  import java.util.Map;
> +import java.util.Set;
>
>  import org.apache.commons.lang.StringUtils;
>  import org.apache.commons.lang.ObjectUtils;
> @@ -74,6 +76,7 @@
>      private final ClassMetaData _meta;
>      private final boolean _readOnly;
>      private List _includes;
> +    private Set  _containedBy;
>      private Map _depths;
>      private Boolean _postLoad;
>
> @@ -171,6 +174,32 @@
>              }
>          }
>          return false;
> +    }
> +
> +    /**
> +     * Sets this receiver as one of the included fetch groups of the given
> +     * parent.
> +     * The parent fecth grop must include this receiver before this call.
> +     *
> +     * @see #includes(String, boolean)
> +     * @see #addDeclaredInclude(String)
> +     */
> +    public boolean setContainedBy(FetchGroup parent) {
> +       parent.addDeclaredInclude(this.getName());
> +       if (_containedBy==null)
> +               _containedBy = new HashSet();
> +       return _containedBy.add(parent.getName());
> +    }
> +
> +    /**
> +     * Gets the name of the fetch groups in which this receiver has been
> +     * included.
> +     *
> +     * @see #setContainedBy(FetchGroup)
> +     */
> +    public String[] getContainedBy() {
> +       return (_containedBy == null) ? new String[0]
> +            : (String[]) _containedBy.toArray(new String[_containedBy.size()]);
>      }
>
>      /**
>
> Modified: openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java
> URL: http://svn.apache.org/viewvc/openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java?rev=573750&r1=573749&r2=573750&view=diff
> ==============================================================================
> --- openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java (original)
> +++ openjpa/trunk/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java Fri Sep  7 16:08:02 2007
> @@ -875,6 +875,14 @@
>
>      /**
>       * Create fetch groups.
> +     * If FetchGroup A includes FetchGroup B, then a bi-link is set between
> +     * A and B. Both A and B must be declared in the same Class.
> +     * <br>
> +     * Call {@link #parseFetchAttribute(ClassMetaData,
> +     * org.apache.openjpa.meta.FetchGroup, FetchAttribute) only after the
> +     * bi-links have been established, because a field f will not only add the
> +     * fetch group A which explictly includes f to its custom fetch groups but
> +     * also will also add any fetch group B that includes A.
>       */
>      private void parseFetchGroups(ClassMetaData meta, FetchGroup... groups) {
>          org.apache.openjpa.meta.FetchGroup fg;
> @@ -885,12 +893,25 @@
>              fg = meta.addDeclaredFetchGroup(group.name());
>              if (group.postLoad())
>                  fg.setPostLoad(true);
> -            for (String s : group.fetchGroups())
> +            for (String s : group.fetchGroups()) {
>                  fg.addDeclaredInclude(s);
> +            }
> +        }
> +
> +        for (FetchGroup group:groups) {
> +               fg = meta.getFetchGroup(group.name());
> +               String[] includedFetchGropNames = fg.getDeclaredIncludes();
> +               for (String includedFectchGroupName:includedFetchGropNames)
> +                   meta.getFetchGroup(includedFectchGroupName).setContainedBy(fg);
> +        }
> +
> +        for (FetchGroup group : groups) {
> +            fg = meta.getFetchGroup(group.name());
>              for (FetchAttribute attr : group.attributes())
>                  parseFetchAttribute(meta, fg, attr);
>          }
>      }
> +
>
>      /**
>       * Set a field's fetch group.
> @@ -904,6 +925,9 @@
>                  meta, attr.name()));
>
>          field.setInFetchGroup(fg.getName(), true);
> +        String[] parentFetchGroups = fg.getContainedBy();
> +        for (String parentFetchGroup:parentFetchGroups)
> +               field.setInFetchGroup(parentFetchGroup, true);
>          if (attr.recursionDepth() != Integer.MIN_VALUE)
>              fg.setRecursionDepth(field, attr.recursionDepth());
>      }
>
>
>


-- 
Patrick Linskey
202 669 5907