diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java index 1dd65d02f..0d1f17766 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCEnhancer.java @@ -97,7 +97,6 @@ import org.apache.openjpa.util.StringId; import org.apache.openjpa.util.UserException; import org.apache.openjpa.util.asm.AsmHelper; import org.apache.openjpa.util.asm.ClassWriterTracker; -import org.apache.xbean.asm9.ClassReader; import org.apache.xbean.asm9.Opcodes; import org.apache.xbean.asm9.Type; import org.apache.xbean.asm9.tree.AbstractInsnNode; @@ -225,9 +224,9 @@ public class PCEnhancer { private Set _violations = null; private File _dir = null; private BytecodeWriter _writer = null; - private Map _backingFields = null; // map of set / get names => field names - private Map _attrsToFields = null; // map of attr names => field names - private Map _fieldsToAttrs = null; // map of field names => attr names + private Map _backingFields = null; // map of set / get names => field names + private Map _attrsToFields = null; // map of attr names => field names + private Map _fieldsToAttrs = null; // map of field names => attr names private boolean _isAlreadyRedefined = false; private boolean _isAlreadySubclassed = false; private boolean _bcsConfigured = false; @@ -573,12 +572,14 @@ public class PCEnhancer { _log.trace(_loc.get("enhance-start", type)); } - configureBCs(); + final ClassNode classNode = AsmHelper.readClassNode(_managedType.toByteArray()); + + configureBCs(classNode); // validate properties before replacing field access so that // we build up a record of backing fields, etc if (isPropertyAccess(_meta)) { - validateProperties(); + validateProperties(classNode); if (getCreateSubclass()) { addAttributeTranslation(); } @@ -607,22 +608,21 @@ public class PCEnhancer { } } - private void configureBCs() { + private void configureBCs(final ClassNode classNode) { if (!_bcsConfigured) { if (getRedefine()) { - if (_managedType.getAttribute(REDEFINED_ATTRIBUTE) == null) + if (_managedType.getAttribute(REDEFINED_ATTRIBUTE) == null) { _managedType.addAttribute(REDEFINED_ATTRIBUTE); - else + } else { _isAlreadyRedefined = true; + } } if (getCreateSubclass()) { - PCSubclassValidator val = new PCSubclassValidator( - _meta, _managedType, _log, _fail); + PCSubclassValidator val = new PCSubclassValidator(_meta, classNode, _managedType, _log, _fail); val.assertCanSubclass(); - _pc = _managedType.getProject().loadClass( - toPCSubclassName(_managedType.getType())); + _pc = _managedType.getProject().loadClass(toPCSubclassName(_managedType.getType())); if (_pc.getSuperclassBC() != _managedType) { _pc.setSuperclass(_managedType); _pc.setAbstract(_managedType.isAbstract()); @@ -699,12 +699,14 @@ public class PCEnhancer { * written correctly. This method also gathers information on each * property's backing field. */ - private void validateProperties() { + private void validateProperties(ClassNode classNode) { FieldMetaData[] fmds; - if (getCreateSubclass()) + if (getCreateSubclass()) { fmds = _meta.getFields(); - else + } + else { fmds = _meta.getDeclaredFields(); + } Method meth; BCMethod bcGetter, bcSetter; @@ -728,6 +730,7 @@ public class PCEnhancer { } meth = (Method) fmd.getBackingMember(); + // ##### this will fail if we override and don't call super. BCClass declaringType = _managedType.getProject() .loadClass(fmd.getDeclaringType()); @@ -739,12 +742,13 @@ public class PCEnhancer { continue; } bcReturned = getReturnedField_old(bcGetter); - returned = getReturnedField(meth); + getter = meth; + returned = getReturnedField(classNode, getter); //X TODO remove PCEnhancer.assertSameField(returned, bcReturned); - if (bcReturned != null) { + if (returned != null) { registerBackingFieldInfo(fmd, bcGetter, bcReturned); } @@ -774,7 +778,7 @@ public class PCEnhancer { if (bcSetter != null) { bcAssigned = getAssignedField_old(bcSetter); - assigned = getAssignedField(getMethod(fmd.getDeclaringType(), fmd.getSetterName(), new Class[]{fmd.getDeclaredType()})); + assigned = getAssignedField(classNode, getMethod(fmd.getDeclaringType(), fmd.getSetterName(), new Class[]{fmd.getDeclaredType()})); assertSameField(assigned, bcAssigned); } @@ -790,18 +794,20 @@ public class PCEnhancer { } } - private void registerBackingFieldInfo(FieldMetaData fmd, BCMethod method, - BCField field) { - if (_backingFields == null) + private void registerBackingFieldInfo(FieldMetaData fmd, BCMethod method, BCField field) { + if (_backingFields == null) { _backingFields = new HashMap(); + } _backingFields.put(method.getName(), field.getName()); - if (_attrsToFields == null) + if (_attrsToFields == null) { _attrsToFields = new HashMap(); + } _attrsToFields.put(fmd.getName(), field.getName()); - if (_fieldsToAttrs == null) + if (_fieldsToAttrs == null) { _fieldsToAttrs = new HashMap(); + } _fieldsToAttrs.put(field.getName(), fmd.getName()); } @@ -886,13 +892,14 @@ public class PCEnhancer { * Return the field returned by the given method, or null if none. * Package-protected and static for testing. */ + @Deprecated static BCField getReturnedField_old(BCMethod meth) { return findField_old(meth, new Code().xreturn() .setType(meth.getReturnType()), false); } - static Field getReturnedField(Method meth) { - return findField(meth, (ain) -> ain.getOpcode() == AsmHelper.getReturnInsn(meth.getReturnType()), false); + static Field getReturnedField(ClassNode classNode, Method meth) { + return findField(classNode, meth, (ain) -> ain.getOpcode() == AsmHelper.getReturnInsn(meth.getReturnType()), false); } @@ -900,13 +907,14 @@ public class PCEnhancer { * Return the field assigned in the given method, or null if none. * Package-protected and static for testing. */ + @Deprecated static BCField getAssignedField_old(BCMethod meth) { return findField_old(meth, (AccessController.doPrivileged( J2DoPrivHelper.newCodeAction())).putfield(), true); } - static Field getAssignedField(Method meth) { - return findField(meth, (ain) -> ain.getOpcode() == Opcodes.PUTFIELD, true); + static Field getAssignedField(ClassNode classNode, Method meth) { + return findField(classNode, meth, (ain) -> ain.getOpcode() == Opcodes.PUTFIELD, true); } /** @@ -914,6 +922,7 @@ public class PCEnhancer { * null if non-fields (methods, literals, parameters, variables) are * returned, or if non-parameters are assigned to fields. */ + @Deprecated private static BCField findField_old(BCMethod meth, Instruction template, boolean findAccessed) { // ignore any static methods. OpenJPA only currently supports // non-static setters and getters @@ -980,24 +989,17 @@ public class PCEnhancer { return field; } - - private static Field findField(Method meth, Predicate ain, boolean findAccessed) { + private static Field findField(ClassNode classNode, Method meth, Predicate ain, boolean findAccessed) { // ignore any static methods. OpenJPA only currently supports // non-static setters and getters if (Modifier.isStatic(meth.getModifiers())) { return null; } - ClassReader cr; - final String classResourceName = meth.getDeclaringClass().getName().replace(".", "/") + ".class"; - try (final InputStream classBytesStream = meth.getDeclaringClass().getClassLoader().getResourceAsStream(classResourceName)) { - cr = new ClassReader(classBytesStream); + if (meth.getDeclaringClass().isInterface()) { + return null; } - catch (IOException e) { - throw new RuntimeException(e); - } - ClassNode classNode = new ClassNode(); - cr.accept(classNode, 0); + final MethodNode methodNode = classNode.methods.stream() .filter(mn -> mn.name.equals(meth.getName()) && mn.desc.equals(Type.getMethodDescriptor(meth))) .findAny().get(); @@ -3437,7 +3439,7 @@ public class PCEnhancer { /** * Adds a custom readObject method that delegates to the - * {@link ObjectInputStream#readObject} method. + * {@link ObjectInputStream#readObject()} method. */ private void modifyReadObjectMethod(BCMethod method, boolean full) { Code code = method.getCode(true); @@ -4935,7 +4937,7 @@ public class PCEnhancer { *
    *
  • -properties/-p <properties file>: The path to a OpenJPA * properties file containing information as outlined in - * {@link Configuration}; optional.
    • + * {@link org.apache.openjpa.lib.conf.Configuration}; optional. *
  • -<property name> <property value>: All bean * properties of the standard OpenJPA {@link OpenJPAConfiguration} can be * set by using their names and supplying a value; for example: diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCSubclassValidator.java b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCSubclassValidator.java index a71a581ce..33303a843 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCSubclassValidator.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCSubclassValidator.java @@ -27,6 +27,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collection; +import java.util.Objects; import org.apache.openjpa.lib.log.Log; import org.apache.openjpa.lib.util.Localizer; @@ -36,6 +37,7 @@ import org.apache.openjpa.meta.AccessCode; import org.apache.openjpa.meta.ClassMetaData; import org.apache.openjpa.meta.FieldMetaData; import org.apache.openjpa.util.UserException; +import org.apache.xbean.asm9.tree.ClassNode; import serp.bytecode.BCClass; import serp.bytecode.BCField; @@ -83,6 +85,7 @@ public class PCSubclassValidator { Localizer.forPackage(PCSubclassValidator.class); private final ClassMetaData meta; + private final ClassNode classNode; private final BCClass bc; private final Log log; private final boolean failOnContractViolations; @@ -90,9 +93,10 @@ public class PCSubclassValidator { private Collection errors; private Collection contractViolations; - public PCSubclassValidator(ClassMetaData meta, BCClass bc, Log log, + public PCSubclassValidator(ClassMetaData meta, ClassNode classNode, BCClass bc, Log log, boolean enforceContractViolations) { this.meta = meta; + this.classNode = classNode; this.bc = bc; this.log = log; this.failOnContractViolations = enforceContractViolations; @@ -148,7 +152,7 @@ public class PCSubclassValidator { fmd.getName()), fmd); continue; } - BCField returnedField = checkGetterIsSubclassable(getter, fmd); + Field returnedField = checkGetterIsSubclassable(getter, fmd); Method setter = setterForField(fmd); if (setter == null) { @@ -156,12 +160,12 @@ public class PCSubclassValidator { fmd); continue; } - BCField assignedField = checkSetterIsSubclassable(setter, fmd); + Field assignedField = checkSetterIsSubclassable(setter, fmd); if (assignedField == null) { continue; } - if (assignedField != returnedField) { + if (!Objects.equals(assignedField, returnedField)) { addContractViolation(loc.get("subclasser-setter-getter-field-mismatch", fmd.getName(), returnedField, assignedField), fmd); @@ -199,20 +203,20 @@ public class PCSubclassValidator { * null if something other than a single field is * returned, or if it cannot be determined what is returned. */ - private BCField checkGetterIsSubclassable(Method meth, FieldMetaData fmd) { + private Field checkGetterIsSubclassable(Method meth, FieldMetaData fmd) { checkMethodIsSubclassable(meth, fmd); BCField bcField = PCEnhancer.getReturnedField_old(getBCMethod(meth)); - Field field = PCEnhancer.getReturnedField(meth); + Field field = PCEnhancer.getReturnedField(classNode, meth); //X TODO remove PCEnhancer.assertSameField(field, bcField); - if (bcField == null) { + if (field == null) { addContractViolation(loc.get("subclasser-invalid-getter", fmd.getName()), fmd); return null; } else { - return bcField; + return field; } } @@ -221,20 +225,20 @@ public class PCSubclassValidator { * null if something other than a single field is * set, or if it cannot be determined what is set. */ - private BCField checkSetterIsSubclassable(Method meth, FieldMetaData fmd) { + private Field checkSetterIsSubclassable(Method meth, FieldMetaData fmd) { checkMethodIsSubclassable(meth, fmd); BCField bcField = PCEnhancer.getAssignedField_old(getBCMethod(meth)); - Field field = PCEnhancer.getAssignedField(meth); + Field field = PCEnhancer.getAssignedField(classNode, meth); //X TODO remove PCEnhancer.assertSameField(field, bcField); - if (bcField == null) { + if (field == null) { addContractViolation(loc.get("subclasser-invalid-setter", fmd.getName()), fmd); return null; } else { - return bcField; + return field; } } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/util/asm/AsmHelper.java b/openjpa-kernel/src/main/java/org/apache/openjpa/util/asm/AsmHelper.java index fcebd4c49..1f44aba54 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/util/asm/AsmHelper.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/util/asm/AsmHelper.java @@ -17,6 +17,8 @@ package org.apache.openjpa.util.asm; import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.lang.reflect.Method; import org.apache.xbean.asm9.ClassReader; @@ -24,6 +26,7 @@ import org.apache.xbean.asm9.ClassWriter; import org.apache.xbean.asm9.Opcodes; import org.apache.xbean.asm9.Type; import org.apache.xbean.asm9.tree.AbstractInsnNode; +import org.apache.xbean.asm9.tree.ClassNode; import org.apache.xbean.asm9.tree.VarInsnNode; import serp.bytecode.BCClass; @@ -39,6 +42,41 @@ public final class AsmHelper { // utility class ct } + + /** + * Read the binary bytecode from the class with the given name + * @param classBytes the binary of the class + * @return the ClassNode constructed from that class + */ + public static ClassNode readClassNode(byte[] classBytes) { + ClassReader cr = new ClassReader(classBytes); + ClassNode classNode = new ClassNode(); + cr.accept(classNode, 0); + + return classNode; + } + + /** + * Read the binary bytecode from the class with the given name + * @param classLoader the ClassLoader to use + * @param className the fully qualified class name to read. e.g. "org.mycorp.mypackage.MyEntity" + * @return the ClassNode constructed from that class + */ + public static ClassNode readClassNode(ClassLoader classLoader, String className) { + ClassReader cr; + final String classResourceName = className.replace(".", "/") + ".class"; + try (final InputStream classBytesStream = classLoader.getResourceAsStream(classResourceName)) { + cr = new ClassReader(classBytesStream); + } + catch (IOException e) { + throw new RuntimeException(e); + } + ClassNode classNode = new ClassNode(); + cr.accept(classNode, 0); + + return classNode; + } + /** * temporary helper class to convert BCClass to ASM * @deprecated must get removed when done with migrating from Serp to ASM diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestSubclassValidator.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestSubclassValidator.java index 681ca6a11..70c1fb1c0 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestSubclassValidator.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestSubclassValidator.java @@ -24,9 +24,9 @@ import org.apache.openjpa.meta.MetaDataModes; import org.apache.openjpa.meta.MetaDataRepository; import org.apache.openjpa.persistence.common.apps.Department; import org.apache.openjpa.persistence.common.apps.RuntimeTest2; -import org.apache.openjpa.enhance.UnenhancedPropertyAccess; import org.apache.openjpa.persistence.test.SingleEMFTestCase; -import org.apache.openjpa.util.UserException; +import org.apache.openjpa.util.asm.AsmHelper; +import org.apache.xbean.asm9.tree.ClassNode; import org.junit.Test; import jakarta.persistence.Access; @@ -34,6 +34,7 @@ import jakarta.persistence.AccessType; import jakarta.persistence.Basic; import jakarta.persistence.Entity; import jakarta.persistence.Id; +import jakarta.persistence.MappedSuperclass; import serp.bytecode.BCClass; import serp.bytecode.Project; @@ -43,7 +44,7 @@ public class TestSubclassValidator extends SingleEMFTestCase { setUp("openjpa.RuntimeUnenhancedClasses", "supported", Department.class, RuntimeTest2.class, - EnhancableGetterEntity.class, + EnhanceableGetterEntity.class, UnenhancedPropertyAccess.class, CLEAR_TABLES); } @@ -80,29 +81,31 @@ public class TestSubclassValidator extends SingleEMFTestCase { */ { - final BCClass bcClass = project.loadClass(EnhancableGetterEntity.class.getName(), tempCl); - final ClassMetaData meta = repos.getMetaData(tempCl.loadClass(EnhancableGetterEntity.class.getName()), tempCl, false); - PCSubclassValidator subclassValidator = new PCSubclassValidator(meta, bcClass, log, true); + ClassNode classNode = AsmHelper.readClassNode(EnhanceableGetterEntity.class.getClassLoader(), EnhanceableGetterEntity.class.getName()); + final BCClass bcClass = project.loadClass(EnhanceableGetterEntity.class.getName(), tempCl); + final ClassMetaData meta = repos.getMetaData(tempCl.loadClass(EnhanceableGetterEntity.class.getName()), tempCl, false); + PCSubclassValidator subclassValidator = new PCSubclassValidator(meta, classNode, bcClass, log, true); subclassValidator.assertCanSubclass(); } { + ClassNode classNode = AsmHelper.readClassNode(UnenhancedPropertyAccess.class.getClassLoader(), UnenhancedPropertyAccess.class.getName()); final BCClass bcClass = project.loadClass(UnenhancedPropertyAccess.class.getName(), tempCl); final ClassMetaData meta = repos.getMetaData(tempCl.loadClass(UnenhancedPropertyAccess.class.getName()), tempCl, false); - PCSubclassValidator subclassValidator = new PCSubclassValidator(meta, bcClass, log, true); + PCSubclassValidator subclassValidator = new PCSubclassValidator(meta, classNode, bcClass, log, true); subclassValidator.assertCanSubclass(); } } @Entity @Access(AccessType.PROPERTY) - public static class EnhancableGetterEntity { + public static class EnhanceableGetterEntity { @Id private String id; @Basic - private String name; + protected String name; @Basic private String another;