Merge pull request #3075 from ebean-orm/feature/3062-dtoQuery

#3062 - DtoQuery may match the wrong constructor

ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMeta.java

@@ -1,9 +1,6 @@
 package io.ebeaninternal.server.dto;
 
-import java.util.Arrays;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 /**
  * Holds property and constructor meta-data for a given DTO bean type.
@@ -18,7 +15,7 @@ final class DtoMeta {
   private final DtoMetaConstructor defaultConstructor;
   private final DtoMetaConstructor maxArgConstructor;
 
-  DtoMeta(Class<?> dtoType, List<DtoMetaConstructor> constructors, List<DtoMetaProperty> properties) {
+  DtoMeta(Class<?> dtoType, Collection<DtoMetaConstructor> constructors, List<DtoMetaProperty> properties) {
     this.dtoType = dtoType;
     for (DtoMetaProperty property : properties) {
       propMap.put(property.getName().toUpperCase(), property);
@@ -27,7 +24,7 @@ final class DtoMeta {
     DtoMetaConstructor defaultConstructor = null;
     DtoMetaConstructor maxArgConstructor = null;
     for (DtoMetaConstructor constructor : constructors) {
-      int args = constructor.getArgCount();
+      int args = constructor.argCount();
       constructorMap.put(args, constructor);
       if (args == 0) {
         defaultConstructor = constructor;
@@ -47,7 +44,7 @@ final class DtoMeta {
     if (constructor != null) {
       return new DtoQueryPlanConstructor(request, constructor);
     }
-    if (maxArgConstructor != null && colLen > maxArgConstructor.getArgCount()) {
+    if (maxArgConstructor != null && colLen > maxArgConstructor.argCount()) {
       // maxArgConst + setters
       return matchMaxArgPlusSetters(request);
     }
@@ -61,7 +58,7 @@ final class DtoMeta {
   }
 
   private DtoQueryPlanConPlus matchMaxArgPlusSetters(DtoMappingRequest request) {
-    DtoReadSet[] setterProps = request.mapArgPlusSetters(this, maxArgConstructor.getArgCount());
+    DtoReadSet[] setterProps = request.mapArgPlusSetters(this, maxArgConstructor.argCount());
     return new DtoQueryPlanConPlus(request, maxArgConstructor, setterProps);
   }
 

ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMetaBuilder.java

@@ -6,8 +6,7 @@ import io.ebeaninternal.server.type.TypeManager;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.*;
 
 import static java.lang.System.Logger.Level.DEBUG;
 
@@ -21,7 +20,7 @@ final class DtoMetaBuilder {
   private final TypeManager typeManager;
   private final Class<?> dtoType;
   private final List<DtoMetaProperty> properties = new ArrayList<>();
-  private final List<DtoMetaConstructor> constructorList = new ArrayList<>();
+  private final Map<Integer, DtoMetaConstructor> constructorMap = new HashMap<>();
 
   DtoMetaBuilder(Class<?> dtoType, TypeManager typeManager) {
     this.dtoType = dtoType;
@@ -31,7 +30,7 @@ final class DtoMetaBuilder {
   DtoMeta build() {
     readConstructors();
     readProperties();
-    return new DtoMeta(dtoType, constructorList, properties);
+    return new DtoMeta(dtoType, constructorMap.values(), properties);
   }
 
   private void readProperties() {
@@ -74,14 +73,23 @@ final class DtoMetaBuilder {
   }
 
   private void readConstructors() {
+    final Set<Integer> removal = new HashSet<>();
     for (Constructor<?> constructor : dtoType.getConstructors()) {
       try {
-        constructorList.add(new DtoMetaConstructor(typeManager, constructor, dtoType));
+        final var meta = new DtoMetaConstructor(typeManager, constructor, dtoType);
+        final var conflicting = constructorMap.put(meta.argCount(), meta);
+        if (conflicting != null) {
+          removal.add(meta.argCount());
+        }
       } catch (Exception e) {
         // we don't want that constructor
         CoreLog.log.log(DEBUG, "exclude on " + dtoType + " constructor " + constructor, e);
       }
     }
+    // remove the constructors that conflicted by argument count
+    for (Integer key : removal) {
+      constructorMap.remove(key);
+    }
   }
 
 }

ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMetaConstructor.java

@@ -30,11 +30,7 @@ final class DtoMetaConstructor {
     return MethodType.methodType(void.class, types);
   }
 
-  Class<?>[] getTypes() {
-    return types;
-  }
-
-  int getArgCount() {
+  int argCount() {
     return types.length;
   }
 

ebean-test/src/test/java/io/ebean/xtest/base/DtoQuery3Test.java

@@ -0,0 +1,76 @@
+package io.ebean.xtest.base;
+
+import io.ebean.xtest.BaseTestCase;
+import org.junit.jupiter.api.Test;
+import org.tests.model.basic.ResetBasicData;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class DtoQuery3Test extends BaseTestCase {
+
+  @Test
+  void dtoQuery_when_constructorsClash() {
+    ResetBasicData.reset();
+
+    List<DCust> list = server().findDto(DCust.class, "select id, name, status from o_customer").findList();
+
+    assertThat(list).isNotEmpty();
+    for (DCust cust: list) {
+      assertThat(cust.id()).isNotNull();
+      assertThat(cust.name()).isNotNull();
+      assertThat(cust.status()).isNotNull();
+    }
+  }
+
+  public static class DCust {
+
+    final Integer id;
+    String name;
+
+    String status;
+
+    public DCust(Integer id) {
+      this.id = id;
+    }
+
+    /** Constructor with 3 args - clashes with the one below */
+    public DCust(Integer id, String name, List<String> notUsed) {
+      this.id = id;
+      this.name = name;
+    }
+
+    /** Constructor with 3 args - clashes with the one above */
+    public DCust(Integer id, String name, String status) {
+      this.id = id;
+      this.name = name;
+      this.status = status;
+    }
+
+    @Override
+    public String toString() {
+      return "id:" + id + " name:" + name;
+    }
+
+    public Integer id() {
+      return id;
+    }
+
+    public String name() {
+      return name;
+    }
+
+    public void name(String name) {
+      this.name = name;
+    }
+
+    public String status() {
+      return status;
+    }
+
+    public void setStatus(String status) {
+      this.status = status;
+    }
+  }
+}