This is an automated email from the ASF dual-hosted git repository.
struberg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/openjpa.git
commit 3ea2412003028d37f2a69971a47bb20abf589f8b
Author: Mark Struberg <strub...@apache.org>
AuthorDate: Thu May 25 18:07:41 2023 +0200
OPENJPA-2911 move PCSubclassValidator to ASM
---
.../org/apache/openjpa/enhance/PCEnhancer.java | 84 +++++++++++-----------
.../openjpa/enhance/PCSubclassValidator.java | 28 ++++----
.../org/apache/openjpa/util/asm/AsmHelper.java | 38 ++++++++++
.../openjpa/enhance/TestSubclassValidator.java | 21 +++---
4 files changed, 109 insertions(+), 62 deletions(-)
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<String, String> _backingFields = null; // map of set / get
names => field names
+ private Map<String, String> _attrsToFields = null; // map of attr names =>
field names
+ private Map<String, String> _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<AbstractInsnNode>
ain, boolean findAccessed) {
+ private static Field findField(ClassNode classNode, Method meth,
Predicate<AbstractInsnNode> 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);
- }
- catch (IOException e) {
- throw new RuntimeException(e);
+ if (meth.getDeclaringClass().isInterface()) {
+ return null;
}
- 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 {
* <ul>
* <li><i>-properties/-p <properties file></i>: The path to a OpenJPA
* properties file containing information as outlined in
- * {@link Configuration}; optional.</li>
+ * {@link org.apache.openjpa.lib.conf.Configuration}; optional.</li>
* <li><i>-<property name> <property value></i>: 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 {
* <code>null</code> 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 {
* <code>null</code> 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;
