Add IntervalDescriptor and change DurationConverter to use Interval datatype (#653)

* Add use of interval data type

* Add support for Millis

* Use Java-object type

* Change column type for relock_duration

* add years and months

* Add tests for hours, minutes, and seconds

* Add javadoc describing how joda duration is stored

* Add test for lots of days

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']

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

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

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

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

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

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

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());
   }
 }

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.
+ *
+ * <p>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<Duration, Long> {
+public class DurationConverter implements AttributeConverter<Duration, PGInterval> {
 
   @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();
   }
 }

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 <a
+ *     href="https://docs.jboss.org/hibernate/orm/current/userguide/html_single/Hibernate_User_Guide.html#basic-jpa-convert">JPA
+ *     2.1 AttributeConverters</a>
+ */
+public class IntervalDescriptor extends AbstractTypeDescriptor<PGInterval>
+    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> X unwrap(PGInterval value, Class<X> type, WrapperOptions options) {
+    if (value == null) {
+      return null;
+    }
+    if (PGInterval.class.isAssignableFrom(type)) {
+      return (X) value;
+    }
+    throw unknownUnwrap(type);
+  }
+
+  @Override
+  public <X> 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 <X> ValueBinder<X> getBinder(JavaTypeDescriptor<X> javaTypeDescriptor) {
+    return new BasicBinder<X>(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 <X> ValueExtractor<X> getExtractor(JavaTypeDescriptor<X> javaTypeDescriptor) {
+    return new BasicExtractor<X>(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);
+      }
+    };
+  }
+}

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;
     }
   }

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;
+

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,

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
 );