From 5369ae5d43f5d6210e7bfe2ef179fb71e1e6758f Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Wed, 24 Jul 2019 12:34:51 -0700 Subject: [PATCH 1/2] Revert "HHH-13266 HHH-13357 : Skip OffsetTimeTest#nativeWriteThenRead and #writeThenRead in some cases due to HHH-13357" This reverts commit b7b8f44298191db050f12819ca0594a30124df75. (cherry picked from commit 352b029404877822468738e693136e9a5ced6342) --- .../test/type/AbstractJavaTimeTypeTest.java | 9 +++---- .../hibernate/test/type/OffsetTimeTest.java | 24 ------------------- 2 files changed, 5 insertions(+), 28 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/test/type/AbstractJavaTimeTypeTest.java b/hibernate-core/src/test/java/org/hibernate/test/type/AbstractJavaTimeTypeTest.java index 1c5db51aeab8..1c83b62cc4e6 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/type/AbstractJavaTimeTypeTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/type/AbstractJavaTimeTypeTest.java @@ -20,11 +20,16 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.function.Consumer; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.hibernate.cfg.AvailableSettings; import org.hibernate.cfg.Configuration; import org.hibernate.dialect.Dialect; import org.hibernate.dialect.H2Dialect; +import org.hibernate.dialect.MariaDB10Dialect; +import org.hibernate.dialect.Oracle8iDialect; +import org.hibernate.dialect.PostgreSQL81Dialect; import org.hibernate.type.descriptor.sql.SqlTypeDescriptor; import org.hibernate.testing.TestForIssue; @@ -216,10 +221,6 @@ else if ( cause instanceof Error ) { } } - protected final ZoneId getDefaultJvmTimeZone() { - return env.defaultJvmTimeZone; - } - protected final Class getRemappingDialectClass() { return env.remappingDialectClass; } diff --git a/hibernate-core/src/test/java/org/hibernate/test/type/OffsetTimeTest.java b/hibernate-core/src/test/java/org/hibernate/test/type/OffsetTimeTest.java index c2e2df87e710..8a4203cbfc16 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/type/OffsetTimeTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/type/OffsetTimeTest.java @@ -18,7 +18,6 @@ import java.time.ZoneOffset; import java.util.Arrays; import java.util.List; -import java.util.TimeZone; import javax.persistence.Basic; import javax.persistence.Column; import javax.persistence.Entity; @@ -31,7 +30,6 @@ import org.hibernate.type.descriptor.sql.TimestampTypeDescriptor; import org.hibernate.testing.SkipForDialect; -import org.hibernate.testing.SkipLog; import org.junit.Test; import org.junit.runners.Parameterized; @@ -183,17 +181,6 @@ protected Object getActualJdbcValue(ResultSet resultSet, int columnIndex) throws return resultSet.getTimestamp( columnIndex ); } - @Override - @Test - public void nativeWriteThenRead() { - if ( TimeAsTimestampRemappingH2Dialect.class.equals( getRemappingDialectClass() ) && - !ZONE_GMT.equals( getDefaultJvmTimeZone() ) ) { - SkipLog.reportSkip( "OffsetTimeType remapped as timestamp only works reliably with GMT default JVM for nativeWriteThenRead; see HHH-13357" ); - return; - } - super.nativeWriteThenRead(); - } - @Override @Test @SkipForDialect(value = AbstractHANADialect.class, comment = "HANA seems to return a java.sql.Timestamp instead of a java.sql.Time") @@ -201,17 +188,6 @@ public void writeThenNativeRead() { super.writeThenNativeRead(); } - @Override - @Test - public void writeThenRead() { - if ( TimeAsTimestampRemappingH2Dialect.class.equals( getRemappingDialectClass() ) && - !ZONE_GMT.equals( getDefaultJvmTimeZone() ) ) { - SkipLog.reportSkip( "OffsetTimeType remapped as timestamp only works reliably with GMT default JVM for writeThenRead; see HHH-13357" ); - return; - } - super.writeThenRead(); - } - @Entity(name = ENTITY_NAME) static final class EntityWithOffsetTime { @Id From e357c5b8406d3295ee6bf1cda04b2fe8882615be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 16 Apr 2019 09:01:06 +0200 Subject: [PATCH 2/2] HHH-13357 Fix OffsetDateTime ending up with a different offset than the JVM default when loading from a Timestamp This bug only affects users that override the type descriptor for OffsetDateTime, and only affects reading. Since I had to change how we extract the local time from the timestamp, I also took this opportunity to apply the fix for HHH-13266, which should make data loading more resilient when databases contain weird values representing time, like 1650-04-15T14:45:49 or 0000-00-00T14:45:49. (cherry picked from commit 0f4c7ec0f2f1cb1d822ab37b6b14707a14951685) --- .../java/OffsetTimeJavaDescriptor.java | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetTimeJavaDescriptor.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetTimeJavaDescriptor.java index 2e3947f57e8a..65f0099a0c4f 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetTimeJavaDescriptor.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetTimeJavaDescriptor.java @@ -12,7 +12,7 @@ import java.time.LocalDate; import java.time.OffsetDateTime; import java.time.OffsetTime; -import java.time.ZoneId; +import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.util.Calendar; import java.util.Date; @@ -65,6 +65,12 @@ public X unwrap(OffsetTime offsetTime, Class type, WrapperOptions options final ZonedDateTime zonedDateTime = offsetTime.atDate( LocalDate.of( 1970, 1, 1 ) ).toZonedDateTime(); if ( Timestamp.class.isAssignableFrom( type ) ) { + /* + * Workaround for HHH-13266 (JDK-8061577). + * Ideally we'd want to use Timestamp.from( offsetDateTime.toInstant() ), but this won't always work. + * Timestamp.from() assumes the number of milliseconds since the epoch + * means the same thing in Timestamp and Instant, but it doesn't, in particular before 1900. + */ return (X) Timestamp.valueOf( zonedDateTime.toLocalDateTime() ); } @@ -95,22 +101,44 @@ public OffsetTime wrap(X value, WrapperOptions options) { return (OffsetTime) value; } + /* + * Also, in order to fix HHH-13357, and to be consistent with the conversion to Time (see above), + * we set the offset to the current offset of the JVM (OffsetDateTime.now().getOffset()). + * This is different from setting the *zone* to the current *zone* of the JVM (ZoneId.systemDefault()), + * since a zone has a varying offset over time, + * thus the zone might have a different offset for the given timezone than it has for the current date/time. + * For example, if the timestamp represents 1970-01-01TXX:YY, + * and the JVM is set to use Europe/Paris as a timezone, and the current time is 2019-04-16-08:53, + * then applying the JVM timezone to the timestamp would result in the offset +01:00, + * but applying the JVM offset would result in the offset +02:00, since DST is in effect at 2019-04-16-08:53. + * + * Of course none of this would be a problem if we just stored the offset in the database, + * but I guess there are historical reasons that explain why we don't. + */ + ZoneOffset offset = OffsetDateTime.now().getOffset(); + if ( Time.class.isInstance( value ) ) { - return ( (Time) value ).toLocalTime().atOffset( OffsetDateTime.now().getOffset() ); + return ( (Time) value ).toLocalTime().atOffset( offset ); } if ( Timestamp.class.isInstance( value ) ) { final Timestamp ts = (Timestamp) value; - return OffsetTime.ofInstant( ts.toInstant(), ZoneId.systemDefault() ); + /* + * Workaround for HHH-13266 (JDK-8061577). + * Ideally we'd want to use OffsetDateTime.ofInstant( ts.toInstant(), ... ), but this won't always work. + * ts.toInstant() assumes the number of milliseconds since the epoch + * means the same thing in Timestamp and Instant, but it doesn't, in particular before 1900. + */ + return ts.toLocalDateTime().toLocalTime().atOffset( offset ); } if ( Date.class.isInstance( value ) ) { final Date date = (Date) value; - return OffsetTime.ofInstant( date.toInstant(), ZoneId.systemDefault() ); + return OffsetTime.ofInstant( date.toInstant(), offset ); } if ( Long.class.isInstance( value ) ) { - return OffsetTime.ofInstant( Instant.ofEpochMilli( (Long) value ), ZoneId.systemDefault() ); + return OffsetTime.ofInstant( Instant.ofEpochMilli( (Long) value ), offset ); } if ( Calendar.class.isInstance( value ) ) {