Skip to content

Commit 12cff17

Browse files
rbygraverbygrave
authored
Merge pull request #220 from ebean-orm/feature/detect-oneToMany-init-empty
Support initialisation of ToMany collections using List.of() etc
2 parents cfff738 + af1a203 commit 12cff17

File tree

13 files changed

+261
-48
lines changed

13 files changed

+261
-48
lines changed

‎ebean-agent/pom.xml‎

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
<dependency>
6060
<groupId>io.avaje</groupId>
6161
<artifactId>junit</artifactId>
62-
<version>1.1</version>
62+
<version>1.5</version>
6363
<scope>test</scope>
6464
</dependency>
6565

@@ -92,23 +92,9 @@
9292
</dependency>
9393

9494
<dependency>
95-
<groupId>org.avaje.composite</groupId>
96-
<artifactId>logback</artifactId>
97-
<version>1.1</version>
98-
<scope>test</scope>
99-
</dependency>
100-
101-
<dependency>
102-
<groupId>org.mockito</groupId>
103-
<artifactId>mockito-core</artifactId>
104-
<version>3.11.2</version>
105-
<scope>test</scope>
106-
</dependency>
107-
108-
<dependency>
109-
<groupId>org.avaje.composite</groupId>
110-
<artifactId>composite-testing</artifactId>
111-
<version>3.1</version>
95+
<groupId>ch.qos.logback</groupId>
96+
<artifactId>logback-classic</artifactId>
97+
<version>1.5.17</version>
11298
<scope>test</scope>
11399
</dependency>
114100

‎ebean-agent/src/main/java/io/ebean/enhance/EnhancementException.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package io.ebean.enhance;
2+
3+
/**
4+
* Some unexpected bytecode that can't be supported.
5+
* <p>
6+
* For example, some unsupported OneToMany collection initialisation.
7+
*/
8+
public class EnhancementException extends RuntimeException {
9+
public EnhancementException(String message) {
10+
super(message);
11+
}
12+
}

‎ebean-agent/src/main/java/io/ebean/enhance/Transformer.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,9 @@ public byte[] transform(ClassLoader loader, String className, Class<?> classBein
191191
// the class is an interface
192192
log(8, className, "No Enhancement required " + e.getMessage());
193193
return null;
194+
} catch (EnhancementException e) {
195+
enhanceContext.log(className, "Transform error " + e.getMessage());
196+
throw e;
194197
} catch (IllegalArgumentException | IllegalStateException e) {
195198
log(2, className, "No enhancement on class due to " + e);
196199
return null;

‎ebean-agent/src/main/java/io/ebean/enhance/ant/OfflineFileTransform.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.ebean.enhance.ant;
22

3+
import io.ebean.enhance.EnhancementException;
34
import io.ebean.enhance.Transformer;
45
import io.ebean.enhance.common.InputStreamTransform;
56

@@ -60,7 +61,6 @@ private String trimSlash(String dir) {
6061
* Process the packageNames as comma delimited string.
6162
*/
6263
public void process(String packageNames) {
63-
6464
if (packageNames == null) {
6565
// just process all directories
6666
processPackage("");
@@ -69,7 +69,6 @@ public void process(String packageNames) {
6969

7070
Set<String> pkgNames = new LinkedHashSet<>();
7171
Collections.addAll(pkgNames, packageNames.split(","));
72-
7372
process(pkgNames);
7473
}
7574

@@ -81,7 +80,6 @@ public void process(String packageNames) {
8180
* </p>
8281
*/
8382
public void process(Set<String> packageNames) {
84-
8583
if (packageNames == null || packageNames.isEmpty()) {
8684
// just process all directories
8785
inputStreamTransform.log(2, "processing all directories (as no explicit packages)");
@@ -90,17 +88,14 @@ public void process(Set<String> packageNames) {
9088
}
9189

9290
for (String pkgName : packageNames) {
93-
9491
String pkg = pkgName.trim().replace('.', '/');
95-
9692
if (pkg.endsWith("**")) {
9793
pkg = pkg.substring(0, pkg.length() - 2);
9894
} else if (pkg.endsWith("*")) {
9995
pkg = pkg.substring(0, pkg.length() - 1);
10096
}
10197

10298
pkg = trimSlash(pkg);
103-
10499
processPackage(pkg);
105100
}
106101
}
@@ -142,15 +137,20 @@ private void processPackage(String dir) {
142137
}
143138

144139
private void transformFile(File file) throws IOException, IllegalClassFormatException {
145-
146140
String className = getClassName(file);
147-
148-
byte[] result = inputStreamTransform.transform(className, file);
149-
150-
if (result != null) {
151-
InputStreamTransform.writeBytes(result, file);
152-
if (listener != null && logLevel > 0) {
153-
listener.logEvent("Enhanced " + file);
141+
try {
142+
byte[] result = inputStreamTransform.transform(className, file);
143+
if (result != null) {
144+
InputStreamTransform.writeBytes(result, file);
145+
if (listener != null && logLevel > 0) {
146+
listener.logEvent("Enhanced " + file);
147+
}
148+
}
149+
} catch (EnhancementException e) {
150+
if (listener != null) {
151+
listener.logError("Error enhancing class " + className + " " + e.getMessage());
152+
} else {
153+
throw e;
154154
}
155155
}
156156
}

‎ebean-agent/src/main/java/io/ebean/enhance/common/AgentManifest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ public final class AgentManifest {
2828
private boolean transientInternalFields;
2929
private boolean transientInit;
3030
private boolean transientInitThrowError;
31+
private boolean unsupportedInitThrowError = true;
3132
private boolean checkNullManyFields = true;
3233
private boolean enableProfileLocation = true;
3334
private boolean enableEntityFieldAccess;
@@ -151,6 +152,10 @@ public boolean isTransientInitThrowError() {
151152
return transientInitThrowError;
152153
}
153154

155+
public boolean isUnsupportedInitThrowError() {
156+
return unsupportedInitThrowError;
157+
}
158+
154159
/**
155160
* Return true if we should use transient internal fields.
156161
*/
@@ -315,6 +320,7 @@ private void readOptions(Attributes attributes) {
315320
transientInternalFields = bool("transient-internal-fields", transientInternalFields, attributes);
316321
checkNullManyFields = bool("check-null-many-fields", checkNullManyFields, attributes);
317322
allowNullableDbArray = bool("allow-nullable-dbarray", allowNullableDbArray, attributes);
323+
unsupportedInitThrowError = bool("unsupported-init-error", unsupportedInitThrowError, attributes);
318324
}
319325

320326
private boolean bool(String key, boolean defaultValue, Attributes attributes) {

‎ebean-agent/src/main/java/io/ebean/enhance/common/ClassMeta.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public class ClassMeta {
5050
/**
5151
* If enhancement is adding a default constructor - only default constructors are supported initialising transient fields.
5252
*/
53+
private final Set<String> unsupportedInitMany = new LinkedHashSet<>();
5354
private final Set<String> unsupportedTransientInitialisation = new LinkedHashSet<>();
5455
private final Map<String, CapturedInitCode> transientInitCode = new LinkedHashMap<>();
5556
private final LinkedHashMap<String, FieldMeta> fields = new LinkedHashMap<>();
@@ -447,6 +448,20 @@ public Collection<CapturedInitCode> transientInit() {
447448
return transientInitCode.values();
448449
}
449450

451+
public void addUnsupportedInitMany(String name) {
452+
unsupportedInitMany.add(name);
453+
}
454+
455+
public boolean hasUnsupportedInitMany() {
456+
return !unsupportedInitMany.isEmpty();
457+
}
458+
459+
public String initFieldErrorMessage() {
460+
return "ERROR: Unsupported initialisation of @OneToMany or @ManyToMany on: "
461+
+ className + " fields: " + unsupportedInitMany
462+
+ " Refer: https://ebean.io/docs/trouble-shooting#initialisation-error";
463+
}
464+
450465
public void addUnsupportedTransientInit(String name) {
451466
unsupportedTransientInitialisation.add(name);
452467
}

‎ebean-agent/src/main/java/io/ebean/enhance/common/EnhanceContext.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,10 @@ public boolean isTransientInitThrowError() {
344344
return manifest.isTransientInitThrowError();
345345
}
346346

347+
public boolean isUnsupportedInitThrowError() {
348+
return manifest.isUnsupportedInitThrowError();
349+
}
350+
347351
/**
348352
* Return true if internal ebean fields in entity classes should be transient.
349353
*/

‎ebean-agent/src/main/java/io/ebean/enhance/entity/ClassAdapterEntity.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.ebean.enhance.entity;
22

3+
import io.ebean.enhance.EnhancementException;
34
import io.ebean.enhance.asm.AnnotationVisitor;
45
import io.ebean.enhance.asm.ClassVisitor;
56
import io.ebean.enhance.asm.FieldVisitor;
@@ -205,6 +206,14 @@ public void visitEnd() {
205206
if (!classMeta.isEntityEnhancementRequired()) {
206207
throw new NoEnhancementRequiredException();
207208
}
209+
if (classMeta.hasUnsupportedInitMany()) {
210+
if (classMeta.context().isUnsupportedInitThrowError()) {
211+
throw new EnhancementException(classMeta.initFieldErrorMessage());
212+
} else {
213+
// the default constructor being added will leave some transient fields uninitialised (null, 0, false etc)
214+
System.err.println(classMeta.initFieldErrorMessage());
215+
}
216+
}
208217
if (!classMeta.hasStaticInit()) {
209218
IndexFieldWeaver.addPropertiesInit(cv, classMeta);
210219
}

‎ebean-agent/src/main/java/io/ebean/enhance/entity/ConstructorDeferredCode.java‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ enum State {
4545
INVOKE_SPECIAL,
4646
KT_CHECKCAST, // optional kotlin state
4747
KT_LABEL, // optional kotlin state
48-
KT_EMPTYLIST
48+
EMPTY,
49+
MAYBE_UNSUPPORTED
4950
}
5051

5152
private static final ALoad ALOAD_INSTRUCTION = new ALoad();
@@ -130,20 +131,53 @@ boolean deferVisitMethodInsn(int opcode, String owner, String name, String desc,
130131
stateInitialiseType = owner;
131132
return true;
132133
}
133-
if (opcode == INVOKESTATIC && stateAload() && kotlinEmptyList(owner, name, desc)) {
134-
codes.add(new NoArgInit(opcode, owner, name, desc, itf));
135-
state = State.KT_EMPTYLIST;
136-
stateInitialiseType = "java/util/ArrayList";
137-
return true;
134+
if (opcode == INVOKESTATIC && stateAload()) {
135+
if (kotlinEmptyList(owner, name, desc) || emptyList(owner, name, desc)) {
136+
codes.add(new NoArgInit(opcode, owner, name, desc, itf));
137+
state = State.EMPTY;
138+
stateInitialiseType = "java/util/ArrayList";
139+
return true;
140+
}
141+
if (emptySet(owner, name, desc)) {
142+
codes.add(new NoArgInit(opcode, owner, name, desc, itf));
143+
state = State.EMPTY;
144+
stateInitialiseType = "java/util/LinkedHashSet";
145+
return true;
146+
}
147+
if (emptyMap(owner, name, desc)) {
148+
codes.add(new NoArgInit(opcode, owner, name, desc, itf));
149+
state = State.EMPTY;
150+
stateInitialiseType = "java/util/LinkedHashMap";
151+
return true;
152+
}
138153
}
139154
flush();
155+
state = State.MAYBE_UNSUPPORTED;
140156
return false;
141157
}
142158

143159
private boolean isNoArgInit(String name, String desc) {
144160
return name.equals(INIT) && desc.equals(NOARG_VOID);
145161
}
146162

163+
private boolean emptyList(String owner, String name, String desc) {
164+
return desc.equals("()Ljava/util/List;")
165+
&& ((owner.equals("java/util/List") && name.equals("of"))
166+
|| (owner.equals("java/util/Collections") && name.equals("emptyList")));
167+
}
168+
169+
private boolean emptySet(String owner, String name, String desc) {
170+
return desc.equals("()Ljava/util/Set;")
171+
&& ((owner.equals("java/util/Set") && name.equals("of"))
172+
|| (owner.equals("java/util/Collections") && name.equals("emptySet")));
173+
}
174+
175+
private boolean emptyMap(String owner, String name, String desc) {
176+
return desc.equals("()Ljava/util/Map;")
177+
&& ((owner.equals("java/util/Map") && name.equals("of"))
178+
|| (owner.equals("java/util/Collections") && name.equals("emptyMap")));
179+
}
180+
147181
private boolean kotlinEmptyList(String owner, String name, String desc) {
148182
return owner.equals("kotlin/collections/CollectionsKt")
149183
&& name.equals("emptyList")
@@ -155,6 +189,12 @@ private boolean kotlinEmptyList(String owner, String name, String desc) {
155189
*/
156190
boolean consumeVisitFieldInsn(int opcode, String owner, String name, String desc) {
157191
if (opcode == PUTFIELD) {
192+
if (state == State.MAYBE_UNSUPPORTED && meta.isConsumeInitMany(name)) {
193+
// a OneToMany/ManyToMany is initialised in an unsupported manor
194+
meta.addUnsupportedInitMany(name);
195+
flush();
196+
return false;
197+
}
158198
if (stateConsumeDeferred()) {
159199
if (meta.isConsumeInitMany(name) && isConsumeManyType()) {
160200
if (meta.isLog(3)) {
@@ -232,17 +272,17 @@ private boolean stateInvokeSpecial() {
232272
}
233273

234274
private boolean stateConsumeDeferred() {
235-
return state == State.INVOKE_SPECIAL || state == State.KT_CHECKCAST || state == State.KT_EMPTYLIST;
275+
return state == State.INVOKE_SPECIAL || state == State.KT_CHECKCAST || state == State.EMPTY;
236276
}
237277

238278
/**
239279
* Return true if the type being initialised is valid for auto initialisation of ToMany or DbArray.
240280
*/
241281
private boolean isConsumeManyType() {
242-
return ("java/util/ArrayList".equals(stateInitialiseType)
282+
return "java/util/ArrayList".equals(stateInitialiseType)
243283
|| "java/util/LinkedHashSet".equals(stateInitialiseType)
244-
|| "java/util/HashSet".equals(stateInitialiseType));
245-
//|| "java/util/LinkedHashMap".equals(stateInitialiseType)
284+
|| "java/util/HashSet".equals(stateInitialiseType)
285+
|| "java/util/LinkedHashMap".equals(stateInitialiseType);
246286
//|| "java/util/HashMap".equals(stateInitialiseType));
247287
}
248288

‎test/pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
<properties>
1616
<ebean.version>13.4.0</ebean.version>
17+
<java.version>11</java.version>
1718
</properties>
1819

1920
<dependencies>

0 commit comments

Comments
 (0)