From 483f0398b2d9ded9685b7566ac8b8ca4c13f97cf Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Wed, 24 Nov 2021 14:37:45 +0100 Subject: [PATCH 1/5] Fix: 'No ScalarType registered for class java.util.GregorianCalendar' - check all registered scalar type if no exact match was found --- .../io/ebeaninternal/server/type/DefaultTypeManager.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 544d8433e..42c9e2ded 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 @@ -43,6 +43,7 @@ import java.sql.Timestamp; import java.sql.Types; import java.time.*; import java.util.*; +import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; /** @@ -281,8 +282,11 @@ public final class DefaultTypeManager implements TypeManager { } private ScalarType checkInterfaceTypes(Class type) { - if (java.nio.file.Path.class.isAssignableFrom(type)) { - return typeMap.get(java.nio.file.Path.class); + for (Entry, ScalarType> entry : typeMap.entrySet()) { + if (entry.getKey().isAssignableFrom(type)) { + typeMap.put(type, entry.getValue()); + return entry.getValue(); + } } return null; } From 84ac737ceb7d62da9f46eea06d5f2bcde6d4fce3 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 2 Dec 2021 10:13:15 +0100 Subject: [PATCH 2/5] ADD: Testcase for Calendar --- .../io/ebeaninternal/server/type/TestTypeManager.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ebean-test/src/test/java/io/ebeaninternal/server/type/TestTypeManager.java b/ebean-test/src/test/java/io/ebeaninternal/server/type/TestTypeManager.java index c28da92a7..383671363 100644 --- a/ebean-test/src/test/java/io/ebeaninternal/server/type/TestTypeManager.java +++ b/ebean-test/src/test/java/io/ebeaninternal/server/type/TestTypeManager.java @@ -15,6 +15,8 @@ import org.tests.model.ivo.converter.MoneyTypeConverter; import javax.persistence.EnumType; import java.sql.SQLException; import java.sql.Types; +import java.util.Calendar; +import java.util.GregorianCalendar; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; @@ -29,7 +31,6 @@ public class TestTypeManager extends BaseTestCase { DefaultTypeManager typeManager = createTypeManager(); ScalarType type = typeManager.createEnumScalarType(MyEnum.class, null); - typeManager.addEnumType(type, MyEnum.class); Object val = type.read(new DummyDataReader("A")); assertThat(val).isEqualTo(MyEnum.Aval); @@ -131,6 +132,13 @@ public class TestTypeManager extends BaseTestCase { return new DefaultTypeManager(config, bootupClasses); } + @Test + public void testCalendar() throws SQLException { + + DefaultTypeManager typeManager = createTypeManager(); + ScalarType typeB = typeManager.getScalarType(GregorianCalendar.class); + assertThat(typeB).isInstanceOf(ScalarTypeCalendar.class); + } /** * Test double DataReader implementation. */ From f830662cce91e550f08a02f6b6646977a8817cef Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 2 Dec 2021 11:18:22 +0100 Subject: [PATCH 3/5] Improve TypeManager - cache typeNotFound information. --- .../server/type/DefaultTypeManager.java | 76 +++++----- .../server/type/ScalarTypeNotFound.java | 140 ++++++++++++++++++ .../server/type/TypeManager.java | 6 - 3 files changed, 179 insertions(+), 43 deletions(-) create mode 100644 ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeNotFound.java 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 42c9e2ded..941d5745c 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 @@ -43,7 +43,6 @@ import java.sql.Timestamp; import java.sql.Types; import java.time.*; import java.util.*; -import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; /** @@ -194,7 +193,7 @@ public final class DefaultTypeManager implements TypeManager { List> types = plugin.createTypes(config, objectMapper); for (ScalarType type : types) { log.debug("adding ScalarType {}", type.getClass()); - addCustomType(type); + add(type); } } } @@ -212,25 +211,6 @@ public final class DefaultTypeManager implements TypeManager { logAdd(scalarType); } - /** - * Register the ScalarType for an enum. This is special in the sense that an Enum - * can have many classes if it uses method overrides and we need to register all - * the variations/classes for the enum. - */ - @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()) { - mappedClasses.add(value.getClass()); - } - for (Class cls : mappedClasses) { - typeMap.put(cls, scalarType); - } - logAdd(scalarType); - } - private void logAdd(ScalarType scalarType) { if (log.isTraceEnabled()) { String msg = "ScalarType register [" + scalarType.getClass().getName() + "]"; @@ -270,25 +250,50 @@ public final class DefaultTypeManager implements TypeManager { @Override public ScalarType getScalarType(Class type) { ScalarType found = typeMap.get(type); - if (found == null) { + if (found == null || found == ScalarTypeNotFound.INSTANCE) { + found = null; if (type.getName().equals("org.joda.time.LocalTime")) { throw new IllegalStateException( "ScalarType of Joda LocalTime not defined. You need to set DatabaseConfig.jodaLocalTimeMode to" + " either 'normal' or 'utc'. UTC is the old mode using UTC timezone but local time zone is now preferred as 'normal' mode."); } - found = checkInterfaceTypes(type); + found = checkInheritedTypes(type); } return found; } - private ScalarType checkInterfaceTypes(Class type) { - for (Entry, ScalarType> entry : typeMap.entrySet()) { - if (entry.getKey().isAssignableFrom(type)) { - typeMap.put(type, entry.getValue()); - return entry.getValue(); + /** + * Checks the typeMap for inherited types. + * + * If type is a GregorianCalendar, then this method + * will check the class hierarchy and will probably return a + * ScalarTypeCalendar To speed up a second lookup, it will write + * back the found scalarType to typeMap. + * + * @param type + * @return + */ + private ScalarType checkInheritedTypes(Class type) { + // first step loop through inheritance chain + Class parent = type; + while (parent != null && parent != Object.class) { + ScalarType found = typeMap.get(parent); + if (found != null && found != ScalarTypeNotFound.INSTANCE) { + typeMap.put(type, found); // store type for next lookup + return found; + } + parent = parent.getSuperclass(); + } + // second step - loop through interface chain + for (Class iface: type.getInterfaces()) { + ScalarType found = checkInheritedTypes(iface); + if (found != null && found != ScalarTypeNotFound.INSTANCE) { + typeMap.put(type, found); // store type for next lookup + return found; } } - return null; + typeMap.put(type, ScalarTypeNotFound.INSTANCE); + return null; // no success } @Override @@ -394,8 +399,8 @@ public final class DefaultTypeManager implements TypeManager { private DocPropertyType getDocType(Type genericType) { if (genericType instanceof Class) { - ScalarType found = typeMap.get(genericType); - if (found != null) { + ScalarType found = getScalarType((Class)genericType); + if (found != null && found != ScalarTypeNotFound.INSTANCE) { return found.getDocType(); } } @@ -452,7 +457,7 @@ public final class DefaultTypeManager implements TypeManager { return scalarType; } - scalarType = typeMap.get(type); + scalarType = getScalarType(type); if (scalarType != null) { if (jdbcType == 0 || scalarType.getJdbcType() == jdbcType) { // matching type @@ -570,7 +575,7 @@ public final class DefaultTypeManager implements TypeManager { // use JPA normal Enum type (without mapping) scalarEnum = createEnumScalarTypePerSpec(enumType, type); } - addEnumType(scalarEnum, enumType); + add(scalarEnum); return scalarEnum; } @@ -660,16 +665,13 @@ public final class DefaultTypeManager implements TypeManager { scalarType = cls.getDeclaredConstructor().newInstance(); } } - addCustomType(scalarType); + add(scalarType); } catch (Exception e) { log.error("Error loading ScalarType [" + cls.getName() + "]", e); } } } - private void addCustomType(ScalarType scalarType) { - add(scalarType); - } private Object initObjectMapper(DatabaseConfig config) { Object objectMapper = config.getObjectMapper(); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeNotFound.java b/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeNotFound.java new file mode 100644 index 000000000..7fbb5b5e9 --- /dev/null +++ b/ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeNotFound.java @@ -0,0 +1,140 @@ +package io.ebeaninternal.server.type; + +import java.io.DataInput; +import java.io.DataOutput; +import java.io.IOException; +import java.sql.SQLException; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; + +import io.ebean.core.type.DataBinder; +import io.ebean.core.type.DataReader; +import io.ebean.core.type.DocPropertyType; +import io.ebean.core.type.ScalarType; + +/** + * Class is required as "null" key in ConcurrentHashMap. + * @author Roland Praml, FOCONIS AG + * + */ +class ScalarTypeNotFound implements ScalarType { + + public static final ScalarTypeNotFound INSTANCE = new ScalarTypeNotFound(); + private ScalarTypeNotFound() { } + @Override + public boolean isBinaryType() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isMutable() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isDirty(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public int getLength() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isJdbcNative() { + throw new UnsupportedOperationException(); + } + + @Override + public int getJdbcType() { + throw new UnsupportedOperationException(); + } + + @Override + public Class getType() { + throw new UnsupportedOperationException(); + } + + @Override + public Void read(DataReader reader) throws SQLException { + throw new UnsupportedOperationException(); + } + + @Override + public void loadIgnore(DataReader reader) { + throw new UnsupportedOperationException(); + } + + @Override + public void bind(DataBinder binder, Void value) throws SQLException { + throw new UnsupportedOperationException(); + } + + @Override + public Object toJdbcType(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public Void toBeanType(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public String formatValue(Void value) { + throw new UnsupportedOperationException(); + } + + @Override + public String format(Object value) { + throw new UnsupportedOperationException(); + } + + @Override + public Void parse(String value) { + throw new UnsupportedOperationException(); + } + + @Override + public DocPropertyType getDocType() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isDateTimeCapable() { + throw new UnsupportedOperationException(); + } + + @Override + public long asVersion(Void value) { + throw new UnsupportedOperationException(); + } + + @Override + public Void convertFromMillis(long dateTime) { + throw new UnsupportedOperationException(); + } + + @Override + public Void readData(DataInput dataInput) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void writeData(DataOutput dataOutput, Void v) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public Void jsonRead(JsonParser parser) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void jsonWrite(JsonGenerator writer, Void value) throws IOException { + throw new UnsupportedOperationException(); + } + +} diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/type/TypeManager.java b/ebean-core/src/main/java/io/ebeaninternal/server/type/TypeManager.java index 692e65681..079782972 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/type/TypeManager.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/type/TypeManager.java @@ -17,12 +17,6 @@ public interface TypeManager { */ void add(ScalarType scalarType); - /** - * Register a ScalarType for an Enum with can have multiple classes. - */ - @SuppressWarnings("rawtypes") - void addEnumType(ScalarType type, Class myEnumClass); - /** * Return the scalar type for the given logical type. */ From eb21b41fba722854d4a73b4b92082a7241ecd7b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20P=C3=B6hler?= Date: Wed, 5 Jan 2022 14:13:09 +0100 Subject: [PATCH 4/5] Refactor and fixes for interfaces (like Path) + extend existing tests --- .../server/type/DefaultTypeManager.java | 35 +++++++++---------- .../tests/model/types/SomeNewTypesBean.java | 11 ++++++ .../java/org/tests/types/TestNewTypes.java | 9 ++++- 3 files changed, 36 insertions(+), 19 deletions(-) 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 401ad2216..81d6a1ea2 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 @@ -250,8 +250,7 @@ public final class DefaultTypeManager implements TypeManager { @Override public ScalarType getScalarType(Class type) { ScalarType found = typeMap.get(type); - if (found == null || found == ScalarTypeNotFound.INSTANCE) { - found = null; + if (found == null) { if (type.getName().equals("org.joda.time.LocalTime")) { throw new IllegalStateException( "ScalarType of Joda LocalTime not defined. You need to set DatabaseConfig.jodaLocalTimeMode to" @@ -259,19 +258,19 @@ public final class DefaultTypeManager implements TypeManager { } found = checkInheritedTypes(type); } - return found; + return found != ScalarTypeNotFound.INSTANCE ? found : null; // Do not return ScalarTypeNotFound, otherwise checks will fail } /** * Checks the typeMap for inherited types. - * - * If type is a GregorianCalendar, then this method + * + * If e.g. type is a GregorianCalendar, then this method * will check the class hierarchy and will probably return a * ScalarTypeCalendar To speed up a second lookup, it will write * back the found scalarType to typeMap. - * - * @param type - * @return + * + * @param type the for which to search for a ScalarType + * @return either a valid ScalarType if one could be found or {@link ScalarTypeNotFound#INSTANCE} if not */ private ScalarType checkInheritedTypes(Class type) { // first step loop through inheritance chain @@ -282,18 +281,18 @@ public final class DefaultTypeManager implements TypeManager { typeMap.put(type, found); // store type for next lookup return found; } + // second step - loop through interfaces of this type + for (Class iface: parent.getInterfaces()) { + found = checkInheritedTypes(iface); + if (found != null && found != ScalarTypeNotFound.INSTANCE) { + typeMap.put(type, found); // store type for next lookup + return found; + } + } parent = parent.getSuperclass(); } - // second step - loop through interface chain - for (Class iface: type.getInterfaces()) { - ScalarType found = checkInheritedTypes(iface); - if (found != null && found != ScalarTypeNotFound.INSTANCE) { - typeMap.put(type, found); // store type for next lookup - return found; - } - } typeMap.put(type, ScalarTypeNotFound.INSTANCE); - return null; // no success + return ScalarTypeNotFound.INSTANCE; // no success } @Override @@ -399,7 +398,7 @@ public final class DefaultTypeManager implements TypeManager { private DocPropertyType getDocType(Type genericType) { if (genericType instanceof Class) { ScalarType found = getScalarType((Class)genericType); - if (found != null && found != ScalarTypeNotFound.INSTANCE) { + if (found != null) { return found.getDocType(); } } diff --git a/ebean-test/src/test/java/org/tests/model/types/SomeNewTypesBean.java b/ebean-test/src/test/java/org/tests/model/types/SomeNewTypesBean.java index 2596dbcba..1a0fee0bf 100644 --- a/ebean-test/src/test/java/org/tests/model/types/SomeNewTypesBean.java +++ b/ebean-test/src/test/java/org/tests/model/types/SomeNewTypesBean.java @@ -7,6 +7,7 @@ import javax.persistence.Version; import java.nio.file.Path; import java.sql.Date; import java.time.*; +import java.util.Calendar; @Entity public class SomeNewTypesBean { @@ -58,6 +59,8 @@ public class SomeNewTypesBean { Duration duration; + Calendar calendar; + public Long getId() { return id; } @@ -217,4 +220,12 @@ public class SomeNewTypesBean { public void setDuration(Duration duration) { this.duration = duration; } + + public Calendar getCalendar() { + return calendar; + } + + public void setCalendar(final Calendar calendar) { + this.calendar = calendar; + } } diff --git a/ebean-test/src/test/java/org/tests/types/TestNewTypes.java b/ebean-test/src/test/java/org/tests/types/TestNewTypes.java index 35732801e..a40c673ea 100644 --- a/ebean-test/src/test/java/org/tests/types/TestNewTypes.java +++ b/ebean-test/src/test/java/org/tests/types/TestNewTypes.java @@ -10,6 +10,8 @@ import org.tests.model.types.SomeNewTypesBean; import java.io.File; import java.nio.file.Paths; import java.time.*; +import java.util.Calendar; +import java.util.GregorianCalendar; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -44,6 +46,7 @@ public class TestNewTypes extends BaseTestCase { bean.setPath(Paths.get(TEMP_PATH)); bean.setPeriod(Period.of(4,3,2)); bean.setDuration(Duration.ofMinutes(5)); + bean.setCalendar(Calendar.getInstance()); DB.save(bean); @@ -104,6 +107,9 @@ public class TestNewTypes extends BaseTestCase { list = DB.find(SomeNewTypesBean.class).where().eq("duration", Duration.ofMinutes(5)).findList(); assertThat(list).isNotEmpty(); + list = DB.find(SomeNewTypesBean.class).where().le("calendar", GregorianCalendar.getInstance()).findList(); + assertThat(list).isNotEmpty(); + SomeNewTypesBean fetched = DB.find(SomeNewTypesBean.class, bean.getId()); assertEquals(bean.getZoneId(), fetched.getZoneId()); @@ -122,6 +128,7 @@ public class TestNewTypes extends BaseTestCase { assertEquals(bean.getPath(), fetched.getPath()); assertEquals(bean.getPeriod(), fetched.getPeriod()); assertEquals(bean.getDuration(), fetched.getDuration()); + assertEquals(bean.getCalendar(), fetched.getCalendar()); String asJson = DB.json().toJson(fetched); @@ -139,10 +146,10 @@ public class TestNewTypes extends BaseTestCase { assertThat(toBean.getOffsetDateTime()).isEqualToIgnoringNanos(bean.getOffsetDateTime()); assertEquals(bean.getLocalTime().toSecondOfDay(), toBean.getLocalTime().toSecondOfDay()); assertEquals(bean.getInstant().toEpochMilli() / 1000, toBean.getInstant().toEpochMilli() / 1000); - // FIXME: This test fails on Windows with: expected:<\tmp> but was: assertEquals(bean.getPath(), toBean.getPath()); assertEquals(bean.getPeriod(), toBean.getPeriod()); assertEquals(bean.getDuration(), toBean.getDuration()); + assertEquals(bean.getCalendar(), toBean.getCalendar()); } From e7f9c6350261c0b2e24c44fd06d115aa8f5d5beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20P=C3=B6hler?= Date: Wed, 5 Jan 2022 15:46:34 +0100 Subject: [PATCH 5/5] Clear up some possible confusion in the test --- ebean-test/src/test/java/org/tests/types/TestNewTypes.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ebean-test/src/test/java/org/tests/types/TestNewTypes.java b/ebean-test/src/test/java/org/tests/types/TestNewTypes.java index a40c673ea..90ffd842a 100644 --- a/ebean-test/src/test/java/org/tests/types/TestNewTypes.java +++ b/ebean-test/src/test/java/org/tests/types/TestNewTypes.java @@ -11,7 +11,6 @@ import java.io.File; import java.nio.file.Paths; import java.time.*; import java.util.Calendar; -import java.util.GregorianCalendar; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -107,7 +106,8 @@ public class TestNewTypes extends BaseTestCase { list = DB.find(SomeNewTypesBean.class).where().eq("duration", Duration.ofMinutes(5)).findList(); assertThat(list).isNotEmpty(); - list = DB.find(SomeNewTypesBean.class).where().le("calendar", GregorianCalendar.getInstance()).findList(); + // Calendar.getInstance() returns an implementation, which then has to be remapped to ScalarTypeCalendar + list = DB.find(SomeNewTypesBean.class).where().le("calendar", Calendar.getInstance()).findList(); assertThat(list).isNotEmpty(); SomeNewTypesBean fetched = DB.find(SomeNewTypesBean.class, bean.getId());