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 @@ public DtoQueryPlan match(DtoMappingRequest request) {
  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 @@ public DtoQueryPlan match(DtoMappingRequest request) {
  }
 
  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 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 @@ static boolean includeMethod(Method method) {
  }
 
  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 @@ private MethodType typeFor(Class<?>[] types) {
  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;
+ }
+ }
+}