From edd262ac19c08a1be7556eaab158d3e9bf11d25c Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 4 Mar 2025 12:26:37 +1300 Subject: [PATCH 1/4] Support initialisation of ToMany collections using List.of() etc - Add Support for List.of() & Collections.emptyList() - Add Support for Set.of() & Collections.emptySet() - Add Support for Map.of() & Collections.emptyMap() --- .../entity/ConstructorDeferredCode.java | 52 ++++++++-- test/pom.xml | 1 + .../WithInitialisedCollections2Test.java | 40 ++++++++ .../WithInitialisedCollectionsTest.java | 11 +-- .../model/WithInitialisedCollections2.java | 98 +++++++++++++++++++ 5 files changed, 186 insertions(+), 16 deletions(-) create mode 100644 test/src/test/java/test/enhancement/WithInitialisedCollections2Test.java create mode 100644 test/src/test/java/test/model/WithInitialisedCollections2.java 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..4f669468 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,7 @@ enum State { INVOKE_SPECIAL, KT_CHECKCAST, // optional kotlin state KT_LABEL, // optional kotlin state - KT_EMPTYLIST + EMPTY } private static final ALoad ALOAD_INSTRUCTION = new ALoad(); @@ -130,11 +130,25 @@ 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 (emptyList(owner, name, desc) || kotlinEmptyList(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(); return false; @@ -144,6 +158,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") @@ -232,17 +264,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; + } +} From 3377e64a29b96403958d9b4a6f494d4671105c62 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 4 Mar 2025 13:32:44 +1300 Subject: [PATCH 2/4] Tidy up test dependencies --- ebean-agent/pom.xml | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) 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 From 20b1bc456e3338be2150c4621cf0045f6246a5f1 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 4 Mar 2025 23:11:23 +1300 Subject: [PATCH 3/4] Add EnhancementException to throw when unsupported OneToMany initialisation is detected maven plugin will also be updated to fail the build when this exception is thrown --- .../ebean/enhance/EnhancementException.java | 12 +++ .../java/io/ebean/enhance/Transformer.java | 3 + .../enhance/ant/OfflineFileTransform.java | 28 +++---- .../ebean/enhance/common/AgentManifest.java | 6 ++ .../io/ebean/enhance/common/ClassMeta.java | 15 ++++ .../ebean/enhance/common/EnhanceContext.java | 4 + .../enhance/entity/ClassAdapterEntity.java | 9 ++ .../entity/ConstructorDeferredCode.java | 83 ++++++++++--------- 8 files changed, 108 insertions(+), 52 deletions(-) create mode 100644 ebean-agent/src/main/java/io/ebean/enhance/EnhancementException.java 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 4f669468..00b3b3d5 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 - EMPTY + EMPTY, + MAYBE_UNSUPPORTED } private static final ALoad ALOAD_INSTRUCTION = new ALoad(); @@ -131,26 +132,27 @@ boolean deferVisitMethodInsn(int opcode, String owner, String name, String desc, return true; } if (opcode == INVOKESTATIC && stateAload()) { - if (emptyList(owner, name, desc) || kotlinEmptyList(owner, name, desc)) { + 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; - } +// 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; } @@ -158,23 +160,23 @@ 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 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") @@ -188,13 +190,18 @@ private boolean kotlinEmptyList(String owner, String name, String desc) { boolean consumeVisitFieldInsn(int opcode, String owner, String name, String desc) { if (opcode == PUTFIELD) { if (stateConsumeDeferred()) { - if (meta.isConsumeInitMany(name) && isConsumeManyType()) { - if (meta.isLog(3)) { - meta.log("... consumed init of many: " + name); + if (meta.isConsumeInitMany(name)) { + if (isConsumeManyType()) { + if (meta.isLog(3)) { + meta.log("... consumed init of many: " + name); + } + state = State.UNSET; + codes.clear(); + return true; + } else { + // a OneToMany/ManyToMany is initialised in an unsupported manor + meta.addUnsupportedInitMany(name); } - state = State.UNSET; - codes.clear(); - return true; } else if (meta.isInitTransient(name)) { // keep the initialisation code for transient to 'replay' // it when adding a default constructor if needed @@ -264,7 +271,7 @@ private boolean stateInvokeSpecial() { } private boolean stateConsumeDeferred() { - return state == State.INVOKE_SPECIAL || state == State.KT_CHECKCAST || state == State.EMPTY; + return state == State.INVOKE_SPECIAL || state == State.KT_CHECKCAST || state == State.EMPTY || state == State.MAYBE_UNSUPPORTED; } /** From af1a203baa1f1334a4113093cd0dab8f84611aac Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 4 Mar 2025 23:59:32 +1300 Subject: [PATCH 4/4] Add back support for List.of() and Collections.emptyList() as initialisation for OneToMany etc The thinking is that many folks are not reading the JPA spec, and there are some folks "suggesting" to initialise OneToMany collections with Collections.emptyList() ... even though it isn't as per JPA spec. Hmmm. --- .../entity/ConstructorDeferredCode.java | 85 ++++++++++--------- 1 file changed, 43 insertions(+), 42 deletions(-) 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 00b3b3d5..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 @@ -132,24 +132,24 @@ boolean deferVisitMethodInsn(int opcode, String owner, String name, String desc, return true; } if (opcode == INVOKESTATIC && stateAload()) { - if (kotlinEmptyList(owner, name, desc)) { // emptyList(owner, name, desc) || + 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; -// } + 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; @@ -160,23 +160,23 @@ 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 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") @@ -189,19 +189,20 @@ 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)) { - if (isConsumeManyType()) { - if (meta.isLog(3)) { - meta.log("... consumed init of many: " + name); - } - state = State.UNSET; - codes.clear(); - return true; - } else { - // a OneToMany/ManyToMany is initialised in an unsupported manor - meta.addUnsupportedInitMany(name); + if (meta.isConsumeInitMany(name) && isConsumeManyType()) { + if (meta.isLog(3)) { + meta.log("... consumed init of many: " + name); } + state = State.UNSET; + codes.clear(); + return true; } else if (meta.isInitTransient(name)) { // keep the initialisation code for transient to 'replay' // it when adding a default constructor if needed @@ -271,7 +272,7 @@ private boolean stateInvokeSpecial() { } private boolean stateConsumeDeferred() { - return state == State.INVOKE_SPECIAL || state == State.KT_CHECKCAST || state == State.EMPTY || state == State.MAYBE_UNSUPPORTED; + return state == State.INVOKE_SPECIAL || state == State.KT_CHECKCAST || state == State.EMPTY; } /**