HHH-18532 Suggested fix to conversion of epoch time in milliseconds to java.sql.Time

cigaly · cigaly · commit 11fecb6925bd · 2024-11-01T09:53:47.000+01:00
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/AbstractTemporalJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/AbstractTemporalJavaType.java
@@ -4,6 +4,10 @@
  */
 package org.hibernate.type.descriptor.java;
 
+import java.sql.Time;
+import java.time.Instant;
+import java.time.LocalTime;
+import java.time.ZoneId;
 import java.util.Comparator;
 
 import jakarta.persistence.TemporalType;
@@ -60,6 +64,13 @@ private <X> TemporalJavaType<X> forMissingPrecision(TypeConfiguration typeConfig
 		return (TemporalJavaType) this;
 	}
 
+	public static Time millisToSqlTime(long millis) {
+		final LocalTime localTime = Instant.ofEpochMilli( millis ).atZone( ZoneId.systemDefault() ).toLocalTime();
+		final Time time = Time.valueOf( localTime );
+		time.setTime( time.getTime() + localTime.getNano() / 1_000_000 );
+		return time;
+	}
+
 	protected <X> TemporalJavaType<X> forTimestampPrecision(TypeConfiguration typeConfiguration) {
 		throw new UnsupportedOperationException(
 				this + " as `jakarta.persistence.TemporalType.TIMESTAMP` not supported"
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CalendarDateJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CalendarDateJavaType.java
@@ -100,7 +100,7 @@ public <X> X unwrap(Calendar value, Class<X> type, WrapperOptions options) {
 			return (X) new java.sql.Date( value.getTimeInMillis() );
 		}
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
-			return (X) new java.sql.Time( value.getTimeInMillis() % 86_400_000 );
+			return (X) millisToSqlTime( value.getTimeInMillis() );
 		}
 		if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {
 			return (X) new java.sql.Timestamp( value.getTimeInMillis() );
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CalendarJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CalendarJavaType.java
@@ -117,7 +117,7 @@ public <X> X unwrap(Calendar value, Class<X> type, WrapperOptions options) {
 			return (X) new java.sql.Date( value.getTimeInMillis() );
 		}
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
-			return (X) new java.sql.Time( value.getTimeInMillis() % 86_400_000 );
+			return (X) millisToSqlTime( value.getTimeInMillis() );
 		}
 		if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {
 			return (X) new java.sql.Timestamp( value.getTimeInMillis() );
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CalendarTimeJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CalendarTimeJavaType.java
@@ -102,7 +102,7 @@ public <X> X unwrap(Calendar value, Class<X> type, WrapperOptions options) {
 			return (X) new java.sql.Date( value.getTimeInMillis() );
 		}
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
-			return (X) new java.sql.Time( value.getTimeInMillis() % 86_400_000 );
+			return (X) millisToSqlTime( value.getTimeInMillis() );
 		}
 		if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {
 			return (X) new java.sql.Timestamp( value.getTimeInMillis() );
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/DateJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/DateJavaType.java
@@ -124,7 +124,7 @@ public <X> X unwrap(Date value, Class<X> type, WrapperOptions options) {
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
 			final java.sql.Time rtn = value instanceof java.sql.Time
 					? ( java.sql.Time ) value
-					: new java.sql.Time( value.getTime() % 86_400_000 );
+					: millisToSqlTime( value.getTime() );
 			return (X) rtn;
 		}
 		if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/InstantJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/InstantJavaType.java
@@ -126,7 +126,7 @@ public <X> X unwrap(Instant instant, Class<X> type, WrapperOptions options) {
 		}
 
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
-			return (X) new java.sql.Time( instant.toEpochMilli() % 86_400_000 );
+			return (X) millisToSqlTime( instant.toEpochMilli() );
 		}
 
 		if ( Date.class.isAssignableFrom( type ) ) {
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/JdbcTimeJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/JdbcTimeJavaType.java
@@ -120,7 +120,7 @@ public Object unwrap(Date value, Class type, WrapperOptions options) {
 		if ( LocalTime.class.isAssignableFrom( type ) ) {
 			final Time time = value instanceof java.sql.Time
 					? ( (java.sql.Time) value )
-					: new java.sql.Time( value.getTime() % 86_400_000 );
+					: millisToSqlTime( value.getTime() );
 			final LocalTime localTime = time.toLocalTime();
 			long millis = time.getTime() % 1000;
 			if ( millis == 0 ) {
@@ -135,9 +135,7 @@ public Object unwrap(Date value, Class type, WrapperOptions options) {
 		}
 
 		if ( Time.class.isAssignableFrom( type ) ) {
-			return value instanceof Time
-					? value
-					: new Time( value.getTime() % 86_400_000 );
+			return millisToSqlTime( value.getTime() );
 		}
 
 		if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {
@@ -175,12 +173,8 @@ public Date wrap(Object value, WrapperOptions options) {
 			return null;
 		}
 
-		if ( value instanceof Time time ) {
-			return time;
-		}
-
 		if ( value instanceof Date date ) {
-			return new Time( date.getTime() % 86_400_000 );
+			return millisToSqlTime( date.getTime() );
 		}
 
 		if ( value instanceof LocalTime localTime ) {
@@ -197,7 +191,7 @@ public Date wrap(Object value, WrapperOptions options) {
 		}
 
 		if ( value instanceof Calendar calendar ) {
-			return new Time( calendar.getTimeInMillis() % 86_400_000 );
+			return millisToSqlTime( ( calendar.getTimeInMillis() );
 		}
 
 		throw unknownWrap( value.getClass() );
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/JdbcTimestampJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/JdbcTimestampJavaType.java
@@ -137,7 +137,7 @@ public Object unwrap(Date value, Class type, WrapperOptions options) {
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
 			return value instanceof java.sql.Time
 					? ( java.sql.Time ) value
-					: new java.sql.Time( value.getTime() % 86_400_000 );
+					: millisToSqlTime( value.getTime() );
 		}
 
 		if ( Date.class.isAssignableFrom( type ) ) {
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/LocalDateTimeJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/LocalDateTimeJavaType.java
@@ -102,7 +102,7 @@ public <X> X unwrap(LocalDateTime value, Class<X> type, WrapperOptions options)
 
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
 			Instant instant = value.atZone( ZoneId.systemDefault() ).toInstant();
-			return (X) new java.sql.Time( instant.toEpochMilli() % 86_400_000 );
+			return (X) millisToSqlTime( instant.toEpochMilli() );
 		}
 
 		if ( Date.class.isAssignableFrom( type ) ) {
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetDateTimeJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetDateTimeJavaType.java
@@ -157,7 +157,7 @@ public <X> X unwrap(OffsetDateTime offsetDateTime, Class<X> type, WrapperOptions
 		}
 
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
-			return (X) new java.sql.Time( offsetDateTime.toInstant().toEpochMilli() % 86_400_000 );
+			return (X) millisToSqlTime( offsetDateTime.toInstant().toEpochMilli() );
 		}
 
 		if ( Date.class.isAssignableFrom( type ) ) {
diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/ZonedDateTimeJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/ZonedDateTimeJavaType.java
@@ -124,7 +124,7 @@ public <X> X unwrap(ZonedDateTime zonedDateTime, Class<X> type, WrapperOptions o
 		}
 
 		if ( java.sql.Time.class.isAssignableFrom( type ) ) {
-			return (X) new java.sql.Time( zonedDateTime.toInstant().toEpochMilli() % 86_400_000 );
+			return (X) millisToSqlTime( zonedDateTime.toInstant().toEpochMilli() );
 		}
 
 		if ( Date.class.isAssignableFrom( type ) ) {
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/DateSubclassesUnwrapTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/DateSubclassesUnwrapTest.java
@@ -1,8 +1,6 @@
 /*
- * Hibernate, Relational Persistence for Idiomatic Java
- *
- * License: GNU Lesser General Public License (LGPL), version 2.1 or later
- * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ * Copyright Red Hat Inc. and Hibernate Authors
  */
 package org.hibernate.orm.test.mapping.type.java;
 
@@ -17,15 +15,8 @@
 import java.time.ZonedDateTime;
 import java.util.Calendar;
 import java.util.Date;
-import java.util.TimeZone;
 
 import org.hibernate.HibernateException;
-import org.hibernate.dialect.Dialect;
-import org.hibernate.dialect.H2Dialect;
-import org.hibernate.engine.jdbc.LobCreator;
-import org.hibernate.engine.jdbc.NonContextualLobCreator;
-import org.hibernate.engine.spi.SessionFactoryImplementor;
-import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.type.descriptor.WrapperOptions;
 import org.hibernate.type.descriptor.java.CalendarDateJavaType;
 import org.hibernate.type.descriptor.java.CalendarJavaType;
@@ -173,7 +164,7 @@ void testOffsetTimeJavaType(SessionFactoryScope scope) {
 
 		assertInstanceOf( Timestamp.class, javaType.unwrap( time, Timestamp.class, wrapperOptions ) );
 		assertThrows( IllegalArgumentException.class,
-					  () -> javaType.unwrap( time, java.sql.Date.class, wrapperOptions ) );
+					() -> javaType.unwrap( time, java.sql.Date.class, wrapperOptions ) );
 		assertInstanceOf( Time.class, javaType.unwrap( time, Time.class, wrapperOptions ) );
 		assertInstanceOf( Date.class, javaType.unwrap( time, Date.class, wrapperOptions ) );
 	}
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/GoodBadUglyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/GoodBadUglyTest.java
@@ -1,3 +1,7 @@
+/*
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
 package org.hibernate.orm.test.mapping.type.java;
 
 import java.sql.Time;
@@ -43,7 +47,6 @@
 @DomainModel(annotatedClasses = GoodBadUglyTest.Times.class)
 @SessionFactory
 @SkipForDialect(dialectClass = InformixDialect.class)
-@Disabled
 public class GoodBadUglyTest {
 
 	@BeforeEach

Original file line number	Diff line number	Diff line change
`@@ -100,7 +100,7 @@ public <X> X unwrap(Calendar value, Class<X> type, WrapperOptions options) {`
`100`	`100`	`return (X) new java.sql.Date( value.getTimeInMillis() );`
`101`	`101`	`}`
`102`	`102`	`if ( java.sql.Time.class.isAssignableFrom( type ) ) {`
`103`		`- return (X) new java.sql.Time( value.getTimeInMillis() % 86_400_000 );`
	`103`	`+ return (X) millisToSqlTime( value.getTimeInMillis() );`
`104`	`104`	`}`
`105`	`105`	`if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {`
`106`	`106`	`return (X) new java.sql.Timestamp( value.getTimeInMillis() );`
Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,7 @@ public <X> X unwrap(Calendar value, Class<X> type, WrapperOptions options) {`
`117`	`117`	`return (X) new java.sql.Date( value.getTimeInMillis() );`
`118`	`118`	`}`
`119`	`119`	`if ( java.sql.Time.class.isAssignableFrom( type ) ) {`
`120`		`- return (X) new java.sql.Time( value.getTimeInMillis() % 86_400_000 );`
	`120`	`+ return (X) millisToSqlTime( value.getTimeInMillis() );`
`121`	`121`	`}`
`122`	`122`	`if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {`
`123`	`123`	`return (X) new java.sql.Timestamp( value.getTimeInMillis() );`
Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,7 @@ public <X> X unwrap(Calendar value, Class<X> type, WrapperOptions options) {`
`102`	`102`	`return (X) new java.sql.Date( value.getTimeInMillis() );`
`103`	`103`	`}`
`104`	`104`	`if ( java.sql.Time.class.isAssignableFrom( type ) ) {`
`105`		`- return (X) new java.sql.Time( value.getTimeInMillis() % 86_400_000 );`
	`105`	`+ return (X) millisToSqlTime( value.getTimeInMillis() );`
`106`	`106`	`}`
`107`	`107`	`if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {`
`108`	`108`	`return (X) new java.sql.Timestamp( value.getTimeInMillis() );`
Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@ public <X> X unwrap(Date value, Class<X> type, WrapperOptions options) {`
`124`	`124`	`if ( java.sql.Time.class.isAssignableFrom( type ) ) {`
`125`	`125`	`final java.sql.Time rtn = value instanceof java.sql.Time`
`126`	`126`	`? ( java.sql.Time ) value`
`127`		`- : new java.sql.Time( value.getTime() % 86_400_000 );`
	`127`	`+ : millisToSqlTime( value.getTime() );`
`128`	`128`	`return (X) rtn;`
`129`	`129`	`}`
`130`	`130`	`if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {`
Original file line number	Diff line number	Diff line change
`@@ -126,7 +126,7 @@ public <X> X unwrap(Instant instant, Class<X> type, WrapperOptions options) {`
`126`	`126`	`}`
`127`	`127`
`128`	`128`	`if ( java.sql.Time.class.isAssignableFrom( type ) ) {`
`129`		`- return (X) new java.sql.Time( instant.toEpochMilli() % 86_400_000 );`
	`129`	`+ return (X) millisToSqlTime( instant.toEpochMilli() );`
`130`	`130`	`}`
`131`	`131`
`132`	`132`	`if ( Date.class.isAssignableFrom( type ) ) {`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ public Object unwrap(Date value, Class type, WrapperOptions options) {`
`120`	`120`	`if ( LocalTime.class.isAssignableFrom( type ) ) {`
`121`	`121`	`final Time time = value instanceof java.sql.Time`
`122`	`122`	`? ( (java.sql.Time) value )`
`123`		`- : new java.sql.Time( value.getTime() % 86_400_000 );`
	`123`	`+ : millisToSqlTime( value.getTime() );`
`124`	`124`	`final LocalTime localTime = time.toLocalTime();`
`125`	`125`	`long millis = time.getTime() % 1000;`
`126`	`126`	`if ( millis == 0 ) {`
`@@ -135,9 +135,7 @@ public Object unwrap(Date value, Class type, WrapperOptions options) {`
`135`	`135`	`}`
`136`	`136`
`137`	`137`	`if ( Time.class.isAssignableFrom( type ) ) {`
`138`		`- return value instanceof Time`
`139`		`- ? value`
`140`		`- : new Time( value.getTime() % 86_400_000 );`
	`138`	`+ return millisToSqlTime( value.getTime() );`
`141`	`139`	`}`
`142`	`140`
`143`	`141`	`if ( java.sql.Timestamp.class.isAssignableFrom( type ) ) {`
`@@ -175,12 +173,8 @@ public Date wrap(Object value, WrapperOptions options) {`
`175`	`173`	`return null;`
`176`	`174`	`}`
`177`	`175`
`178`		`- if ( value instanceof Time time ) {`
`179`		`- return time;`
`180`		`- }`
`181`		`-`
`182`	`176`	`if ( value instanceof Date date ) {`
`183`		`- return new Time( date.getTime() % 86_400_000 );`
	`177`	`+ return millisToSqlTime( date.getTime() );`
`184`	`178`	`}`
`185`	`179`
`186`	`180`	`if ( value instanceof LocalTime localTime ) {`
`@@ -197,7 +191,7 @@ public Date wrap(Object value, WrapperOptions options) {`
`197`	`191`	`}`
`198`	`192`
`199`	`193`	`if ( value instanceof Calendar calendar ) {`
`200`		`- return new Time( calendar.getTimeInMillis() % 86_400_000 );`
	`194`	`+ return millisToSqlTime( ( calendar.getTimeInMillis() );`
`201`	`195`	`}`
`202`	`196`
`203`	`197`	`throw unknownWrap( value.getClass() );`
Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ public Object unwrap(Date value, Class type, WrapperOptions options) {`
`137`	`137`	`if ( java.sql.Time.class.isAssignableFrom( type ) ) {`
`138`	`138`	`return value instanceof java.sql.Time`
`139`	`139`	`? ( java.sql.Time ) value`
`140`		`- : new java.sql.Time( value.getTime() % 86_400_000 );`
	`140`	`+ : millisToSqlTime( value.getTime() );`
`141`	`141`	`}`
`142`	`142`
`143`	`143`	`if ( Date.class.isAssignableFrom( type ) ) {`
Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,7 @@ public <X> X unwrap(OffsetDateTime offsetDateTime, Class<X> type, WrapperOptions`
`157`	`157`	`}`
`158`	`158`
`159`	`159`	`if ( java.sql.Time.class.isAssignableFrom( type ) ) {`
`160`		`- return (X) new java.sql.Time( offsetDateTime.toInstant().toEpochMilli() % 86_400_000 );`
	`160`	`+ return (X) millisToSqlTime( offsetDateTime.toInstant().toEpochMilli() );`
`161`	`161`	`}`
`162`	`162`
`163`	`163`	`if ( Date.class.isAssignableFrom( type ) ) {`