You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by "Teresa Kan (JIRA)" <ji...@apache.org> on 2007/09/14 21:18:32 UTC

[jira] Updated: (OPENJPA-370) LoadFetchGroup annotation was not recognized during the fetch1

     [ https://issues.apache.org/jira/browse/OPENJPA-370?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Teresa Kan updated OPENJPA-370:
-------------------------------

    Attachment: TestFetchGroup.zip

Attach the patch. See the following changes:
Index: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/DelegatingFetchConfiguration.java
===================================================================
--- openjpa-kernel/src/main/java/org/apache/openjpa/kernel/DelegatingFetchConfiguration.java	(revision 574344)
+++ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/DelegatingFetchConfiguration.java	(working copy)
@@ -287,6 +287,9 @@
         }
     }
 
+    public void removeLoadFetchGroup(){
+        
+    }
     public Set getFields() {
         try {
             return _fetch.getFields();
Index: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfiguration.java
===================================================================
--- openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfiguration.java	(revision 574344)
+++ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfiguration.java	(working copy)
@@ -192,7 +192,10 @@
     public FetchConfiguration clearFetchGroups();
 
     /**
-     * Resets the set of fetch groups to the list in the global configuration.
+     * Resets the set of fetch groups to the list in the global configuration. 
+     * The global configuration will be the default plus any fetch groups 
+     * that are added by the openjpa.FetchGroups property in the persistence.xml
+     * file.
      */
     public FetchConfiguration resetFetchGroups();
 
@@ -197,6 +200,11 @@
     public FetchConfiguration resetFetchGroups();
 
     /**
+     * Remove the loadFetchGroup if there is any loadFetchGroup.
+     */   
+    public void removeLoadFetchGroup();
+    
+    /**
      * Returns the set of fully-qualified field names that this component
      * will use when loading objects. Defaults to the empty set.  This set is
      * not thread safe.
Index: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java
===================================================================
--- openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java	(revision 575491)
+++ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchConfigurationImpl.java	(working copy)
@@ -92,6 +92,7 @@
     private boolean _load = true;
     private int _availableRecursion;
     private int _availableDepth;
+    private String _lfg = null;
 
     public FetchConfigurationImpl() {
         this(null);
@@ -287,7 +288,14 @@
                 getFetchGroupsList()));
         return this;
     }
+      
+    public void removeLoadFetchGroup() {
+        if (_lfg != null)
+            removeFetchGroup(_lfg);
+        _lfg = null;
+    }
 
+
     public Set getFields() {
         return (_state.fields == null) ? Collections.EMPTY_SET : _state.fields;
     }
@@ -568,8 +576,16 @@
             return true;
         String[] fgs = fmd.getCustomFetchGroups();
         for (int i = 0; i < fgs.length; i++)
-            if (hasFetchGroup(fgs[i]))
+            if (hasFetchGroup(fgs[i])) {
+                String fg = fmd.getLoadFetchGroup();
+                if (fg != null) {
+                    if (!hasFetchGroup(fg)) {
+                        addFetchGroup(fg);
+                        _lfg = fg;
+                    }
+                }
                 return true;
+            }
         return false; 
     }
 
@@ -576,7 +592,7 @@
     /**
      * Return the available recursion depth via the given field for the
      * given type.
-     *
+     * 
      * @param traverse whether we're traversing the field
      */
     private int getAvailableRecursionDepth(FieldMetaData fm, Class type, 
Index: openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java
===================================================================
--- openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java	(revision 575494)
+++ openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateManagerImpl.java	(working copy)
@@ -358,6 +358,10 @@
         // is loaded, etc
         int lockLevel = calculateLockLevel(active, forWrite, fetch);
         boolean ret = loadFields(fields, fetch, lockLevel, sdata, forWrite);
+        // call back to FetchConfiguration to clean up any loadFetchGroup.
+        // The loadFetchGroup was set by the FetchConfigurationImpl.includes()
+        // during the process of the getUnloadedInternal method. 
+        fetch.removeLoadFetchGroup();
         obtainLocks(active, forWrite, lockLevel, fetch, sdata);
         return ret;
     }
Index: openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
===================================================================
--- openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java	(revision 574693)
+++ openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java	(working copy)
@@ -710,6 +710,8 @@
     /**
      * The fetch group that is to be loaded when this receiver is loaded, or
      * null if none set.
+     * The LoadFetchGroup does not relate to the FetchGroup annotation. Therefore
+     * resets a fetch group will not remove this LoadFetchGroup.
      */
     public void setLoadFetchGroup (String lfg) {
         if ("".equals(lfg))


> LoadFetchGroup annotation was not recognized during the fetch1
> --------------------------------------------------------------
>
>                 Key: OPENJPA-370
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-370
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.1.0
>            Reporter: Teresa Kan
>         Attachments: TestFetchGroup.zip
>
>
> Employee class has a LoadFetchGroup annotation defined on the Rating field, when getRating was called, the address should be returned also. However, openjpa did not handle the LoadFetchGroup correctly, therefore, address was not eargly fetched.
> public class FGEmployee{
>     @Id
>     private int id;
>  
>     @OneToOne(fetch=FetchType.LAZY) 
>     private FGAddress address;
>  
>     @Basic(fetch=FetchType.LAZY)
>     @LoadFetchGroup("AddressFetchGroup")
>     private String rating;
>  
>     @ManyToOne(fetch=FetchType.LAZY)
>     private FGManager manager;
> ..
> }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


RE: [jira] Updated: (OPENJPA-370) LoadFetchGroup annotation was not recognized during the fetch1

Posted by Pinaki Poddar <pp...@bea.com>.
You are possibly right.

In general, declaration scope of metadata elements both standard (e.g.
SQLResultSetMapping) or non-standard (e.g. FetchGroup) has been an
outstanding issue, especially OpenJPA's load-on-demand philosophy (which
is a desirable feature). 
In any case, this declartion scope aspect should be strengthened in
future version of JPA spec.    


Pinaki Poddar
972.834.2865
 

>-----Original Message-----
>From: Patrick Linskey [mailto:plinskey@gmail.com] 
>Sent: Monday, September 17, 2007 12:48 PM
>To: dev@openjpa.apache.org
>Subject: Re: [jira] Updated: (OPENJPA-370) LoadFetchGroup 
>annotation was not recognized during the fetch1
>
>> during the parsing phase. If we restrict that if Field f specifies 
>> LoadFetchGroup L then L must be declared in the scope of the same 
>> parsing unit, then capturing that 'indirect' information becomes 
>> plausible.
>
>It could be that we do it the way it's done currently to allow 
>subclasses to contribute to the LoadFetchGroup.
>
>-Patrick
>
>On 9/17/07, Pinaki Poddar <pp...@bea.com> wrote:
>> I am guessing the reason is in the way our data structures 
>> (FieldMetaData and FetchConfiguration) capture the information.
>> They are designed to answer the 'direct' question:
>>     FetchConfiguration.hasField(String fullyQualifiedFieldName) and 
>> FieldMetaData.isInFetchGroup(String fg).
>>
>>  But with LoadFetchGroup the answer is 'indirect' -- should 
>field f be 
>> included because of someone else?
>> To answer that question efficiently and in a localized manner -- I 
>> think we need to capture more info into FieldMetaData during the 
>> parsing phase. If we restrict that if Field f specifies 
>LoadFetchGroup 
>> L then L must be declared in the scope of the same parsing 
>unit, then 
>> capturing that 'indirect' information becomes plausible.
>>
>>  As the question could not be answered in a localized manner with 
>> current data structures -- the approach was to check if 
>everything is 
>> loaded before being returned to the caller. And if not, then add the 
>> LoadFetchGroups temporarily for loading.
>>
>>
>> Pinaki Poddar
>> 972.834.2865
>>
>>
>> >-----Original Message-----
>> >From: Patrick Linskey [mailto:plinskey@gmail.com]
>> >Sent: Monday, September 17, 2007 12:22 PM
>> >To: dev@openjpa.apache.org
>> >Subject: Re: [jira] Updated: (OPENJPA-370) LoadFetchGroup 
>annotation 
>> >was not recognized during the fetch1
>> >
>> >> Also if a field f is included in current FectConfiguration
>> >can then be
>> >> decided uniformly in one place i.e.
>> >> FecthConfigurationImpl.includes(FieldMetaData fmd);
>> >
>> >Generally-speaking, that sounds like a good goal. I'm not sure if 
>> >there is a reason why the fetch group handling is done differently 
>> >during the StateManagerImpl.loadField() call, but all things being 
>> >equal, it would be good to reduce the different ways that fetch 
>> >groups are configured where possible.
>> >
>> >-Patrick
>> >
>> >On 9/17/07, Pinaki Poddar <pp...@bea.com> wrote:
>> >>
>> >> Hi,
>> >>   1. Within the load sequence, the LoadFetchGroup is handled in 
>> >> org.apache.openjpa.kernel.StateManagerImpl
>> >>
>> >>     protected void loadField(int, int, boolean, boolean);
>> >>
>> >>    This method checks for any associated LoadFetchGroup L
>> >for a field
>> >> f, adds L to the current FetchConfiguration temporarily and
>> >removes L
>> >> after load. This codepath is not getting executed withh your
>> >test case
>> >> -- I have not debugged it to find out why.
>> >>
>> >>   2. This method follows a similar approach as in your 
>approach i.e.
>> >> add L temporarily and then remove it.
>> >>
>> >>   3. The other line of attck will be to capture enough 
>information 
>> >> during annotation metadata parsing so that FecthConfigurationImpl 
>> >> method
>> >>         private boolean includes(FieldMetaData fmd); can
>> >affirm if fmd
>> >> is included in the currrent FetchConfiguration if not
>> >directly (as it
>> >> does now) then indirectly via LoadFetchGroup.
>> >> In this way, we can get rid of both 
>temporarily-add-and-then-remove 
>> >> and
>> >> check-during-StateManagerImpl.loadField() approach.
>> >> Also if a field f is included in current FectConfiguration
>> >can then be
>> >> decided uniformly in one place i.e.
>> >> FecthConfigurationImpl.includes(FieldMetaData fmd);
>> >>
>> >>
>> >>
>> >>
>> >> Pinaki Poddar
>> >> 972.834.2865
>> >>
>> >>
>> >> >-----Original Message-----
>> >> >From: Teresa Kan (JIRA) [mailto:jira@apache.org]
>> >> >Sent: Friday, September 14, 2007 2:19 PM
>> >> >To: dev@openjpa.apache.org
>> >> >Subject: [jira] Updated: (OPENJPA-370) LoadFetchGroup
>> >annotation was
>> >> >not recognized during the fetch1
>> >> >
>> >> >
>> >> >     [
>> >> >https://issues.apache.org/jira/browse/OPENJPA-370?page=com.atla
>> >> >ssian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>> >> >
>> >> >Teresa Kan updated OPENJPA-370:
>> >> >-------------------------------
>> >> >
>> >> >    Attachment: TestFetchGroup.zip
>> >> >
>> >> >Attach the patch. See the following changes:
>> >> >Index:
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>> >> >ngFetchConfiguration.java
>> >> 
>>==================================================================
>> >> >=
>> >> >---
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>> >> >ngFetchConfiguration.java      (revision 574344)
>> >> >+++
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>> >> >ngFetchConfiguration.java      (working copy)
>> >> >@@ -287,6 +287,9 @@
>> >> >         }
>> >> >     }
>> >> >
>> >> >+    public void removeLoadFetchGroup(){
>> >> >+
>> >> >+    }
>> >> >     public Set getFields() {
>> >> >         try {
>> >> >             return _fetch.getFields();
>> >> >Index:
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >> >figuration.java
>> >> 
>>==================================================================
>> >> >=
>> >> >---
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >> >figuration.java        (revision 574344)
>> >> >+++
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >> >figuration.java        (working copy)
>> >> >@@ -192,7 +192,10 @@
>> >> >     public FetchConfiguration clearFetchGroups();
>> >> >
>> >> >     /**
>> >> >-     * Resets the set of fetch groups to the list in the
>> >> >global configuration.
>> >> >+     * Resets the set of fetch groups to the list in the
>> >> >global configuration.
>> >> >+     * The global configuration will be the default plus any
>> >> >fetch groups
>> >> >+     * that are added by the openjpa.FetchGroups property in
>> >> >the persistence.xml
>> >> >+     * file.
>> >> >      */
>> >> >     public FetchConfiguration resetFetchGroups();
>> >> >
>> >> >@@ -197,6 +200,11 @@
>> >> >     public FetchConfiguration resetFetchGroups();
>> >> >
>> >> >     /**
>> >> >+     * Remove the loadFetchGroup if there is any loadFetchGroup.
>> >> >+     */
>> >> >+    public void removeLoadFetchGroup();
>> >> >+
>> >> >+    /**
>> >> >      * Returns the set of fully-qualified field names that this 
>> >> >component
>> >> >      * will use when loading objects. Defaults to the empty set.
>> >> >This set is
>> >> >      * not thread safe.
>> >> >Index:
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >> >figurationImpl.java
>> >> 
>>==================================================================
>> >> >=
>> >> >---
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >> >figurationImpl.java    (revision 575491)
>> >> >+++
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >> >figurationImpl.java    (working copy)
>> >> >@@ -92,6 +92,7 @@
>> >> >     private boolean _load = true;
>> >> >     private int _availableRecursion;
>> >> >     private int _availableDepth;
>> >> >+    private String _lfg = null;
>> >> >
>> >> >     public FetchConfigurationImpl() {
>> >> >         this(null);
>> >> >@@ -287,7 +288,14 @@
>> >> >                 getFetchGroupsList()));
>> >> >         return this;
>> >> >     }
>> >> >+
>> >> >+    public void removeLoadFetchGroup() {
>> >> >+        if (_lfg != null)
>> >> >+            removeFetchGroup(_lfg);
>> >> >+        _lfg = null;
>> >> >+    }
>> >> >
>> >> >+
>> >> >     public Set getFields() {
>> >> >         return (_state.fields == null) ?
>> >> >Collections.EMPTY_SET : _state.fields;
>> >> >     }
>> >> >@@ -568,8 +576,16 @@
>> >> >             return true;
>> >> >         String[] fgs = fmd.getCustomFetchGroups();
>> >> >         for (int i = 0; i < fgs.length; i++)
>> >> >-            if (hasFetchGroup(fgs[i]))
>> >> >+            if (hasFetchGroup(fgs[i])) {
>> >> >+                String fg = fmd.getLoadFetchGroup();
>> >> >+                if (fg != null) {
>> >> >+                    if (!hasFetchGroup(fg)) {
>> >> >+                        addFetchGroup(fg);
>> >> >+                        _lfg = fg;
>> >> >+                    }
>> >> >+                }
>> >> >                 return true;
>> >> >+            }
>> >> >         return false;
>> >> >     }
>> >> >
>> >> >@@ -576,7 +592,7 @@
>> >> >     /**
>> >> >      * Return the available recursion depth via the given
>> >field for
>> >> >the
>> >> >      * given type.
>> >> >-     *
>> >> >+     *
>> >> >      * @param traverse whether we're traversing the field
>> >> >      */
>> >> >     private int getAvailableRecursionDepth(FieldMetaData fm, 
>> >> >Class type,
>> >> >Index:
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>> >> >agerImpl.java
>> >> 
>>==================================================================
>> >> >=
>> >> >---
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>> >> >agerImpl.java  (revision 575494)
>> >> >+++
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>> >> >agerImpl.java  (working copy)
>> >> >@@ -358,6 +358,10 @@
>> >> >         // is loaded, etc
>> >> >         int lockLevel = calculateLockLevel(active,
>> >forWrite, fetch);
>> >> >         boolean ret = loadFields(fields, fetch, 
>lockLevel, sdata, 
>> >> >forWrite);
>> >> >+        // call back to FetchConfiguration to clean up any
>> >> >loadFetchGroup.
>> >> >+        // The loadFetchGroup was set by the
>> >> >FetchConfigurationImpl.includes()
>> >> >+        // during the process of the getUnloadedInternal method.
>> >> >+        fetch.removeLoadFetchGroup();
>> >> >         obtainLocks(active, forWrite, lockLevel, fetch, sdata);
>> >> >         return ret;
>> >> >     }
>> >> >Index:
>> >>
>> 
>>>openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.j
>> >>a
>> >> >va
>> >> 
>>==================================================================
>> >> >=
>> >> >---
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
>> >> >ata.java       (revision 574693)
>> >> >+++
>> >> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
>> >> >ata.java       (working copy)
>> >> >@@ -710,6 +710,8 @@
>> >> >     /**
>> >> >      * The fetch group that is to be loaded when this 
>receiver is 
>> >> >loaded, or
>> >> >      * null if none set.
>> >> >+     * The LoadFetchGroup does not relate to the FetchGroup
>> >> >annotation. Therefore
>> >> >+     * resets a fetch group will not remove this LoadFetchGroup.
>> >> >      */
>> >> >     public void setLoadFetchGroup (String lfg) {
>> >> >         if ("".equals(lfg))
>> >> >
>> >> >
>> >> >> LoadFetchGroup annotation was not recognized during the fetch1
>> >> >> --------------------------------------------------------------
>> >> >>
>> >> >>                 Key: OPENJPA-370
>> >> >>                 URL:
>> >> >https://issues.apache.org/jira/browse/OPENJPA-370
>> >> >>             Project: OpenJPA
>> >> >>          Issue Type: Bug
>> >> >>          Components: kernel
>> >> >>    Affects Versions: 1.1.0
>> >> >>            Reporter: Teresa Kan
>> >> >>         Attachments: TestFetchGroup.zip
>> >> >>
>> >> >>
>> >> >> Employee class has a LoadFetchGroup annotation defined on
>> >> >the Rating field, when getRating was called, the address 
>should be 
>> >> >returned also. However, openjpa did not handle the 
>LoadFetchGroup 
>> >> >correctly, therefore, address was not eargly fetched.
>> >> >> public class FGEmployee{
>> >> >>     @Id
>> >> >>     private int id;
>> >> >>
>> >> >>     @OneToOne(fetch=FetchType.LAZY)
>> >> >>     private FGAddress address;
>> >> >>
>> >> >>     @Basic(fetch=FetchType.LAZY)
>> >> >>     @LoadFetchGroup("AddressFetchGroup")
>> >> >>     private String rating;
>> >> >>
>> >> >>     @ManyToOne(fetch=FetchType.LAZY)
>> >> >>     private FGManager manager;
>> >> >> ..
>> >> >> }
>> >> >
>> >> >--
>> >> >This message is automatically generated by JIRA.
>> >> >-
>> >> >You can reply to this email to add a comment to the issue online.
>> >> >
>> >> >
>> >>
>> >> 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
>> >
>>
>> 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
>

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: [jira] Updated: (OPENJPA-370) LoadFetchGroup annotation was not recognized during the fetch1

Posted by Patrick Linskey <pl...@gmail.com>.
> during the parsing phase. If we restrict that if Field f specifies
> LoadFetchGroup L then L must be declared in the scope of the same
> parsing unit, then capturing that 'indirect' information becomes
> plausible.

It could be that we do it the way it's done currently to allow
subclasses to contribute to the LoadFetchGroup.

-Patrick

On 9/17/07, Pinaki Poddar <pp...@bea.com> wrote:
> I am guessing the reason is in the way our data structures
> (FieldMetaData and FetchConfiguration) capture the information.
> They are designed to answer the 'direct' question:
>     FetchConfiguration.hasField(String fullyQualifiedFieldName)
> and FieldMetaData.isInFetchGroup(String fg).
>
>  But with LoadFetchGroup the answer is 'indirect' -- should field f be
> included because of someone else?
> To answer that question efficiently and in a localized manner -- I think
> we need to capture more info into FieldMetaData
> during the parsing phase. If we restrict that if Field f specifies
> LoadFetchGroup L then L must be declared in the scope of the same
> parsing unit, then capturing that 'indirect' information becomes
> plausible.
>
>  As the question could not be answered in a localized manner with
> current data structures -- the approach was to check if everything is
> loaded before being returned to the caller. And if not, then add the
> LoadFetchGroups temporarily for loading.
>
>
> Pinaki Poddar
> 972.834.2865
>
>
> >-----Original Message-----
> >From: Patrick Linskey [mailto:plinskey@gmail.com]
> >Sent: Monday, September 17, 2007 12:22 PM
> >To: dev@openjpa.apache.org
> >Subject: Re: [jira] Updated: (OPENJPA-370) LoadFetchGroup
> >annotation was not recognized during the fetch1
> >
> >> Also if a field f is included in current FectConfiguration
> >can then be
> >> decided uniformly in one place i.e.
> >> FecthConfigurationImpl.includes(FieldMetaData fmd);
> >
> >Generally-speaking, that sounds like a good goal. I'm not sure
> >if there is a reason why the fetch group handling is done
> >differently during the StateManagerImpl.loadField() call, but
> >all things being equal, it would be good to reduce the
> >different ways that fetch groups are configured where possible.
> >
> >-Patrick
> >
> >On 9/17/07, Pinaki Poddar <pp...@bea.com> wrote:
> >>
> >> Hi,
> >>   1. Within the load sequence, the LoadFetchGroup is handled in
> >> org.apache.openjpa.kernel.StateManagerImpl
> >>
> >>     protected void loadField(int, int, boolean, boolean);
> >>
> >>    This method checks for any associated LoadFetchGroup L
> >for a field
> >> f, adds L to the current FetchConfiguration temporarily and
> >removes L
> >> after load. This codepath is not getting executed withh your
> >test case
> >> -- I have not debugged it to find out why.
> >>
> >>   2. This method follows a similar approach as in your approach i.e.
> >> add L temporarily and then remove it.
> >>
> >>   3. The other line of attck will be to capture enough information
> >> during annotation metadata parsing so that FecthConfigurationImpl
> >> method
> >>         private boolean includes(FieldMetaData fmd); can
> >affirm if fmd
> >> is included in the currrent FetchConfiguration if not
> >directly (as it
> >> does now) then indirectly via LoadFetchGroup.
> >> In this way, we can get rid of both temporarily-add-and-then-remove
> >> and
> >> check-during-StateManagerImpl.loadField() approach.
> >> Also if a field f is included in current FectConfiguration
> >can then be
> >> decided uniformly in one place i.e.
> >> FecthConfigurationImpl.includes(FieldMetaData fmd);
> >>
> >>
> >>
> >>
> >> Pinaki Poddar
> >> 972.834.2865
> >>
> >>
> >> >-----Original Message-----
> >> >From: Teresa Kan (JIRA) [mailto:jira@apache.org]
> >> >Sent: Friday, September 14, 2007 2:19 PM
> >> >To: dev@openjpa.apache.org
> >> >Subject: [jira] Updated: (OPENJPA-370) LoadFetchGroup
> >annotation was
> >> >not recognized during the fetch1
> >> >
> >> >
> >> >     [
> >> >https://issues.apache.org/jira/browse/OPENJPA-370?page=com.atla
> >> >ssian.jira.plugin.system.issuetabpanels:all-tabpanel ]
> >> >
> >> >Teresa Kan updated OPENJPA-370:
> >> >-------------------------------
> >> >
> >> >    Attachment: TestFetchGroup.zip
> >> >
> >> >Attach the patch. See the following changes:
> >> >Index:
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >> >ngFetchConfiguration.java
> >> >===================================================================
> >> >---
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >> >ngFetchConfiguration.java      (revision 574344)
> >> >+++
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >> >ngFetchConfiguration.java      (working copy)
> >> >@@ -287,6 +287,9 @@
> >> >         }
> >> >     }
> >> >
> >> >+    public void removeLoadFetchGroup(){
> >> >+
> >> >+    }
> >> >     public Set getFields() {
> >> >         try {
> >> >             return _fetch.getFields();
> >> >Index:
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >> >figuration.java
> >> >===================================================================
> >> >---
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >> >figuration.java        (revision 574344)
> >> >+++
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >> >figuration.java        (working copy)
> >> >@@ -192,7 +192,10 @@
> >> >     public FetchConfiguration clearFetchGroups();
> >> >
> >> >     /**
> >> >-     * Resets the set of fetch groups to the list in the
> >> >global configuration.
> >> >+     * Resets the set of fetch groups to the list in the
> >> >global configuration.
> >> >+     * The global configuration will be the default plus any
> >> >fetch groups
> >> >+     * that are added by the openjpa.FetchGroups property in
> >> >the persistence.xml
> >> >+     * file.
> >> >      */
> >> >     public FetchConfiguration resetFetchGroups();
> >> >
> >> >@@ -197,6 +200,11 @@
> >> >     public FetchConfiguration resetFetchGroups();
> >> >
> >> >     /**
> >> >+     * Remove the loadFetchGroup if there is any loadFetchGroup.
> >> >+     */
> >> >+    public void removeLoadFetchGroup();
> >> >+
> >> >+    /**
> >> >      * Returns the set of fully-qualified field names that this
> >> >component
> >> >      * will use when loading objects. Defaults to the empty set.
> >> >This set is
> >> >      * not thread safe.
> >> >Index:
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >> >figurationImpl.java
> >> >===================================================================
> >> >---
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >> >figurationImpl.java    (revision 575491)
> >> >+++
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >> >figurationImpl.java    (working copy)
> >> >@@ -92,6 +92,7 @@
> >> >     private boolean _load = true;
> >> >     private int _availableRecursion;
> >> >     private int _availableDepth;
> >> >+    private String _lfg = null;
> >> >
> >> >     public FetchConfigurationImpl() {
> >> >         this(null);
> >> >@@ -287,7 +288,14 @@
> >> >                 getFetchGroupsList()));
> >> >         return this;
> >> >     }
> >> >+
> >> >+    public void removeLoadFetchGroup() {
> >> >+        if (_lfg != null)
> >> >+            removeFetchGroup(_lfg);
> >> >+        _lfg = null;
> >> >+    }
> >> >
> >> >+
> >> >     public Set getFields() {
> >> >         return (_state.fields == null) ?
> >> >Collections.EMPTY_SET : _state.fields;
> >> >     }
> >> >@@ -568,8 +576,16 @@
> >> >             return true;
> >> >         String[] fgs = fmd.getCustomFetchGroups();
> >> >         for (int i = 0; i < fgs.length; i++)
> >> >-            if (hasFetchGroup(fgs[i]))
> >> >+            if (hasFetchGroup(fgs[i])) {
> >> >+                String fg = fmd.getLoadFetchGroup();
> >> >+                if (fg != null) {
> >> >+                    if (!hasFetchGroup(fg)) {
> >> >+                        addFetchGroup(fg);
> >> >+                        _lfg = fg;
> >> >+                    }
> >> >+                }
> >> >                 return true;
> >> >+            }
> >> >         return false;
> >> >     }
> >> >
> >> >@@ -576,7 +592,7 @@
> >> >     /**
> >> >      * Return the available recursion depth via the given
> >field for
> >> >the
> >> >      * given type.
> >> >-     *
> >> >+     *
> >> >      * @param traverse whether we're traversing the field
> >> >      */
> >> >     private int getAvailableRecursionDepth(FieldMetaData fm, Class
> >> >type,
> >> >Index:
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >> >agerImpl.java
> >> >===================================================================
> >> >---
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >> >agerImpl.java  (revision 575494)
> >> >+++
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >> >agerImpl.java  (working copy)
> >> >@@ -358,6 +358,10 @@
> >> >         // is loaded, etc
> >> >         int lockLevel = calculateLockLevel(active,
> >forWrite, fetch);
> >> >         boolean ret = loadFields(fields, fetch, lockLevel, sdata,
> >> >forWrite);
> >> >+        // call back to FetchConfiguration to clean up any
> >> >loadFetchGroup.
> >> >+        // The loadFetchGroup was set by the
> >> >FetchConfigurationImpl.includes()
> >> >+        // during the process of the getUnloadedInternal method.
> >> >+        fetch.removeLoadFetchGroup();
> >> >         obtainLocks(active, forWrite, lockLevel, fetch, sdata);
> >> >         return ret;
> >> >     }
> >> >Index:
> >>
> >>openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.ja
> >> >va
> >> >===================================================================
> >> >---
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
> >> >ata.java       (revision 574693)
> >> >+++
> >> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
> >> >ata.java       (working copy)
> >> >@@ -710,6 +710,8 @@
> >> >     /**
> >> >      * The fetch group that is to be loaded when this receiver is
> >> >loaded, or
> >> >      * null if none set.
> >> >+     * The LoadFetchGroup does not relate to the FetchGroup
> >> >annotation. Therefore
> >> >+     * resets a fetch group will not remove this LoadFetchGroup.
> >> >      */
> >> >     public void setLoadFetchGroup (String lfg) {
> >> >         if ("".equals(lfg))
> >> >
> >> >
> >> >> LoadFetchGroup annotation was not recognized during the fetch1
> >> >> --------------------------------------------------------------
> >> >>
> >> >>                 Key: OPENJPA-370
> >> >>                 URL:
> >> >https://issues.apache.org/jira/browse/OPENJPA-370
> >> >>             Project: OpenJPA
> >> >>          Issue Type: Bug
> >> >>          Components: kernel
> >> >>    Affects Versions: 1.1.0
> >> >>            Reporter: Teresa Kan
> >> >>         Attachments: TestFetchGroup.zip
> >> >>
> >> >>
> >> >> Employee class has a LoadFetchGroup annotation defined on
> >> >the Rating field, when getRating was called, the address should be
> >> >returned also. However, openjpa did not handle the LoadFetchGroup
> >> >correctly, therefore, address was not eargly fetched.
> >> >> public class FGEmployee{
> >> >>     @Id
> >> >>     private int id;
> >> >>
> >> >>     @OneToOne(fetch=FetchType.LAZY)
> >> >>     private FGAddress address;
> >> >>
> >> >>     @Basic(fetch=FetchType.LAZY)
> >> >>     @LoadFetchGroup("AddressFetchGroup")
> >> >>     private String rating;
> >> >>
> >> >>     @ManyToOne(fetch=FetchType.LAZY)
> >> >>     private FGManager manager;
> >> >> ..
> >> >> }
> >> >
> >> >--
> >> >This message is automatically generated by JIRA.
> >> >-
> >> >You can reply to this email to add a comment to the issue online.
> >> >
> >> >
> >>
> >> 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
> >
>
> 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: [jira] Updated: (OPENJPA-370) LoadFetchGroup annotation was not recognized during the fetch1

Posted by Pinaki Poddar <pp...@bea.com>.
I am guessing the reason is in the way our data structures
(FieldMetaData and FetchConfiguration) capture the information. 
They are designed to answer the 'direct' question: 
    FetchConfiguration.hasField(String fullyQualifiedFieldName) 
and FieldMetaData.isInFetchGroup(String fg).

 But with LoadFetchGroup the answer is 'indirect' -- should field f be
included because of someone else? 
To answer that question efficiently and in a localized manner -- I think
we need to capture more info into FieldMetaData
during the parsing phase. If we restrict that if Field f specifies
LoadFetchGroup L then L must be declared in the scope of the same
parsing unit, then capturing that 'indirect' information becomes
plausible.  

 As the question could not be answered in a localized manner with
current data structures -- the approach was to check if everything is
loaded before being returned to the caller. And if not, then add the
LoadFetchGroups temporarily for loading. 


Pinaki Poddar
972.834.2865
 

>-----Original Message-----
>From: Patrick Linskey [mailto:plinskey@gmail.com] 
>Sent: Monday, September 17, 2007 12:22 PM
>To: dev@openjpa.apache.org
>Subject: Re: [jira] Updated: (OPENJPA-370) LoadFetchGroup 
>annotation was not recognized during the fetch1
>
>> Also if a field f is included in current FectConfiguration 
>can then be 
>> decided uniformly in one place i.e.
>> FecthConfigurationImpl.includes(FieldMetaData fmd);
>
>Generally-speaking, that sounds like a good goal. I'm not sure 
>if there is a reason why the fetch group handling is done 
>differently during the StateManagerImpl.loadField() call, but 
>all things being equal, it would be good to reduce the 
>different ways that fetch groups are configured where possible.
>
>-Patrick
>
>On 9/17/07, Pinaki Poddar <pp...@bea.com> wrote:
>>
>> Hi,
>>   1. Within the load sequence, the LoadFetchGroup is handled in 
>> org.apache.openjpa.kernel.StateManagerImpl
>>
>>     protected void loadField(int, int, boolean, boolean);
>>
>>    This method checks for any associated LoadFetchGroup L 
>for a field 
>> f, adds L to the current FetchConfiguration temporarily and 
>removes L 
>> after load. This codepath is not getting executed withh your 
>test case 
>> -- I have not debugged it to find out why.
>>
>>   2. This method follows a similar approach as in your approach i.e. 
>> add L temporarily and then remove it.
>>
>>   3. The other line of attck will be to capture enough information 
>> during annotation metadata parsing so that FecthConfigurationImpl 
>> method
>>         private boolean includes(FieldMetaData fmd); can 
>affirm if fmd 
>> is included in the currrent FetchConfiguration if not 
>directly (as it 
>> does now) then indirectly via LoadFetchGroup.
>> In this way, we can get rid of both temporarily-add-and-then-remove 
>> and
>> check-during-StateManagerImpl.loadField() approach.
>> Also if a field f is included in current FectConfiguration 
>can then be 
>> decided uniformly in one place i.e.
>> FecthConfigurationImpl.includes(FieldMetaData fmd);
>>
>>
>>
>>
>> Pinaki Poddar
>> 972.834.2865
>>
>>
>> >-----Original Message-----
>> >From: Teresa Kan (JIRA) [mailto:jira@apache.org]
>> >Sent: Friday, September 14, 2007 2:19 PM
>> >To: dev@openjpa.apache.org
>> >Subject: [jira] Updated: (OPENJPA-370) LoadFetchGroup 
>annotation was 
>> >not recognized during the fetch1
>> >
>> >
>> >     [
>> >https://issues.apache.org/jira/browse/OPENJPA-370?page=com.atla
>> >ssian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>> >
>> >Teresa Kan updated OPENJPA-370:
>> >-------------------------------
>> >
>> >    Attachment: TestFetchGroup.zip
>> >
>> >Attach the patch. See the following changes:
>> >Index:
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>> >ngFetchConfiguration.java
>> >===================================================================
>> >---
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>> >ngFetchConfiguration.java      (revision 574344)
>> >+++
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>> >ngFetchConfiguration.java      (working copy)
>> >@@ -287,6 +287,9 @@
>> >         }
>> >     }
>> >
>> >+    public void removeLoadFetchGroup(){
>> >+
>> >+    }
>> >     public Set getFields() {
>> >         try {
>> >             return _fetch.getFields();
>> >Index:
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >figuration.java
>> >===================================================================
>> >---
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >figuration.java        (revision 574344)
>> >+++
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >figuration.java        (working copy)
>> >@@ -192,7 +192,10 @@
>> >     public FetchConfiguration clearFetchGroups();
>> >
>> >     /**
>> >-     * Resets the set of fetch groups to the list in the
>> >global configuration.
>> >+     * Resets the set of fetch groups to the list in the
>> >global configuration.
>> >+     * The global configuration will be the default plus any
>> >fetch groups
>> >+     * that are added by the openjpa.FetchGroups property in
>> >the persistence.xml
>> >+     * file.
>> >      */
>> >     public FetchConfiguration resetFetchGroups();
>> >
>> >@@ -197,6 +200,11 @@
>> >     public FetchConfiguration resetFetchGroups();
>> >
>> >     /**
>> >+     * Remove the loadFetchGroup if there is any loadFetchGroup.
>> >+     */
>> >+    public void removeLoadFetchGroup();
>> >+
>> >+    /**
>> >      * Returns the set of fully-qualified field names that this 
>> >component
>> >      * will use when loading objects. Defaults to the empty set.  
>> >This set is
>> >      * not thread safe.
>> >Index:
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >figurationImpl.java
>> >===================================================================
>> >---
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >figurationImpl.java    (revision 575491)
>> >+++
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>> >figurationImpl.java    (working copy)
>> >@@ -92,6 +92,7 @@
>> >     private boolean _load = true;
>> >     private int _availableRecursion;
>> >     private int _availableDepth;
>> >+    private String _lfg = null;
>> >
>> >     public FetchConfigurationImpl() {
>> >         this(null);
>> >@@ -287,7 +288,14 @@
>> >                 getFetchGroupsList()));
>> >         return this;
>> >     }
>> >+
>> >+    public void removeLoadFetchGroup() {
>> >+        if (_lfg != null)
>> >+            removeFetchGroup(_lfg);
>> >+        _lfg = null;
>> >+    }
>> >
>> >+
>> >     public Set getFields() {
>> >         return (_state.fields == null) ?
>> >Collections.EMPTY_SET : _state.fields;
>> >     }
>> >@@ -568,8 +576,16 @@
>> >             return true;
>> >         String[] fgs = fmd.getCustomFetchGroups();
>> >         for (int i = 0; i < fgs.length; i++)
>> >-            if (hasFetchGroup(fgs[i]))
>> >+            if (hasFetchGroup(fgs[i])) {
>> >+                String fg = fmd.getLoadFetchGroup();
>> >+                if (fg != null) {
>> >+                    if (!hasFetchGroup(fg)) {
>> >+                        addFetchGroup(fg);
>> >+                        _lfg = fg;
>> >+                    }
>> >+                }
>> >                 return true;
>> >+            }
>> >         return false;
>> >     }
>> >
>> >@@ -576,7 +592,7 @@
>> >     /**
>> >      * Return the available recursion depth via the given 
>field for 
>> >the
>> >      * given type.
>> >-     *
>> >+     *
>> >      * @param traverse whether we're traversing the field
>> >      */
>> >     private int getAvailableRecursionDepth(FieldMetaData fm, Class 
>> >type,
>> >Index:
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>> >agerImpl.java
>> >===================================================================
>> >---
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>> >agerImpl.java  (revision 575494)
>> >+++
>> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>> >agerImpl.java  (working copy)
>> >@@ -358,6 +358,10 @@
>> >         // is loaded, etc
>> >         int lockLevel = calculateLockLevel(active, 
>forWrite, fetch);
>> >         boolean ret = loadFields(fields, fetch, lockLevel, sdata, 
>> >forWrite);
>> >+        // call back to FetchConfiguration to clean up any
>> >loadFetchGroup.
>> >+        // The loadFetchGroup was set by the
>> >FetchConfigurationImpl.includes()
>> >+        // during the process of the getUnloadedInternal method.
>> >+        fetch.removeLoadFetchGroup();
>> >         obtainLocks(active, forWrite, lockLevel, fetch, sdata);
>> >         return ret;
>> >     }
>> >Index:
>> 
>>openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.ja
>> >va 
>> >===================================================================
>> >---
>> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
>> >ata.java       (revision 574693)
>> >+++
>> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
>> >ata.java       (working copy)
>> >@@ -710,6 +710,8 @@
>> >     /**
>> >      * The fetch group that is to be loaded when this receiver is 
>> >loaded, or
>> >      * null if none set.
>> >+     * The LoadFetchGroup does not relate to the FetchGroup
>> >annotation. Therefore
>> >+     * resets a fetch group will not remove this LoadFetchGroup.
>> >      */
>> >     public void setLoadFetchGroup (String lfg) {
>> >         if ("".equals(lfg))
>> >
>> >
>> >> LoadFetchGroup annotation was not recognized during the fetch1
>> >> --------------------------------------------------------------
>> >>
>> >>                 Key: OPENJPA-370
>> >>                 URL:
>> >https://issues.apache.org/jira/browse/OPENJPA-370
>> >>             Project: OpenJPA
>> >>          Issue Type: Bug
>> >>          Components: kernel
>> >>    Affects Versions: 1.1.0
>> >>            Reporter: Teresa Kan
>> >>         Attachments: TestFetchGroup.zip
>> >>
>> >>
>> >> Employee class has a LoadFetchGroup annotation defined on
>> >the Rating field, when getRating was called, the address should be 
>> >returned also. However, openjpa did not handle the LoadFetchGroup 
>> >correctly, therefore, address was not eargly fetched.
>> >> public class FGEmployee{
>> >>     @Id
>> >>     private int id;
>> >>
>> >>     @OneToOne(fetch=FetchType.LAZY)
>> >>     private FGAddress address;
>> >>
>> >>     @Basic(fetch=FetchType.LAZY)
>> >>     @LoadFetchGroup("AddressFetchGroup")
>> >>     private String rating;
>> >>
>> >>     @ManyToOne(fetch=FetchType.LAZY)
>> >>     private FGManager manager;
>> >> ..
>> >> }
>> >
>> >--
>> >This message is automatically generated by JIRA.
>> >-
>> >You can reply to this email to add a comment to the issue online.
>> >
>> >
>>
>> 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
>

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: [jira] Updated: (OPENJPA-370) LoadFetchGroup annotation was not recognized during the fetch1

Posted by Patrick Linskey <pl...@gmail.com>.
> Also if a field f is included in current FectConfiguration can then be
> decided uniformly in one place i.e.
> FecthConfigurationImpl.includes(FieldMetaData fmd);

Generally-speaking, that sounds like a good goal. I'm not sure if
there is a reason why the fetch group handling is done differently
during the StateManagerImpl.loadField() call, but all things being
equal, it would be good to reduce the different ways that fetch groups
are configured where possible.

-Patrick

On 9/17/07, Pinaki Poddar <pp...@bea.com> wrote:
>
> Hi,
>   1. Within the load sequence, the LoadFetchGroup is handled in
> org.apache.openjpa.kernel.StateManagerImpl
>
>     protected void loadField(int, int, boolean, boolean);
>
>    This method checks for any associated LoadFetchGroup L for a field f,
> adds L to the current FetchConfiguration temporarily and removes L after
> load. This codepath is not getting executed withh your test case -- I
> have not debugged it to find out why.
>
>   2. This method follows a similar approach as in your approach i.e. add
> L temporarily and then remove it.
>
>   3. The other line of attck will be to capture enough information
> during annotation metadata parsing so that
> FecthConfigurationImpl method
>         private boolean includes(FieldMetaData fmd);
> can affirm if fmd is included in the currrent FetchConfiguration if not
> directly (as it does now)
> then indirectly via LoadFetchGroup.
> In this way, we can get rid of both temporarily-add-and-then-remove and
> check-during-StateManagerImpl.loadField() approach.
> Also if a field f is included in current FectConfiguration can then be
> decided uniformly in one place i.e.
> FecthConfigurationImpl.includes(FieldMetaData fmd);
>
>
>
>
> Pinaki Poddar
> 972.834.2865
>
>
> >-----Original Message-----
> >From: Teresa Kan (JIRA) [mailto:jira@apache.org]
> >Sent: Friday, September 14, 2007 2:19 PM
> >To: dev@openjpa.apache.org
> >Subject: [jira] Updated: (OPENJPA-370) LoadFetchGroup
> >annotation was not recognized during the fetch1
> >
> >
> >     [
> >https://issues.apache.org/jira/browse/OPENJPA-370?page=com.atla
> >ssian.jira.plugin.system.issuetabpanels:all-tabpanel ]
> >
> >Teresa Kan updated OPENJPA-370:
> >-------------------------------
> >
> >    Attachment: TestFetchGroup.zip
> >
> >Attach the patch. See the following changes:
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >ngFetchConfiguration.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >ngFetchConfiguration.java      (revision 574344)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >ngFetchConfiguration.java      (working copy)
> >@@ -287,6 +287,9 @@
> >         }
> >     }
> >
> >+    public void removeLoadFetchGroup(){
> >+
> >+    }
> >     public Set getFields() {
> >         try {
> >             return _fetch.getFields();
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figuration.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figuration.java        (revision 574344)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figuration.java        (working copy)
> >@@ -192,7 +192,10 @@
> >     public FetchConfiguration clearFetchGroups();
> >
> >     /**
> >-     * Resets the set of fetch groups to the list in the
> >global configuration.
> >+     * Resets the set of fetch groups to the list in the
> >global configuration.
> >+     * The global configuration will be the default plus any
> >fetch groups
> >+     * that are added by the openjpa.FetchGroups property in
> >the persistence.xml
> >+     * file.
> >      */
> >     public FetchConfiguration resetFetchGroups();
> >
> >@@ -197,6 +200,11 @@
> >     public FetchConfiguration resetFetchGroups();
> >
> >     /**
> >+     * Remove the loadFetchGroup if there is any loadFetchGroup.
> >+     */
> >+    public void removeLoadFetchGroup();
> >+
> >+    /**
> >      * Returns the set of fully-qualified field names that
> >this component
> >      * will use when loading objects. Defaults to the empty
> >set.  This set is
> >      * not thread safe.
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figurationImpl.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figurationImpl.java    (revision 575491)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figurationImpl.java    (working copy)
> >@@ -92,6 +92,7 @@
> >     private boolean _load = true;
> >     private int _availableRecursion;
> >     private int _availableDepth;
> >+    private String _lfg = null;
> >
> >     public FetchConfigurationImpl() {
> >         this(null);
> >@@ -287,7 +288,14 @@
> >                 getFetchGroupsList()));
> >         return this;
> >     }
> >+
> >+    public void removeLoadFetchGroup() {
> >+        if (_lfg != null)
> >+            removeFetchGroup(_lfg);
> >+        _lfg = null;
> >+    }
> >
> >+
> >     public Set getFields() {
> >         return (_state.fields == null) ?
> >Collections.EMPTY_SET : _state.fields;
> >     }
> >@@ -568,8 +576,16 @@
> >             return true;
> >         String[] fgs = fmd.getCustomFetchGroups();
> >         for (int i = 0; i < fgs.length; i++)
> >-            if (hasFetchGroup(fgs[i]))
> >+            if (hasFetchGroup(fgs[i])) {
> >+                String fg = fmd.getLoadFetchGroup();
> >+                if (fg != null) {
> >+                    if (!hasFetchGroup(fg)) {
> >+                        addFetchGroup(fg);
> >+                        _lfg = fg;
> >+                    }
> >+                }
> >                 return true;
> >+            }
> >         return false;
> >     }
> >
> >@@ -576,7 +592,7 @@
> >     /**
> >      * Return the available recursion depth via the given
> >field for the
> >      * given type.
> >-     *
> >+     *
> >      * @param traverse whether we're traversing the field
> >      */
> >     private int getAvailableRecursionDepth(FieldMetaData fm,
> >Class type,
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >agerImpl.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >agerImpl.java  (revision 575494)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >agerImpl.java  (working copy)
> >@@ -358,6 +358,10 @@
> >         // is loaded, etc
> >         int lockLevel = calculateLockLevel(active, forWrite, fetch);
> >         boolean ret = loadFields(fields, fetch, lockLevel,
> >sdata, forWrite);
> >+        // call back to FetchConfiguration to clean up any
> >loadFetchGroup.
> >+        // The loadFetchGroup was set by the
> >FetchConfigurationImpl.includes()
> >+        // during the process of the getUnloadedInternal method.
> >+        fetch.removeLoadFetchGroup();
> >         obtainLocks(active, forWrite, lockLevel, fetch, sdata);
> >         return ret;
> >     }
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
> >ata.java       (revision 574693)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
> >ata.java       (working copy)
> >@@ -710,6 +710,8 @@
> >     /**
> >      * The fetch group that is to be loaded when this
> >receiver is loaded, or
> >      * null if none set.
> >+     * The LoadFetchGroup does not relate to the FetchGroup
> >annotation. Therefore
> >+     * resets a fetch group will not remove this LoadFetchGroup.
> >      */
> >     public void setLoadFetchGroup (String lfg) {
> >         if ("".equals(lfg))
> >
> >
> >> LoadFetchGroup annotation was not recognized during the fetch1
> >> --------------------------------------------------------------
> >>
> >>                 Key: OPENJPA-370
> >>                 URL:
> >https://issues.apache.org/jira/browse/OPENJPA-370
> >>             Project: OpenJPA
> >>          Issue Type: Bug
> >>          Components: kernel
> >>    Affects Versions: 1.1.0
> >>            Reporter: Teresa Kan
> >>         Attachments: TestFetchGroup.zip
> >>
> >>
> >> Employee class has a LoadFetchGroup annotation defined on
> >the Rating field, when getRating was called, the address
> >should be returned also. However, openjpa did not handle the
> >LoadFetchGroup correctly, therefore, address was not eargly fetched.
> >> public class FGEmployee{
> >>     @Id
> >>     private int id;
> >>
> >>     @OneToOne(fetch=FetchType.LAZY)
> >>     private FGAddress address;
> >>
> >>     @Basic(fetch=FetchType.LAZY)
> >>     @LoadFetchGroup("AddressFetchGroup")
> >>     private String rating;
> >>
> >>     @ManyToOne(fetch=FetchType.LAZY)
> >>     private FGManager manager;
> >> ..
> >> }
> >
> >--
> >This message is automatically generated by JIRA.
> >-
> >You can reply to this email to add a comment to the issue online.
> >
> >
>
> 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: [jira] Updated: (OPENJPA-370) LoadFetchGroup annotation was not recognized during the fetch1

Posted by Teresa Kan <tc...@gmail.com>.
Can anyone help me understand the following methods in the StateManagerImpl?
loadFields and loadField?
I thought that the loadFields will load all necessary fields. It should call
loadField to load individual field. However, it did not call loadField
method. Instead, the loadField() calls the loadFields method..
Should we make the loadFields to call the loadField to load individual field
then all the loadFetchGroup will be covered under the loadField call.. But I
like to understand why it did not do it at the first place?

Thanks,
Teresa


On 9/17/07, Teresa Kan <tc...@gmail.com> wrote:
>
> Pinaki,
> Yes, I realized that the loadField in the StateManagerImpl has all I
> needed. But I found out that this method was never called in my test.. So I
> simulated the logic in my patch.. I also looked at the includes method in
> the FetchConfigurationImpl too.. I will look at it again to see the reason
> why I can't put the code in one place..
> Teresa
>
>
> On 9/17/07, Pinaki Poddar <pp...@bea.com> wrote:
> >
> >
> > Hi,
> > 1. Within the load sequence, the LoadFetchGroup is handled in
> > org.apache.openjpa.kernel.StateManagerImpl
> >
> >    protected void loadField(int, int, boolean, boolean);
> >
> >   This method checks for any associated LoadFetchGroup L for a field f,
> > adds L to the current FetchConfiguration temporarily and removes L after
> > load. This codepath is not getting executed withh your test case -- I
> > have not debugged it to find out why.
> >
> > 2. This method follows a similar approach as in your approach i.e. add
> > L temporarily and then remove it.
> >
> > 3. The other line of attck will be to capture enough information
> > during annotation metadata parsing so that
> > FecthConfigurationImpl method
> >        private boolean includes(FieldMetaData fmd);
> > can affirm if fmd is included in the currrent FetchConfiguration if not
> > directly (as it does now)
> > then indirectly via LoadFetchGroup.
> > In this way, we can get rid of both temporarily-add-and-then-remove and
> > check-during-StateManagerImpl.loadField() approach.
> > Also if a field f is included in current FectConfiguration can then be
> > decided uniformly in one place i.e.
> > FecthConfigurationImpl.includes(FieldMetaData fmd);
> >
> >
> >
> >
> > Pinaki Poddar
> > 972.834.2865
> >
> >
> > >-----Original Message-----
> > >From: Teresa Kan (JIRA) [mailto: jira@apache.org]
> > >Sent: Friday, September 14, 2007 2:19 PM
> > >To: dev@openjpa.apache.org
> > >Subject: [jira] Updated: (OPENJPA-370) LoadFetchGroup
> > >annotation was not recognized during the fetch1
> > >
> > >
> > >     [
> > >https://issues.apache.org/jira/browse/OPENJPA-370?page=com.atla
> > >ssian.jira.plugin.system.issuetabpanels:all-tabpanel ]
> > >
> > >Teresa Kan updated OPENJPA-370:
> > >-------------------------------
> > >
> > >    Attachment: TestFetchGroup.zip
> > >
> > >Attach the patch. See the following changes:
> > >Index:
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> > >ngFetchConfiguration.java
> > >===================================================================
> > >---
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> > >ngFetchConfiguration.java      (revision 574344)
> > >+++
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> > >ngFetchConfiguration.java      (working copy)
> > >@@ -287,6 +287,9 @@
> > >         }
> > >     }
> > >
> > >+    public void removeLoadFetchGroup(){
> > >+
> > >+    }
> > >     public Set getFields() {
> > >         try {
> > >             return _fetch.getFields();
> > >Index:
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> > >figuration.java
> > >===================================================================
> > >---
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> > >figuration.java        (revision 574344)
> > >+++
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> > >figuration.java        (working copy)
> > >@@ -192,7 +192,10 @@
> > >     public FetchConfiguration clearFetchGroups();
> > >
> > >     /**
> > >-     * Resets the set of fetch groups to the list in the
> > >global configuration.
> > >+     * Resets the set of fetch groups to the list in the
> > >global configuration.
> > >+     * The global configuration will be the default plus any
> > >fetch groups
> > >+     * that are added by the openjpa.FetchGroups property in
> > >the persistence.xml
> > >+     * file.
> > >      */
> > >     public FetchConfiguration resetFetchGroups();
> > >
> > >@@ -197,6 +200,11 @@
> > >     public FetchConfiguration resetFetchGroups();
> > >
> > >     /**
> > >+     * Remove the loadFetchGroup if there is any loadFetchGroup.
> > >+     */
> > >+    public void removeLoadFetchGroup();
> > >+
> > >+    /**
> > >      * Returns the set of fully-qualified field names that
> > >this component
> > >      * will use when loading objects. Defaults to the empty
> > >set.  This set is
> > >      * not thread safe.
> > >Index:
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> > >figurationImpl.java
> > >===================================================================
> > >---
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> > >figurationImpl.java    (revision 575491)
> > >+++
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> > >figurationImpl.java    (working copy)
> > >@@ -92,6 +92,7 @@
> > >     private boolean _load = true;
> > >     private int _availableRecursion;
> > >     private int _availableDepth;
> > >+    private String _lfg = null;
> > >
> > >     public FetchConfigurationImpl() {
> > >         this(null);
> > >@@ -287,7 +288,14 @@
> > >                 getFetchGroupsList()));
> > >         return this;
> > >     }
> > >+
> > >+    public void removeLoadFetchGroup() {
> > >+        if (_lfg != null)
> > >+            removeFetchGroup(_lfg);
> > >+        _lfg = null;
> > >+    }
> > >
> > >+
> > >     public Set getFields() {
> > >         return (_state.fields == null) ?
> > >Collections.EMPTY_SET : _state.fields;
> > >     }
> > >@@ -568,8 +576,16 @@
> > >             return true;
> > >         String[] fgs = fmd.getCustomFetchGroups();
> > >         for (int i = 0; i < fgs.length; i++)
> > >-            if (hasFetchGroup(fgs[i]))
> > >+            if (hasFetchGroup(fgs[i])) {
> > >+                String fg = fmd.getLoadFetchGroup();
> > >+                if (fg != null) {
> > >+                    if (!hasFetchGroup(fg)) {
> > >+                        addFetchGroup(fg);
> > >+                        _lfg = fg;
> > >+                    }
> > >+                }
> > >                 return true;
> > >+            }
> > >         return false;
> > >     }
> > >
> > >@@ -576,7 +592,7 @@
> > >     /**
> > >      * Return the available recursion depth via the given
> > >field for the
> > >      * given type.
> > >-     *
> > >+     *
> > >      * @param traverse whether we're traversing the field
> > >      */
> > >     private int getAvailableRecursionDepth(FieldMetaData fm,
> > >Class type,
> > >Index:
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> > >agerImpl.java
> > >===================================================================
> > >---
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> > >agerImpl.java  (revision 575494)
> > >+++
> > >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> > >agerImpl.java  (working copy)
> > >@@ -358,6 +358,10 @@
> > >         // is loaded, etc
> > >         int lockLevel = calculateLockLevel(active, forWrite, fetch);
> > >         boolean ret = loadFields(fields, fetch, lockLevel,
> > >sdata, forWrite);
> > >+        // call back to FetchConfiguration to clean up any
> > >loadFetchGroup.
> > >+        // The loadFetchGroup was set by the
> > >FetchConfigurationImpl.includes()
> > >+        // during the process of the getUnloadedInternal method.
> > >+        fetch.removeLoadFetchGroup ();
> > >         obtainLocks(active, forWrite, lockLevel, fetch, sdata);
> > >         return ret;
> > >     }
> > >Index:
> > >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
> > >===================================================================
> > >---
> > >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
> > >ata.java       (revision 574693)
> > >+++
> > >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
> > >ata.java       (working copy)
> > >@@ -710,6 +710,8 @@
> > >     /**
> > >      * The fetch group that is to be loaded when this
> > >receiver is loaded, or
> > >      * null if none set.
> > >+     * The LoadFetchGroup does not relate to the FetchGroup
> > >annotation. Therefore
> > >+     * resets a fetch group will not remove this LoadFetchGroup.
> > >      */
> > >     public void setLoadFetchGroup (String lfg) {
> > >         if ("".equals(lfg))
> > >
> > >
> > >> LoadFetchGroup annotation was not recognized during the fetch1
> > >> --------------------------------------------------------------
> > >>
> > >>                 Key: OPENJPA-370
> > >>                 URL:
> > >https://issues.apache.org/jira/browse/OPENJPA-370
> > >>             Project: OpenJPA
> > >>          Issue Type: Bug
> > >>          Components: kernel
> > >>    Affects Versions: 1.1.0
> > >>            Reporter: Teresa Kan
> > >>         Attachments: TestFetchGroup.zip
> > >>
> > >>
> > >> Employee class has a LoadFetchGroup annotation defined on
> > >the Rating field, when getRating was called, the address
> > >should be returned also. However, openjpa did not handle the
> > >LoadFetchGroup correctly, therefore, address was not eargly fetched.
> > >> public class FGEmployee{
> > >>     @Id
> > >>     private int id;
> > >>
> > >>     @OneToOne(fetch=FetchType.LAZY)
> > >>     private FGAddress address;
> > >>
> > >>     @Basic(fetch=FetchType.LAZY)
> > >>     @LoadFetchGroup("AddressFetchGroup")
> > >>     private String rating;
> > >>
> > >>     @ManyToOne(fetch=FetchType.LAZY)
> > >>     private FGManager manager;
> > >> ..
> > >> }
> > >
> > >--
> > >This message is automatically generated by JIRA.
> > >-
> > >You can reply to this email to add a comment to the issue online.
> > >
> > >
> >
> > 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: [jira] Updated: (OPENJPA-370) LoadFetchGroup annotation was not recognized during the fetch1

Posted by Teresa Kan <tc...@gmail.com>.
Pinaki,
Yes, I realized that the loadField in the StateManagerImpl has all I needed.
But I found out that this method was never called in my test.. So I
simulated the logic in my patch.. I also looked at the includes method in
the FetchConfigurationImpl too.. I will look at it again to see the reason
why I can't put the code in one place..
Teresa


On 9/17/07, Pinaki Poddar <pp...@bea.com> wrote:
>
>
> Hi,
> 1. Within the load sequence, the LoadFetchGroup is handled in
> org.apache.openjpa.kernel.StateManagerImpl
>
>    protected void loadField(int, int, boolean, boolean);
>
>   This method checks for any associated LoadFetchGroup L for a field f,
> adds L to the current FetchConfiguration temporarily and removes L after
> load. This codepath is not getting executed withh your test case -- I
> have not debugged it to find out why.
>
> 2. This method follows a similar approach as in your approach i.e. add
> L temporarily and then remove it.
>
> 3. The other line of attck will be to capture enough information
> during annotation metadata parsing so that
> FecthConfigurationImpl method
>        private boolean includes(FieldMetaData fmd);
> can affirm if fmd is included in the currrent FetchConfiguration if not
> directly (as it does now)
> then indirectly via LoadFetchGroup.
> In this way, we can get rid of both temporarily-add-and-then-remove and
> check-during-StateManagerImpl.loadField() approach.
> Also if a field f is included in current FectConfiguration can then be
> decided uniformly in one place i.e.
> FecthConfigurationImpl.includes(FieldMetaData fmd);
>
>
>
>
> Pinaki Poddar
> 972.834.2865
>
>
> >-----Original Message-----
> >From: Teresa Kan (JIRA) [mailto:jira@apache.org]
> >Sent: Friday, September 14, 2007 2:19 PM
> >To: dev@openjpa.apache.org
> >Subject: [jira] Updated: (OPENJPA-370) LoadFetchGroup
> >annotation was not recognized during the fetch1
> >
> >
> >     [
> >https://issues.apache.org/jira/browse/OPENJPA-370?page=com.atla
> >ssian.jira.plugin.system.issuetabpanels:all-tabpanel ]
> >
> >Teresa Kan updated OPENJPA-370:
> >-------------------------------
> >
> >    Attachment: TestFetchGroup.zip
> >
> >Attach the patch. See the following changes:
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >ngFetchConfiguration.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >ngFetchConfiguration.java      (revision 574344)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
> >ngFetchConfiguration.java      (working copy)
> >@@ -287,6 +287,9 @@
> >         }
> >     }
> >
> >+    public void removeLoadFetchGroup(){
> >+
> >+    }
> >     public Set getFields() {
> >         try {
> >             return _fetch.getFields();
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figuration.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figuration.java        (revision 574344)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figuration.java        (working copy)
> >@@ -192,7 +192,10 @@
> >     public FetchConfiguration clearFetchGroups();
> >
> >     /**
> >-     * Resets the set of fetch groups to the list in the
> >global configuration.
> >+     * Resets the set of fetch groups to the list in the
> >global configuration.
> >+     * The global configuration will be the default plus any
> >fetch groups
> >+     * that are added by the openjpa.FetchGroups property in
> >the persistence.xml
> >+     * file.
> >      */
> >     public FetchConfiguration resetFetchGroups();
> >
> >@@ -197,6 +200,11 @@
> >     public FetchConfiguration resetFetchGroups();
> >
> >     /**
> >+     * Remove the loadFetchGroup if there is any loadFetchGroup.
> >+     */
> >+    public void removeLoadFetchGroup();
> >+
> >+    /**
> >      * Returns the set of fully-qualified field names that
> >this component
> >      * will use when loading objects. Defaults to the empty
> >set.  This set is
> >      * not thread safe.
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figurationImpl.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figurationImpl.java    (revision 575491)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
> >figurationImpl.java    (working copy)
> >@@ -92,6 +92,7 @@
> >     private boolean _load = true;
> >     private int _availableRecursion;
> >     private int _availableDepth;
> >+    private String _lfg = null;
> >
> >     public FetchConfigurationImpl() {
> >         this(null);
> >@@ -287,7 +288,14 @@
> >                 getFetchGroupsList()));
> >         return this;
> >     }
> >+
> >+    public void removeLoadFetchGroup() {
> >+        if (_lfg != null)
> >+            removeFetchGroup(_lfg);
> >+        _lfg = null;
> >+    }
> >
> >+
> >     public Set getFields() {
> >         return (_state.fields == null) ?
> >Collections.EMPTY_SET : _state.fields;
> >     }
> >@@ -568,8 +576,16 @@
> >             return true;
> >         String[] fgs = fmd.getCustomFetchGroups();
> >         for (int i = 0; i < fgs.length; i++)
> >-            if (hasFetchGroup(fgs[i]))
> >+            if (hasFetchGroup(fgs[i])) {
> >+                String fg = fmd.getLoadFetchGroup();
> >+                if (fg != null) {
> >+                    if (!hasFetchGroup(fg)) {
> >+                        addFetchGroup(fg);
> >+                        _lfg = fg;
> >+                    }
> >+                }
> >                 return true;
> >+            }
> >         return false;
> >     }
> >
> >@@ -576,7 +592,7 @@
> >     /**
> >      * Return the available recursion depth via the given
> >field for the
> >      * given type.
> >-     *
> >+     *
> >      * @param traverse whether we're traversing the field
> >      */
> >     private int getAvailableRecursionDepth(FieldMetaData fm,
> >Class type,
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >agerImpl.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >agerImpl.java  (revision 575494)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
> >agerImpl.java  (working copy)
> >@@ -358,6 +358,10 @@
> >         // is loaded, etc
> >         int lockLevel = calculateLockLevel(active, forWrite, fetch);
> >         boolean ret = loadFields(fields, fetch, lockLevel,
> >sdata, forWrite);
> >+        // call back to FetchConfiguration to clean up any
> >loadFetchGroup.
> >+        // The loadFetchGroup was set by the
> >FetchConfigurationImpl.includes()
> >+        // during the process of the getUnloadedInternal method.
> >+        fetch.removeLoadFetchGroup();
> >         obtainLocks(active, forWrite, lockLevel, fetch, sdata);
> >         return ret;
> >     }
> >Index:
> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
> >===================================================================
> >---
> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
> >ata.java       (revision 574693)
> >+++
> >openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
> >ata.java       (working copy)
> >@@ -710,6 +710,8 @@
> >     /**
> >      * The fetch group that is to be loaded when this
> >receiver is loaded, or
> >      * null if none set.
> >+     * The LoadFetchGroup does not relate to the FetchGroup
> >annotation. Therefore
> >+     * resets a fetch group will not remove this LoadFetchGroup.
> >      */
> >     public void setLoadFetchGroup (String lfg) {
> >         if ("".equals(lfg))
> >
> >
> >> LoadFetchGroup annotation was not recognized during the fetch1
> >> --------------------------------------------------------------
> >>
> >>                 Key: OPENJPA-370
> >>                 URL:
> >https://issues.apache.org/jira/browse/OPENJPA-370
> >>             Project: OpenJPA
> >>          Issue Type: Bug
> >>          Components: kernel
> >>    Affects Versions: 1.1.0
> >>            Reporter: Teresa Kan
> >>         Attachments: TestFetchGroup.zip
> >>
> >>
> >> Employee class has a LoadFetchGroup annotation defined on
> >the Rating field, when getRating was called, the address
> >should be returned also. However, openjpa did not handle the
> >LoadFetchGroup correctly, therefore, address was not eargly fetched.
> >> public class FGEmployee{
> >>     @Id
> >>     private int id;
> >>
> >>     @OneToOne(fetch=FetchType.LAZY)
> >>     private FGAddress address;
> >>
> >>     @Basic(fetch=FetchType.LAZY)
> >>     @LoadFetchGroup("AddressFetchGroup")
> >>     private String rating;
> >>
> >>     @ManyToOne(fetch=FetchType.LAZY)
> >>     private FGManager manager;
> >> ..
> >> }
> >
> >--
> >This message is automatically generated by JIRA.
> >-
> >You can reply to this email to add a comment to the issue online.
> >
> >
>
> 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: [jira] Updated: (OPENJPA-370) LoadFetchGroup annotation was not recognized during the fetch1

Posted by Pinaki Poddar <pp...@bea.com>.
Hi,
  1. Within the load sequence, the LoadFetchGroup is handled in
org.apache.openjpa.kernel.StateManagerImpl

    protected void loadField(int, int, boolean, boolean);

   This method checks for any associated LoadFetchGroup L for a field f,
adds L to the current FetchConfiguration temporarily and removes L after
load. This codepath is not getting executed withh your test case -- I
have not debugged it to find out why.

  2. This method follows a similar approach as in your approach i.e. add
L temporarily and then remove it. 

  3. The other line of attck will be to capture enough information
during annotation metadata parsing so that 
FecthConfigurationImpl method
	private boolean includes(FieldMetaData fmd);
can affirm if fmd is included in the currrent FetchConfiguration if not
directly (as it does now) 
then indirectly via LoadFetchGroup. 
In this way, we can get rid of both temporarily-add-and-then-remove and
check-during-StateManagerImpl.loadField() approach.
Also if a field f is included in current FectConfiguration can then be
decided uniformly in one place i.e.
FecthConfigurationImpl.includes(FieldMetaData fmd);

 
  

Pinaki Poddar
972.834.2865
 

>-----Original Message-----
>From: Teresa Kan (JIRA) [mailto:jira@apache.org] 
>Sent: Friday, September 14, 2007 2:19 PM
>To: dev@openjpa.apache.org
>Subject: [jira] Updated: (OPENJPA-370) LoadFetchGroup 
>annotation was not recognized during the fetch1
>
>
>     [ 
>https://issues.apache.org/jira/browse/OPENJPA-370?page=com.atla
>ssian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>
>Teresa Kan updated OPENJPA-370:
>-------------------------------
>
>    Attachment: TestFetchGroup.zip
>
>Attach the patch. See the following changes:
>Index: 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>ngFetchConfiguration.java
>===================================================================
>--- 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>ngFetchConfiguration.java	(revision 574344)
>+++ 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/Delegati
>ngFetchConfiguration.java	(working copy)
>@@ -287,6 +287,9 @@
>         }
>     }
> 
>+    public void removeLoadFetchGroup(){
>+        
>+    }
>     public Set getFields() {
>         try {
>             return _fetch.getFields();
>Index: 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>figuration.java
>===================================================================
>--- 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>figuration.java	(revision 574344)
>+++ 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>figuration.java	(working copy)
>@@ -192,7 +192,10 @@
>     public FetchConfiguration clearFetchGroups();
> 
>     /**
>-     * Resets the set of fetch groups to the list in the 
>global configuration.
>+     * Resets the set of fetch groups to the list in the 
>global configuration. 
>+     * The global configuration will be the default plus any 
>fetch groups 
>+     * that are added by the openjpa.FetchGroups property in 
>the persistence.xml
>+     * file.
>      */
>     public FetchConfiguration resetFetchGroups();
> 
>@@ -197,6 +200,11 @@
>     public FetchConfiguration resetFetchGroups();
> 
>     /**
>+     * Remove the loadFetchGroup if there is any loadFetchGroup.
>+     */   
>+    public void removeLoadFetchGroup();
>+    
>+    /**
>      * Returns the set of fully-qualified field names that 
>this component
>      * will use when loading objects. Defaults to the empty 
>set.  This set is
>      * not thread safe.
>Index: 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>figurationImpl.java
>===================================================================
>--- 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>figurationImpl.java	(revision 575491)
>+++ 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/FetchCon
>figurationImpl.java	(working copy)
>@@ -92,6 +92,7 @@
>     private boolean _load = true;
>     private int _availableRecursion;
>     private int _availableDepth;
>+    private String _lfg = null;
> 
>     public FetchConfigurationImpl() {
>         this(null);
>@@ -287,7 +288,14 @@
>                 getFetchGroupsList()));
>         return this;
>     }
>+      
>+    public void removeLoadFetchGroup() {
>+        if (_lfg != null)
>+            removeFetchGroup(_lfg);
>+        _lfg = null;
>+    }
> 
>+
>     public Set getFields() {
>         return (_state.fields == null) ? 
>Collections.EMPTY_SET : _state.fields;
>     }
>@@ -568,8 +576,16 @@
>             return true;
>         String[] fgs = fmd.getCustomFetchGroups();
>         for (int i = 0; i < fgs.length; i++)
>-            if (hasFetchGroup(fgs[i]))
>+            if (hasFetchGroup(fgs[i])) {
>+                String fg = fmd.getLoadFetchGroup();
>+                if (fg != null) {
>+                    if (!hasFetchGroup(fg)) {
>+                        addFetchGroup(fg);
>+                        _lfg = fg;
>+                    }
>+                }
>                 return true;
>+            }
>         return false; 
>     }
> 
>@@ -576,7 +592,7 @@
>     /**
>      * Return the available recursion depth via the given 
>field for the
>      * given type.
>-     *
>+     *
>      * @param traverse whether we're traversing the field
>      */
>     private int getAvailableRecursionDepth(FieldMetaData fm, 
>Class type,
>Index: 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>agerImpl.java
>===================================================================
>--- 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>agerImpl.java	(revision 575494)
>+++ 
>openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StateMan
>agerImpl.java	(working copy)
>@@ -358,6 +358,10 @@
>         // is loaded, etc
>         int lockLevel = calculateLockLevel(active, forWrite, fetch);
>         boolean ret = loadFields(fields, fetch, lockLevel, 
>sdata, forWrite);
>+        // call back to FetchConfiguration to clean up any 
>loadFetchGroup.
>+        // The loadFetchGroup was set by the 
>FetchConfigurationImpl.includes()
>+        // during the process of the getUnloadedInternal method. 
>+        fetch.removeLoadFetchGroup();
>         obtainLocks(active, forWrite, lockLevel, fetch, sdata);
>         return ret;
>     }
>Index: 
>openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaData.java
>===================================================================
>--- 
>openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
>ata.java	(revision 574693)
>+++ 
>openjpa-kernel/src/main/java/org/apache/openjpa/meta/FieldMetaD
>ata.java	(working copy)
>@@ -710,6 +710,8 @@
>     /**
>      * The fetch group that is to be loaded when this 
>receiver is loaded, or
>      * null if none set.
>+     * The LoadFetchGroup does not relate to the FetchGroup 
>annotation. Therefore
>+     * resets a fetch group will not remove this LoadFetchGroup.
>      */
>     public void setLoadFetchGroup (String lfg) {
>         if ("".equals(lfg))
>
>
>> LoadFetchGroup annotation was not recognized during the fetch1
>> --------------------------------------------------------------
>>
>>                 Key: OPENJPA-370
>>                 URL: 
>https://issues.apache.org/jira/browse/OPENJPA-370
>>             Project: OpenJPA
>>          Issue Type: Bug
>>          Components: kernel
>>    Affects Versions: 1.1.0
>>            Reporter: Teresa Kan
>>         Attachments: TestFetchGroup.zip
>>
>>
>> Employee class has a LoadFetchGroup annotation defined on 
>the Rating field, when getRating was called, the address 
>should be returned also. However, openjpa did not handle the 
>LoadFetchGroup correctly, therefore, address was not eargly fetched.
>> public class FGEmployee{
>>     @Id
>>     private int id;
>>  
>>     @OneToOne(fetch=FetchType.LAZY) 
>>     private FGAddress address;
>>  
>>     @Basic(fetch=FetchType.LAZY)
>>     @LoadFetchGroup("AddressFetchGroup")
>>     private String rating;
>>  
>>     @ManyToOne(fetch=FetchType.LAZY)
>>     private FGManager manager;
>> ..
>> }
>
>--
>This message is automatically generated by JIRA.
>-
>You can reply to this email to add a comment to the issue online.
>
>

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.