Create a converter for sets of InetAddresses and use it in HostResource (#612)

* Create a converter for sets of inetAddresses and use it in HostResource

This can just be a set of strings where each string represents an
address;  there's no need for it to be a separate table. This allows
for simplification of the SQL schema.

* Regenerate golden SQL file after renaming v28 -> v29

* Add more tests and rename a typo in the file

* Refactor common test code and use tm methods

* Use JUnit5 API

* Rename test entity

core/src/main/java/google/registry/model/host/HostResource.java

@@ -40,7 +40,6 @@ import java.net.InetAddress;
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nullable;
-import javax.persistence.ElementCollection;
 import org.joda.time.DateTime;
 
 /**
@@ -70,7 +69,7 @@ public class HostResource extends EppResource
   String fullyQualifiedHostName;
 
   /** IP Addresses for this host. Can be null if this is an external host. */
-  @Index @ElementCollection Set<InetAddress> inetAddresses;
+  @Index Set<InetAddress> inetAddresses;
 
   /** The superordinate domain of this host, or null if this is an external host. */
   @Index

core/src/main/java/google/registry/persistence/converter/InetAddressSetConverter.java

@@ -0,0 +1,33 @@
+// 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 com.google.common.net.InetAddresses;
+import java.net.InetAddress;
+import javax.persistence.Converter;
+
+@Converter(autoApply = true)
+public class InetAddressSetConverter extends StringSetConverterBase<InetAddress> {
+
+  @Override
+  String toString(InetAddress element) {
+    return InetAddresses.toAddrString(element);
+  }
+
+  @Override
+  InetAddress fromString(String value) {
+    return InetAddresses.forString(value);
+  }
+}

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

@@ -50,6 +50,7 @@
     <class>google.registry.persistence.converter.CurrencyUnitConverter</class>
     <class>google.registry.persistence.converter.DateTimeConverter</class>
     <class>google.registry.persistence.converter.DurationConverter</class>
+    <class>google.registry.persistence.converter.InetAddressSetConverter</class>
     <class>google.registry.persistence.converter.PostalInfoChoiceListConverter</class>
     <class>google.registry.persistence.converter.RegistrarPocSetConverter</class>
     <class>google.registry.persistence.converter.StatusValueSetConverter</class>

core/src/test/java/google/registry/persistence/converter/InetAddressSetConverterTest.java

@@ -0,0 +1,86 @@
+// 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 static com.google.common.truth.Truth.assertThat;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.net.InetAddresses;
+import google.registry.model.ImmutableObject;
+import google.registry.persistence.VKey;
+import google.registry.schema.replay.EntityTest.EntityForTesting;
+import google.registry.testing.AppEngineRule;
+import java.net.InetAddress;
+import java.util.Set;
+import javax.annotation.Nullable;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+/** Unit tests for {@link google.registry.persistence.converter.InetAddressSetConverter}. */
+public class InetAddressSetConverterTest {
+
+  @RegisterExtension
+  public final AppEngineRule appEngine =
+      AppEngineRule.builder()
+          .withDatastoreAndCloudSql()
+          .withJpaUnitTestEntities(InetAddressSetTestEntity.class)
+          .build();
+
+  @Test
+  public void roundTripConversion_returnsSameAddresses() {
+    verifySaveAndLoad(
+        ImmutableSet.of(
+            InetAddresses.forString("0.0.0.0"),
+            InetAddresses.forString("192.168.0.1"),
+            InetAddresses.forString("2001:41d0:1:a41e:0:0:0:1"),
+            InetAddresses.forString("2041:0:140F::875B:131B")));
+  }
+
+  @Test
+  public void roundTrip_emptySet() {
+    verifySaveAndLoad(ImmutableSet.of());
+  }
+
+  @Test
+  public void roundTrip_null() {
+    verifySaveAndLoad(null);
+  }
+
+  private void verifySaveAndLoad(@Nullable Set<InetAddress> inetAddresses) {
+    InetAddressSetTestEntity testEntity = new InetAddressSetTestEntity(inetAddresses);
+    jpaTm().transact(() -> jpaTm().saveNew(testEntity));
+    InetAddressSetTestEntity persisted =
+        jpaTm().transact(() -> jpaTm().load(VKey.createSql(InetAddressSetTestEntity.class, "id")));
+    assertThat(persisted.addresses).isEqualTo(inetAddresses);
+  }
+
+  @Entity(name = "TestEntity") // Override entity name to avoid the nested class reference.
+  @EntityForTesting
+  private static class InetAddressSetTestEntity extends ImmutableObject {
+
+    @Id String name = "id";
+
+    Set<InetAddress> addresses;
+
+    private InetAddressSetTestEntity() {}
+
+    private InetAddressSetTestEntity(Set<InetAddress> addresses) {
+      this.addresses = addresses;
+    }
+  }
+}

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

@@ -0,0 +1,17 @@
+-- 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 "HostResource" ADD COLUMN inet_addresses text[];
+
+DROP TABLE IF EXISTS "HostResource_inetAddresses";

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

@@ -225,17 +225,13 @@
         last_epp_update_time timestamptz,
         statuses text[],
         fully_qualified_host_name text,
+        inet_addresses text[],
         last_superordinate_change timestamptz,
         last_transfer_time timestamptz,
         superordinate_domain text,
         primary key (repo_id)
     );
 
-    create table "HostResource_inetAddresses" (
-       host_resource_repo_id text not null,
-        inet_addresses bytea
-    );
-
     create table "Lock" (
        resource_name text not null,
         tld text not null,
@@ -437,11 +433,6 @@ create index reservedlist_name_idx on "ReservedList" (name);
        foreign key (domain_repo_id) 
        references "Domain";
 
-    alter table if exists "HostResource_inetAddresses" 
-       add constraint FK6unwhfkcu3oq6q347fxvpagv 
-       foreign key (host_resource_repo_id) 
-       references "HostResource";
-
     alter table if exists "PremiumEntry" 
        add constraint FKo0gw90lpo1tuee56l0nb6y6g5 
        foreign key (revision_id) 

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

@@ -348,17 +348,8 @@ CREATE TABLE public."HostResource" (
     fully_qualified_host_name text,
     last_superordinate_change timestamp with time zone,
     last_transfer_time timestamp with time zone,
-    superordinate_domain text
-);
-
-
---
--- Name: HostResource_inetAddresses; Type: TABLE; Schema: public; Owner: -
---
-
-CREATE TABLE public."HostResource_inetAddresses" (
-    host_resource_repo_id text NOT NULL,
-    inet_addresses bytea
+    superordinate_domain text,
+    inet_addresses text[]
 );
 
 
@@ -1095,14 +1086,6 @@ ALTER TABLE ONLY public."ClaimsEntry"
     ADD CONSTRAINT fk6sc6at5hedffc0nhdcab6ivuq FOREIGN KEY (revision_id) REFERENCES public."ClaimsList"(revision_id);
 
 
---
--- Name: HostResource_inetAddresses fk6unwhfkcu3oq6q347fxvpagv; Type: FK CONSTRAINT; Schema: public; Owner: -
---
-
-ALTER TABLE ONLY public."HostResource_inetAddresses"
-    ADD CONSTRAINT fk6unwhfkcu3oq6q347fxvpagv FOREIGN KEY (host_resource_repo_id) REFERENCES public."HostResource"(repo_id);
-
-
 --
 -- Name: Contact fk93c185fx7chn68uv7nl6uv2s0; Type: FK CONSTRAINT; Schema: public; Owner: -
 --