diff --git a/core/build.gradle b/core/build.gradle index 8f57d1816..6fbc5a6fe 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -258,7 +258,7 @@ dependencies { compile deps['org.joda:joda-money'] compile deps['org.json:json'] testCompile deps['org.mortbay.jetty:jetty'] - runtimeOnly deps['org.postgresql:postgresql'] + compile deps['org.postgresql:postgresql'] testCompile deps['org.seleniumhq.selenium:selenium-api'] testCompile deps['org.seleniumhq.selenium:selenium-chrome-driver'] testCompile deps['org.seleniumhq.selenium:selenium-java'] diff --git a/core/gradle/dependency-locks/compile.lockfile b/core/gradle/dependency-locks/compile.lockfile index 306a7782a..c679590d0 100644 --- a/core/gradle/dependency-locks/compile.lockfile +++ b/core/gradle/dependency-locks/compile.lockfile @@ -228,6 +228,7 @@ org.ow2.asm:asm-commons:7.1 org.ow2.asm:asm-tree:8.0.1 org.ow2.asm:asm-util:8.0.1 org.ow2.asm:asm:8.0.1 +org.postgresql:postgresql:42.2.14 org.rnorth.duct-tape:duct-tape:1.0.8 org.rnorth.visible-assertions:visible-assertions:2.1.2 org.rnorth:tcp-unix-socket-proxy:1.0.2 diff --git a/core/gradle/dependency-locks/compileClasspath.lockfile b/core/gradle/dependency-locks/compileClasspath.lockfile index 8b7a2c214..231a2cdb0 100644 --- a/core/gradle/dependency-locks/compileClasspath.lockfile +++ b/core/gradle/dependency-locks/compileClasspath.lockfile @@ -223,6 +223,7 @@ org.ow2.asm:asm-commons:7.1 org.ow2.asm:asm-tree:8.0.1 org.ow2.asm:asm-util:8.0.1 org.ow2.asm:asm:8.0.1 +org.postgresql:postgresql:42.2.14 org.rnorth.duct-tape:duct-tape:1.0.8 org.rnorth.visible-assertions:visible-assertions:2.1.2 org.rnorth:tcp-unix-socket-proxy:1.0.2 diff --git a/core/gradle/dependency-locks/nonprodCompile.lockfile b/core/gradle/dependency-locks/nonprodCompile.lockfile index 306a7782a..c679590d0 100644 --- a/core/gradle/dependency-locks/nonprodCompile.lockfile +++ b/core/gradle/dependency-locks/nonprodCompile.lockfile @@ -228,6 +228,7 @@ org.ow2.asm:asm-commons:7.1 org.ow2.asm:asm-tree:8.0.1 org.ow2.asm:asm-util:8.0.1 org.ow2.asm:asm:8.0.1 +org.postgresql:postgresql:42.2.14 org.rnorth.duct-tape:duct-tape:1.0.8 org.rnorth.visible-assertions:visible-assertions:2.1.2 org.rnorth:tcp-unix-socket-proxy:1.0.2 diff --git a/core/gradle/dependency-locks/nonprodCompileClasspath.lockfile b/core/gradle/dependency-locks/nonprodCompileClasspath.lockfile index 960f4e217..339676671 100644 --- a/core/gradle/dependency-locks/nonprodCompileClasspath.lockfile +++ b/core/gradle/dependency-locks/nonprodCompileClasspath.lockfile @@ -226,6 +226,7 @@ org.ow2.asm:asm-commons:7.1 org.ow2.asm:asm-tree:8.0.1 org.ow2.asm:asm-util:8.0.1 org.ow2.asm:asm:8.0.1 +org.postgresql:postgresql:42.2.14 org.rnorth.duct-tape:duct-tape:1.0.8 org.rnorth.visible-assertions:visible-assertions:2.1.2 org.rnorth:tcp-unix-socket-proxy:1.0.2 diff --git a/core/gradle/dependency-locks/testCompile.lockfile b/core/gradle/dependency-locks/testCompile.lockfile index 4edc95acc..a213885cf 100644 --- a/core/gradle/dependency-locks/testCompile.lockfile +++ b/core/gradle/dependency-locks/testCompile.lockfile @@ -266,6 +266,7 @@ org.ow2.asm:asm-commons:7.1 org.ow2.asm:asm-tree:8.0.1 org.ow2.asm:asm-util:8.0.1 org.ow2.asm:asm:8.0.1 +org.postgresql:postgresql:42.2.14 org.rnorth.duct-tape:duct-tape:1.0.8 org.rnorth.visible-assertions:visible-assertions:2.1.2 org.rnorth:tcp-unix-socket-proxy:1.0.2 diff --git a/core/gradle/dependency-locks/testCompileClasspath.lockfile b/core/gradle/dependency-locks/testCompileClasspath.lockfile index 7d2df0a24..e6182722b 100644 --- a/core/gradle/dependency-locks/testCompileClasspath.lockfile +++ b/core/gradle/dependency-locks/testCompileClasspath.lockfile @@ -264,6 +264,7 @@ org.ow2.asm:asm-commons:7.1 org.ow2.asm:asm-tree:8.0.1 org.ow2.asm:asm-util:8.0.1 org.ow2.asm:asm:8.0.1 +org.postgresql:postgresql:42.2.14 org.rnorth.duct-tape:duct-tape:1.0.8 org.rnorth.visible-assertions:visible-assertions:2.1.2 org.rnorth:tcp-unix-socket-proxy:1.0.2 diff --git a/core/src/main/java/google/registry/persistence/NomulusPostgreSQLDialect.java b/core/src/main/java/google/registry/persistence/NomulusPostgreSQLDialect.java index 6f60d7c38..f90123113 100644 --- a/core/src/main/java/google/registry/persistence/NomulusPostgreSQLDialect.java +++ b/core/src/main/java/google/registry/persistence/NomulusPostgreSQLDialect.java @@ -13,6 +13,7 @@ // limitations under the License. package google.registry.persistence; +import google.registry.persistence.converter.IntervalDescriptor; import google.registry.persistence.converter.StringCollectionDescriptor; import google.registry.persistence.converter.StringMapDescriptor; import java.sql.Types; @@ -30,6 +31,7 @@ public class NomulusPostgreSQLDialect extends PostgreSQL95Dialect { registerColumnType(StringMapDescriptor.COLUMN_TYPE, StringMapDescriptor.COLUMN_NAME); registerColumnType( StringCollectionDescriptor.COLUMN_TYPE, StringCollectionDescriptor.COLUMN_DDL_NAME); + registerColumnType(IntervalDescriptor.COLUMN_TYPE, IntervalDescriptor.COLUMN_NAME); } @Override @@ -40,5 +42,7 @@ public class NomulusPostgreSQLDialect extends PostgreSQL95Dialect { typeContributions.contributeSqlTypeDescriptor(StringCollectionDescriptor.getInstance()); typeContributions.contributeJavaTypeDescriptor(StringMapDescriptor.getInstance()); typeContributions.contributeSqlTypeDescriptor(StringMapDescriptor.getInstance()); + typeContributions.contributeJavaTypeDescriptor(IntervalDescriptor.getInstance()); + typeContributions.contributeSqlTypeDescriptor(IntervalDescriptor.getInstance()); } } diff --git a/core/src/main/java/google/registry/persistence/converter/DurationConverter.java b/core/src/main/java/google/registry/persistence/converter/DurationConverter.java index 8808bc371..35b85d67e 100644 --- a/core/src/main/java/google/registry/persistence/converter/DurationConverter.java +++ b/core/src/main/java/google/registry/persistence/converter/DurationConverter.java @@ -14,24 +14,67 @@ package google.registry.persistence.converter; +import java.sql.SQLException; import javax.annotation.Nullable; import javax.persistence.AttributeConverter; import javax.persistence.Converter; import org.joda.time.Duration; +import org.joda.time.Period; +import org.postgresql.util.PGInterval; -/** JPA converter to for storing/retrieving {@link org.joda.time.DateTime} objects. */ +/** + * JPA converter to for storing/retrieving {@link org.joda.time.Duration} objects. + * + *

The Joda Time Duration is simply a number of milliseconds representing a length of time. This + * can be converted into a PGInterval, but only for the fields that have a standard number of + * milliseconds. Therefore, there is no way to populate the months or years field of a PGInterval + * and be confident that it is representing the exact number of milliseconds it was intended to + * represent. + */ @Converter(autoApply = true) -public class DurationConverter implements AttributeConverter { +public class DurationConverter implements AttributeConverter { @Override @Nullable - public Long convertToDatabaseColumn(@Nullable Duration duration) { - return duration == null ? null : duration.getMillis(); + public PGInterval convertToDatabaseColumn(@Nullable Duration duration) { + if (duration == null) { + return new PGInterval(); + } + PGInterval interval = new PGInterval(); + Period period = new Period(duration); + // For some reason when the period is created from the duration, it does not set days, but + // instead just a total number of hours. Years and months are not created because those can + // differ in length of milliseconds. + interval.setDays(period.getHours() / 24); + interval.setHours(period.getHours() % 24); + interval.setMinutes(period.getMinutes()); + double millis = (double) period.getMillis() / 1000; + interval.setSeconds(period.getSeconds() + millis); + return interval; } @Override @Nullable - public Duration convertToEntityAttribute(@Nullable Long dbData) { - return dbData == null ? null : new Duration(dbData); + public Duration convertToEntityAttribute(@Nullable PGInterval dbData) { + if (dbData == null) { + return null; + } + PGInterval interval = null; + try { + interval = new PGInterval(dbData.toString()); + } catch (SQLException e) { + throw new RuntimeException(e); + } + + if (interval.equals(new PGInterval())) { + return null; + } + + final int days = interval.getDays(); + final int hours = interval.getHours(); + final int mins = interval.getMinutes(); + final int secs = (int) interval.getSeconds(); + final int millis = interval.getMicroSeconds() / 1000; + return new Period(0, 0, 0, days, hours, mins, secs, millis).toStandardDuration(); } } diff --git a/core/src/main/java/google/registry/persistence/converter/IntervalDescriptor.java b/core/src/main/java/google/registry/persistence/converter/IntervalDescriptor.java new file mode 100644 index 000000000..9994f20cf --- /dev/null +++ b/core/src/main/java/google/registry/persistence/converter/IntervalDescriptor.java @@ -0,0 +1,139 @@ +// Copyright 2020 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.persistence.converter; + +import java.sql.CallableStatement; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Types; +import org.hibernate.type.descriptor.ValueBinder; +import org.hibernate.type.descriptor.ValueExtractor; +import org.hibernate.type.descriptor.WrapperOptions; +import org.hibernate.type.descriptor.java.AbstractTypeDescriptor; +import org.hibernate.type.descriptor.java.JavaTypeDescriptor; +import org.hibernate.type.descriptor.spi.JdbcRecommendedSqlTypeMappingContext; +import org.hibernate.type.descriptor.sql.BasicBinder; +import org.hibernate.type.descriptor.sql.BasicExtractor; +import org.hibernate.type.descriptor.sql.SqlTypeDescriptor; +import org.postgresql.util.PGInterval; + +/** + * The {@link JavaTypeDescriptor} and {@link SqlTypeDescriptor} for {@link PGInterval}. + * + * @see JPA + * 2.1 AttributeConverters + */ +public class IntervalDescriptor extends AbstractTypeDescriptor + implements SqlTypeDescriptor { + public static final int COLUMN_TYPE = Types.JAVA_OBJECT; + public static final String COLUMN_NAME = "interval"; + private static final IntervalDescriptor INSTANCE = new IntervalDescriptor(); + + private IntervalDescriptor() { + super(PGInterval.class); + } + + public static IntervalDescriptor getInstance() { + return INSTANCE; + } + + @Override + public PGInterval fromString(String string) { + throw new UnsupportedOperationException( + "Constructing IntervalDescriptor from string is not allowed"); + } + + @Override + public X unwrap(PGInterval value, Class type, WrapperOptions options) { + if (value == null) { + return null; + } + if (PGInterval.class.isAssignableFrom(type)) { + return (X) value; + } + throw unknownUnwrap(type); + } + + @Override + public PGInterval wrap(X value, WrapperOptions options) { + if (value == null) { + return null; + } + if (value instanceof PGInterval) { + try { + return new PGInterval(value.toString()); + } catch (SQLException e) { + throw new RuntimeException(e); + } + } + throw unknownWrap(value.getClass()); + } + + @Override + public int getSqlType() { + return COLUMN_TYPE; + } + + @Override + public SqlTypeDescriptor getJdbcRecommendedSqlType(JdbcRecommendedSqlTypeMappingContext context) { + return this; + } + + @Override + public boolean canBeRemapped() { + return false; + } + + @Override + public ValueBinder getBinder(JavaTypeDescriptor javaTypeDescriptor) { + return new BasicBinder(javaTypeDescriptor, this) { + @Override + protected void doBind(PreparedStatement st, X value, int index, WrapperOptions options) + throws SQLException { + st.setObject(index, new PGInterval(value.toString())); + } + + @Override + protected void doBind(CallableStatement st, X value, String name, WrapperOptions options) + throws SQLException { + st.setObject(name, new PGInterval(value.toString())); + } + }; + } + + @Override + public ValueExtractor getExtractor(JavaTypeDescriptor javaTypeDescriptor) { + return new BasicExtractor(javaTypeDescriptor, this) { + @Override + protected X doExtract(ResultSet rs, String name, WrapperOptions options) throws SQLException { + return javaTypeDescriptor.wrap(rs.getObject(name), options); + } + + @Override + protected X doExtract(CallableStatement statement, int index, WrapperOptions options) + throws SQLException { + return javaTypeDescriptor.wrap(statement.getObject(index), options); + } + + @Override + protected X doExtract(CallableStatement statement, String name, WrapperOptions options) + throws SQLException { + return javaTypeDescriptor.wrap(statement.getObject(name), options); + } + }; + } +} diff --git a/core/src/test/java/google/registry/persistence/converter/DurationConverterTest.java b/core/src/test/java/google/registry/persistence/converter/DurationConverterTest.java index 01f2d1992..46766371b 100644 --- a/core/src/test/java/google/registry/persistence/converter/DurationConverterTest.java +++ b/core/src/test/java/google/registry/persistence/converter/DurationConverterTest.java @@ -21,58 +21,65 @@ import google.registry.model.ImmutableObject; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaUnitTestExtension; import google.registry.schema.replay.EntityTest.EntityForTesting; -import java.math.BigInteger; import javax.persistence.Entity; import javax.persistence.Id; import org.joda.time.Duration; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.postgresql.util.PGInterval; /** Unit tests for {@link DurationConverter}. */ public class DurationConverterTest { @RegisterExtension public final JpaUnitTestExtension jpaExtension = - new JpaTestRules.Builder().withEntityClass(TestEntity.class).buildUnitTestRule(); + new JpaTestRules.Builder().withEntityClass(DurationTestEntity.class).buildUnitTestRule(); private final DurationConverter converter = new DurationConverter(); @Test - void testNulls() { - assertThat(converter.convertToDatabaseColumn(null)).isNull(); - assertThat(converter.convertToEntityAttribute(null)).isNull(); + public void testNulls() { + assertThat(converter.convertToDatabaseColumn(null)).isEqualTo(new PGInterval()); + assertThat(converter.convertToEntityAttribute(new PGInterval())).isNull(); } @Test void testRoundTrip() { - TestEntity entity = new TestEntity(Duration.standardDays(6)); + Duration testDuration = + Duration.standardDays(6) + .plus(Duration.standardHours(10)) + .plus(Duration.standardMinutes(30)) + .plus(Duration.standardSeconds(15)) + .plus(Duration.millis(7)); + DurationTestEntity entity = new DurationTestEntity(testDuration); jpaTm().transact(() -> jpaTm().getEntityManager().persist(entity)); - assertThat( - jpaTm() - .transact( - () -> - jpaTm() - .getEntityManager() - .createNativeQuery( - "SELECT duration FROM \"TestEntity\" WHERE name = 'id'") - .getResultList())) - .containsExactly(BigInteger.valueOf(Duration.standardDays(6).getMillis())); - TestEntity persisted = - jpaTm().transact(() -> jpaTm().getEntityManager().find(TestEntity.class, "id")); - assertThat(persisted.duration).isEqualTo(Duration.standardDays(6)); + DurationTestEntity persisted = + jpaTm().transact(() -> jpaTm().getEntityManager().find(DurationTestEntity.class, "id")); + assertThat(persisted.duration.getMillis()).isEqualTo(testDuration.getMillis()); + } + + @Test + void testRoundTripLargeNumberOfDays() { + Duration testDuration = + Duration.standardDays(10001).plus(Duration.standardHours(100)).plus(Duration.millis(790)); + DurationTestEntity entity = new DurationTestEntity(testDuration); + jpaTm().transact(() -> jpaTm().getEntityManager().persist(entity)); + DurationTestEntity persisted = + jpaTm().transact(() -> jpaTm().getEntityManager().find(DurationTestEntity.class, "id")); + assertThat(persisted.duration.getMillis()).isEqualTo(testDuration.getMillis()); } @Entity(name = "TestEntity") // Override entity name to avoid the nested class reference. @EntityForTesting - public static class TestEntity extends ImmutableObject { + public static class DurationTestEntity extends ImmutableObject { @Id String name = "id"; Duration duration; - public TestEntity() {} + public DurationTestEntity() {} - TestEntity(Duration duration) { + DurationTestEntity(Duration duration) { this.duration = duration; } } diff --git a/db/src/main/resources/sql/flyway/V43__update_relock_duration_type.sql b/db/src/main/resources/sql/flyway/V43__update_relock_duration_type.sql new file mode 100644 index 000000000..89d90687c --- /dev/null +++ b/db/src/main/resources/sql/flyway/V43__update_relock_duration_type.sql @@ -0,0 +1,18 @@ +-- Copyright 2020 The Nomulus Authors. All Rights Reserved. +-- +-- Licensed under the Apache License, Version 2.0 (the "License"); +-- you may not use this file except in compliance with the License. +-- You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. + +ALTER TABLE "RegistryLock" DROP COLUMN relock_duration; + +ALTER TABLE "RegistryLock" ADD COLUMN relock_duration interval; + diff --git a/db/src/main/resources/sql/schema/db-schema.sql.generated b/db/src/main/resources/sql/schema/db-schema.sql.generated index fced5c322..ffb4dd34c 100644 --- a/db/src/main/resources/sql/schema/db-schema.sql.generated +++ b/db/src/main/resources/sql/schema/db-schema.sql.generated @@ -459,7 +459,7 @@ create sequence history_id_sequence start 1 increment 1; lock_request_timestamp timestamptz not null, registrar_id text not null, registrar_poc_id text, - relock_duration int8, + relock_duration interval, repo_id text not null, unlock_completion_timestamp timestamptz, unlock_request_timestamp timestamptz, diff --git a/db/src/main/resources/sql/schema/nomulus.golden.sql b/db/src/main/resources/sql/schema/nomulus.golden.sql index 9d1aa2191..704e2423d 100644 --- a/db/src/main/resources/sql/schema/nomulus.golden.sql +++ b/db/src/main/resources/sql/schema/nomulus.golden.sql @@ -670,7 +670,7 @@ CREATE TABLE public."RegistryLock" ( unlock_completion_timestamp timestamp with time zone, last_update_timestamp timestamp with time zone, relock_revision_id bigint, - relock_duration bigint + relock_duration interval );