From 75e69a0a233c210a2f5620c15af85260989ece2c Mon Sep 17 00:00:00 2001 From: "robin.bygrave" Date: Thu, 25 May 2023 21:57:54 +1200 Subject: [PATCH] #3062 - DtoQuery may match the wrong constructor When 2 or more Dto constructors clash by argument count then we need to ignore those constructors. --- .../io/ebeaninternal/server/dto/DtoMeta.java | 13 ++-- .../server/dto/DtoMetaBuilder.java | 18 +++-- .../server/dto/DtoMetaConstructor.java | 6 +- .../io/ebean/xtest/base/DtoQuery3Test.java | 76 +++++++++++++++++++ 4 files changed, 95 insertions(+), 18 deletions(-) create mode 100644 ebean-test/src/test/java/io/ebean/xtest/base/DtoQuery3Test.java diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMeta.java b/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMeta.java index fed2ff4c1..ce2c94a8c 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMeta.java +++ b/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 constructors, List properties) { + DtoMeta(Class dtoType, Collection constructors, List 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); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMetaBuilder.java b/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMetaBuilder.java index 67f4d9912..06929cefe 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMetaBuilder.java +++ b/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 properties = new ArrayList<>(); - private final List constructorList = new ArrayList<>(); + private final Map 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 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); + } } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMetaConstructor.java b/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMetaConstructor.java index 04f213ef0..48b05808d 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/dto/DtoMetaConstructor.java +++ b/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; } diff --git a/ebean-test/src/test/java/io/ebean/xtest/base/DtoQuery3Test.java b/ebean-test/src/test/java/io/ebean/xtest/base/DtoQuery3Test.java new file mode 100644 index 000000000..a717e0052 --- /dev/null +++ b/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 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 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; + } + } +}