Add a converter for CurrencyUnits stored in the database (#334)

* Add a converter for CurrencyUnits stored in the database

This uses the well-known String representation for currency units. It also
provides a base class for other converters that will be persisting the
toString() representation.

* Add DB and formatting changes

* Add tests, make minor fixes

core/src/main/java/google/registry/persistence/CurrencyUnitConverter.java

@@ -0,0 +1,29 @@
+// Copyright 2019 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;
+
+import javax.annotation.Nullable;
+import javax.persistence.Converter;
+import org.joda.money.CurrencyUnit;
+
+/** JPA converter for {@link CurrencyUnit}s. */
+@Converter(autoApply = true)
+public class CurrencyUnitConverter extends ToStringConverterBase<CurrencyUnit> {
+
+  @Override
+  @Nullable
+  public CurrencyUnit convertToEntityAttribute(@Nullable String columnValue) {
+    return (columnValue == null) ? null : CurrencyUnit.of(columnValue);
+  }
+}

core/src/main/java/google/registry/persistence/ToStringConverterBase.java

@@ -0,0 +1,27 @@
+// Copyright 2019 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;
+
+import javax.annotation.Nullable;
+import javax.persistence.AttributeConverter;
+
+/** Abstract JPA converter for objects that are stored by their toString() value. */
+abstract class ToStringConverterBase<T> implements AttributeConverter<T, String> {
+
+  @Override
+  @Nullable
+  public String convertToDatabaseColumn(@Nullable T entity) {
+    return (entity == null) ? null : entity.toString();
+  }
+}

core/src/main/resources/META-INF/persistence.xml

@@ -35,6 +35,7 @@
     <!-- Customized type converters -->
     <class>google.registry.persistence.BloomFilterConverter</class>
     <class>google.registry.persistence.CreateAutoTimestampConverter</class>
+    <class>google.registry.persistence.CurrencyUnitConverter</class>
     <class>google.registry.persistence.UpdateAutoTimestampConverter</class>
     <class>google.registry.persistence.ZonedDateTimeConverter</class>
 

core/src/test/java/google/registry/persistence/BloomFilterConverterTest.java

@@ -30,6 +30,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
+/** Unit tests for {@link BloomFilterConverter}. */
 @RunWith(JUnit4.class)
 public class BloomFilterConverterTest {
 
@@ -60,7 +61,7 @@ public class BloomFilterConverterTest {
 
     public TestEntity() {}
 
-    public TestEntity(BloomFilter<String> bloomFilter) {
+    TestEntity(BloomFilter<String> bloomFilter) {
       this.bloomFilter = bloomFilter;
     }
   }

core/src/test/java/google/registry/persistence/CreateAutoTimestampConverterTest.java

@@ -28,6 +28,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
+/** Unit tests for {@link CreateAutoTimestampConverter}. */
 @RunWith(JUnit4.class)
 public class CreateAutoTimestampConverterTest {
 
@@ -70,7 +71,7 @@ public class CreateAutoTimestampConverterTest {
 
     public TestEntity() {}
 
-    public TestEntity(String name, CreateAutoTimestamp cat) {
+    TestEntity(String name, CreateAutoTimestamp cat) {
       this.name = name;
       this.cat = cat;
     }

core/src/test/java/google/registry/persistence/CurrencyUnitConverterTest.java

@@ -0,0 +1,98 @@
+// Copyright 2019 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;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.testing.JUnitBackports.assertThrows;
+
+import google.registry.model.ImmutableObject;
+import google.registry.model.transaction.JpaTransactionManagerRule;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.PersistenceException;
+import org.hibernate.cfg.Environment;
+import org.joda.money.CurrencyUnit;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Unit tests for {@link CurrencyUnitConverter}. */
+@RunWith(JUnit4.class)
+public class CurrencyUnitConverterTest {
+
+  @Rule
+  public final JpaTransactionManagerRule jpaTmRule =
+      new JpaTransactionManagerRule.Builder()
+          .withEntityClass(TestEntity.class)
+          .withProperty(Environment.HBM2DDL_AUTO, "update")
+          .build();
+
+  @Test
+  public void roundTripConversion() {
+    TestEntity entity = new TestEntity(CurrencyUnit.EUR);
+    jpaTm().transact(() -> jpaTm().getEntityManager().persist(entity));
+    assertThat(
+            jpaTm()
+                .transact(
+                    () ->
+                        jpaTm()
+                            .getEntityManager()
+                            .createNativeQuery("SELECT currency FROM TestEntity WHERE name = 'id'")
+                            .getResultList()))
+        .containsExactly("EUR");
+    TestEntity persisted =
+        jpaTm().transact(() -> jpaTm().getEntityManager().find(TestEntity.class, "id"));
+    assertThat(persisted.currency).isEqualTo(CurrencyUnit.EUR);
+  }
+
+  @Test
+  public void invalidCurrency() {
+    jpaTm()
+        .transact(
+            () ->
+                jpaTm()
+                    .getEntityManager()
+                    .createNativeQuery(
+                        "INSERT INTO TestEntity (name, currency) VALUES('id', 'XXXX')")
+                    .executeUpdate());
+    PersistenceException thrown =
+        assertThrows(
+            PersistenceException.class,
+            () ->
+                jpaTm()
+                    .transact(
+                        () -> jpaTm().getEntityManager().find(TestEntity.class, "id").currency));
+    assertThat(thrown)
+        .hasCauseThat()
+        .hasCauseThat()
+        .hasMessageThat()
+        .isEqualTo("Unknown currency 'XXXX'");
+  }
+
+  @Entity(name = "TestEntity") // Override entity name to avoid the nested class reference.
+  public static class TestEntity extends ImmutableObject {
+
+    @Id String name = "id";
+
+    CurrencyUnit currency;
+
+    public TestEntity() {}
+
+    TestEntity(CurrencyUnit currency) {
+      this.currency = currency;
+    }
+  }
+}

core/src/test/java/google/registry/persistence/UpdateAutoTimestampConverterTest.java

@@ -27,6 +27,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
+/** Unit tests for {@link UpdateAutoTimestampConverter}. */
 @RunWith(JUnit4.class)
 public class UpdateAutoTimestampConverterTest {
 
@@ -81,7 +82,7 @@ public class UpdateAutoTimestampConverterTest {
 
     public TestEntity() {}
 
-    public TestEntity(String name, UpdateAutoTimestamp uat) {
+    TestEntity(String name, UpdateAutoTimestamp uat) {
       this.name = name;
       this.uat = uat;
     }

core/src/test/java/google/registry/persistence/ZonedDateTimeConverterTest.java

@@ -113,7 +113,7 @@ public class ZonedDateTimeConverterTest {
 
     public TestEntity() {}
 
-    public TestEntity(String name, ZonedDateTime zdt) {
+    TestEntity(String name, ZonedDateTime zdt) {
       this.name = name;
       this.zdt = zdt;
     }

core/src/test/java/google/registry/schema/integration/SqlIntegrationMembershipTest.java

@@ -45,10 +45,7 @@ public class SqlIntegrationMembershipTest {
   public void sqlIntegrationMembershipComplete() {
     ImmutableSet<String> sqlDependentTests;
     try (ScanResult scanResult =
-        new ClassGraph()
+        new ClassGraph().enableAnnotationInfo().whitelistPackages("google.registry").scan()) {
-            .enableAnnotationInfo()
-            .whitelistPackages("google.registry")
-            .scan()) {
       sqlDependentTests =
           scanResult.getClassesWithAnnotation(RunWith.class.getName()).stream()
               .filter(clazz -> clazz.getSimpleName().endsWith("Test"))

core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java

@@ -20,6 +20,7 @@ import google.registry.model.transaction.JpaTransactionManagerImplTest;
 import google.registry.model.transaction.JpaTransactionManagerRuleTest;
 import google.registry.persistence.BloomFilterConverterTest;
 import google.registry.persistence.CreateAutoTimestampConverterTest;
+import google.registry.persistence.CurrencyUnitConverterTest;
 import google.registry.persistence.UpdateAutoTimestampConverterTest;
 import google.registry.persistence.ZonedDateTimeConverterTest;
 import google.registry.schema.tld.PremiumListDaoTest;
@@ -42,6 +43,7 @@ import org.junit.runners.Suite.SuiteClasses;
   BloomFilterConverterTest.class,
   ClaimsListDaoTest.class,
   CreateAutoTimestampConverterTest.class,
+  CurrencyUnitConverterTest.class,
   JpaTransactionManagerImplTest.class,
   JpaTransactionManagerRuleTest.class,
   PremiumListDaoTest.class,

db/src/main/resources/sql/flyway/V9__premium_list_currency_type.sql

@@ -0,0 +1,19 @@
+-- Copyright 2019 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.
+
+-- Note: We're OK with dropping this data since it's not live in production yet.
+alter table "PremiumList" drop column if exists currency;
+
+-- TODO(mcilwain): Add a subsequent schema change to remove this default.
+alter table "PremiumList" add column currency text not null default 'USD';

db/src/main/resources/sql/schema/db-schema.sql.generated

@@ -133,7 +133,7 @@
        revision_id  bigserial not null,
         bloom_filter bytea not null,
         creation_timestamp timestamptz not null,
-        currency bytea not null,
+        currency text not null,
         name text not null,
         primary key (revision_id)
     );

db/src/main/resources/sql/schema/nomulus.golden.sql

@@ -92,9 +92,9 @@ CREATE TABLE public."PremiumEntry" (
 CREATE TABLE public."PremiumList" (
     revision_id bigint NOT NULL,
     creation_timestamp timestamp with time zone NOT NULL,
-    currency bytea NOT NULL,
     name text NOT NULL,
-    bloom_filter bytea NOT NULL
+    bloom_filter bytea NOT NULL,
+    currency text DEFAULT 'USD'::text NOT NULL
 );
 
 

proxy/src/main/java/google/registry/proxy/metric/BaseMetrics.java

@@ -35,7 +35,7 @@ public abstract class BaseMetrics {
    * nomulus -e production list_registrars -f clientCertificateHash | grep $HASH
    * </pre>
    */
-   protected static final ImmutableSet<LabelDescriptor> LABELS =
+  protected static final ImmutableSet<LabelDescriptor> LABELS =
       ImmutableSet.of(
           LabelDescriptor.create("protocol", "Name of the protocol."),
           LabelDescriptor.create(

proxy/src/test/java/google/registry/proxy/handler/FrontendMetricsHandlerTest.java

@@ -158,12 +158,9 @@ public class FrontendMetricsHandlerTest {
     Duration latency2 = new Duration(requestTime2, responseTime2);
     Duration latency3 = new Duration(requestTime3, responseTime3);
 
-    verify(metrics)
+    verify(metrics).responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency1);
-        .responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency1);
+    verify(metrics).responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency2);
-    verify(metrics)
+    verify(metrics).responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency3);
-        .responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency2);
-    verify(metrics)
-        .responseSent(PROTOCOL_NAME, CLIENT_CERT_HASH, latency3);
     verifyNoMoreInteractions(metrics);
   }
 }

util/src/main/java/google/registry/util/Retrier.java

@@ -146,9 +146,7 @@ public class Retrier implements Serializable {
   }
 
   private <V> V callWithRetry(
-      Callable<V> callable,
+      Callable<V> callable, FailureReporter failureReporter, Predicate<Throwable> isRetryable) {
-      FailureReporter failureReporter,
-      Predicate<Throwable> isRetryable) {
     int failures = 0;
     while (true) {
       try {