From f055a0d0cb91364052ca4687e65b60fca3235894 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Wed, 17 Feb 2016 21:34:52 +1300 Subject: [PATCH] #420 - Review JodaLocalTime (being stored and fetched using UTC) --- .../com/avaje/ebean/config/ServerConfig.java | 17 +++++ .../server/type/DefaultTypeManager.java | 23 ++++++- .../server/type/ScalarTypeJodaLocalTime.java | 12 ++-- .../type/ScalarTypeJodaLocalTimeUTC.java | 62 +++++++++++++++++++ .../type/ScalarTypeJodaLocalTimeTest.java | 14 +++++ .../com/avaje/tests/basic/TestJodaType.java | 19 +++++- src/test/resources/test-ebean.properties | 4 ++ 7 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTimeUTC.java diff --git a/src/main/java/com/avaje/ebean/config/ServerConfig.java b/src/main/java/com/avaje/ebean/config/ServerConfig.java index a4e2e1168..d13fe2c97 100644 --- a/src/main/java/com/avaje/ebean/config/ServerConfig.java +++ b/src/main/java/com/avaje/ebean/config/ServerConfig.java @@ -387,6 +387,8 @@ public class ServerConfig { */ private boolean expressionEqualsWithNullAsNoop; + private String jodaLocalTimeMode; + /** * Construct a Server Configuration for programmatically creating an EbeanServer. */ @@ -1640,6 +1642,20 @@ public class ServerConfig { this.disableClasspathSearch = disableClasspathSearch; } + /** + * Return the mode to use for Joda LocalTime support 'normal' or 'utc'. + */ + public String getJodaLocalTimeMode() { + return jodaLocalTimeMode; + } + + /** + * Set the mode to use for Joda LocalTime support 'normal' or 'utc'. + */ + public void setJodaLocalTimeMode(String jodaLocalTimeMode) { + this.jodaLocalTimeMode = jodaLocalTimeMode; + } + /** * Programmatically add classes (typically entities) that this server should * use. @@ -2286,6 +2302,7 @@ public class ServerConfig { dbUuid = DbUuid.BINARY; } localTimeWithNanos = p.getBoolean("localTimeWithNanos", localTimeWithNanos); + jodaLocalTimeMode = p.get("jodaLocalTimeMode", jodaLocalTimeMode); lazyLoadBatchSize = p.getInt("lazyLoadBatchSize", lazyLoadBatchSize); queryBatchSize = p.getInt("queryBatchSize", queryBatchSize); diff --git a/src/main/java/com/avaje/ebeaninternal/server/type/DefaultTypeManager.java b/src/main/java/com/avaje/ebeaninternal/server/type/DefaultTypeManager.java index dbdc4be17..8d3be52df 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/type/DefaultTypeManager.java +++ b/src/main/java/com/avaje/ebeaninternal/server/type/DefaultTypeManager.java @@ -7,7 +7,6 @@ import com.avaje.ebean.annotation.EnumValue; import com.avaje.ebean.config.*; import com.avaje.ebean.config.dbplatform.DatabasePlatform; import com.avaje.ebean.config.dbplatform.DbType; -import com.avaje.ebeaninternal.api.ClassUtil; import com.avaje.ebeaninternal.server.core.BootupClasses; import com.avaje.ebeaninternal.server.lib.util.StringHelper; import com.avaje.ebeaninternal.server.type.reflect.*; @@ -285,7 +284,15 @@ public final class DefaultTypeManager implements TypeManager, KnownImmutable { */ @SuppressWarnings("unchecked") public ScalarType getScalarType(Class type) { - return (ScalarType) typeMap.get(type); + ScalarType found = (ScalarType) typeMap.get(type); + if (found == null) { + if (type.getName().equals("org.joda.time.LocalTime")) { + throw new IllegalStateException( + "ScalarType of Joda LocalTime not defined. You need to set ServerConfig.jodaLocalTimeMode to" + + " either 'normal' or 'utc'. UTC is the old mode using UTC timezone but local time zone is now preferred as 'normal' mode."); + } + } + return found; } public ScalarDataReader getScalarDataReader(Class propertyType, int sqlType) { @@ -805,8 +812,18 @@ public final class DefaultTypeManager implements TypeManager, KnownImmutable { typeMap.put(LocalDateTime.class, new ScalarTypeJodaLocalDateTime(mode)); typeMap.put(DateTime.class, new ScalarTypeJodaDateTime(mode)); typeMap.put(LocalDate.class, new ScalarTypeJodaLocalDate()); - typeMap.put(LocalTime.class, new ScalarTypeJodaLocalTime()); typeMap.put(DateMidnight.class, new ScalarTypeJodaDateMidnight()); + + String jodaLocalTimeMode = config.getJodaLocalTimeMode(); + if ("normal".equalsIgnoreCase(jodaLocalTimeMode)) { + // use the expected/normal local time zone + typeMap.put(LocalTime.class, new ScalarTypeJodaLocalTime()); + logger.debug("registered ScalarTypeJodaLocalTime"); + } else if ("utc".equalsIgnoreCase(jodaLocalTimeMode)) { + // use the old UTC based + typeMap.put(LocalTime.class, new ScalarTypeJodaLocalTimeUTC()); + logger.debug("registered ScalarTypeJodaLocalTimeUTC"); + } } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTime.java b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTime.java index eff2bb282..a41d4d823 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTime.java +++ b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTime.java @@ -28,8 +28,7 @@ public class ScalarTypeJodaLocalTime extends ScalarTypeBase { if (value == null) { b.setNull(Types.TIME); } else { - Time sqlTime = new Time(value.getMillisOfDay()); - b.setTime(sqlTime); + b.setTime(new Time(value.getHourOfDay(), value.getMinuteOfHour(), value.getSecondOfMinute())); } } @@ -40,14 +39,15 @@ public class ScalarTypeJodaLocalTime extends ScalarTypeBase { if (sqlTime == null) { return null; } else { - return new LocalTime(sqlTime, DateTimeZone.UTC); + return new LocalTime(sqlTime, DateTimeZone.getDefault()); } } @Override public Object toJdbcType(Object value) { if (value instanceof LocalTime) { - return new Time(((LocalTime) value).getMillisOfDay()); + LocalTime lt = (LocalTime) value; + return new Time(lt.getHourOfDay(), lt.getMinuteOfHour(), lt.getSecondOfMinute()); } return BasicTypeConverter.toTime(value); } @@ -55,7 +55,7 @@ public class ScalarTypeJodaLocalTime extends ScalarTypeBase { @Override public LocalTime toBeanType(Object value) { if (value instanceof java.util.Date) { - return new LocalTime(value, DateTimeZone.UTC); + return new LocalTime(value, DateTimeZone.getDefault()); } return (LocalTime) value; } @@ -88,7 +88,7 @@ public class ScalarTypeJodaLocalTime extends ScalarTypeBase { @Override public LocalTime convertFromMillis(long systemTimeMillis) { - return new LocalTime(systemTimeMillis); + return new LocalTime(systemTimeMillis, DateTimeZone.getDefault()); } @Override diff --git a/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTimeUTC.java b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTimeUTC.java new file mode 100644 index 000000000..7f4c2549f --- /dev/null +++ b/src/main/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTimeUTC.java @@ -0,0 +1,62 @@ +package com.avaje.ebeaninternal.server.type; + +import com.avaje.ebeaninternal.server.core.BasicTypeConverter; +import org.joda.time.DateTimeZone; +import org.joda.time.LocalTime; + +import java.sql.SQLException; +import java.sql.Time; +import java.sql.Types; + +/** + * ScalarType for Joda LocalTime. This maps to a JDBC Time. + */ +public class ScalarTypeJodaLocalTimeUTC extends ScalarTypeJodaLocalTime { + + public ScalarTypeJodaLocalTimeUTC() { + super(); + } + + @Override + public void bind(DataBind b, LocalTime value) throws SQLException { + if (value == null) { + b.setNull(Types.TIME); + } else { + Time sqlTime = new Time(value.getMillisOfDay()); + b.setTime(sqlTime); + } + } + + @Override + public LocalTime read(DataReader dataReader) throws SQLException { + + Time sqlTime = dataReader.getTime(); + if (sqlTime == null) { + return null; + } else { + return new LocalTime(sqlTime, DateTimeZone.UTC); + } + } + + @Override + public Object toJdbcType(Object value) { + if (value instanceof LocalTime) { + return new Time(((LocalTime) value).getMillisOfDay()); + } + return BasicTypeConverter.toTime(value); + } + + @Override + public LocalTime toBeanType(Object value) { + if (value instanceof java.util.Date) { + return new LocalTime(value, DateTimeZone.UTC); + } + return (LocalTime) value; + } + + @Override + public LocalTime convertFromMillis(long systemTimeMillis) { + return new LocalTime(systemTimeMillis); + } + +} diff --git a/src/test/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTimeTest.java b/src/test/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTimeTest.java index 952fc765c..d36c284b1 100644 --- a/src/test/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTimeTest.java +++ b/src/test/java/com/avaje/ebeaninternal/server/type/ScalarTypeJodaLocalTimeTest.java @@ -2,14 +2,28 @@ package com.avaje.ebeaninternal.server.type; import org.joda.time.DateTimeZone; import org.joda.time.LocalDateTime; +import org.joda.time.LocalTime; import org.junit.Test; import java.sql.Timestamp; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; public class ScalarTypeJodaLocalTimeTest { + ScalarTypeJodaLocalTime type = new ScalarTypeJodaLocalTime(); + + @Test + public void toJdbcType_toBeanType() { + + LocalTime localTime0 = new LocalTime(); + Object time = type.toJdbcType(localTime0); + LocalTime localTime1 = type.toBeanType(time); + + assertThat(localTime0).isEqualTo(localTime1); + } + @Test public void test() { diff --git a/src/test/java/com/avaje/tests/basic/TestJodaType.java b/src/test/java/com/avaje/tests/basic/TestJodaType.java index 1bafb0dfd..57c90eaff 100644 --- a/src/test/java/com/avaje/tests/basic/TestJodaType.java +++ b/src/test/java/com/avaje/tests/basic/TestJodaType.java @@ -1,5 +1,6 @@ package com.avaje.tests.basic; +import org.joda.time.LocalTime; import org.junit.Assert; import org.junit.Test; @@ -11,6 +12,8 @@ import com.avaje.ebeaninternal.server.deploy.BeanProperty; import com.avaje.ebeaninternal.server.type.ScalarType; import com.avaje.tests.model.basic.TJodaEntity; +import static org.assertj.core.api.Assertions.assertThat; + public class TestJodaType extends BaseTestCase { @Test @@ -23,5 +26,19 @@ public class TestJodaType extends BaseTestCase { Assert.assertNotNull(scalarType); } - + + @Test + public void test_insert_find() { + + LocalTime now = new LocalTime().withMillisOfSecond(0); + + TJodaEntity bean = new TJodaEntity(); + bean.setLocalTime(now); + Ebean.save(bean); + + TJodaEntity foundBean = Ebean.find(TJodaEntity.class, bean.getId()); + + assertThat(foundBean.getLocalTime()).isEqualTo(bean.getLocalTime()); + } + } diff --git a/src/test/resources/test-ebean.properties b/src/test/resources/test-ebean.properties index ed085d404..3ee993b51 100644 --- a/src/test/resources/test-ebean.properties +++ b/src/test/resources/test-ebean.properties @@ -1 +1,5 @@ datasource.h2.username=sa + + +ebean.jodaLocalTimeMode=normal +#ebean.jodaLocalTimeMode=utc \ No newline at end of file