Merge pull request #295 from SentryMan/constructor

Improve Constructor Detection
avaje · Dec 9, 2024 · ac1f01d · ac1f01d
2 parents 029de5d + f662334
commit ac1f01d
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 34 deletions.
diff --git a/jsonb-generator/pom.xml b/jsonb-generator/pom.xml
@@ -11,7 +11,7 @@
   <name>jsonb generator</name>
   <description>annotation processor generating source code json adapters for avaje-jsonb</description>
   <properties>
-    <avaje.prisms.version>1.31</avaje.prisms.version>
+    <avaje.prisms.version>1.36</avaje.prisms.version>
   </properties>
 
   <dependencies>

diff --git a/jsonb-generator/src/main/java/io/avaje/jsonb/generator/FieldReader.java b/jsonb-generator/src/main/java/io/avaje/jsonb/generator/FieldReader.java
@@ -10,6 +10,7 @@ final class FieldReader {
 
   private final Map<String, TypeSubTypeMeta> subTypes = new LinkedHashMap<>();
 
+  private final Element element;
   private final FieldProperty property;
   private final String propertyName;
   private final boolean serialize;
@@ -24,23 +25,24 @@ final class FieldReader {
   private boolean isCreatorParam;
 
   FieldReader(
-    Element element,
-    NamingConvention namingConvention,
-    TypeSubTypeMeta subType,
-    List<String> genericTypeParams,
-    Integer frequency) {
+      Element element,
+      NamingConvention namingConvention,
+      TypeSubTypeMeta subType,
+      List<String> genericTypeParams,
+      Integer frequency) {
 
     this(element, namingConvention, subType, genericTypeParams, frequency, false);
   }
 
   FieldReader(
-    Element element,
-    NamingConvention namingConvention,
-    TypeSubTypeMeta subType,
-    List<String> genericTypeParams,
-    Integer frequency,
-    boolean jsonCreatorPresent) {
-
+      Element element,
+      NamingConvention namingConvention,
+      TypeSubTypeMeta subType,
+      List<String> genericTypeParams,
+      Integer frequency,
+      boolean jsonCreatorPresent) {
+
+    this.element = element;
     num = frequency == 0 ? "" : frequency.toString();
     addSubType(subType);
     var isMethod = element instanceof ExecutableElement;
@@ -296,4 +298,7 @@ List<String> aliases() {
     return aliases;
   }
 
+  Element element() {
+    return element;
+  }
 }
diff --git a/jsonb-generator/src/main/java/io/avaje/jsonb/generator/TypeReader.java b/jsonb-generator/src/main/java/io/avaje/jsonb/generator/TypeReader.java
@@ -4,6 +4,8 @@
 import javax.lang.model.type.TypeMirror;
 import javax.lang.model.util.ElementFilter;
 
+import static java.util.stream.Collectors.toSet;
+
 import java.util.*;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
@@ -41,11 +43,8 @@ final class TypeReader {
   private boolean nonAccessibleField;
 
   private final Map<String, Element> mixInFields;
-
   private final String typePropertyKey;
-
   private final Map<String, Integer> frequencyMap = new HashMap<>();
-
   private final List<MethodProperty> methodProperties = new ArrayList<>();
 
   private boolean optional;
@@ -139,10 +138,9 @@ void read(TypeElement type) {
       for (var param : constructor.getParams()) {
         var name = param.name();
         var element = param.element();
-        var matchingField =
-            localFields.stream()
-                .filter(f -> f.propertyName().equals(name) || f.fieldName().equals(name))
-                .findFirst();
+        var matchingField = localFields.stream()
+          .filter(f -> f.propertyName().equals(name) || f.fieldName().equals(name))
+          .findFirst();
         matchingField.ifPresentOrElse(f -> f.readParam(element), () -> readField(element, localFields));
       }
     }
@@ -161,7 +159,6 @@ void read(TypeElement type) {
         } else {
           commonField.addSubType(currentSubType);
         }
-
         if (commonField == null && currentSubType != null) {
           localField.setSubTypeField();
         }
@@ -172,22 +169,19 @@ void read(TypeElement type) {
   private void readField(Element element, List<FieldReader> localFields) {
     final Element mixInField = mixInFields.get(element.getSimpleName().toString());
     if (mixInField != null && APContext.types().isSameType(mixInField.asType(), element.asType())) {
-
       var mixinModifiers = new HashSet<>(mixInField.getModifiers());
       var modifiers = new HashSet<>(mixInField.getModifiers());
 
       Arrays.stream(Modifier.values())
-          .filter(m -> m != Modifier.PRIVATE || m != Modifier.PROTECTED || m != Modifier.PUBLIC)
-          .forEach(
-              m -> {
-                modifiers.remove(m);
-                mixinModifiers.remove(m);
-              });
+        .filter(m -> m != Modifier.PRIVATE || m != Modifier.PROTECTED || m != Modifier.PUBLIC)
+        .forEach(m -> {
+          modifiers.remove(m);
+          mixinModifiers.remove(m);
+        });
 
       if (!modifiers.equals(mixinModifiers)) {
        APContext.logError(mixInField, "mixIn fields must have the same modifiers as the target class");
       }
-
       element = mixInField;
     }
     if (element.asType().toString().contains("java.util.Optional")) {
@@ -304,13 +298,17 @@ private void matchFieldsToSetterOrConstructor() {
   }
 
   private void matchFieldToSetter(FieldReader field) {
-    if (!matchFieldToSetter2(field, false)
+    if (hasNoSetter(field)) {
+      logError("Non public field " + baseType + " " + field.fieldName() + " with no matching setter or constructor?");
+    }
+  }
+
+  private boolean hasNoSetter(FieldReader field) {
+    return !matchFieldToSetter2(field, false)
       && !matchFieldToSetter2(field, true)
       && !matchFieldToSetterByParam(field)
       && !field.isPublicField()
-      && !field.isSubTypeField()) {
-      logError("Non public field " + baseType + " " + field.fieldName() + " with no matching setter or constructor?");
-    }
+      && !field.isSubTypeField();
   }
 
   private boolean matchFieldToSetterByParam(FieldReader field) {
@@ -458,7 +456,26 @@ private MethodReader determineConstructor() {
       // fallback to the single public constructor
       return allPublic.get(0);
     }
-    // find the largest constructor
+
+    // find the right constructor
+    var constructorFields = allFields.stream()
+      .filter(FieldReader::includeFromJson)
+      .filter(this::hasNoSetter)
+      .map(f -> f.element().asType().toString())
+      .map(Util::trimAnnotations)
+      .collect(toSet());
+
+    return allPublic.stream()
+      .filter(c -> c.getParams().size() == constructorFields.size())
+      .filter(c -> c.getParams().stream()
+        .map(p -> p.element().asType().toString())
+        .map(Util::trimAnnotations)
+        .allMatch(constructorFields::contains))
+      .findFirst()
+      .orElseGet(this::largest);
+  }
+
+  private MethodReader largest() {
     int argCount = 0;
     MethodReader largestConstructor = null;
     for (MethodReader ctor : publicConstructors) {

diff --git a/jsonb-generator/src/test/java/io/avaje/jsonb/generator/models/valid/MultiConstruct.java b/jsonb-generator/src/test/java/io/avaje/jsonb/generator/models/valid/MultiConstruct.java
@@ -0,0 +1,36 @@
+package io.avaje.jsonb.generator.models.valid;
+
+import java.util.List;
+
+import io.avaje.jsonb.Json;
+
+@Json
+public class MultiConstruct {
+
+  private final List<String> body;
+  private String setter;
+
+  public MultiConstruct(String string) {
+    this(List.of(string));
+  }
+
+  public MultiConstruct(List<String> body) {
+    this.body = body;
+  }
+
+  public MultiConstruct(List<String> body, int somethin) {
+    this.body = body;
+  }
+
+  public String getSetter() {
+    return setter;
+  }
+
+  public void setSetter(String setter) {
+    this.setter = setter;
+  }
+
+  public List<String> getBody() {
+    return body;
+  }
+}