From dda106b0af6f9155157b59a728678e878c9253ac Mon Sep 17 00:00:00 2001 From: Scott Marlow Date: Tue, 23 Apr 2019 16:00:23 -0400 Subject: [PATCH] WFLY-11991 Applications that extend certain Hibernate classes should be updated to use type SharedSessionContractImplementor instead of SessionImplementor --- .../_developer-guide/JPA_Reference_Guide.adoc | 2 +- .../Hibernate51CompatibilityTransformer.java | 229 +++--------------- .../org/jboss/as/hibernate/MethodAdapter.java | 20 +- 3 files changed, 48 insertions(+), 203 deletions(-) diff --git a/docs/src/main/asciidoc/_developer-guide/JPA_Reference_Guide.adoc b/docs/src/main/asciidoc/_developer-guide/JPA_Reference_Guide.adoc index 13bd700dec8a..3d64ecd5a575 100644 --- a/docs/src/main/asciidoc/_developer-guide/JPA_Reference_Guide.adoc +++ b/docs/src/main/asciidoc/_developer-guide/JPA_Reference_Guide.adoc @@ -711,7 +711,7 @@ Hibernate51CompatibilityTransformer transformed application classes in 'deployme |Hibernate51CompatibilityTransformer.disableAmbiguousChanges| disable transformation of org.hibernate.Query.setFirstResult(int) + org.hibernate.Query.setMaxResults(int), since these methods are in both Hibernate ORM 5.1.x + 5.3.x|false -|Hibernate51CompatibilityTransformer.showTransformedClassFolder| specifies the folder name to create bytecode level description of applications run through the transformer. If the transformer actually makes a change to your application, a (boolean) class variable will be added "$_org_jboss_as_hibernate_Hibernate51CompatibilityTransformer_transformed_$", that ensures that the application is only transformed once|disabled +|Hibernate51CompatibilityTransformer.showTransformedClassFolder| specifies the folder name to create bytecode level description of applications run through the transformer. The specified folder must already exist. If the transformer actually makes a change to your application, a (boolean) class variable will be added "$_org_jboss_as_hibernate_Hibernate51CompatibilityTransformer_transformed_$", that ensures that the application is only transformed once|disabled |  ---- ---- diff --git a/jpa/hibernate-transformer/src/main/java/org/jboss/as/hibernate/Hibernate51CompatibilityTransformer.java b/jpa/hibernate-transformer/src/main/java/org/jboss/as/hibernate/Hibernate51CompatibilityTransformer.java index ca88011d8a8a..7c1331e51543 100644 --- a/jpa/hibernate-transformer/src/main/java/org/jboss/as/hibernate/Hibernate51CompatibilityTransformer.java +++ b/jpa/hibernate-transformer/src/main/java/org/jboss/as/hibernate/Hibernate51CompatibilityTransformer.java @@ -18,6 +18,7 @@ package org.jboss.as.hibernate; +import static org.objectweb.asm.ClassWriter.COMPUTE_FRAMES; import static org.objectweb.asm.Opcodes.ACC_PUBLIC; import static org.objectweb.asm.Opcodes.ACC_STATIC; @@ -74,15 +75,27 @@ public static Hibernate51CompatibilityTransformer getInstance() { @Override public byte[] transform(final ClassLoader loader, final String className, final Class classBeingRedefined, final ProtectionDomain protectionDomain, final byte[] classfileBuffer) { + + // ignore these non-application classes + if(className.startsWith("org/jboss/arquillian") || + className.startsWith("org/jboss/as/arquillian") || + className.startsWith("org/junit") || + className.startsWith("org/jboss/shrinkwrap")) { + logger.debugf("Hibernate51CompatibilityTransformer ignoring class '%s' from '%s'", className, getModuleName(loader)); + return null; + } logger.debugf("Hibernate51CompatibilityTransformer transforming deployment class '%s' from '%s'", className, getModuleName(loader)); final Set parentClassesAndInterfaces = new HashSet<>(); + collectClassesAndInterfaces(parentClassesAndInterfaces, loader, className); logger.tracef("Class %s extends or implements %s", className, parentClassesAndInterfaces); + final boolean rewriteSessionImplementor = parentExpectsSharedSessionContractImplementor(parentClassesAndInterfaces); + final TransformedState transformedState = new TransformedState(); final ClassReader classReader = new ClassReader(classfileBuffer); - final ClassWriter classWriter = new ClassWriter(classReader, 0); + final ClassWriter classWriter = new ClassWriter(classReader, COMPUTE_FRAMES); ClassVisitor traceClassVisitor = classWriter; PrintWriter tracePrintWriter = null; try { @@ -128,203 +141,15 @@ public void visitEnd() { public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) { // Handle changing SessionImplementor parameter to SharedSessionContractImplementor - boolean rewriteSessionImplementor = false; final String descOrig = desc; logger.tracef("method %s, description %s, signature %s", name, desc, signature); - if (parentClassesAndInterfaces.contains("org/hibernate/usertype/UserType")) { - if (name.equals("nullSafeGet") && - "(Ljava/sql/ResultSet;[Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("nullSafeSet") && - "(Ljava/sql/PreparedStatement;Ljava/lang/Object;ILorg/hibernate/engine/spi/SessionImplementor;)V".equals(desc)) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/usertype/CompositeUserType")) { - if (name.equals("nullSafeGet") && - "(Ljava/sql/ResultSet;[Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("nullSafeSet") && - "(Ljava/sql/PreparedStatement;Ljava/lang/Object;ILorg/hibernate/engine/spi/SessionImplementor;)V".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("assemble") && - "(Ljava/io/Serializable;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("disassemble") && - "(Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/io/Serializable;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("replace") && - "(Ljava/lang/Object;Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/usertype/UserCollectionType")) { - if (name.equals("instantiate") && - "(Lorg/hibernate/engine/spi/SessionImplementor;Lorg/hibernate/persister/collection/CollectionPersister;)Lorg/hibernate/collection/spi/PersistentCollection;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("replaceElements") && - "(Ljava/lang/Object;Ljava/lang/Object;Lorg/hibernate/persister/collection/CollectionPersister;Ljava/lang/Object;Ljava/util/Map;Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("wrap") && - "(Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Lorg/hibernate/collection/spi/PersistentCollection;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/usertype/UserVersionType")) { - if (name.equals("seed") && - "(Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("next") && - "(Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/type/Type")) { - if (name.equals("assemble") && - "(Ljava/io/Serializable;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("disassemble") && - "(Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/io/Serializable;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("beforeAssemble") && - "(Ljava/io/Serializable;Lorg/hibernate/engine/spi/SessionImplementor;)V".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("hydrate") && - "(Ljava/sql/ResultSet;[Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("isDirty") && - "(Ljava/lang/Object;Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;)Z".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("isDirty") && - "(Ljava/lang/Object;Ljava/lang/Object;[ZLorg/hibernate/engine/spi/SessionImplementor;)Z".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("isModified") && - "(Ljava/lang/Object;Ljava/lang/Object;[ZLorg/hibernate/engine/spi/SessionImplementor;)Z".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("nullSafeGet") && - "(Ljava/sql/ResultSet;[Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("nullSafeGet") && - "(Ljava/sql/ResultSet;Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("nullSafeSet") && - "(Ljava/sql/PreparedStatement;Ljava/lang/Object;I[ZLorg/hibernate/engine/spi/SessionImplementor;)V".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("nullSafeSet") && - "(Ljava/sql/PreparedStatement;Ljava/lang/Object;ILorg/hibernate/engine/spi/SessionImplementor;)V".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("replace") && - "(Ljava/lang/Object;Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;Ljava/util/Map;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("replace") && - "(Ljava/lang/Object;Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;Ljava/util/Map;Lorg/hibernate/type/ForeignKeyDirection;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("resolve") && - "(Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("resolve") && - "(Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;Ljava/lang/Boolean;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("semiResolve") && - "(Ljava/lang/Object;Lorg/hibernate/engine/spi/SessionImplementor;Ljava/lang/Object;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/type/SingleColumnType")) { - if (name.equals("nullSafeGet") && - "(Ljava/sql/ResultSet;Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("get") && - "(Ljava/sql/ResultSet;Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("set") && - "(Ljava/sql/PreparedStatement;Ljava/lang/Object;ILorg/hibernate/engine/spi/SessionImplementor;)V".equals(desc)) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - if (parentClassesAndInterfaces.contains("org/hibernate/type/AbstractStandardBasicType")) { - if (name.equals("get") && - "(Ljava/sql/ResultSet;Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("nullSafeGet") && - "(Ljava/sql/ResultSet;Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;)Ljava/lang/Object;".equals(desc)) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("set") && - "(Ljava/sql/PreparedStatement;Ljava/lang/Object;ILorg/hibernate/engine/spi/SessionImplementor;)V".equals(desc)) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/type/ProcedureParameterExtractionAware")) { - if (name.equals("extract") - && (desc.startsWith( - "(Ljava/sql/CallableStatement;ILorg/hibernate/engine/spi/SessionImplementor;)") - || desc.startsWith( - "(Ljava/sql/CallableStatement;[Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;)"))) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/type/ProcedureParameterNamedBinder")) { - if (name.equals("nullSafeSet") && - "(Ljava/sql/CallableStatement;Ljava/lang/Object;Ljava/lang/String;Lorg/hibernate/engine/spi/SessionImplementor;)V".equals(desc)) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/type/VersionType")) { - if (name.equals("seed") && - desc.startsWith("(Lorg/hibernate/engine/spi/SessionImplementor;)")) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("next") && - desc.contains("Lorg/hibernate/engine/spi/SessionImplementor;")) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (parentClassesAndInterfaces.contains("org/hibernate/collection/spi/PersistentCollection")) { - if (name.equals("unsetSession") && - desc.equals("(Lorg/hibernate/engine/spi/SessionImplementor;)Z")) { - desc = replaceSessionImplementor(desc); - } else if (name.equals("setCurrentSession") && - desc.equals("(Lorg/hibernate/engine/spi/SessionImplementor;)Z")) { - desc = replaceSessionImplementor(desc); - } - - rewriteSessionImplementor = true; - } - - if (rewriteSessionImplementor && name.equals("")) { - // update constructor methods to use SharedSessionContractImplementor instead of SessionImplementor + // If application class has certain checked Hibernate ORM classes as super class, + // replace method parameters of type 'SessionImplementor', to instead use type 'SharedSessionContractImplementor'. + if (rewriteSessionImplementor && desc.contains("org/hibernate/engine/spi/SessionImplementor")) { desc = replaceSessionImplementor(desc); } - if (descOrig != desc) { // if we are changing from type SessionImplementor to SharedSessionContractImplementor // mark the class as transformed transformedState.setClassTransformed(true); @@ -334,7 +159,7 @@ public MethodVisitor visitMethod(int access, String name, String desc, String si } }, 0); if (!transformedState.transformationsMade()) { - // no change was actually made, indicate so by returning null + // no change was made, indicate so by returning null return null; } return classWriter.toByteArray(); @@ -360,11 +185,26 @@ private static String replaceSessionImplementor(String desc) { "Lorg/hibernate/engine/spi/SharedSessionContractImplementor;"); } + private boolean parentExpectsSharedSessionContractImplementor(Set parentClassesAndInterfaces) { + return (parentClassesAndInterfaces.contains("org/hibernate/usertype/UserType") + || parentClassesAndInterfaces.contains("org/hibernate/usertype/CompositeUserType") + || parentClassesAndInterfaces.contains("org/hibernate/usertype/UserCollectionType") + || parentClassesAndInterfaces.contains("org/hibernate/usertype/UserVersionType") + || parentClassesAndInterfaces.contains("org/hibernate/type/Type") + || parentClassesAndInterfaces.contains("org/hibernate/type/SingleColumnType") + || parentClassesAndInterfaces.contains("org/hibernate/type/AbstractStandardBasicType") + || parentClassesAndInterfaces.contains("org/hibernate/type/ProcedureParameterExtractionAware") + || parentClassesAndInterfaces.contains("org/hibernate/type/ProcedureParameterNamedBinder") + || parentClassesAndInterfaces.contains("org/hibernate/type/VersionType") + || parentClassesAndInterfaces.contains("org/hibernate/collection/spi/PersistentCollection")); + } + private void collectClassesAndInterfaces(Set classesAndInterfaces, ClassLoader classLoader, String className) { if (className == null || "java/lang/Object".equals(className)) { return; } + try (InputStream is = classLoader.getResourceAsStream(className.replace('.', '/') + ".class")) { ClassReader classReader = new ClassReader(is); classReader.accept(new ClassVisitor(useASM7 ? Opcodes.ASM7 : Opcodes.ASM6) { @@ -392,7 +232,7 @@ public void visitInnerClass(String name, String outerName, String innerName, int }, 0); } catch (IOException e) { - logger.warn("Unable to open class file %1$s", className, e); + logger.debugf("Unable to open class file %s, due to exception %s", className, e); } } @@ -407,5 +247,6 @@ private static int getMajorJavaVersion() { } return major; } + } diff --git a/jpa/hibernate-transformer/src/main/java/org/jboss/as/hibernate/MethodAdapter.java b/jpa/hibernate-transformer/src/main/java/org/jboss/as/hibernate/MethodAdapter.java index d28a701a1338..2b561ded57a4 100644 --- a/jpa/hibernate-transformer/src/main/java/org/jboss/as/hibernate/MethodAdapter.java +++ b/jpa/hibernate-transformer/src/main/java/org/jboss/as/hibernate/MethodAdapter.java @@ -36,7 +36,6 @@ class MethodAdapter extends MethodVisitor { public static final BasicLogger logger = Logger.getLogger("org.jboss.as.hibernate.transformer"); private final boolean rewriteSessionImplementor; private final TransformedState transformedState; - private final MethodVisitor mv; private final String moduleName; private final String className; @@ -44,7 +43,6 @@ class MethodAdapter extends MethodVisitor { final String moduleName, final String className, TransformedState transformedState) { super(api, mv); this.rewriteSessionImplementor = rewriteSessionImplementor; - this.mv = mv; this.moduleName = moduleName; this.className = className; this.transformedState = transformedState; @@ -58,14 +56,14 @@ class MethodAdapter extends MethodVisitor { public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) { if (rewriteSessionImplementor && hasSessionImplementor(desc) && - (opcode == Opcodes.INVOKESPECIAL || opcode == Opcodes.INVOKEVIRTUAL)) { + methodCall(opcode)) { // if we have a user type calling a method from org.hibernate, we rewrite it to use SharedSessionContractImplementor logger.debugf("Deprecated Hibernate51CompatibilityTransformer transformed application classes in '%s', " + "class '%s' is calling method %s.%s, which must be changed to use SharedSessionContractImplementor as parameter.", moduleName, className, owner, name); mv.visitMethodInsn(opcode, owner, name, replaceSessionImplementor(desc), itf); transformedState.setClassTransformed(true); - } else if (opcode == Opcodes.INVOKEINTERFACE && + } else if (methodCall(opcode) && (owner.equals("org/hibernate/Session") || owner.equals("org/hibernate/BasicQueryContract")) && name.equals("getFlushMode") && desc.equals("()Lorg/hibernate/FlushMode;")) { logger.debugf("Deprecated Hibernate51CompatibilityTransformer transformed application classes in '%s', " + @@ -74,7 +72,7 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, name = "getHibernateFlushMode"; mv.visitMethodInsn(opcode, owner, name, desc, itf); transformedState.setClassTransformed(true); - } else if (opcode == Opcodes.INVOKEINTERFACE && + } else if (methodCall(opcode) && owner.equals("org/hibernate/Query") && name.equals("getFirstResult") && desc.equals("()Ljava/lang/Integer;")) { @@ -86,7 +84,7 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, name = "getHibernateFirstResult"; mv.visitMethodInsn(opcode, owner, name, desc, itf); transformedState.setClassTransformed(true); - } else if (opcode == Opcodes.INVOKEINTERFACE && + } else if (methodCall(opcode) && owner.equals("org/hibernate/Query") && name.equals("getMaxResults") && desc.equals("()Ljava/lang/Integer;")) { @@ -98,7 +96,7 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, name = "getHibernateMaxResults"; mv.visitMethodInsn(opcode, owner, name, desc, itf); transformedState.setClassTransformed(true); - } else if (!disableAmbiguousChanges && opcode == Opcodes.INVOKEINTERFACE && + } else if (!disableAmbiguousChanges && methodCall(opcode) && owner.equals("org/hibernate/Query") && name.equals("setFirstResult") && desc.equals("(I)Lorg/hibernate/Query;")) { @@ -110,7 +108,7 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, name = "setHibernateFirstResult"; mv.visitMethodInsn(opcode, owner, name, desc, itf); transformedState.setClassTransformed(true); - } else if (!disableAmbiguousChanges && opcode == Opcodes.INVOKEINTERFACE && + } else if (!disableAmbiguousChanges && methodCall(opcode) && owner.equals("org/hibernate/Query") && name.equals("setMaxResults") && desc.equals("(I)Lorg/hibernate/Query;")) { @@ -124,7 +122,13 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, } else { mv.visitMethodInsn(opcode, owner, name, desc, itf); } + } + private boolean methodCall(int opcode) { + return opcode == Opcodes.INVOKESPECIAL || + opcode == Opcodes.INVOKEVIRTUAL || + opcode == Opcodes.INVOKEINTERFACE || + opcode == Opcodes.INVOKESTATIC; } @Override