OPENJPA-297

git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@560304 13f79535-47bb-0310-9956-ffa450edef68

openjpa-kernel-5/src/main/java/org/apache/openjpa/enhance/ManagedClassSubclasser.java

@@ -35,6 +35,8 @@ import org.apache.openjpa.lib.log.Log;
 import org.apache.openjpa.util.GeneratedClasses;
 import org.apache.openjpa.util.InternalException;
 import org.apache.openjpa.meta.ClassMetaData;
+import org.apache.openjpa.meta.FieldMetaData;
+import org.apache.openjpa.meta.JavaTypes;
 import serp.bytecode.BCClass;
 
 /**
@@ -74,7 +76,8 @@ public class ManagedClassSubclasser {
             return Collections.EMPTY_LIST;
 
         Log log = conf.getLog(OpenJPAConfiguration.LOG_ENHANCE);
-        if (ClassRedefiner.canRedefineClasses())
+        boolean redefine = ClassRedefiner.canRedefineClasses();
+        if (redefine)
             log.info(_loc.get("enhance-and-subclass-no-redef-start",
                 classes));
         else
@@ -88,21 +91,24 @@ public class ManagedClassSubclasser {
             final Class cls = (Class) iter.next();
             final PCEnhancer enhancer = new PCEnhancer(conf, cls);
 
-            // set this before enhancement as well as after since enhancement
-            // uses a different metadata repository, and the value of this
-            // setting matters in the enhancement contract.
-            setDetachedState(enhancer.getMetaData());
-
             enhancer.setBytecodeWriter(new BytecodeWriter() {
                 public void write(BCClass bc) throws IOException {
                     ManagedClassSubclasser.write(bc, enhancer, map,
                         cls, subs, ints);
                 }
             });
-            if (ClassRedefiner.canRedefineClasses())
+            if (redefine)
                 enhancer.setRedefine(true);
             enhancer.setCreateSubclass(true);
             enhancer.setAddDefaultConstructor(true);
+
+            // set this before enhancement as well as after since enhancement
+            // uses a different metadata repository, and the metadata config
+            // matters in the enhancement contract. Don't do any warning here,
+            // since we'll issue warnings when we do the final metadata
+            // reconfiguration at the end of this method.
+            configureMetaData(enhancer.getMetaData(), conf, redefine, false);
+
             enhancer.run();
             try {
                 enhancer.record();
@@ -115,16 +121,54 @@ public class ManagedClassSubclasser {
         ClassRedefiner.redefineClasses(conf, map);
         for (Class cls : map.keySet()) {
             setIntercepting(conf, envLoader, cls);
-            configureMetaData(conf, envLoader, cls);
+            configureMetaData(conf, envLoader, cls, redefine);
         }
         for (Class cls : (Collection<Class>) subs)
-            configureMetaData(conf, envLoader, cls);
+            configureMetaData(conf, envLoader, cls, redefine);
         for (Class cls : (Collection<Class>) ints)
             setIntercepting(conf, envLoader, cls);
 
         return subs;
     }
 
+    private static void configureMetaData(OpenJPAConfiguration conf,
+        ClassLoader envLoader, Class cls, boolean redefineAvailable) {
+        ClassMetaData meta = conf.getMetaDataRepositoryInstance()
+            .getMetaData(cls, envLoader, true);
+        configureMetaData(meta, conf, redefineAvailable, true);
+    }
+
+    private static void configureMetaData(ClassMetaData meta,
+        OpenJPAConfiguration conf, boolean redefineAvailable, boolean warn) {
+
+        setDetachedState(meta);
+
+        if (warn && meta.getAccessType() == ClassMetaData.ACCESS_FIELD
+            && !redefineAvailable) {
+            // only warn about declared fields; superclass fields will be
+            // warned about when the superclass is handled
+            for (FieldMetaData fmd : meta.getDeclaredFields()) {
+                switch (fmd.getTypeCode()) {
+                    case JavaTypes.COLLECTION:
+                    case JavaTypes.MAP:
+                        // we can lazily load these, since we own the
+                        // relationship container
+                        break;
+                    default:
+                        if (!fmd.isInDefaultFetchGroup()
+                            && !(fmd.isVersion() || fmd.isPrimaryKey())) {
+                            Log log = conf.getLog(
+                                OpenJPAConfiguration.LOG_ENHANCE);
+                            log.warn(_loc.get("subclasser-fetch-group-override",
+                                meta.getDescribedType().getName(),
+                                fmd.getName()));
+                            fmd.setInDefaultFetchGroup(true);
+                        }
+                }
+            }
+        }
+    }
+
     private static void write(BCClass bc, PCEnhancer enhancer,
         Map<Class, byte[]> map, Class cls, List subs, List ints)
         throws IOException {
@@ -158,13 +202,6 @@ public class ManagedClassSubclasser {
         meta.setIntercepting(true);
     }
 
-    private static void configureMetaData(OpenJPAConfiguration conf,
-        ClassLoader envLoader, Class cls) {
-        ClassMetaData meta = conf.getMetaDataRepositoryInstance()
-            .getMetaData(cls, envLoader, true);
-        setDetachedState(meta);
-    }
-
     /**
      * If the metadata is configured to use a synthetic
      * detached state, reset it to not use a detached

openjpa-kernel/src/main/resources/org/apache/openjpa/enhance/localizer.properties

@@ -187,3 +187,7 @@ subclasser-native-methods-not-allowed: The method {1} in type {0} is native. \
     OpenJPA requires methods in unenhanced instances to be non-native.
 subclasser-static-methods-not-supported: The method {1} in type {0} is static. \
     OpenJPA requires methods in unenhanced instances to be non-static.
+subclasser-fetch-group-override: The field {1} in type {0} is configured to be \
+    lazily loaded, but lazy loading is not available for classes that use field\
+    access when not running the OpenJPA enhancer or when dynamic class \
+    redefinition is not available.

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/AbstractUnenhancedClassTest.java

@@ -39,7 +39,6 @@ public abstract class AbstractUnenhancedClassTest
 
     // ##### To do:
     // - clearing in pnew property-access without redefinition
-    // - lazy loading override for -to-one field types
     // - figure out how to auto-test the redefinition code, either in Java 5
     //   or in Java 6
     // - run CTS in the following combinations:
@@ -48,8 +47,7 @@ public abstract class AbstractUnenhancedClassTest
     //   * Java 5 without javaagent
 
     public void setUp() {
-        setUp(getUnenhancedClass(), getUnenhancedSubclass(), CLEAR_TABLES,
+        setUp(getUnenhancedClass(), getUnenhancedSubclass(), CLEAR_TABLES);
-            "openjpa.Log", "Enhance=TRACE");
         // trigger class redefinition
         emf.createEntityManager().close();
     }
@@ -237,17 +235,17 @@ public abstract class AbstractUnenhancedClassTest
             PCEnhancer.toPCSubclassName(getUnenhancedClass()));
     }
 
-    public void testLazyLoadingInUserCreatedInstance()
+    public void testEvictionInUserCreatedInstance()
         throws NoSuchFieldException, IllegalAccessException {
-        lazyLoading(true);
+        evictionHelper(true);
     }
 
-    public void testLazyLoadingInOpenJPACreatedInstance()
+    public void testEvictionInOpenJPACreatedInstance()
         throws NoSuchFieldException, IllegalAccessException {
-        lazyLoading(false);
+        evictionHelper(false);
     }
 
-    private void lazyLoading(boolean userDefined)
+    private void evictionHelper(boolean userDefined)
         throws NoSuchFieldException, IllegalAccessException {
         OpenJPAEntityManager em = emf.createEntityManager();
         UnenhancedType un = newUnenhancedInstance();
@@ -303,6 +301,52 @@ public abstract class AbstractUnenhancedClassTest
         em.close();
     }
 
+    public void testLazyLoading()
+        throws NoSuchFieldException, IllegalAccessException {
+        OpenJPAEntityManager em = emf.createEntityManager();
+        UnenhancedType un = newUnenhancedInstance();
+        em.getTransaction().begin();
+        em.persist(un);
+        em.getTransaction().commit();
+        em.close();
+
+        em = emf.createEntityManager();
+        un = em.find(getUnenhancedClass(), un.getId());
+        assertTrue(getUnenhancedClass() != un.getClass());
+        OpenJPAStateManager sm = (OpenJPAStateManager)
+            ImplHelper.toPersistenceCapable(un, null).pcGetStateManager();
+
+        // we only expect lazy loading to work when we can redefine classes
+        // or when accessing a property-access record that OpenJPA created.
+        if (ClassRedefiner.canRedefineClasses()
+            || (sm.getMetaData().getAccessType() != ClassMetaData.ACCESS_FIELD))
+        {
+            assertFalse(sm.getLoaded()
+                .get(sm.getMetaData().getField("lazyField").getIndex()));
+
+            // make sure that the value was cleared
+            Field field = getUnenhancedClass().getDeclaredField("lazyField");
+            field.setAccessible(true);
+            assertEquals(null, field.get(un));
+        } else {
+            // unredefined field access
+            assertTrue(sm.getLoaded()
+                .get(sm.getMetaData().getField("lazyField").getIndex()));
+
+            // make sure that the value was loaded already
+            Field field = getUnenhancedClass().getDeclaredField("lazyField");
+            field.setAccessible(true);
+            assertEquals("lazy", field.get(un));
+        }
+
+        // make sure that the value is available, one way or another
+        assertEquals("lazy", un.getLazyField());
+        assertTrue(sm.getLoaded()
+            .get(sm.getMetaData().getField("lazyField").getIndex()));
+
+        em.close();
+    }
+
     public void testSerializationOfUserDefinedInstance()
         throws IOException, ClassNotFoundException {
         serializationHelper(true, false);

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedFieldAccess.java

@@ -26,6 +26,8 @@ import javax.persistence.GeneratedValue;
 import javax.persistence.Inheritance;
 import javax.persistence.InheritanceType;
 import javax.persistence.Table;
+import javax.persistence.Basic;
+import javax.persistence.FetchType;
 
 import org.apache.openjpa.persistence.DetachedState;
 
@@ -39,6 +41,9 @@ public class UnenhancedFieldAccess
     @Version public int version;
     protected String stringField = "foo";
 
+    @Basic(fetch = FetchType.LAZY)
+    private String lazyField = "lazy";
+
     public int getId() {
         return id;
     }
@@ -51,6 +56,10 @@ public class UnenhancedFieldAccess
         return stringField;
     }
 
+    public String getLazyField() {
+        return lazyField;
+    }
+
     public boolean equals(Object o) {
         if (o == this)
             return true;

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedPropertyAccess.java

@@ -27,8 +27,7 @@ import javax.persistence.Basic;
 import javax.persistence.Inheritance;
 import javax.persistence.InheritanceType;
 import javax.persistence.Table;
-
+import javax.persistence.FetchType;
-import org.apache.openjpa.persistence.DetachedState;
 
 @Entity
 @Table(name="UN_PROP")
@@ -39,6 +38,7 @@ public class UnenhancedPropertyAccess
     private int id;
     private int version;
     private String sf = "foo";
+    private String lazyField = "lazy";
 
     @Id @GeneratedValue
     public int getId() {
@@ -58,13 +58,22 @@ public class UnenhancedPropertyAccess
         version = v;
     }
 
+    @Basic
+    public String getStringField() {
+        return sf;
+    }
+
     public void setStringField(String s) {
         sf = s;
     }
 
-    @Basic
+    @Basic(fetch = FetchType.LAZY)
-    public String getStringField() {
+    public String getLazyField() {
-        return sf;
+        return lazyField;
+    }
+
+    public void setLazyField(String s) {
+        lazyField = s;
     }
 
     public boolean equals(Object o) {

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedType.java

@@ -23,9 +23,12 @@ package org.apache.openjpa.enhance;
  */
 public interface UnenhancedType {
 
+    int getId();
+
     void setStringField(String s);
     String getStringField();
-    int getId();
+
+    String getLazyField();
 
     Object clone() throws CloneNotSupportedException;
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceMetaDataDefaults.java

@@ -50,6 +50,7 @@ import javax.persistence.Transient;
 import org.apache.commons.lang.StringUtils;
 import org.apache.openjpa.lib.util.J2DoPrivHelper;
 import org.apache.openjpa.lib.util.Localizer;
+import org.apache.openjpa.lib.log.Log;
 import org.apache.openjpa.meta.AbstractMetaDataDefaults;
 import org.apache.openjpa.meta.ClassMetaData;
 import org.apache.openjpa.meta.FieldMetaData;
@@ -57,6 +58,7 @@ import org.apache.openjpa.meta.JavaTypes;
 import org.apache.openjpa.meta.ValueMetaData;
 import static org.apache.openjpa.persistence.PersistenceStrategy.*;
 import org.apache.openjpa.util.MetaDataException;
+import org.apache.openjpa.conf.OpenJPAConfiguration;
 
 /**
  * JPA-based metadata defaults.
@@ -294,10 +296,13 @@ public class PersistenceMetaDataDefaults
                         meta.getDescribedType(), "set" +
                         StringUtils.capitalize(name), new Class[] { 
                             ((Method) member).getReturnType() }));
-                if (setter == null)
+                if (setter == null) {
+                    logNoSetter(meta, name, null);
                     return false;
+                }
             } catch (Exception e) {
                 // e.g., NoSuchMethodException
+                logNoSetter(meta, name, e);
                 return false;
             }
         }
@@ -307,4 +312,16 @@ public class PersistenceMetaDataDefaults
             return false;
         return true;
 	}
+
+    private void logNoSetter(ClassMetaData meta, String name, Exception e) {
+        Log log = meta.getRepository().getConfiguration()
+            .getLog(OpenJPAConfiguration.LOG_METADATA);
+        if (log.isWarnEnabled())
+            log.warn(_loc.get("no-setter-for-getter", name,
+                meta.getDescribedType().getName()));
+        else if (log.isTraceEnabled())
+            // log the exception, if any, if we're in trace-level debugging
+            log.warn(_loc.get("no-setter-for-getter", name,
+                meta.getDescribedType().getName()), e);
+    }
 }

openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties

@@ -121,6 +121,10 @@ map-persistent-types-skipping-non-url: Skipping persistent type location \
 map-persistent-types-skipping-class: Skipping persistent type location \
     association for location "{0}" since it is a class, and will not \
     need to be re-parsed later.
+no-setter-for-getter: No setter was found for method {0} in type {1} while \
+    searching for persistent properties. This method will be ignored. If you \
+    intended for this to be persistent, please add a corresponding setter, \
+    or switch to field access for this type hierarchy.
 
 EntityManagerFactory-name: EntityManagerFactory implementation
 EntityManagerFactory-desc: Allows extension of standard \