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 31681982de16a3496e0ebb6aa1efa3901ac6e6ef
Author: Mark Struberg <strub...@apache.org>
AuthorDate: Wed May 24 10:38:44 2023 +0200
OPENJPA-2911 move PCSubclassValidator to ASM
work in progress.
For now we compare old and new mechanism to catch errors.
---
.../org/apache/openjpa/enhance/PCEnhancer.java | 153 +++++++++++--
.../openjpa/enhance/PCSubclassValidator.java | 253 +++++++++++----------
.../openjpa/meta/AbstractMetaDataDefaults.java | 82 +++----
.../org/apache/openjpa/util/asm/AsmHelper.java | 30 +++
.../openjpa/enhance/TestPCEnhancerFindField.java | 2 +-
.../openjpa/enhance/TestSubclassValidator.java | 126 ++++++++++
6 files changed, 470 insertions(+), 176 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 72b6380c8..092c8ef9b 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
@@ -52,6 +52,7 @@ import java.util.Locale;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
+import java.util.function.Predicate;
import org.apache.openjpa.conf.OpenJPAConfiguration;
import org.apache.openjpa.conf.OpenJPAConfigurationImpl;
@@ -96,6 +97,14 @@ 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;
+import org.apache.xbean.asm9.tree.ClassNode;
+import org.apache.xbean.asm9.tree.FieldInsnNode;
+import org.apache.xbean.asm9.tree.MethodNode;
+import org.apache.xbean.asm9.tree.VarInsnNode;
import serp.bytecode.BCClass;
import serp.bytecode.BCField;
@@ -541,14 +550,15 @@ public class PCEnhancer {
Class<?> type = _managedType.getType();
try {
// if enum, skip, no need of any meta
- if (_pc.isEnum())
+ if (type.isEnum())
return ENHANCE_NONE;
// if managed interface, skip
- if (_pc.isInterface())
+ if (type.isInterface())
return ENHANCE_INTERFACE;
// check if already enhanced
+ // we cannot simply use instanceof or isAssignableFrom as we have
a temp ClassLoader inbetween
ClassLoader loader =
AccessController.doPrivileged(J2DoPrivHelper.getClassLoaderAction(type));
for (String iface : _managedType.getDeclaredInterfaceNames()) {
if (iface.equals(PCTYPE.getName())) {
@@ -558,19 +568,20 @@ public class PCEnhancer {
return ENHANCE_NONE;
}
}
+
if (_log.isTraceEnabled()) {
- _log.trace(_loc.get("enhance-start", type, loader));
+ _log.trace(_loc.get("enhance-start", type));
}
-
configureBCs();
// validate properties before replacing field access so that
// we build up a record of backing fields, etc
if (isPropertyAccess(_meta)) {
validateProperties();
- if (getCreateSubclass())
+ if (getCreateSubclass()) {
addAttributeTranslation();
+ }
}
replaceAndValidateFieldAccess();
processViolations();
@@ -710,7 +721,7 @@ public class PCEnhancer {
true);
continue;
}
- returned = getReturnedField(getter);
+ returned = getReturnedField_old(getter);
if (returned != null)
registerBackingFieldInfo(fmd, getter, returned);
@@ -738,7 +749,7 @@ public class PCEnhancer {
}
if (setter != null)
- assigned = getAssignedField(setter);
+ assigned = getAssignedField_old(setter);
if (assigned != null) {
if (setter != null)
@@ -848,28 +859,35 @@ public class PCEnhancer {
* Return the field returned by the given method, or null if none.
* Package-protected and static for testing.
*/
- static BCField getReturnedField(BCMethod meth) {
- return findField(meth, (AccessController.doPrivileged(
- J2DoPrivHelper.newCodeAction())).xreturn()
+ 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);
+ }
+
+
/**
* Return the field assigned in the given method, or null if none.
* Package-protected and static for testing.
*/
- static BCField getAssignedField(BCMethod meth) {
- return findField(meth, (AccessController.doPrivileged(
+ 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);
+ }
+
/**
* Return the field returned / assigned by <code>meth</code>. Returns
* null if non-fields (methods, literals, parameters, variables) are
* returned, or if non-parameters are assigned to fields.
*/
- private static BCField findField(BCMethod meth, Instruction template,
- boolean findAccessed) {
+ private static BCField findField_old(BCMethod meth, Instruction template,
boolean findAccessed) {
// ignore any static methods. OpenJPA only currently supports
// non-static setters and getters
if (meth.isStatic())
@@ -918,8 +936,7 @@ public class PCEnhancer {
&& ((LoadInstruction) prevIns).getParam() == 0) {
final FieldInstruction fTemplateIns =
(FieldInstruction) templateIns;
- cur = AccessController.doPrivileged(J2DoPrivHelper
- .getFieldInstructionFieldAction(fTemplateIns));
+ cur = fTemplateIns.getField();
} else
return null;
@@ -936,6 +953,103 @@ public class PCEnhancer {
return field;
}
+
+ private static Field findField(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;
+ try {
+ cr = new ClassReader(meth.getDeclaringClass().getName());
+ }
+ 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();
+
+ Field field = null;
+ Field cur;
+ AbstractInsnNode prevInsn, earlierInsn;
+ for (AbstractInsnNode insn : methodNode.instructions) {
+ if (!ain.test(insn)) {
+ continue;
+ }
+
+ prevInsn = insn.getPrevious();
+ if (prevInsn == null) {
+ return null;
+ }
+
+ if (prevInsn.getOpcode() == Opcodes.INSTANCEOF &&
prevInsn.getPrevious() != null ) {
+ prevInsn = prevInsn.getPrevious();
+ }
+
+ if (prevInsn.getPrevious() == null) {
+ return null;
+ }
+
+ earlierInsn = prevInsn.getPrevious();
+
+ // if the opcode two before the template was an aload_0, check
+ // against the middle instruction based on what type of find
+ // we're doing
+ if (!AsmHelper.isLoadInsn(earlierInsn)
+ || !AsmHelper.isThisInsn(earlierInsn)) {
+ return null;
+ }
+
+ // if the middle instruction was a getfield, then it's the
+ // field that's being accessed
+ if (!findAccessed && prevInsn.getOpcode() == Opcodes.GETFIELD) {
+ final FieldInsnNode fieldInsn = (FieldInsnNode) prevInsn;
+
+ cur = getField(meth.getDeclaringClass(), fieldInsn);
+
+ // if the middle instruction was an xload_1, then the
+ // matched instruction is the field that's being set.
+ } else if (findAccessed && AsmHelper.isLoadInsn(prevInsn)
+ && ((VarInsnNode) prevInsn).var == 1) {
+ final FieldInsnNode fieldInsn = (FieldInsnNode)insn;
+ cur = getField(meth.getDeclaringClass(), fieldInsn);
+ } else {
+ return null;
+ }
+
+
+ if (field != null && cur != field) {
+ return null;
+ }
+ field = cur;
+ }
+
+
+ return field;
+ }
+
+ private static Field getField(Class<?> clazz, FieldInsnNode fieldInsn) {
+ try {
+ final Class<?> fieldOwner =
clazz.getClassLoader().loadClass(fieldInsn.owner.replace("/", "."));
+ return fieldOwner.getDeclaredField(fieldInsn.name);
+ }
+ catch (NoSuchFieldException e) {
+ if (clazz.getSuperclass() == Object.class) {
+ throw new IllegalStateException("Cannot find field " +
fieldInsn + " in Class " + clazz);
+ }
+ return getField(clazz.getSuperclass(), fieldInsn);
+ }
+ catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+
/**
* Record a violation of the property access restrictions.
*/
@@ -4895,8 +5009,8 @@ public class PCEnhancer {
if (args == null || args.length == 0) {
classes = repos.getPersistentTypeNames(true, loader);
if (classes == null) {
- log.warn(_loc.get("no-class-to-enhance"));
- return false;
+ log.warn(_loc.get("no-class-to-enhance"));
+ return false;
}
} else {
ClassArgParser cap = conf.getMetaDataRepositoryInstance().
@@ -4923,8 +5037,9 @@ public class PCEnhancer {
else
bc = project.loadClass((Class) o);
enhancer = new PCEnhancer(conf, bc, repos, loader);
- if (writer != null)
+ if (writer != null) {
enhancer.setBytecodeWriter(writer);
+ }
enhancer.setDirectory(flags.directory);
enhancer.setAddDefaultConstructor(flags.addDefaultConstructor);
status = enhancer.run();
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 cee8f03a4..19018417e 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
@@ -21,6 +21,7 @@ package org.apache.openjpa.enhance;
import java.io.Externalizable;
import java.io.ObjectInput;
import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
@@ -34,7 +35,6 @@ import org.apache.openjpa.lib.util.StringUtil;
import org.apache.openjpa.meta.AccessCode;
import org.apache.openjpa.meta.ClassMetaData;
import org.apache.openjpa.meta.FieldMetaData;
-import org.apache.openjpa.util.InternalException;
import org.apache.openjpa.util.UserException;
import serp.bytecode.BCClass;
@@ -42,48 +42,48 @@ import serp.bytecode.BCField;
import serp.bytecode.BCMethod;
/**
- * <p>Validates that a given type meets the JPA contract, plus a few
- * OpenJPA-specific additions for subclassing / redefinition:
+ * <p>Validates that a given type meets the JPA contract, plus a few
+ * OpenJPA-specific additions for subclassing / redefinition:
*
- * <ul>
- * <li>must have an accessible no-args constructor</li>
- * <li>must be a public or protected class</li>
- * <li>must not be final</li>
- * <li>must not extend an enhanced class</li>
- * <li>all persistent data represented by accessible setter/getter
- * methods (persistent properties)</li>
- * <li>if versioning is to be used, exactly one persistent property for
- * the numeric version data</li> <!-- ##### is this true? -->
+ * <ul>
+ * <li>must have an accessible no-args constructor</li>
+ * <li>must be a public or protected class</li>
+ * <li>must not be final</li>
+ * <li>must not extend an enhanced class</li>
+ * <li>all persistent data represented by accessible setter/getter
+ * methods (persistent properties)</li>
+ * <li>if versioning is to be used, exactly one persistent property for
+ * the numeric version data</li> <!-- ##### is this true? -->
*
- * <li>When using property access, the backing field for a persistent
- * property must be:
- * <ul>
- * <!-- ##### JPA validation of these needs to be tested -->
- * <li>private</li>
- * <li>set only in the designated setter,
- * in the constructor, or in {@link Object#clone()},
- * <code>readObject(ObjectInputStream)</code>, or
- * {@link Externalizable#readExternal(ObjectInput)}.</li>
- * <li>read only in the designated getter and the
- * constructor.</li>
- * </ul>
- * </li>
- * </ul>
+ * <li>When using property access, the backing field for a persistent
+ * property must be:
+ * <ul>
+ * <!-- ##### JPA validation of these needs to be tested -->
+ * <li>private</li>
+ * <li>set only in the designated setter,
+ * in the constructor, or in {@link Object#clone()},
+ * <code>readObject(ObjectInputStream)</code>, or
+ * {@link Externalizable#readExternal(ObjectInput)}.</li>
+ * <li>read only in the designated getter and the
+ * constructor.</li>
+ * </ul>
+ * </li>
+ * </ul>
*
- * <p>If you use this technique and use the <code>new</code> keyword instead
- * of a OpenJPA-supplied construction routine, OpenJPA will need to do extra
- * work with persistent-new-flushed instances, since OpenJPA cannot in this
- * case track what happens to such an instance.</p>
- *
- * @since 1.0.0
+ * <p>If you use this technique and use the <code>new</code> keyword instead
+ * of a OpenJPA-supplied construction routine, OpenJPA will need to do extra
+ * work with persistent-new-flushed instances, since OpenJPA cannot in this
+ * case track what happens to such an instance.</p>
+ * <p>
+ * @since 1.0.0
*/
public class PCSubclassValidator {
private static final Localizer loc =
- Localizer.forPackage(PCSubclassValidator.class);
+ Localizer.forPackage(PCSubclassValidator.class);
private final ClassMetaData meta;
- private final BCClass pc;
+ private final BCClass bc;
private final Log log;
private final boolean failOnContractViolations;
@@ -91,52 +91,51 @@ public class PCSubclassValidator {
private Collection contractViolations;
public PCSubclassValidator(ClassMetaData meta, BCClass bc, Log log,
- boolean enforceContractViolations) {
+ boolean enforceContractViolations) {
this.meta = meta;
- this.pc = bc;
+ this.bc = bc;
this.log = log;
this.failOnContractViolations = enforceContractViolations;
}
public void assertCanSubclass() {
- Class superclass = meta.getDescribedType();
+ Class<?> superclass = meta.getDescribedType();
String name = superclass.getName();
- if (superclass.isInterface())
+ if (superclass.isInterface()) {
addError(loc.get("subclasser-no-ifaces", name), meta);
- if (Modifier.isFinal(superclass.getModifiers()))
+ }
+ if (Modifier.isFinal(superclass.getModifiers())) {
addError(loc.get("subclasser-no-final-classes", name), meta);
- if (Modifier.isPrivate(superclass.getModifiers()))
+ }
+ if (Modifier.isPrivate(superclass.getModifiers())) {
addError(loc.get("subclasser-no-private-classes", name), meta);
- if (PersistenceCapable.class.isAssignableFrom(superclass))
+ }
+ if (PersistenceCapable.class.isAssignableFrom(superclass)) {
addError(loc.get("subclasser-super-already-pc", name), meta);
+ }
try {
- Constructor c = superclass.getDeclaredConstructor(new Class[0]);
+ Constructor c = superclass.getDeclaredConstructor();
if (!(Modifier.isProtected(c.getModifiers())
- || Modifier.isPublic(c.getModifiers())))
+ || Modifier.isPublic(c.getModifiers()))) {
addError(loc.get("subclasser-private-ctor", name), meta);
+ }
}
catch (NoSuchMethodException e) {
- addError(loc.get("subclasser-no-void-ctor", name),
- meta);
+ addError(loc.get("subclasser-no-void-ctor", name), meta);
}
- // if the BCClass we loaded is already pc and the superclass is not,
- // then we should never get here, so let's make sure that the
- // calling context is caching correctly by throwing an exception.
- if (pc.isInstanceOf(PersistenceCapable.class) &&
- !PersistenceCapable.class.isAssignableFrom(superclass))
- throw new InternalException(
- loc.get("subclasser-class-already-pc", name));
-
- if (AccessCode.isProperty(meta.getAccessType()))
+ if (AccessCode.isProperty(meta.getAccessType())) {
checkPropertiesAreInterceptable();
+ }
- if (errors != null && !errors.isEmpty())
+ if (errors != null && !errors.isEmpty()) {
throw new UserException(errors.toString());
+ }
else if (contractViolations != null &&
- !contractViolations.isEmpty() && log.isWarnEnabled())
+ !contractViolations.isEmpty() && log.isWarnEnabled()) {
log.warn(contractViolations.toString());
+ }
}
private void checkPropertiesAreInterceptable() {
@@ -146,7 +145,7 @@ public class PCSubclassValidator {
Method getter = getBackingMember(fmd);
if (getter == null) {
addError(loc.get("subclasser-no-getter",
- fmd.getName()), fmd);
+ fmd.getName()), fmd);
continue;
}
BCField returnedField = checkGetterIsSubclassable(getter, fmd);
@@ -154,18 +153,19 @@ public class PCSubclassValidator {
Method setter = setterForField(fmd);
if (setter == null) {
addError(loc.get("subclasser-no-setter", fmd.getName()),
- fmd);
+ fmd);
continue;
}
BCField assignedField = checkSetterIsSubclassable(setter, fmd);
- if (assignedField == null)
+ if (assignedField == null) {
continue;
+ }
- if (assignedField != returnedField)
- addContractViolation(loc.get
- ("subclasser-setter-getter-field-mismatch",
- fmd.getName(), returnedField,
assignedField),
- fmd);
+ if (assignedField != returnedField) {
+
addContractViolation(loc.get("subclasser-setter-getter-field-mismatch",
+ fmd.getName(), returnedField,
assignedField),
+ fmd);
+ }
// ### scan through all the rest of the class to make sure it
// ### doesn't use the field.
@@ -173,22 +173,21 @@ public class PCSubclassValidator {
}
private Method getBackingMember(FieldMetaData fmd) {
- Member back = fmd.getBackingMember();
- if (Method.class.isInstance(back))
- return (Method)back;
-
- Method getter = Reflection.findGetter(meta.getDescribedType(),
- fmd.getName(), false);
- if (getter != null)
- fmd.backingMember(getter);
- return getter;
+ Member back = fmd.getBackingMember();
+ if (back instanceof Method) {
+ return (Method) back;
+ }
+
+ Method getter = Reflection.findGetter(meta.getDescribedType(),
fmd.getName(), false);
+ if (getter != null) {
+ fmd.backingMember(getter);
+ }
+ return getter;
}
private Method setterForField(FieldMetaData fmd) {
try {
- return fmd.getDeclaringType().getDeclaredMethod(
- "set" + StringUtil.capitalize(fmd.getName()),
- new Class[]{ fmd.getDeclaredType() });
+ return fmd.getDeclaringType().getDeclaredMethod("set" +
StringUtil.capitalize(fmd.getName()), fmd.getDeclaredType());
}
catch (NoSuchMethodException e) {
return null;
@@ -197,93 +196,111 @@ public class PCSubclassValidator {
/**
* @return the name of the field that is returned by <code>meth</code>, or
- * <code>null</code> if something other than a single field is
- * returned, or if it cannot be determined what is returned.
+ * <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) {
checkMethodIsSubclassable(meth, fmd);
- BCField field = PCEnhancer.getReturnedField(getBCMethod(meth));
- if (field == null) {
- addContractViolation(loc.get("subclasser-invalid-getter",
- fmd.getName()), fmd);
+ BCField bcField = PCEnhancer.getReturnedField_old(getBCMethod(meth));
+ Field field = PCEnhancer.getReturnedField(meth);
+
+ //X TODO remove
+ if (field == null && bcField != null || field != null && bcField ==
null ||
+ field != null && bcField != null &&
!field.getName().equals(bcField.getName())) {
+ throw new IllegalStateException("MSX ASMTODO " + bcField + " " +
field);
+ }
+
+ if (bcField == null) {
+ addContractViolation(loc.get("subclasser-invalid-getter",
fmd.getName()), fmd);
return null;
- } else {
- return field;
+ }
+ else {
+ return bcField;
}
}
/**
* @return the field that is set in <code>meth</code>, or
- * <code>null</code> if something other than a single field is
- * set, or if it cannot be determined what is set.
+ * <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) {
checkMethodIsSubclassable(meth, fmd);
- BCField field = PCEnhancer.getAssignedField(getBCMethod(meth));
- if (field == null) {
- addContractViolation(loc.get("subclasser-invalid-setter",
- fmd.getName()), fmd);
+ BCField bcField = PCEnhancer.getAssignedField_old(getBCMethod(meth));
+ Field field = PCEnhancer.getAssignedField(meth);
+
+ //X TODO remove
+ if (field == null && bcField != null || field != null && bcField ==
null ||
+ field != null && bcField != null &&
!field.getName().equals(bcField.getName())) {
+ throw new IllegalStateException("MSX ASMTODO " + bcField + " " +
field);
+ }
+
+ if (bcField == null) {
+ addContractViolation(loc.get("subclasser-invalid-setter",
fmd.getName()), fmd);
return null;
- } else {
- return field;
+ }
+ else {
+ return bcField;
}
}
private BCMethod getBCMethod(Method meth) {
- BCClass bc = pc.getProject().loadClass(meth.getDeclaringClass());
+ BCClass bc = this.bc.getProject().loadClass(meth.getDeclaringClass());
return bc.getDeclaredMethod(meth.getName(), meth.getParameterTypes());
}
private void checkMethodIsSubclassable(Method meth, FieldMetaData fmd) {
String className = fmd.getDefiningMetaData().
- getDescribedType().getName();
+ getDescribedType().getName();
if (!(Modifier.isProtected(meth.getModifiers())
- || Modifier.isPublic(meth.getModifiers())))
- addError(loc.get("subclasser-private-accessors-unsupported",
- className, meth.getName()), fmd);
- if (Modifier.isFinal(meth.getModifiers()))
- addError(loc.get("subclasser-final-methods-not-allowed",
- className, meth.getName()), fmd);
- if (Modifier.isNative(meth.getModifiers()))
- addContractViolation(loc.get
- ("subclasser-native-methods-not-allowed", className,
- meth.getName()),
- fmd);
- if (Modifier.isStatic(meth.getModifiers()))
- addError(loc.get("subclasser-static-methods-not-supported",
- className, meth.getName()), fmd);
+ || Modifier.isPublic(meth.getModifiers()))) {
+ addError(loc.get("subclasser-private-accessors-unsupported",
className, meth.getName()), fmd);
+ }
+ if (Modifier.isFinal(meth.getModifiers())) {
+ addError(loc.get("subclasser-final-methods-not-allowed",
className, meth.getName()), fmd);
+ }
+ if (Modifier.isNative(meth.getModifiers())) {
+
addContractViolation(loc.get("subclasser-native-methods-not-allowed",
className, meth.getName()), fmd);
+ }
+ if (Modifier.isStatic(meth.getModifiers())) {
+ addError(loc.get("subclasser-static-methods-not-supported",
className, meth.getName()), fmd);
+ }
}
private void addError(Message s, ClassMetaData cls) {
- if (errors == null)
+ if (errors == null) {
errors = new ArrayList();
+ }
errors.add(loc.get("subclasser-error-meta", s,
- cls.getDescribedType().getName(),
- cls.getSourceFile()));
+ cls.getDescribedType().getName(),
+ cls.getSourceFile()));
}
private void addError(Message s, FieldMetaData fmd) {
- if (errors == null)
+ if (errors == null) {
errors = new ArrayList();
+ }
errors.add(loc.get("subclasser-error-field", s,
- fmd.getFullName(),
- fmd.getDeclaringMetaData().getSourceFile()));
+ fmd.getFullName(),
+ fmd.getDeclaringMetaData().getSourceFile()));
}
private void addContractViolation(Message m, FieldMetaData fmd) {
// add the violation as an error in case we're processing violations
// as errors; this keeps them in the order that they were found rather
// than just adding the violations to the end of the list.
- if (failOnContractViolations)
+ if (failOnContractViolations) {
addError(m, fmd);
+ }
- if (contractViolations == null)
+ if (contractViolations == null) {
contractViolations = new ArrayList();
+ }
contractViolations.add(loc.get
- ("subclasser-contract-violation-field", m.getMessage(),
- fmd.getFullName(),
fmd.getDeclaringMetaData().getSourceFile()));
+ ("subclasser-contract-violation-field", m.getMessage(),
+ fmd.getFullName(),
fmd.getDeclaringMetaData().getSourceFile()));
}
}
diff --git
a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractMetaDataDefaults.java
b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractMetaDataDefaults.java
index 6108d7daa..6c7b73340 100644
---
a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractMetaDataDefaults.java
+++
b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractMetaDataDefaults.java
@@ -187,11 +187,11 @@ public abstract class AbstractMetaDataDefaults
Member member;
FieldMetaData fmd;
for (int i = 0; i < fieldNames.length; i ++) {
- String property = fieldNames[i];
- member = getMemberByProperty(meta, property,
- AccessCode.UNKNOWN, true);
- if (member == null) // transient or indeterminable access
- continue;
+ String property = fieldNames[i];
+ member = getMemberByProperty(meta, property,
AccessCode.UNKNOWN, true);
+ if (member == null) { // transient or indeterminable access
+ continue;
+ }
fmd = meta.addDeclaredField(property, fieldTypes[i]);
fmd.backingMember(member);
populate(fmd);
@@ -285,21 +285,27 @@ public abstract class AbstractMetaDataDefaults
* the next letter lower-cased. For other methods, returns null.
*/
public static String getFieldName(Member member) {
- if (member instanceof Field)
+ if (member instanceof Field) {
return member.getName();
- if (!(member instanceof Method))
- return null;
+ }
+ if (!(member instanceof Method)) {
+ return null;
+ }
Method method = (Method) member;
String name = method.getName();
- if (isNormalGetter(method))
- name = name.substring("get".length());
- else if (isBooleanGetter(method))
- name = name.substring("is".length());
- else
+ if (isNormalGetter(method)) {
+ name = name.substring("get".length());
+ }
+ else if (isBooleanGetter(method)) {
+ name = name.substring("is".length());
+ }
+ else {
return null;
+ }
- if (name.length() == 1)
+ if (name.length() == 1) {
return name.toLowerCase();
+ }
return Character.toLowerCase(name.charAt(0)) + name.substring(1);
}
@@ -335,7 +341,7 @@ public abstract class AbstractMetaDataDefaults
if (fmd == null)
return null;
if (fmd.getBackingMember() != null)
- return fmd.getBackingMember();
+ return fmd.getBackingMember();
return getMemberByProperty(fmd.getDeclaringMetaData(), fmd.getName(),
fmd.getAccessType(), true);
}
@@ -362,10 +368,10 @@ public abstract class AbstractMetaDataDefaults
* where T is any non-void type.
*/
public static boolean isNormalGetter(Method method) {
- String methodName = method.getName();
- return startsWith(methodName, "get")
- && method.getParameterTypes().length == 0
- && method.getReturnType() != void.class;
+ String methodName = method.getName();
+ return startsWith(methodName, "get")
+ && method.getParameterTypes().length == 0
+ && method.getReturnType() != void.class;
}
/**
@@ -374,10 +380,10 @@ public abstract class AbstractMetaDataDefaults
* <code> public Boolean isXXX() </code>
*/
public static boolean isBooleanGetter(Method method) {
- String methodName = method.getName();
- return startsWith(methodName, "is")
- && method.getParameterTypes().length == 0
- && isBoolean(method.getReturnType());
+ String methodName = method.getName();
+ return startsWith(methodName, "is")
+ && method.getParameterTypes().length == 0
+ && isBoolean(method.getReturnType());
}
/**
@@ -388,16 +394,16 @@ public abstract class AbstractMetaDataDefaults
* <code> public T isXXX()</code> where T is boolean or Boolean.<br>
*/
public static boolean isGetter(Method method, boolean includePrivate) {
- if (method == null)
- return false;
- int mods = method.getModifiers();
- if (!(Modifier.isPublic(mods)
- || Modifier.isProtected(mods)
- || (Modifier.isPrivate(mods) && includePrivate))
- || Modifier.isNative(mods)
- || Modifier.isStatic(mods))
- return false;
- return isNormalGetter(method) || isBooleanGetter(method);
+ if (method == null)
+ return false;
+ int mods = method.getModifiers();
+ if (!(Modifier.isPublic(mods)
+ || Modifier.isProtected(mods)
+ || (Modifier.isPrivate(mods) && includePrivate))
+ || Modifier.isNative(mods)
+ || Modifier.isStatic(mods))
+ return false;
+ return isNormalGetter(method) || isBooleanGetter(method);
}
/**
@@ -409,14 +415,14 @@ public abstract class AbstractMetaDataDefaults
}
public static boolean isBoolean(Class<?> cls) {
- return cls == boolean.class || cls == Boolean.class;
+ return cls == boolean.class || cls == Boolean.class;
}
public static List<String> toNames(List<? extends Member> members) {
- List<String> result = new ArrayList<>();
- for (Member m : members)
- result.add(m.getName());
- return result;
+ List<String> result = new ArrayList<>();
+ for (Member m : members)
+ result.add(m.getName());
+ return result;
}
}
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 5f6ac6860..fcebd4c49 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
@@ -23,6 +23,8 @@ import org.apache.xbean.asm9.ClassReader;
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.VarInsnNode;
import serp.bytecode.BCClass;
import serp.bytecode.Project;
@@ -211,4 +213,32 @@ public final class AsmHelper {
return types;
}
+ /**
+ * @return true if the instruction is an LOAD instruction
+ */
+ public static boolean isLoadInsn(AbstractInsnNode insn) {
+ return insn.getOpcode() == Opcodes.ALOAD
+ || insn.getOpcode() == Opcodes.ILOAD
+ || insn.getOpcode() == Opcodes.IALOAD
+ || insn.getOpcode() == Opcodes.LLOAD
+ || insn.getOpcode() == Opcodes.LALOAD
+ || insn.getOpcode() == Opcodes.FLOAD
+ || insn.getOpcode() == Opcodes.FALOAD
+ || insn.getOpcode() == Opcodes.DLOAD
+ || insn.getOpcode() == Opcodes.DALOAD
+ || insn.getOpcode() == Opcodes.BALOAD
+ || insn.getOpcode() == Opcodes.CALOAD
+ || insn.getOpcode() == Opcodes.SALOAD;
+
+
+ }
+
+ /**
+ * @return true if the instruction is an ALOAD_0
+ */
+ public static boolean isThisInsn(AbstractInsnNode insn) {
+ return insn instanceof VarInsnNode
+ && insn.getOpcode() == Opcodes.ALOAD
+ && ((VarInsnNode)insn).var == 0;
+ }
}
diff --git
a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestPCEnhancerFindField.java
b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestPCEnhancerFindField.java
index 44077384f..069a54b55 100644
---
a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestPCEnhancerFindField.java
+++
b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestPCEnhancerFindField.java
@@ -40,7 +40,7 @@ public class TestPCEnhancerFindField
Project proj = new Project();
BCClass bc = proj.loadClass(getClass());
BCMethod meth = bc.getMethods("myMethod")[0];
- BCField field = PCEnhancer.getReturnedField(meth);
+ BCField field = PCEnhancer.getReturnedField_old(meth);
assertEquals("field", field.getName());
}
}
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
new file mode 100644
index 000000000..681ca6a11
--- /dev/null
+++
b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/enhance/TestSubclassValidator.java
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openjpa.enhance;
+
+import org.apache.openjpa.conf.OpenJPAConfiguration;
+import org.apache.openjpa.lib.log.Log;
+import org.apache.openjpa.lib.util.TemporaryClassLoader;
+import org.apache.openjpa.meta.ClassMetaData;
+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.junit.Test;
+
+import jakarta.persistence.Access;
+import jakarta.persistence.AccessType;
+import jakarta.persistence.Basic;
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import serp.bytecode.BCClass;
+import serp.bytecode.Project;
+
+public class TestSubclassValidator extends SingleEMFTestCase {
+ @Override
+ public void setUp() throws Exception {
+ setUp("openjpa.RuntimeUnenhancedClasses", "supported",
+ Department.class,
+ RuntimeTest2.class,
+ EnhancableGetterEntity.class,
+ UnenhancedPropertyAccess.class,
+ CLEAR_TABLES);
+ }
+
+ @Test
+ public void testBcSubclassValidator() throws Exception {
+ Project project = new Project();
+ TemporaryClassLoader tempCl = new
TemporaryClassLoader(this.getClass().getClassLoader());
+ final OpenJPAConfiguration conf = emf.getConfiguration();
+
+ final MetaDataRepository repos = conf.newMetaDataRepositoryInstance();
+ repos.setSourceMode(MetaDataModes.MODE_META);
+
+ final Log log = conf.getLog(OpenJPAConfiguration.LOG_ENHANCE);
+/*
+
+ {
+ final BCClass bcClass =
project.loadClass(Department.class.getName(), tempCl);
+ final ClassMetaData meta =
repos.getMetaData(tempCl.loadClass(Department.class.getName()), tempCl, false);
+ PCSubclassValidator subclassValidator = new
PCSubclassValidator(meta, bcClass, log, false);
+ subclassValidator.assertCanSubclass();
+ }
+
+ try {
+ final BCClass bcClass =
project.loadClass(RuntimeTest2.class.getName(), tempCl);
+ final ClassMetaData meta =
repos.getMetaData(tempCl.loadClass(RuntimeTest2.class.getName()), tempCl,
false);
+ PCSubclassValidator subclassValidator = new
PCSubclassValidator(meta, bcClass, log, true);
+ subclassValidator.assertCanSubclass();
+ fail("RuntimeTest2 validation should fail");
+ }
+ catch (UserException ue) {
+ // all fine
+ }
+*/
+
+ {
+ 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);
+ subclassValidator.assertCanSubclass();
+ }
+
+ {
+ 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);
+ subclassValidator.assertCanSubclass();
+ }
+ }
+
+ @Entity
+ @Access(AccessType.PROPERTY)
+ public static class EnhancableGetterEntity {
+
+ @Id
+ private String id;
+
+ @Basic
+ private String name;
+
+ @Basic
+ private String another;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+
+ public String getDifferent() {
+ return another;
+ }
+
+ public void setDifferent(String another) {
+ this.another = another;
+ }
+ }
+}
