diff --git a/openjpa-kernel-5/src/main/java/org/apache/openjpa/enhance/ManagedClassSubclasser.java b/openjpa-kernel-5/src/main/java/org/apache/openjpa/enhance/ManagedClassSubclasser.java index 41489937b..81c464674 100644 --- a/openjpa-kernel-5/src/main/java/org/apache/openjpa/enhance/ManagedClassSubclasser.java +++ b/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) subs) - configureMetaData(conf, envLoader, cls); + configureMetaData(conf, envLoader, cls, redefine); for (Class cls : (Collection) 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 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 diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/enhance/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/enhance/localizer.properties index 5bfc6f805..9df55adc5 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/enhance/localizer.properties +++ b/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. diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/AbstractUnenhancedClassTest.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/AbstractUnenhancedClassTest.java index b49991776..1875aca00 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/AbstractUnenhancedClassTest.java +++ b/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, - "openjpa.Log", "Enhance=TRACE"); + setUp(getUnenhancedClass(), getUnenhancedSubclass(), CLEAR_TABLES); // 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); diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedFieldAccess.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedFieldAccess.java index d2a372a44..a8894f603 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedFieldAccess.java +++ b/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; diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedPropertyAccess.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedPropertyAccess.java index 6eca1f82f..dc0de4b74 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedPropertyAccess.java +++ b/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 org.apache.openjpa.persistence.DetachedState; +import javax.persistence.FetchType; @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 - public String getStringField() { - return sf; + @Basic(fetch = FetchType.LAZY) + public String getLazyField() { + return lazyField; + } + + public void setLazyField(String s) { + lazyField = s; } public boolean equals(Object o) { diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedType.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedType.java index b7c75169a..707eb3139 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/UnenhancedType.java +++ b/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; } diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceMetaDataDefaults.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceMetaDataDefaults.java index bf3b59212..7cff80dbb 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/PersistenceMetaDataDefaults.java +++ b/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); + } } diff --git a/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties b/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties index d7eb58eff..78a22f9b6 100644 --- a/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties +++ b/openjpa-persistence/src/main/resources/org/apache/openjpa/persistence/localizer.properties @@ -121,7 +121,11 @@ 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 \ org.apache.openjpa.persistence.EntityManagerFactoryImpl for custom behavior.