Hibernate
/
hibernate-orm
mirror of https://github.com/hibernate/hibernate-orm
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

HHH-15679 proposed fix to OffsetTime handling 

The idea is: convert all OffsetTimes to the system offset before sending them on
This commit is contained in:
Gavin King 2022-11-10 18:45:12 +01:00
parent 39f85a2dca
commit 5dfb90bb73
3 changed files with 80 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetTimeJavaType.java
View File

@ -11,10 +11,10 @@ import java.sql.Timestamp;
import java.sql.Types;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalTime;
import java.time.OffsetDateTime;
import java.time.OffsetTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Calendar;
import java.util.Date;
@ -76,31 +76,46 @@ public class OffsetTimeJavaType extends AbstractTemporalJavaType<OffsetTime> {
return null;
}
// for java.time types, we assume that the JDBC timezone, if any, is ignored
// (since PS.setObject() doesn't support passing a timezone)
if ( OffsetTime.class.isAssignableFrom( type ) ) {
return (X) offsetTime;
}
if ( Time.class.isAssignableFrom( type ) ) {
return (X) Time.valueOf( offsetTime.toLocalTime() );
if ( LocalTime.class.isAssignableFrom( type ) ) {
return (X) offsetTime.withOffsetSameInstant( getCurrentSystemOffset() ).toLocalTime();
}
final ZonedDateTime zonedDateTime = offsetTime.atDate( LocalDate.of( 1970, 1, 1 ) ).toZonedDateTime();
// for legacy types, we assume that the JDBC timezone is passed to JDBC
// (since PS.setTime() and friends do accept a timezone passed as a Calendar)
final OffsetTime jdbcOffsetTime = offsetTime.withOffsetSameInstant( getCurrentJdbcOffset(options) );
if ( Time.class.isAssignableFrom( type ) ) {
return (X) Time.valueOf( jdbcOffsetTime.toLocalTime() );
}
final OffsetDateTime jdbcOffsetDateTime = jdbcOffsetTime.atDate( LocalDate.EPOCH );
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.
* Ideally we'd want to use Timestamp.from( jdbcOffsetDateTime.toInstant() ),
* but this won't always work since 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() );
return (X) Timestamp.valueOf( jdbcOffsetDateTime.toLocalDateTime() );
}
if ( Calendar.class.isAssignableFrom( type ) ) {
return (X) GregorianCalendar.from( zonedDateTime );
return (X) GregorianCalendar.from( jdbcOffsetDateTime.toZonedDateTime() );
}
final Instant instant = zonedDateTime.toInstant();
// for instants, we assume that the JDBC timezone, if any, is ignored
final Instant instant = offsetTime.atDate( LocalDate.EPOCH ).toInstant();
if ( Long.class.isAssignableFrom( type ) ) {
return (X) Long.valueOf( instant.toEpochMilli() );
@ -119,10 +134,17 @@ public class OffsetTimeJavaType extends AbstractTemporalJavaType<OffsetTime> {
return null;
}
// for java.time types, we assume that the JDBC timezone, if any, is ignored
// (since PS.setObject() doesn't support passing a timezone)
if (value instanceof OffsetTime) {
return (OffsetTime) value;
}
if (value instanceof LocalTime) {
return ((LocalTime) value).atOffset( getCurrentSystemOffset() );
}
/*
* 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()).
@ -137,30 +159,39 @@ public class OffsetTimeJavaType extends AbstractTemporalJavaType<OffsetTime> {
* 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();
// for legacy types, we assume that the JDBC timezone is passed to JDBC
// (since PS.setTime() and friends do accept a timezone passed as a Calendar)
if (value instanceof Time) {
return ( (Time) value ).toLocalTime().atOffset( offset );
final Time time = (Time) value;
return time.toLocalTime().atOffset( getCurrentJdbcOffset(options) )
.withOffsetSameInstant( getCurrentSystemOffset() );
}
if (value instanceof Timestamp) {
final Timestamp ts = (Timestamp) value;
/*
* 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.
* Ideally we'd want to use OffsetDateTime.ofInstant( ts.toInstant(), ... ),
* but this won't always work since 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 );
return ts.toLocalDateTime().toLocalTime().atOffset( getCurrentJdbcOffset(options) )
.withOffsetSameInstant( getCurrentSystemOffset() );
}
if (value instanceof Date) {
final Date date = (Date) value;
return OffsetTime.ofInstant( date.toInstant(), offset );
return OffsetTime.ofInstant( date.toInstant(), getCurrentSystemOffset() );
}
// for instants, we assume that the JDBC timezone, if any, is ignored
if (value instanceof Long) {
return OffsetTime.ofInstant( Instant.ofEpochMilli( (Long) value ), offset );
final long millis = (Long) value;
return OffsetTime.ofInstant( Instant.ofEpochMilli(millis), getCurrentSystemOffset() );
}
if (value instanceof Calendar) {
@ -171,6 +202,19 @@ public class OffsetTimeJavaType extends AbstractTemporalJavaType<OffsetTime> {
throw unknownWrap( value.getClass() );
}
private static ZoneOffset getCurrentJdbcOffset(WrapperOptions options) {
if ( options.getJdbcTimeZone() != null ) {
return OffsetDateTime.now().atZoneSameInstant( options.getJdbcTimeZone().toZoneId() ).getOffset();
}
else {
return getCurrentSystemOffset();
}
}
private static ZoneOffset getCurrentSystemOffset() {
return OffsetDateTime.now().getOffset();
}
@Override
public int getDefaultSqlPrecision(Dialect dialect, JdbcType jdbcType) {
return 0;

11
hibernate-core/src/test/java/org/hibernate/orm/test/type/AbstractJavaTimeTypeTest.java
View File

@ -25,7 +25,6 @@ import java.util.function.Predicate;
import org.hibernate.boot.model.TypeContributions;
import org.hibernate.cfg.AvailableSettings;
import org.hibernate.cfg.Configuration;
import org.hibernate.dialect.DatabaseVersion;
import org.hibernate.dialect.Dialect;
import org.hibernate.dialect.H2Dialect;
import org.hibernate.service.ServiceRegistry;
@ -125,9 +124,10 @@ public abstract class AbstractJavaTimeTypeTest<T, E> extends BaseCoreFunctionalT
} );
inTransaction( session -> {
T read = getActualPropertyValue( session.find( getEntityType(), 1 ) );
T expected = getExpectedPropertyValueAfterHibernateRead();
assertEquals(
"Writing then reading a value should return the original value",
getExpectedPropertyValueAfterHibernateRead(), read
expected, read
);
} );
} );
@ -152,10 +152,10 @@ public abstract class AbstractJavaTimeTypeTest<T, E> extends BaseCoreFunctionalT
try (ResultSet resultSet = statement.getResultSet()) {
resultSet.next();
Object nativeRead = getActualJdbcValue( resultSet, 1 );
Object expected = getExpectedJdbcValueAfterHibernateWrite();
assertEquals(
"Values written by Hibernate ORM should match the original value (same day, hour, ...)",
getExpectedJdbcValueAfterHibernateWrite(),
nativeRead
expected, nativeRead
);
}
}
@ -184,9 +184,10 @@ public abstract class AbstractJavaTimeTypeTest<T, E> extends BaseCoreFunctionalT
} );
inTransaction( session -> {
T read = getActualPropertyValue( session.find( getEntityType(), 1 ) );
T expected = getExpectedPropertyValueAfterHibernateRead();
assertEquals(
"Values written without Hibernate ORM should be read correctly by Hibernate ORM",
getExpectedPropertyValueAfterHibernateRead(), read
expected, read
);
} );
} );

23
hibernate-core/src/test/java/org/hibernate/orm/test/type/OffsetTimeTest.java
View File

@ -12,6 +12,7 @@ import java.sql.SQLException;
import java.sql.Time;
import java.sql.Timestamp;
import java.sql.Types;
import java.time.LocalDate;
import java.time.OffsetDateTime;
import java.time.OffsetTime;
import java.time.ZoneId;
@ -19,6 +20,7 @@ import java.time.ZoneOffset;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import jakarta.persistence.Basic;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
@ -173,11 +175,11 @@ public class OffsetTimeTest extends AbstractJavaTimeTypeTest<OffsetTime, OffsetT
protected OffsetTime getExpectedPropertyValueAfterHibernateRead() {
// For some reason, the offset is not stored, so the restored values use the offset from the default JVM timezone.
if ( TimeAsTimestampRemappingH2Dialect.class.equals( getRemappingDialectClass() ) ) {
return getOriginalPropertyValue().withOffsetSameLocal( OffsetDateTime.now().getOffset() );
return getOriginalPropertyValue().withOffsetSameInstant( OffsetDateTime.now().getOffset() );
}
else {
// When storing time as java.sql.Time, we only get second precision (not nanosecond)
return getOriginalPropertyValue().withNano( 0 ).withOffsetSameLocal( OffsetDateTime.now().getOffset() );
return getOriginalPropertyValue().withNano( 0 ).withOffsetSameInstant( OffsetDateTime.now().getOffset() );
}
}
@ -189,29 +191,28 @@ public class OffsetTimeTest extends AbstractJavaTimeTypeTest<OffsetTime, OffsetT
@Override
protected void setJdbcValueForNonHibernateWrite(PreparedStatement statement, int parameterIndex)
throws SQLException {
OffsetTime offsetTime = OffsetTime.of( hour, minute, second, 0, ZoneOffset.of(offset) )
.withOffsetSameInstant( OffsetDateTime.now().getOffset() );
if ( TimeAsTimestampRemappingH2Dialect.class.equals( getRemappingDialectClass() ) ) {
statement.setTimestamp(
parameterIndex,
new Timestamp(
yearWhenPersistedWithoutHibernate - 1900,
monthWhenPersistedWithoutHibernate - 1,
dayWhenPersistedWithoutHibernate,
hour, minute, second, nanosecond
)
Timestamp.valueOf( offsetTime.atDate( LocalDate.EPOCH ).toLocalDateTime() )
);
}
else {
statement.setTime( parameterIndex, new Time( hour, minute, second ) );
statement.setTime( parameterIndex, Time.valueOf( offsetTime.toLocalTime() ) );
}
}
@Override
protected Object getExpectedJdbcValueAfterHibernateWrite() {
OffsetTime offsetTime = OffsetTime.of( hour, minute, second, 0, ZoneOffset.of(offset) )
.withOffsetSameInstant( OffsetDateTime.now().getOffset() );
if ( TimeAsTimestampRemappingH2Dialect.class.equals( getRemappingDialectClass() ) ) {
return new Timestamp( 1970 - 1900, 0, 1, hour, minute, second, nanosecond );
return Timestamp.valueOf( offsetTime.atDate( LocalDate.EPOCH ).toLocalDateTime() );
}
else {
return new Time( hour, minute, second );
return Time.valueOf( offsetTime.toLocalTime() );
}
}