From 7c477827fe5761c653dfe32886bb216ed82896ad Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Wed, 14 Feb 2018 12:39:35 +1300 Subject: [PATCH] #1259 - Throw explicit mapping error when an Enum is mapped to both ORDINAL and STRING --- .../server/deploy/parse/DeployUtil.java | 34 ++----- .../server/type/DefaultTypeManager.java | 18 ++-- .../server/type/ScalarTypeDayOfWeek.java | 9 ++ .../server/type/ScalarTypeEnum.java | 16 +++- .../server/type/ScalarTypeEnumStandard.java | 11 +++ .../type/ScalarTypeEnumWithMapping.java | 6 ++ .../server/type/ScalarTypeMonth.java | 9 ++ .../io/ebean/server/type/TestTypeManager.java | 20 +++- .../server/type/DefaultTypeManagerTest.java | 91 ++++++++++++++++++- 9 files changed, 176 insertions(+), 38 deletions(-) diff --git a/src/main/java/io/ebeaninternal/server/deploy/parse/DeployUtil.java b/src/main/java/io/ebeaninternal/server/deploy/parse/DeployUtil.java index 8f524f1ac..b821cb89a 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/parse/DeployUtil.java +++ b/src/main/java/io/ebeaninternal/server/deploy/parse/DeployUtil.java @@ -112,30 +112,16 @@ public class DeployUtil { if (!enumType.isEnum()) { throw new IllegalArgumentException("Class [" + enumType + "] is Not a Enum?"); } - ScalarType scalarType = typeManager.getScalarType(enumType); - if (enumOverrideDefaultMapping(enumerated, scalarType)) { - logger.debug("override default enum mapping for type {}", enumType); - scalarType = null; - } - if (scalarType == null) { - // look for @DbEnumValue or @EnumValue annotations etc - Class> enumClass = (Class>) enumType; - EnumType type = enumerated != null ? enumerated.value() : null; - scalarType = typeManager.createEnumScalarType(enumClass, type); - } - prop.setScalarType(scalarType); - prop.setDbType(scalarType.getJdbcType()); - } + try { + Class> enumClass = (Class>) enumType; + EnumType type = enumerated != null ? enumerated.value() : null; + ScalarType scalarType = typeManager.createEnumScalarType(enumClass, type); + prop.setScalarType(scalarType); + prop.setDbType(scalarType.getJdbcType()); - /** - * Return true if there is an existing default mapping for the enum that needs - * to be overridden (for example, DayOfWeek defaults to Integer mapping 1 to 7 - * and some might want this mapped to 'MONDAY' etc). - */ - private boolean enumOverrideDefaultMapping(Enumerated enumerated, ScalarType scalarType) { - return enumerated != null && scalarType != null - && enumerated.value() == EnumType.STRING - && scalarType.getJdbcType() != Types.VARCHAR; + } catch (IllegalStateException e) { + throw new PersistenceException("Error mapping property " + prop.getFullBeanName() + " - " + e.getMessage()); + } } /** @@ -304,7 +290,7 @@ public class DeployUtil { prop.setScalarType(scalarType); } - public boolean isClobType(Class type) { + private boolean isClobType(Class type) { return type.equals(String.class); } diff --git a/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java b/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java index 439b4f347..1e3812b3a 100644 --- a/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java +++ b/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java @@ -271,6 +271,7 @@ public final class DefaultTypeManager implements TypeManager { public void addEnumType(ScalarType scalarType, Class enumClass) { Set> mappedClasses = new HashSet<>(); + mappedClasses.add(enumClass); for (Object value : EnumSet.allOf(enumClass).toArray()) { mappedClasses.add(value.getClass()); } @@ -552,7 +553,7 @@ public final class DefaultTypeManager implements TypeManager { * Return null if the EnumValue annotations are not present/used. *

*/ - private ScalarType createEnumScalarType2(Class enumType) { + private ScalarTypeEnum createEnumScalarType2(Class enumType) { boolean integerType = true; @@ -589,8 +590,11 @@ public final class DefaultTypeManager implements TypeManager { @Override public ScalarType createEnumScalarType(Class> enumType, EnumType type) { - ScalarType scalarType = getScalarType(enumType); - if (scalarType != null) { + ScalarTypeEnum scalarType = (ScalarTypeEnum) getScalarType(enumType); + if (scalarType != null && !scalarType.isOverrideBy(type)) { + if (type != null && !scalarType.isCompatible(type)) { + throw new IllegalStateException("Error mapping Enum type:"+enumType+" It is mapped using 2 different modes when only one is supported (ORDINAL, STRING or an Ebean mapping)"); + } return scalarType; } @@ -603,7 +607,7 @@ public final class DefaultTypeManager implements TypeManager { return scalarType; } - private ScalarType createEnumScalarTypePerSpec(Class enumType, EnumType type) { + private ScalarTypeEnum createEnumScalarTypePerSpec(Class enumType, EnumType type) { if (type == null) { // default as per spec is ORDINAL return new ScalarTypeEnumStandard.OrdinalEnum(enumType); @@ -616,7 +620,7 @@ public final class DefaultTypeManager implements TypeManager { } } - private ScalarType createEnumScalarTypePerExtentions(Class> enumType) { + private ScalarTypeEnum createEnumScalarTypePerExtentions(Class> enumType) { Method[] methods = enumType.getMethods(); for (Method method : methods) { @@ -638,7 +642,7 @@ public final class DefaultTypeManager implements TypeManager { * Return null if the EnumValue annotations are not present/used. *

*/ - private ScalarType createEnumScalarTypeDbValue(Class> enumType, Method method, boolean integerType) { + private ScalarTypeEnum createEnumScalarTypeDbValue(Class> enumType, Method method, boolean integerType) { Map nameValueMap = new HashMap<>(); @@ -664,7 +668,7 @@ public final class DefaultTypeManager implements TypeManager { * length create the ScalarType for the Enum. */ @SuppressWarnings({"unchecked", "rawtypes"}) - private ScalarType createEnumScalarType(Class enumType, Map nameValueMap, boolean integerType, int dbColumnLength) { + private ScalarTypeEnum createEnumScalarType(Class enumType, Map nameValueMap, boolean integerType, int dbColumnLength) { EnumToDbValueMap beanDbMap = EnumToDbValueMap.create(integerType); diff --git a/src/main/java/io/ebeaninternal/server/type/ScalarTypeDayOfWeek.java b/src/main/java/io/ebeaninternal/server/type/ScalarTypeDayOfWeek.java index 31a46c3fc..9bfc8abca 100644 --- a/src/main/java/io/ebeaninternal/server/type/ScalarTypeDayOfWeek.java +++ b/src/main/java/io/ebeaninternal/server/type/ScalarTypeDayOfWeek.java @@ -1,5 +1,6 @@ package io.ebeaninternal.server.type; +import javax.persistence.EnumType; import java.sql.SQLException; import java.sql.Types; import java.time.DayOfWeek; @@ -22,6 +23,14 @@ public class ScalarTypeDayOfWeek extends ScalarTypeEnumWithMapping { super(beanDbMap, DayOfWeek.class, 1); } + /** + * We allow this to be overridden by a JPA EnumType. + */ + @Override + public boolean isOverrideBy(EnumType type) { + return type != null; + } + /** * Bind DayOfWeek enum using getValue(). */ diff --git a/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnum.java b/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnum.java index f01229e9d..1dc3de475 100644 --- a/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnum.java +++ b/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnum.java @@ -1,15 +1,29 @@ package io.ebeaninternal.server.type; +import javax.persistence.EnumType; import java.util.Set; /** * Marker interface for the Enum scalar types. */ -public interface ScalarTypeEnum { +public interface ScalarTypeEnum extends ScalarType { /** * Return the IN values for DB constraint construction. */ Set getDbCheckConstraintValues(); + /** + * Return true if we allow this scalar enum type to be overridden. + * Ability to override the built-in support for java time DayOfWeek and Month. + */ + default boolean isOverrideBy(EnumType type) { + return false; + } + + /** + * Return true if the scalar type is compatible with the specified enum type. + */ + boolean isCompatible(EnumType enumType); + } diff --git a/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumStandard.java b/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumStandard.java index cbcf533ab..17a66a69b 100644 --- a/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumStandard.java +++ b/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumStandard.java @@ -5,6 +5,7 @@ import com.fasterxml.jackson.core.JsonParser; import io.ebean.text.TextException; import io.ebeanservice.docstore.api.mapping.DocPropertyType; +import javax.persistence.EnumType; import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; @@ -40,6 +41,11 @@ public class ScalarTypeEnumStandard { this.length = maxValueLength(enumType); } + @Override + public boolean isCompatible(EnumType enumType) { + return EnumType.STRING == enumType; + } + /** * Return the IN values for DB constraint construction. */ @@ -128,6 +134,11 @@ public class ScalarTypeEnumStandard { this.enumArray = EnumSet.allOf(enumType).toArray(); } + @Override + public boolean isCompatible(EnumType enumType) { + return EnumType.ORDINAL == enumType; + } + /** * Return the IN values for DB constraint construction. */ diff --git a/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java b/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java index 56e3674fc..fe996266b 100644 --- a/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java +++ b/src/main/java/io/ebeaninternal/server/type/ScalarTypeEnumWithMapping.java @@ -1,5 +1,6 @@ package io.ebeaninternal.server.type; +import javax.persistence.EnumType; import java.sql.SQLException; import java.util.Iterator; import java.util.LinkedHashSet; @@ -24,6 +25,11 @@ public class ScalarTypeEnumWithMapping extends ScalarTypeEnumStandard.EnumBase i this.length = length; } + @Override + public boolean isCompatible(EnumType enumType) { + return enumType == null; + } + @Override public long asVersion(Object value) { throw new RuntimeException("not supported"); diff --git a/src/main/java/io/ebeaninternal/server/type/ScalarTypeMonth.java b/src/main/java/io/ebeaninternal/server/type/ScalarTypeMonth.java index c4605cad9..0f86ed315 100644 --- a/src/main/java/io/ebeaninternal/server/type/ScalarTypeMonth.java +++ b/src/main/java/io/ebeaninternal/server/type/ScalarTypeMonth.java @@ -1,5 +1,6 @@ package io.ebeaninternal.server.type; +import javax.persistence.EnumType; import java.sql.SQLException; import java.sql.Types; import java.time.Month; @@ -22,6 +23,14 @@ public class ScalarTypeMonth extends ScalarTypeEnumWithMapping { super(beanDbMap, Month.class, 1); } + /** + * We allow this to be overridden by a JPA EnumType. + */ + @Override + public boolean isOverrideBy(EnumType type) { + return type != null; + } + /** * Bind Month enum value. */ diff --git a/src/test/java/io/ebean/server/type/TestTypeManager.java b/src/test/java/io/ebean/server/type/TestTypeManager.java index 4e552b69f..e5ab13393 100644 --- a/src/test/java/io/ebean/server/type/TestTypeManager.java +++ b/src/test/java/io/ebean/server/type/TestTypeManager.java @@ -12,10 +12,12 @@ import org.junit.Test; import org.tests.model.ivo.Money; import org.tests.model.ivo.converter.MoneyTypeConverter; +import javax.persistence.EnumType; import java.sql.SQLException; import java.sql.Types; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertTrue; public class TestTypeManager extends BaseTestCase { @@ -42,6 +44,13 @@ public class TestTypeManager extends BaseTestCase { assertThat(typeA).isNotNull(); ScalarType typeC = typeManager.getScalarType(MyEnum.Cval.getClass()); assertThat(typeC).isNotNull(); + + try { + typeManager.createEnumScalarType(MyEnum.class, EnumType.STRING); + assertTrue("never get here",false); + } catch (IllegalStateException e) { + assertThat(e.getMessage()).contains("It is mapped using 2 different modes when only one is supported"); + } } @Test @@ -65,6 +74,13 @@ public class TestTypeManager extends BaseTestCase { val = dayOfWeekType.read(new DummyDataReader("FRIDAY ")); assertThat(val).isEqualTo(MyDayOfWeek.FRIDAY); + + try { + typeManager.createEnumScalarType(MyDayOfWeek.class, EnumType.ORDINAL); + assertTrue("never get here",false); + } catch (IllegalStateException e) { + assertThat(e.getMessage()).contains("It is mapped using 2 different modes when only one is supported"); + } } @Test @@ -73,8 +89,8 @@ public class TestTypeManager extends BaseTestCase { DefaultTypeManager typeManager = createTypeManager(); ScalarType scalarType = typeManager.getScalarType(Money.class); - Assert.assertTrue(scalarType.getJdbcType() == Types.DECIMAL); - Assert.assertTrue(!scalarType.isJdbcNative()); + assertTrue(scalarType.getJdbcType() == Types.DECIMAL); + assertTrue(!scalarType.isJdbcNative()); Assert.assertEquals(Money.class, scalarType.getType()); } diff --git a/src/test/java/io/ebeaninternal/server/type/DefaultTypeManagerTest.java b/src/test/java/io/ebeaninternal/server/type/DefaultTypeManagerTest.java index 635f64d04..b03a452f4 100644 --- a/src/test/java/io/ebeaninternal/server/type/DefaultTypeManagerTest.java +++ b/src/test/java/io/ebeaninternal/server/type/DefaultTypeManagerTest.java @@ -5,23 +5,28 @@ import io.ebean.config.dbplatform.postgres.PostgresPlatform; import io.ebeaninternal.server.core.bootup.BootupClasses; import org.junit.Test; +import javax.persistence.EnumType; +import java.time.DayOfWeek; +import java.time.Month; + +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class DefaultTypeManagerTest { - DefaultTypeManager typeManager; - - public DefaultTypeManagerTest() { + private DefaultTypeManager create() { ServerConfig serverConfig = new ServerConfig(); serverConfig.setDatabasePlatform(new PostgresPlatform()); BootupClasses bootupClasses = new BootupClasses(); - typeManager = new DefaultTypeManager(serverConfig, bootupClasses); + return new DefaultTypeManager(serverConfig, bootupClasses); } @Test public void isIntegerType() { + DefaultTypeManager typeManager = create(); + assertTrue(typeManager.isIntegerType("1")); assertTrue(typeManager.isIntegerType("0")); @@ -33,4 +38,82 @@ public class DefaultTypeManagerTest { assertFalse(typeManager.isIntegerType(" A")); } + + @Test + public void enumDayMonth_builtIn_overrideAsString() { + + DefaultTypeManager typeManager = create(); + + ScalarType type = typeManager.createEnumScalarType(Month.class, null); + assertThat(type).isInstanceOf(ScalarTypeEnumWithMapping.class).as("built in type"); + + // 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"); + + } catch (IllegalStateException e) { + assertThat(e.getMessage()).contains("It is mapped using 2 different modes when only one is supported"); + } + } + + @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"); + } + } + + @Test + public void enumDayOfWeek_builtIn_overrideAsString() { + + DefaultTypeManager typeManager = create(); + + ScalarType type = typeManager.createEnumScalarType(DayOfWeek.class, null); + assertThat(type).isInstanceOf(ScalarTypeEnumWithMapping.class).as("built in type"); + + // 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"); + } + } + + @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"); + } + } }