From d68f477e8eed5f6d9986dc9b9805f446ae6f4377 Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Mon, 7 Dec 2020 21:53:03 +1300 Subject: [PATCH] #2120 - DbEnumValue without database constraint --- ebean-api/pom.xml | 2 +- .../server/type/DefaultTypeManager.java | 76 ++----------------- .../type/ScalarTypeEnumWithMapping.java | 15 +++- .../server/type/DefaultTypeManagerTest.java | 43 +++++++---- .../org/tests/model/array/VarcharEnum.java | 2 +- 5 files changed, 49 insertions(+), 89 deletions(-) diff --git a/ebean-api/pom.xml b/ebean-api/pom.xml index 472d240f9..22e0fd394 100644 --- a/ebean-api/pom.xml +++ b/ebean-api/pom.xml @@ -55,7 +55,7 @@ io.ebean ebean-annotation - 6.13 + 6.14-SNAPSHOT diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java b/ebean-core/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java index c41d1121d..7a2d82baa 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java @@ -256,7 +256,6 @@ public final class DefaultTypeManager implements TypeManager { * Load custom scalar types registered via ExtraTypeFactory and ServiceLoader. */ private void loadTypesFromProviders(DatabaseConfig config, Object objectMapper) { - ServiceLoader factories = ServiceLoader.load(ExtraTypeFactory.class); Iterator iterator = factories.iterator(); if (iterator.hasNext()) { @@ -291,7 +290,6 @@ public final class DefaultTypeManager implements TypeManager { @SuppressWarnings({"rawtypes", "unchecked"}) @Override public void addEnumType(ScalarType scalarType, Class enumClass) { - Set> mappedClasses = new HashSet<>(); mappedClasses.add(enumClass); for (Object value : EnumSet.allOf(enumClass).toArray()) { @@ -360,20 +358,16 @@ public final class DefaultTypeManager implements TypeManager { @Override public ScalarType getArrayScalarType(Class type, DbArray dbArray, Type genericType, boolean nullable) { - Type valueType = getValueType(genericType); if (type.equals(List.class)) { return getArrayScalarTypeList(valueType, nullable); - } else if (type.equals(Set.class)) { return getArrayScalarTypeSet(valueType, nullable); - } else { throw new IllegalStateException("Type [" + type + "] not supported for @DbArray"); } } - @SuppressWarnings("rawtypes") private ScalarType getArrayScalarTypeSet(Type valueType, boolean nullable) { if (arrayTypeSetFactory != null) { if (isEnumType(valueType)) { @@ -385,7 +379,6 @@ public final class DefaultTypeManager implements TypeManager { return new ScalarTypeJsonSet.Varchar(getDocType(valueType), nullable); } - @SuppressWarnings("rawtypes") private ScalarType getArrayScalarTypeList(Type valueType, boolean nullable) { if (arrayTypeListFactory != null) { if (isEnumType(valueType)) { @@ -407,10 +400,8 @@ public final class DefaultTypeManager implements TypeManager { @Override public ScalarType getJsonScalarType(DeployBeanProperty prop, int dbType, int dbLength) { - Class type = prop.getPropertyType(); Type genericType = prop.getGenericType(); - boolean hasJacksonAnnotations = objectMapperPresent && checkJacksonAnnotations(prop); if (type.equals(List.class)) { @@ -421,7 +412,6 @@ public final class DefaultTypeManager implements TypeManager { return createJsonObjectMapperType(prop, dbType, docType); } } - if (type.equals(Set.class)) { DocPropertyType docType = getDocType(genericType); if (!hasJacksonAnnotations && isValueTypeSimple(genericType)) { @@ -430,7 +420,6 @@ public final class DefaultTypeManager implements TypeManager { return createJsonObjectMapperType(prop, dbType, docType); } } - if (type.equals(Map.class)) { if (!hasJacksonAnnotations && isMapValueTypeObject(genericType)) { return ScalarTypeJsonMap.typeFor(postgres, dbType); @@ -438,7 +427,6 @@ public final class DefaultTypeManager implements TypeManager { return createJsonObjectMapperType(prop, dbType, DocPropertyType.OBJECT); } } - if (objectMapperPresent) { if (type.equals(JsonNode.class)) { switch (dbType) { @@ -455,7 +443,6 @@ public final class DefaultTypeManager implements TypeManager { } } } - return createJsonObjectMapperType(prop, dbType, DocPropertyType.OBJECT); } @@ -510,11 +497,9 @@ public final class DefaultTypeManager implements TypeManager { *

* Used for java.util.Date and java.util.Calendar which can be mapped to * different jdbcTypes in a single system. - *

*/ @Override public ScalarType getScalarType(Class type, int jdbcType) { - // File is a special Lob so check for that first if (File.class.equals(type)) { return fileType; @@ -554,10 +539,8 @@ public final class DefaultTypeManager implements TypeManager { * Kind of special case because these map multiple jdbc types to single Java * types - like String - Varchar, LongVarchar, Clob. For this reason I check * for the specific Lob types first before looking for a matching type. - *

*/ private ScalarType getLobTypes(int jdbcType) { - return getScalarType(jdbcType); } @@ -601,14 +584,10 @@ public final class DefaultTypeManager implements TypeManager { * Create the Mapping of Enum fields to DB values using EnumValue annotations. *

* Return null if the EnumValue annotations are not present/used. - *

*/ private ScalarTypeEnum createEnumScalarType2(Class enumType) { - boolean integerType = true; - Map nameValueMap = new LinkedHashMap<>(); - Field[] fields = enumType.getDeclaredFields(); for (Field field : fields) { EnumValue enumValue = AnnotationUtil.get(field, EnumValue.class); @@ -624,8 +603,7 @@ public final class DefaultTypeManager implements TypeManager { // Not using EnumValue here return null; } - - return createEnumScalarType(enumType, nameValueMap, integerType, 0); + return createEnumScalarType(enumType, nameValueMap, integerType, 0, true); } /** @@ -635,17 +613,14 @@ public final class DefaultTypeManager implements TypeManager { * such as A,I,N rather than the ACTIVE, INACTIVE, NEW. So there really needs * to be a mapping from the nicely named enumeration values to the typically * much shorter codes used in the DB. - *

*/ @Override public ScalarType createEnumScalarType(Class> enumType, EnumType type) { - ScalarType scalarType = getScalarType(enumType); if (scalarType instanceof ScalarTypeWrapper) { // no override or further mapping required return scalarType; } - ScalarTypeEnum scalarEnum = (ScalarTypeEnum)scalarType; if (scalarEnum != null && !scalarEnum.isOverrideBy(type)) { if (type != null && !scalarEnum.isCompatible(type)) { @@ -653,7 +628,6 @@ public final class DefaultTypeManager implements TypeManager { } return scalarEnum; } - scalarEnum = createEnumScalarTypePerExtentions(enumType); if (scalarEnum == null) { // use JPA normal Enum type (without mapping) @@ -665,33 +639,27 @@ public final class DefaultTypeManager implements TypeManager { private ScalarTypeEnum createEnumScalarTypePerSpec(Class enumType, EnumType type) { if (type == null) { - - if(defaultEnumType == EnumType.ORDINAL) { + if (defaultEnumType == EnumType.ORDINAL) { return new ScalarTypeEnumStandard.OrdinalEnum(enumType); - } else { return new ScalarTypeEnumStandard.StringEnum(enumType); } - } else if (type == EnumType.ORDINAL) { return new ScalarTypeEnumStandard.OrdinalEnum(enumType); - } else { return new ScalarTypeEnumStandard.StringEnum(enumType); } } private ScalarTypeEnum createEnumScalarTypePerExtentions(Class> enumType) { - Method[] methods = enumType.getMethods(); for (Method method : methods) { DbEnumValue dbValue = AnnotationUtil.get(method, DbEnumValue.class); if (dbValue != null) { boolean integerValues = DbEnumType.INTEGER == dbValue.storage(); - return createEnumScalarTypeDbValue(enumType, method, integerValues, dbValue.length()); + return createEnumScalarTypeDbValue(enumType, method, integerValues, dbValue.length(), dbValue.withConstraint()); } } - // look for EnumValue annotations instead return createEnumScalarType2(enumType); } @@ -702,10 +670,8 @@ public final class DefaultTypeManager implements TypeManager { * Return null if the EnumValue annotations are not present/used. *

*/ - private ScalarTypeEnum createEnumScalarTypeDbValue(Class> enumType, Method method, boolean integerType, int length) { - + private ScalarTypeEnum createEnumScalarTypeDbValue(Class> enumType, Method method, boolean integerType, int length, boolean withConstraint) { Map nameValueMap = new LinkedHashMap<>(); - Enum[] enumConstants = enumType.getEnumConstants(); for (Enum enumConstant : enumConstants) { try { @@ -719,8 +685,7 @@ public final class DefaultTypeManager implements TypeManager { // Not using EnumValue here return null; } - - return createEnumScalarType(enumType, nameValueMap, integerType, length); + return createEnumScalarType(enumType, nameValueMap, integerType, length, withConstraint); } /** @@ -728,27 +693,20 @@ public final class DefaultTypeManager implements TypeManager { * length create the ScalarType for the Enum. */ @SuppressWarnings({"unchecked", "rawtypes"}) - private ScalarTypeEnum createEnumScalarType(Class enumType, Map nameValueMap, boolean integerType, int dbColumnLength) { - + private ScalarTypeEnum createEnumScalarType(Class enumType, Map nameValueMap, boolean integerType, int dbColumnLength, boolean withConstraint) { EnumToDbValueMap beanDbMap = EnumToDbValueMap.create(integerType); - int maxValueLen = 0; - for (Map.Entry entry : nameValueMap.entrySet()) { String name = entry.getKey(); String value = entry.getValue(); - maxValueLen = Math.max(maxValueLen, value.length()); - Object enumValue = Enum.valueOf(enumType, name.trim()); beanDbMap.add(enumValue, value, name.trim()); } - if (dbColumnLength == 0 && !integerType) { dbColumnLength = maxValueLen; } - - return new ScalarTypeEnumWithMapping(beanDbMap, enumType, dbColumnLength); + return new ScalarTypeEnumWithMapping(beanDbMap, enumType, dbColumnLength, withConstraint); } /** @@ -760,10 +718,8 @@ public final class DefaultTypeManager implements TypeManager { *

*/ private void initialiseCustomScalarTypes(BootupClasses bootupClasses) { - for (Class> cls : bootupClasses.getScalarTypes()) { try { - ScalarType scalarType; if (objectMapper == null) { scalarType = cls.newInstance(); @@ -776,9 +732,7 @@ public final class DefaultTypeManager implements TypeManager { scalarType = cls.newInstance(); } } - addCustomType(scalarType); - } catch (Exception e) { String msg = "Error loading ScalarType [" + cls.getName() + "]"; logger.error(msg, e); @@ -801,30 +755,23 @@ public final class DefaultTypeManager implements TypeManager { @SuppressWarnings({"unchecked", "rawtypes"}) private void initialiseScalarConverters(BootupClasses bootupClasses) { - List>> foundTypes = bootupClasses.getScalarConverters(); - for (Class> foundType : foundTypes) { try { - Class[] paramTypes = TypeReflectHelper.getParams(foundType, ScalarTypeConverter.class); if (paramTypes.length != 2) { throw new IllegalStateException("Expected 2 generics paramtypes but got: " + Arrays.toString(paramTypes)); } - Class logicalType = paramTypes[0]; Class persistType = paramTypes[1]; - ScalarType wrappedType = getScalarType(persistType); if (wrappedType == null) { throw new IllegalStateException("Could not find ScalarType for: " + paramTypes[1]); } - ScalarTypeConverter converter = foundType.newInstance(); ScalarTypeWrapper stw = new ScalarTypeWrapper(logicalType, wrappedType, converter); logger.debug("Register ScalarTypeWrapper from {} -> {} using:{}", logicalType, persistType, foundType); add(stw); - } catch (Exception e) { logger.error("Error registering ScalarTypeConverter [" + foundType.getName() + "]", e); } @@ -833,30 +780,23 @@ public final class DefaultTypeManager implements TypeManager { @SuppressWarnings({"unchecked", "rawtypes"}) private void initialiseAttributeConverters(BootupClasses bootupClasses) { - List>> foundTypes = bootupClasses.getAttributeConverters(); - for (Class> foundType : foundTypes) { try { - Class[] paramTypes = TypeReflectHelper.getParams(foundType, AttributeConverter.class); if (paramTypes.length != 2) { throw new IllegalStateException("Expected 2 generics paramtypes but got: " + Arrays.toString(paramTypes)); } - Class logicalType = paramTypes[0]; Class persistType = paramTypes[1]; - ScalarType wrappedType = getScalarType(persistType); if (wrappedType == null) { throw new IllegalStateException("Could not find ScalarType for: " + paramTypes[1]); } - AttributeConverter converter = foundType.newInstance(); ScalarTypeWrapper stw = new ScalarTypeWrapper(logicalType, wrappedType, new AttributeConverterAdapter(converter)); logger.debug("Register ScalarTypeWrapper from {} -> {} using:{}", logicalType, persistType, foundType); add(stw); - } catch (Exception e) { logger.error("Error registering AttributeConverter [" + foundType.getName() + "]", e); } @@ -875,12 +815,10 @@ public final class DefaultTypeManager implements TypeManager { jsonNodeVarchar = new ScalarTypeJsonNode.Varchar(mapper); jsonNodeJson = jsonNodeClob; // Default for non-Postgres databases jsonNodeJsonb = jsonNodeClob; // Default for non-Postgres databases - if (isPostgres(config.getDatabasePlatform())) { jsonNodeJson = new ScalarTypeJsonNodePostgres.JSON(mapper); jsonNodeJsonb = new ScalarTypeJsonNodePostgres.JSONB(mapper); } - // add as default mapping for JsonNode (when not annotated with @DbJson etc) typeMap.put(JsonNode.class, jsonNodeJson); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java b/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java index d8ab07e72..598889f49 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java @@ -20,13 +20,20 @@ public class ScalarTypeEnumWithMapping extends ScalarTypeEnumStandard.EnumBase i private final int length; + private final boolean withConstraint; + /** * Create with an explicit mapping of bean to database values. */ - public ScalarTypeEnumWithMapping(EnumToDbValueMap beanDbMap, Class enumType, int length) { + public ScalarTypeEnumWithMapping(EnumToDbValueMap beanDbMap, Class enumType, int length, boolean withConstraint) { super(enumType, false, beanDbMap.getDbType()); this.beanDbMap = beanDbMap; this.length = length; + this.withConstraint = withConstraint; + } + + public ScalarTypeEnumWithMapping(EnumToDbValueMap beanDbMap, Class enumType, int length) { + this(beanDbMap, enumType, length, true); } @Override @@ -49,6 +56,9 @@ public class ScalarTypeEnumWithMapping extends ScalarTypeEnumStandard.EnumBase i */ @Override public Set getDbCheckConstraintValues() { + if (!withConstraint) { + return null; + } LinkedHashSet values = new LinkedHashSet(); Iterator it = beanDbMap.dbValues(); while (it.hasNext()) { @@ -64,9 +74,6 @@ public class ScalarTypeEnumWithMapping extends ScalarTypeEnumStandard.EnumBase i /** * Return the DB column length for storing the enum value. - *

- * This is for enum's mapped to strings. - *

*/ @Override public int getLength() { diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/type/DefaultTypeManagerTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/type/DefaultTypeManagerTest.java index e64bc2b7a..950a805fd 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/type/DefaultTypeManagerTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/type/DefaultTypeManagerTest.java @@ -1,10 +1,13 @@ package io.ebeaninternal.server.type; -import io.ebean.config.ServerConfig; +import io.ebean.config.DatabaseConfig; import io.ebean.config.dbplatform.postgres.PostgresPlatform; import io.ebean.core.type.ScalarType; import io.ebeaninternal.server.core.bootup.BootupClasses; import org.junit.Test; +import org.tests.model.array.IntEnum; +import org.tests.model.array.VarcharEnum; +import org.tests.model.basic.Car; import javax.persistence.EnumType; import java.time.DayOfWeek; @@ -17,7 +20,7 @@ import static org.junit.Assert.assertTrue; public class DefaultTypeManagerTest { private DefaultTypeManager create() { - ServerConfig serverConfig = new ServerConfig(); + DatabaseConfig serverConfig = new DatabaseConfig(); serverConfig.setDatabasePlatform(new PostgresPlatform()); BootupClasses bootupClasses = new BootupClasses(); return new DefaultTypeManager(serverConfig, bootupClasses); @@ -25,7 +28,6 @@ public class DefaultTypeManagerTest { @Test public void isIntegerType() { - DefaultTypeManager typeManager = create(); assertTrue(typeManager.isIntegerType("1")); @@ -42,7 +44,6 @@ public class DefaultTypeManagerTest { @Test public void enumDayMonth_builtIn_overrideAsString() { - DefaultTypeManager typeManager = create(); ScalarType type = typeManager.createEnumScalarType(Month.class, null); @@ -51,7 +52,6 @@ public class DefaultTypeManagerTest { // mapped explicitly as JPA EnumType.STRING type = typeManager.createEnumScalarType(Month.class, EnumType.STRING); assertThat(type).isInstanceOf(ScalarTypeEnumStandard.StringEnum.class).as("override built in type"); - try { typeManager.createEnumScalarType(Month.class, EnumType.ORDINAL); assertThat(true).isFalse().as("never get here"); @@ -63,17 +63,14 @@ public class DefaultTypeManagerTest { @Test public void enumMonth_builtIn_overrideAsOrdinal() { - DefaultTypeManager typeManager = create(); // mapped explicitly as JPA EnumType.STRING ScalarType type = typeManager.createEnumScalarType(Month.class, EnumType.ORDINAL); assertThat(type).isInstanceOf(ScalarTypeEnumStandard.OrdinalEnum.class).as("override built in type"); - try { typeManager.createEnumScalarType(Month.class, EnumType.STRING); assertThat(true).isFalse().as("never get here"); - } catch (IllegalStateException e) { assertThat(e.getMessage()).contains("It is mapped using 2 different modes when only one is supported"); } @@ -81,7 +78,6 @@ public class DefaultTypeManagerTest { @Test public void enumDayOfWeek_builtIn_overrideAsString() { - DefaultTypeManager typeManager = create(); ScalarType type = typeManager.createEnumScalarType(DayOfWeek.class, null); @@ -90,11 +86,9 @@ public class DefaultTypeManagerTest { // mapped explicitly as JPA EnumType.STRING type = typeManager.createEnumScalarType(DayOfWeek.class, EnumType.STRING); assertThat(type).isInstanceOf(ScalarTypeEnumStandard.StringEnum.class).as("override built in type"); - try { typeManager.createEnumScalarType(DayOfWeek.class, EnumType.ORDINAL); assertThat(true).isFalse().as("never get here"); - } catch (IllegalStateException e) { assertThat(e.getMessage()).contains("It is mapped using 2 different modes when only one is supported"); } @@ -102,19 +96,40 @@ public class DefaultTypeManagerTest { @Test public void enumDayOfWeek_builtIn_overrideAsOrdinal() { - DefaultTypeManager typeManager = create(); // mapped explicitly as JPA EnumType.STRING ScalarType type = typeManager.createEnumScalarType(DayOfWeek.class, EnumType.ORDINAL); assertThat(type).isInstanceOf(ScalarTypeEnumStandard.OrdinalEnum.class).as("override built in type"); - try { typeManager.createEnumScalarType(DayOfWeek.class, EnumType.STRING); assertThat(true).isFalse().as("never get here"); - } catch (IllegalStateException e) { assertThat(e.getMessage()).contains("It is mapped using 2 different modes when only one is supported"); } } + + @Test + public void createEnumScalarTypePerExtentions() { + DefaultTypeManager typeManager = create(); + + ScalarType type = typeManager.createEnumScalarType(VarcharEnum.class, EnumType.ORDINAL); + assertThat(type).isInstanceOf(ScalarTypeEnumWithMapping.class); + // withConstraint false + assertThat(((ScalarTypeEnumWithMapping) type).getDbCheckConstraintValues()).isNull(); + + type = typeManager.createEnumScalarType(IntEnum.class, EnumType.ORDINAL); + assertThat(type).isInstanceOf(ScalarTypeEnumWithMapping.class); + ScalarTypeEnumWithMapping enumWithMapping = (ScalarTypeEnumWithMapping) type; + // withConstraint true + assertThat(enumWithMapping.getDbCheckConstraintValues()).hasSize(3); + assertThat(enumWithMapping.getDbCheckConstraintValues()).contains("100", "101", "102"); + + type = typeManager.createEnumScalarType(Car.Size.class, EnumType.ORDINAL); + assertThat(type).isInstanceOf(ScalarTypeEnumWithMapping.class); + enumWithMapping = (ScalarTypeEnumWithMapping) type; + // withConstraint true + assertThat(enumWithMapping.getDbCheckConstraintValues()).hasSize(2); + assertThat(enumWithMapping.getDbCheckConstraintValues()).contains("'L'", "'S'"); + } } diff --git a/ebean-core/src/test/java/org/tests/model/array/VarcharEnum.java b/ebean-core/src/test/java/org/tests/model/array/VarcharEnum.java index 2bbc64daf..f5698d009 100644 --- a/ebean-core/src/test/java/org/tests/model/array/VarcharEnum.java +++ b/ebean-core/src/test/java/org/tests/model/array/VarcharEnum.java @@ -6,7 +6,7 @@ import io.ebean.annotation.DbEnumValue; public enum VarcharEnum { ZERO, ONE, TWO; - @DbEnumValue(storage = DbEnumType.VARCHAR) + @DbEnumValue(storage = DbEnumType.VARCHAR, withConstraint = false) public String dbValue() { return "xXx" + name(); }