diff --git a/ebean-agent/pom.xml b/ebean-agent/pom.xml index f3bd6944..23c3f01b 100644 --- a/ebean-agent/pom.xml +++ b/ebean-agent/pom.xml @@ -59,7 +59,7 @@ io.avaje junit - 1.1 + 1.5 test @@ -92,23 +92,9 @@ - org.avaje.composite - logback - 1.1 - test - - - - org.mockito - mockito-core - 3.11.2 - test - - - - org.avaje.composite - composite-testing - 3.1 + ch.qos.logback + logback-classic + 1.5.17 test diff --git a/ebean-agent/src/main/java/io/ebean/enhance/EnhancementException.java b/ebean-agent/src/main/java/io/ebean/enhance/EnhancementException.java new file mode 100644 index 00000000..dd72e2e2 --- /dev/null +++ b/ebean-agent/src/main/java/io/ebean/enhance/EnhancementException.java @@ -0,0 +1,12 @@ +package io.ebean.enhance; + +/** + * Some unexpected bytecode that can't be supported. + *

+ * For example, some unsupported OneToMany collection initialisation. + */ +public class EnhancementException extends RuntimeException { + public EnhancementException(String message) { + super(message); + } +} diff --git a/ebean-agent/src/main/java/io/ebean/enhance/Transformer.java b/ebean-agent/src/main/java/io/ebean/enhance/Transformer.java index 7b3b605a..afd2a1c4 100644 --- a/ebean-agent/src/main/java/io/ebean/enhance/Transformer.java +++ b/ebean-agent/src/main/java/io/ebean/enhance/Transformer.java @@ -191,6 +191,9 @@ public byte[] transform(ClassLoader loader, String className, Class classBein // the class is an interface log(8, className, "No Enhancement required " + e.getMessage()); return null; + } catch (EnhancementException e) { + enhanceContext.log(className, "Transform error " + e.getMessage()); + throw e; } catch (IllegalArgumentException | IllegalStateException e) { log(2, className, "No enhancement on class due to " + e); return null; diff --git a/ebean-agent/src/main/java/io/ebean/enhance/ant/OfflineFileTransform.java b/ebean-agent/src/main/java/io/ebean/enhance/ant/OfflineFileTransform.java index b7cd1fce..ab17a0d6 100644 --- a/ebean-agent/src/main/java/io/ebean/enhance/ant/OfflineFileTransform.java +++ b/ebean-agent/src/main/java/io/ebean/enhance/ant/OfflineFileTransform.java @@ -1,5 +1,6 @@ package io.ebean.enhance.ant; +import io.ebean.enhance.EnhancementException; import io.ebean.enhance.Transformer; import io.ebean.enhance.common.InputStreamTransform; @@ -60,7 +61,6 @@ private String trimSlash(String dir) { * Process the packageNames as comma delimited string. */ public void process(String packageNames) { - if (packageNames == null) { // just process all directories processPackage(""); @@ -69,7 +69,6 @@ public void process(String packageNames) { Set pkgNames = new LinkedHashSet<>(); Collections.addAll(pkgNames, packageNames.split(",")); - process(pkgNames); } @@ -81,7 +80,6 @@ public void process(String packageNames) { *

*/ public void process(Set packageNames) { - if (packageNames == null || packageNames.isEmpty()) { // just process all directories inputStreamTransform.log(2, "processing all directories (as no explicit packages)"); @@ -90,9 +88,7 @@ public void process(Set packageNames) { } for (String pkgName : packageNames) { - String pkg = pkgName.trim().replace('.', '/'); - if (pkg.endsWith("**")) { pkg = pkg.substring(0, pkg.length() - 2); } else if (pkg.endsWith("*")) { @@ -100,7 +96,6 @@ public void process(Set packageNames) { } pkg = trimSlash(pkg); - processPackage(pkg); } } @@ -142,15 +137,20 @@ private void processPackage(String dir) { } private void transformFile(File file) throws IOException, IllegalClassFormatException { - String className = getClassName(file); - - byte[] result = inputStreamTransform.transform(className, file); - - if (result != null) { - InputStreamTransform.writeBytes(result, file); - if (listener != null && logLevel > 0) { - listener.logEvent("Enhanced " + file); + try { + byte[] result = inputStreamTransform.transform(className, file); + if (result != null) { + InputStreamTransform.writeBytes(result, file); + if (listener != null && logLevel > 0) { + listener.logEvent("Enhanced " + file); + } + } + } catch (EnhancementException e) { + if (listener != null) { + listener.logError("Error enhancing class " + className + " " + e.getMessage()); + } else { + throw e; } } } diff --git a/ebean-agent/src/main/java/io/ebean/enhance/common/AgentManifest.java b/ebean-agent/src/main/java/io/ebean/enhance/common/AgentManifest.java index adb22ecc..d7462e79 100644 --- a/ebean-agent/src/main/java/io/ebean/enhance/common/AgentManifest.java +++ b/ebean-agent/src/main/java/io/ebean/enhance/common/AgentManifest.java @@ -28,6 +28,7 @@ public final class AgentManifest { private boolean transientInternalFields; private boolean transientInit; private boolean transientInitThrowError; + private boolean unsupportedInitThrowError = true; private boolean checkNullManyFields = true; private boolean enableProfileLocation = true; private boolean enableEntityFieldAccess; @@ -151,6 +152,10 @@ public boolean isTransientInitThrowError() { return transientInitThrowError; } + public boolean isUnsupportedInitThrowError() { + return unsupportedInitThrowError; + } + /** * Return true if we should use transient internal fields. */ @@ -315,6 +320,7 @@ private void readOptions(Attributes attributes) { transientInternalFields = bool("transient-internal-fields", transientInternalFields, attributes); checkNullManyFields = bool("check-null-many-fields", checkNullManyFields, attributes); allowNullableDbArray = bool("allow-nullable-dbarray", allowNullableDbArray, attributes); + unsupportedInitThrowError = bool("unsupported-init-error", unsupportedInitThrowError, attributes); } private boolean bool(String key, boolean defaultValue, Attributes attributes) { diff --git a/ebean-agent/src/main/java/io/ebean/enhance/common/ClassMeta.java b/ebean-agent/src/main/java/io/ebean/enhance/common/ClassMeta.java index 85902b71..3613705a 100644 --- a/ebean-agent/src/main/java/io/ebean/enhance/common/ClassMeta.java +++ b/ebean-agent/src/main/java/io/ebean/enhance/common/ClassMeta.java @@ -50,6 +50,7 @@ public class ClassMeta { /** * If enhancement is adding a default constructor - only default constructors are supported initialising transient fields. */ + private final Set unsupportedInitMany = new LinkedHashSet<>(); private final Set unsupportedTransientInitialisation = new LinkedHashSet<>(); private final Map transientInitCode = new LinkedHashMap<>(); private final LinkedHashMap fields = new LinkedHashMap<>(); @@ -447,6 +448,20 @@ public Collection transientInit() { return transientInitCode.values(); } + public void addUnsupportedInitMany(String name) { + unsupportedInitMany.add(name); + } + + public boolean hasUnsupportedInitMany() { + return !unsupportedInitMany.isEmpty(); + } + + public String initFieldErrorMessage() { + return "ERROR: Unsupported initialisation of @OneToMany or @ManyToMany on: " + + className + " fields: " + unsupportedInitMany + + " Refer: https://ebean.io/docs/trouble-shooting#initialisation-error"; + } + public void addUnsupportedTransientInit(String name) { unsupportedTransientInitialisation.add(name); } diff --git a/ebean-agent/src/main/java/io/ebean/enhance/common/EnhanceContext.java b/ebean-agent/src/main/java/io/ebean/enhance/common/EnhanceContext.java index 7c4aa028..bfc08dc8 100644 --- a/ebean-agent/src/main/java/io/ebean/enhance/common/EnhanceContext.java +++ b/ebean-agent/src/main/java/io/ebean/enhance/common/EnhanceContext.java @@ -344,6 +344,10 @@ public boolean isTransientInitThrowError() { return manifest.isTransientInitThrowError(); } + public boolean isUnsupportedInitThrowError() { + return manifest.isUnsupportedInitThrowError(); + } + /** * Return true if internal ebean fields in entity classes should be transient. */ diff --git a/ebean-agent/src/main/java/io/ebean/enhance/entity/ClassAdapterEntity.java b/ebean-agent/src/main/java/io/ebean/enhance/entity/ClassAdapterEntity.java index 558311cd..cf02dc2c 100644 --- a/ebean-agent/src/main/java/io/ebean/enhance/entity/ClassAdapterEntity.java +++ b/ebean-agent/src/main/java/io/ebean/enhance/entity/ClassAdapterEntity.java @@ -1,5 +1,6 @@ package io.ebean.enhance.entity; +import io.ebean.enhance.EnhancementException; import io.ebean.enhance.asm.AnnotationVisitor; import io.ebean.enhance.asm.ClassVisitor; import io.ebean.enhance.asm.FieldVisitor; @@ -205,6 +206,14 @@ public void visitEnd() { if (!classMeta.isEntityEnhancementRequired()) { throw new NoEnhancementRequiredException(); } + if (classMeta.hasUnsupportedInitMany()) { + if (classMeta.context().isUnsupportedInitThrowError()) { + throw new EnhancementException(classMeta.initFieldErrorMessage()); + } else { + // the default constructor being added will leave some transient fields uninitialised (null, 0, false etc) + System.err.println(classMeta.initFieldErrorMessage()); + } + } if (!classMeta.hasStaticInit()) { IndexFieldWeaver.addPropertiesInit(cv, classMeta); } diff --git a/ebean-agent/src/main/java/io/ebean/enhance/entity/ConstructorDeferredCode.java b/ebean-agent/src/main/java/io/ebean/enhance/entity/ConstructorDeferredCode.java index 96a7f704..61cf0f20 100644 --- a/ebean-agent/src/main/java/io/ebean/enhance/entity/ConstructorDeferredCode.java +++ b/ebean-agent/src/main/java/io/ebean/enhance/entity/ConstructorDeferredCode.java @@ -45,7 +45,8 @@ enum State { INVOKE_SPECIAL, KT_CHECKCAST, // optional kotlin state KT_LABEL, // optional kotlin state - KT_EMPTYLIST + EMPTY, + MAYBE_UNSUPPORTED } private static final ALoad ALOAD_INSTRUCTION = new ALoad(); @@ -130,13 +131,28 @@ boolean deferVisitMethodInsn(int opcode, String owner, String name, String desc, stateInitialiseType = owner; return true; } - if (opcode == INVOKESTATIC && stateAload() && kotlinEmptyList(owner, name, desc)) { - codes.add(new NoArgInit(opcode, owner, name, desc, itf)); - state = State.KT_EMPTYLIST; - stateInitialiseType = "java/util/ArrayList"; - return true; + if (opcode == INVOKESTATIC && stateAload()) { + if (kotlinEmptyList(owner, name, desc) || emptyList(owner, name, desc)) { + codes.add(new NoArgInit(opcode, owner, name, desc, itf)); + state = State.EMPTY; + stateInitialiseType = "java/util/ArrayList"; + return true; + } + if (emptySet(owner, name, desc)) { + codes.add(new NoArgInit(opcode, owner, name, desc, itf)); + state = State.EMPTY; + stateInitialiseType = "java/util/LinkedHashSet"; + return true; + } + if (emptyMap(owner, name, desc)) { + codes.add(new NoArgInit(opcode, owner, name, desc, itf)); + state = State.EMPTY; + stateInitialiseType = "java/util/LinkedHashMap"; + return true; + } } flush(); + state = State.MAYBE_UNSUPPORTED; return false; } @@ -144,6 +160,24 @@ private boolean isNoArgInit(String name, String desc) { return name.equals(INIT) && desc.equals(NOARG_VOID); } + private boolean emptyList(String owner, String name, String desc) { + return desc.equals("()Ljava/util/List;") + && ((owner.equals("java/util/List") && name.equals("of")) + || (owner.equals("java/util/Collections") && name.equals("emptyList"))); + } + + private boolean emptySet(String owner, String name, String desc) { + return desc.equals("()Ljava/util/Set;") + && ((owner.equals("java/util/Set") && name.equals("of")) + || (owner.equals("java/util/Collections") && name.equals("emptySet"))); + } + + private boolean emptyMap(String owner, String name, String desc) { + return desc.equals("()Ljava/util/Map;") + && ((owner.equals("java/util/Map") && name.equals("of")) + || (owner.equals("java/util/Collections") && name.equals("emptyMap"))); + } + private boolean kotlinEmptyList(String owner, String name, String desc) { return owner.equals("kotlin/collections/CollectionsKt") && name.equals("emptyList") @@ -155,6 +189,12 @@ private boolean kotlinEmptyList(String owner, String name, String desc) { */ boolean consumeVisitFieldInsn(int opcode, String owner, String name, String desc) { if (opcode == PUTFIELD) { + if (state == State.MAYBE_UNSUPPORTED && meta.isConsumeInitMany(name)) { + // a OneToMany/ManyToMany is initialised in an unsupported manor + meta.addUnsupportedInitMany(name); + flush(); + return false; + } if (stateConsumeDeferred()) { if (meta.isConsumeInitMany(name) && isConsumeManyType()) { if (meta.isLog(3)) { @@ -232,17 +272,17 @@ private boolean stateInvokeSpecial() { } private boolean stateConsumeDeferred() { - return state == State.INVOKE_SPECIAL || state == State.KT_CHECKCAST || state == State.KT_EMPTYLIST; + return state == State.INVOKE_SPECIAL || state == State.KT_CHECKCAST || state == State.EMPTY; } /** * Return true if the type being initialised is valid for auto initialisation of ToMany or DbArray. */ private boolean isConsumeManyType() { - return ("java/util/ArrayList".equals(stateInitialiseType) + return "java/util/ArrayList".equals(stateInitialiseType) || "java/util/LinkedHashSet".equals(stateInitialiseType) - || "java/util/HashSet".equals(stateInitialiseType)); - //|| "java/util/LinkedHashMap".equals(stateInitialiseType) + || "java/util/HashSet".equals(stateInitialiseType) + || "java/util/LinkedHashMap".equals(stateInitialiseType); //|| "java/util/HashMap".equals(stateInitialiseType)); } diff --git a/test/pom.xml b/test/pom.xml index 6da12b36..2f6459a2 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -14,6 +14,7 @@ 13.4.0 + 11 diff --git a/test/src/test/java/test/enhancement/WithInitialisedCollections2Test.java b/test/src/test/java/test/enhancement/WithInitialisedCollections2Test.java new file mode 100644 index 00000000..f31e8b11 --- /dev/null +++ b/test/src/test/java/test/enhancement/WithInitialisedCollections2Test.java @@ -0,0 +1,40 @@ +package test.enhancement; + +import io.ebean.common.BeanList; +import io.ebean.common.BeanMap; +import io.ebean.common.BeanSet; +import org.junit.jupiter.api.Test; +import test.model.Contact; +import test.model.WithInitialisedCollections2; + +import static org.assertj.core.api.Assertions.assertThat; + +class WithInitialisedCollections2Test extends BaseTest { + + @Test + void oneToMany_initialisationCode_expect_removed() { + WithInitialisedCollections2 bean = new WithInitialisedCollections2(); + + assertThat(bean.listOf()).isInstanceOf(BeanList.class); + assertThat(bean.listCollEmpty()).isInstanceOf(BeanList.class); + assertThat(bean.setOf()).isInstanceOf(BeanSet.class); + assertThat(bean.setCollEmpty()).isInstanceOf(BeanSet.class); + assertThat(bean.mapOf()).isInstanceOf(BeanMap.class); + assertThat(bean.mapCollEmpty()).isInstanceOf(BeanMap.class); + + + assertThat(bean.transientList()).isNotInstanceOf(BeanList.class); + assertThat(bean.transientList2()).isNotInstanceOf(BeanList.class); + assertThat(bean.transientSet()).isNotInstanceOf(BeanSet.class); + assertThat(bean.transientSet2()).isNotInstanceOf(BeanSet.class); + assertThat(bean.transientMap()).isNotInstanceOf(BeanMap.class); + assertThat(bean.transientMap2()).isNotInstanceOf(BeanMap.class); + + + // these methods work because the underlying collection is a BeanCollection + bean.listOf().add(new Contact("junk")); + bean.setOf().add(new Contact("junk")); + bean.listCollEmpty().add(new Contact("junk")); + bean.setCollEmpty().add(new Contact("junk")); + } +} diff --git a/test/src/test/java/test/enhancement/WithInitialisedCollectionsTest.java b/test/src/test/java/test/enhancement/WithInitialisedCollectionsTest.java index 26088bb9..7f5aa8ee 100644 --- a/test/src/test/java/test/enhancement/WithInitialisedCollectionsTest.java +++ b/test/src/test/java/test/enhancement/WithInitialisedCollectionsTest.java @@ -7,18 +7,18 @@ import java.util.ArrayList; import java.util.List; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; public class WithInitialisedCollectionsTest extends BaseTest { - @Test public void test() { WithInitialisedCollections bean = new WithInitialisedCollections(); assertNotNull(bean); - assertTrue(bean instanceof EntityBean); + assertInstanceOf(EntityBean.class, bean); EntityBean eb = (EntityBean)bean; String[] props = eb._ebean_getPropertyNames(); @@ -43,7 +43,6 @@ public void test() { assertNotNull(bean.getMyset()); assertNotNull(bean.getMyLinkedSet()); - } @@ -53,7 +52,7 @@ public void test_withTransient() { WithInitialisedCollectionAndTransient bean = new WithInitialisedCollectionAndTransient(); assertNotNull(bean); - assertTrue(bean instanceof EntityBean); + assertInstanceOf(EntityBean.class, bean); assertNotNull(bean.getBuffer()); @@ -69,7 +68,7 @@ public void test_withAtTransient() { WithInitialisedCollectionAndAtTransient bean = new WithInitialisedCollectionAndAtTransient(); assertNotNull(bean); - assertTrue(bean instanceof EntityBean); + assertInstanceOf(EntityBean.class, bean); assertNotNull(bean.getBuffer()); @@ -87,6 +86,6 @@ public void test_withConstructor() { WithInitialisedCollectionsAndConstructor bean = new WithInitialisedCollectionsAndConstructor(contacts); - assertTrue(bean.getContacts().size() == 1); + assertThat(bean.getContacts()).hasSize(1); } } diff --git a/test/src/test/java/test/model/WithInitialisedCollections2.java b/test/src/test/java/test/model/WithInitialisedCollections2.java new file mode 100644 index 00000000..7f406346 --- /dev/null +++ b/test/src/test/java/test/model/WithInitialisedCollections2.java @@ -0,0 +1,98 @@ +package test.model; + + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.OneToMany; +import javax.persistence.Transient; +import java.util.*; + +@Entity +public class WithInitialisedCollections2 extends BaseEntity { + + String name; + + @OneToMany(cascade = CascadeType.PERSIST) + final List listOf = List.of(); + @OneToMany(cascade = CascadeType.PERSIST) + final Set setOf = Set.of(); + @OneToMany(cascade = CascadeType.PERSIST) + Map mapOf = Map.of(); + + @OneToMany(cascade = CascadeType.PERSIST) + final List listCollEmpty = Collections.emptyList(); + @OneToMany(cascade = CascadeType.PERSIST) + final Set setCollEmpty = Collections.emptySet(); + @OneToMany(cascade = CascadeType.PERSIST) + Map mapCollEmpty = Collections.emptyMap(); + + @Transient + List transientList = List.of(); + @Transient + Set transientSet = Set.of(); + @Transient + Map transientMap = Map.of(); + @Transient + List transientList2 = Collections.emptyList(); + @Transient + Set transientSet2 = Collections.emptySet(); + @Transient + Map transientMap2 = Collections.emptyMap(); + + public String name() { + return name; + } + + public WithInitialisedCollections2 setName(String name) { + this.name = name; + return this; + } + + public List listOf() { + return listOf; + } + + public Set setOf() { + return setOf; + } + + public Map mapOf() { + return mapOf; + } + + public List listCollEmpty() { + return listCollEmpty; + } + + public Set setCollEmpty() { + return setCollEmpty; + } + + public Map mapCollEmpty() { + return mapCollEmpty; + } + + public List transientList() { + return transientList; + } + + public Set transientSet() { + return transientSet; + } + + public List transientList2() { + return transientList2; + } + + public Set transientSet2() { + return transientSet2; + } + + public Map transientMap() { + return transientMap; + } + + public Map transientMap2() { + return transientMap2; + } +}