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

Author: rbygrave

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.
+ * <p>
+ * For example, some unsupported OneToMany collection initialisation.
+ */
+public class EnhancementException extends RuntimeException {
+  public EnhancementException(String message) {
+    super(message);
+  }
+}

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;

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<String> pkgNames = new LinkedHashSet<>();
     Collections.addAll(pkgNames, packageNames.split(","));
-
     process(pkgNames);
   }
 
@@ -81,7 +80,6 @@ public void process(String packageNames) {
    * </p>
    */
   public void process(Set<String> packageNames) {
-
     if (packageNames == null || packageNames.isEmpty()) {
       // just process all directories
       inputStreamTransform.log(2, "processing all directories (as no explicit packages)");
@@ -90,17 +88,14 @@ public void process(Set<String> packageNames) {
     }
 
     for (String pkgName : packageNames) {
-
       String pkg = pkgName.trim().replace('.', '/');
-
       if (pkg.endsWith("**")) {
         pkg = pkg.substring(0, pkg.length() - 2);
       } else if (pkg.endsWith("*")) {
         pkg = pkg.substring(0, pkg.length() - 1);
       }
 
       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;
       }
     }
   }

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) {

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<String> unsupportedInitMany = new LinkedHashSet<>();
   private final Set<String> unsupportedTransientInitialisation = new LinkedHashSet<>();
   private final Map<String, CapturedInitCode> transientInitCode = new LinkedHashMap<>();
   private final LinkedHashMap<String, FieldMeta> fields = new LinkedHashMap<>();
@@ -447,6 +448,20 @@ public Collection<CapturedInitCode> 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);
   }

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.
    */

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);
     }

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,50 +132,51 @@ 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;
   }
 
   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;
   }
 
   /**