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 a25ad4fc95bc0d23ba22634a771c330df363017b
Author: Mark Struberg <strub...@apache.org>
AuthorDate: Sun Jul 16 17:24:42 2023 +0200
OPENJPA-2911 getIdClassConstructorParmOrder in ASM
---
.../org/apache/openjpa/enhance/PCEnhancer.java | 156 +++++++--------------
.../org/apache/openjpa/util/asm/AsmHelper.java | 49 +++++--
.../org/apache/openjpa/enhance/TestAsmAdaptor.java | 9 ++
3 files changed, 102 insertions(+), 112 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 7ea61e1c0..dee730d1c 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
@@ -31,6 +31,7 @@ import java.io.ObjectStreamClass;
import java.io.ObjectStreamException;
import java.io.OutputStream;
import java.io.Serializable;
+import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
@@ -3592,19 +3593,11 @@ public class PCEnhancer {
Type.LONG_TYPE.getDescriptor(),
null, uid);
pc.getClassNode().fields.add(serVersField);
-
- /* should be done by ASM FieldNode already
- Code code = getOrCreateClassInitCode(false);
- code.beforeFirst();
- code.constant().setValue(uid.longValue());
- code.putstatic().setField(field);
- */
}
}
- MethodNode writeObjectMeth =
AsmHelper.getMethodNode(pc.getClassNode(), "writeObject",
- void.class,
ObjectOutputStream.class)
- .orElse(null);
+ MethodNode writeObjectMeth =
AsmHelper.getMethodNode(pc.getClassNode(), "writeObject",void.class,
ObjectOutputStream.class)
+ .orElse(null);
boolean full = writeObjectMeth == null;
@@ -4320,32 +4313,6 @@ public class PCEnhancer {
return false;
}
- private boolean setVisibilityToSuperMethod(BCMethod method) {
- BCMethod[] methods = _managedType.getMethods(method.getName(),
- method.getParamTypes());
- if (methods.length == 0)
- throw new UserException(_loc.get("no-accessor",
- _managedType.getName(),
method.getName()));
- BCMethod superMeth = methods[0];
- if (superMeth.isPrivate()) {
- method.makePrivate();
- return true;
- }
- else if (superMeth.isPackage()) {
- method.makePackage();
- return true;
- }
- else if (superMeth.isProtected()) {
- method.makeProtected();
- return true;
- }
- else if (superMeth.isPublic()) {
- method.makePublic();
- return true;
- }
- return false;
- }
-
/**
* Adds a non-static getter that delegates to the super methods, and
* performs any necessary field tracking.
@@ -5363,22 +5330,6 @@ public class PCEnhancer {
}
}
-
- /**
- * Add the {@link Instruction}s to load the instance to modify onto the
- * stack, and return it. If <code>forStatic</code> is set, then
- * <code>code</code> is in an accessor method or another static method;
- * otherwise, it is in one of the PC-specified methods.
- *
- * @return the first instruction added to <code>code</code>.
- */
- @Deprecated
- private Instruction loadManagedInstance(Code code, boolean forStatic,
FieldMetaData fmd) {
- if (forStatic && isFieldAccess(fmd))
- return code.aload().setParam(0);
- return code.aload().setThis();
- }
-
/**
* Add the {@link Instruction}s to load the instance to modify onto the
* stack, and return it. If <code>forStatic</code> is set, then
@@ -5395,18 +5346,6 @@ public class PCEnhancer {
return new VarInsnNode(Opcodes.ALOAD, 0); // this
}
- /**
- * Add the {@link Instruction}s to load the instance to modify onto the
- * stack, and return it. This method should not be used to load static
- * fields.
- *
- * @return the first instruction added to <code>code</code>.
- */
- @Deprecated
- private Instruction loadManagedInstance(Code code, boolean forStatic) {
- return loadManagedInstance(code, forStatic, null);
- }
-
private int getAccessorParameterOffset(FieldMetaData fmd) {
return isFieldAccess(fmd) ? 1 : 0;
}
@@ -5840,67 +5779,78 @@ public class PCEnhancer {
* a matching constructor for the provided pk fields. If a match is
found, it returns
* the order (relative to the field metadata) of the constructor
parameters. If a match
* is not found, returns null.
+ *
+ * We use byte code analysis to find the fields the ct works on.
*/
- private int[] getIdClassConstructorParmOrder(Class<?> oidType,
ArrayList<Integer> pkfields,
- FieldMetaData[] fmds) {
- Project project = new Project();
- BCClass bc = project.loadClass(oidType);
- BCMethod[] methods = bc.getDeclaredMethods("<init>");
- if (methods == null || methods.length == 0) {
+ private int[] getIdClassConstructorParmOrder(Class<?> oidType,
List<Integer> pkfields, FieldMetaData[] fmds) {
+ final ClassNode classNode = AsmHelper.readClassNode(oidType);
+ final List<MethodNode> cts = classNode.methods.stream()
+ .filter(m -> "<init>".equals(m.name))
+ .collect(Collectors.toList());
+
+ if (cts.isEmpty()) {
return null;
}
int parmOrder[] = new int[pkfields.size()];
- for (BCMethod method : methods) {
- // constructor must be public
- if (!method.isPublic()) {
+ for (MethodNode ct : cts) {
+ if ((ct.access & Opcodes.ACC_PUBLIC) == 0) {
+ // ignore non public constructors
continue;
}
- Class<?>[] parmTypes = method.getParamTypes();
+ Type[] argTypes = Type.getArgumentTypes(ct.desc);
+
// make sure the constructors have the same # of parms as
// the number of pk fields
- if (parmTypes.length != pkfields.size()) {
+ if (listSize(pkfields) != argTypes.length) {
continue;
}
int parmOrderIndex = 0;
- Code code = method.getCode(false);
- Instruction[] ins = code.getInstructions();
- for (int i = 0; i < ins.length; i++) {
- if (ins[i] instanceof PutFieldInstruction) {
- PutFieldInstruction pfi = (PutFieldInstruction)ins[i];
- for (int j = 0; j < pkfields.size(); j++) {
- int fieldNum = pkfields.get(j);
- // Compare the field being set with the current pk
field
- String parmName = fmds[fieldNum].getName();
- Class<?> parmType = fmds[fieldNum].getType();
- if (parmName.equals(pfi.getFieldName())) {
- // backup and examine the load instruction parm
- if (i > 0 && ins[i-1] instanceof LoadInstruction) {
- LoadInstruction li = (LoadInstruction)ins[i-1];
- // Get the local index from the instruction.
This will be the index
- // of the constructor parameter. must be less
than or equal to the
- // max parm index to prevent from picking up
locals that could have
- // been produced within the constructor. Also
make sure the parm type
- // matches the fmd type
- int parm = li.getLocal();
- if (parm <= pkfields.size() &&
parmTypes[parm-1].equals(parmType)) {
- parmOrder[parmOrderIndex] = fieldNum;
- parmOrderIndex++;
- }
- } else {
- // Some other instruction found. can't make a
determination of which local/parm
- // is being used on the putfield.
- break;
+ AbstractInsnNode insn = ct.instructions.getFirst();
+ // skip to the next PUTFIELD instruction
+ while ((insn = searchNextInstruction(insn, i -> i.getOpcode() ==
Opcodes.PUTFIELD)) != null) {
+ FieldInsnNode putField = (FieldInsnNode) insn;
+ for (int i = 0; i < pkfields.size(); i++) {
+ int fieldNum = pkfields.get(i);
+ // Compare the field being set with the current pk field
+ String parmName = fmds[fieldNum].getName();
+ Class<?> parmType = fmds[fieldNum].getType();
+ if (parmName.equals(putField.name)) {
+ // backup and examine the load instruction parm
+ if (AsmHelper.isLoadInsn(insn.getPrevious())) {
+ // Get the local index from the instruction. This
will be the index
+ // of the constructor parameter. must be less
than or equal to the
+ // max parm index to prevent from picking up
locals that could have
+ // been produced within the constructor. Also
make sure the parm type
+ // matches the fmd type
+
+ VarInsnNode loadInsn = (VarInsnNode)
insn.getPrevious();
+
+ int parm = AsmHelper.getParamIndex(ct,
loadInsn.var);
+ if (parm < pkfields.size() &&
argTypes[parm].equals(Type.getType(parmType))) {
+ parmOrder[parmOrderIndex] = fieldNum;
+ parmOrderIndex++;
}
+ } else {
+ // Some other instruction found. can't make a
determination of which local/parm
+ // is being used on the putfield.
+ break;
}
}
}
+
+ insn = insn.getNext();
}
if (parmOrderIndex == pkfields.size()) {
return parmOrder;
}
}
+
return null;
}
+
+ private int listSize(Collection<?> coll) {
+ return coll == null ? 0 : coll.size();
+ }
}
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 6c7f8c919..a5e55ca34 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
@@ -57,15 +57,22 @@ public final class AsmHelper {
/**
* Read the binary bytecode from the class with the given name
- * @param classBytes the binary of the class
+ * @param clazz the class to read into the ClassNode
* @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;
+ public static ClassNode readClassNode(Class<?> clazz) {
+ int extPos = clazz.getName().lastIndexOf('.') + 1;
+ String className = clazz.getName().substring(extPos);
+ try (InputStream in = clazz.getResourceAsStream(className + ".class"))
{
+ ClassReader cr = new ClassReader(in);
+ ClassNode classNode = new ClassNode();
+ cr.accept(classNode, 0);
+
+ return classNode;
+ }
+ catch (IOException ioe) {
+ throw new RuntimeException(ioe);
+ }
}
/**
@@ -432,6 +439,30 @@ public final class AsmHelper {
return types;
}
+ /**
+ * Determine the 0-based index of the parameter of LOAD or STORE position
varPos
+ * @param methodNode
+ * @param varPos the position on the stack
+ * @return the index of the parameter which corresponds to this varPos
+ */
+ public static int getParamIndex(MethodNode methodNode, int varPos) {
+ boolean isStatic = (methodNode.access & Opcodes.ACC_STATIC) > 0;
+ if (!isStatic) {
+ // only static methods start with 0
+ // non-static have the this* on pos 0.
+ varPos--;
+ }
+ final Type[] paramTypes = Type.getArgumentTypes(methodNode.desc);
+ int pos = 0;
+ for (int i=0; i<paramTypes.length; i++) {
+ if (pos==varPos) {
+ return i;
+ }
+ pos += paramTypes[i].getSize();
+ }
+ throw new IllegalArgumentException("Cannot determine parameter
position " + varPos + " for method " + methodNode.name);
+ }
+
/**
* @return true if the instruction is an LOAD instruction
*/
@@ -448,10 +479,9 @@ public final class AsmHelper {
|| insn.getOpcode() == Opcodes.BALOAD
|| insn.getOpcode() == Opcodes.CALOAD
|| insn.getOpcode() == Opcodes.SALOAD;
-
-
}
+
/**
* @return true if the instruction is an ALOAD_0
*/
@@ -627,4 +657,5 @@ public final class AsmHelper {
return null;
}
}
+
}
diff --git
a/openjpa-kernel/src/test/java/org/apache/openjpa/enhance/TestAsmAdaptor.java
b/openjpa-kernel/src/test/java/org/apache/openjpa/enhance/TestAsmAdaptor.java
index 970b98143..0d928d3ce 100644
---
a/openjpa-kernel/src/test/java/org/apache/openjpa/enhance/TestAsmAdaptor.java
+++
b/openjpa-kernel/src/test/java/org/apache/openjpa/enhance/TestAsmAdaptor.java
@@ -19,6 +19,7 @@
package org.apache.openjpa.enhance;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -26,10 +27,18 @@ import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import org.apache.openjpa.util.QueryException;
+import org.apache.openjpa.util.asm.AsmHelper;
+import org.apache.xbean.asm9.tree.ClassNode;
import org.junit.Test;
public class TestAsmAdaptor
{
+ @Test
+ public void testAsmHelper() throws Exception {
+ final ClassNode classNode =
AsmHelper.readClassNode(QueryException.class);
+ assertNotNull(classNode);
+ }
@Test
public void isEnhanced()
{
