Get persistence of DomainBase actually working (#438)

* Get persistence of DomainBase actually working

Fix all of the existing problems with DomainBase persistence:
- Remove "final" keywords on getters that cause errors during startup.
- Remove Transient from creationTime (since there's a converter for
  CreateAutoTimestamp)
- Fix DesignatedContext persistence so that it only creates a single table.
  This is a lot more efficient given that these are many-to-one with their
  domains.
- Add a flyway script, update the golden schema.
- Create a unit test, add it to the integration test suite.

* Changes request in review

* Regenerated generated schema file.

* Changes for review

* Persist status value enum set

* Changes in response to review

* Changes requested in review

* Fixes for #456

* Rename Domain "status" column to "statuses"

core/src/main/java/google/registry/model/EppResource.java

@@ -50,8 +50,10 @@ import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
+import javax.persistence.Column;
 import javax.persistence.MappedSuperclass;
 import javax.persistence.Transient;
+import org.hibernate.annotations.Type;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
 
@@ -87,7 +89,7 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
   // Map the method to XML, not the field, because if we map the field (with an adaptor class) it
   // will never be omitted from the xml even if the timestamp inside creationTime is null and we
   // return null from the adaptor. (Instead it gets written as an empty tag.)
-  @Index @Transient CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null);
+  @Index CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null);
 
   /**
    * The time when this resource was or will be deleted.
@@ -105,7 +107,6 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
   @Index
   DateTime deletionTime;
 
-
   /**
    * The time that this resource was last updated.
    *
@@ -116,7 +117,10 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
   DateTime lastEppUpdateTime;
 
   /** Status values associated with this resource. */
-  @Transient Set<StatusValue> status;
+  @Type(type = "google.registry.model.eppcommon.StatusValue$StatusValueSetType")
+  @Column(name = "statuses")
+  // TODO(mmuller): rename to "statuses" once we're off datastore.
+  Set<StatusValue> status;
 
   /**
    * Sorted map of {@link DateTime} keys (modified time) to {@link CommitLogManifest} entries.
@@ -126,6 +130,7 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
    *
    * @see google.registry.model.translators.CommitLogRevisionsTranslatorFactory
    */
+  @Transient
   ImmutableSortedMap<DateTime, Key<CommitLogManifest>> revisions = ImmutableSortedMap.of();
 
   public final String getRepoId() {
@@ -136,15 +141,15 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
     return creationTime.getTimestamp();
   }
 
-  public final String getCreationClientId() {
+  public String getCreationClientId() {
     return creationClientId;
   }
 
-  public final DateTime getLastEppUpdateTime() {
+  public DateTime getLastEppUpdateTime() {
     return lastEppUpdateTime;
   }
 
-  public final String getLastEppUpdateClientId() {
+  public String getLastEppUpdateClientId() {
     return lastEppUpdateClientId;
   }
 
@@ -162,7 +167,7 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
     return nullToEmptyImmutableCopy(status);
   }
 
-  public final DateTime getDeletionTime() {
+  public DateTime getDeletionTime() {
     return deletionTime;
   }
 

core/src/main/java/google/registry/model/domain/DesignatedContact.java

@@ -21,7 +21,7 @@ import com.googlecode.objectify.annotation.Embed;
 import com.googlecode.objectify.annotation.Index;
 import google.registry.model.ImmutableObject;
 import google.registry.model.contact.ContactResource;
-import javax.persistence.Id;
+import javax.persistence.Embeddable;
 import javax.xml.bind.annotation.XmlEnumValue;
 
 /**
@@ -40,7 +40,7 @@ import javax.xml.bind.annotation.XmlEnumValue;
  *     - Contact and Client Identifiers</a>
  */
 @Embed
-@javax.persistence.Entity
+@Embeddable
 public class DesignatedContact extends ImmutableObject {
 
   /**
@@ -67,7 +67,7 @@ public class DesignatedContact extends ImmutableObject {
 
   Type type;
 
-  @Index @Id Key<ContactResource> contact;
+  @Index Key<ContactResource> contact;
 
   public Type getType() {
     return type;

core/src/main/java/google/registry/model/domain/DomainBase.java

@@ -75,6 +75,7 @@ import javax.persistence.AttributeOverrides;
 import javax.persistence.Column;
 import javax.persistence.ElementCollection;
 import javax.persistence.Embedded;
+import javax.persistence.Transient;
 import org.joda.time.DateTime;
 import org.joda.time.Interval;
 
@@ -89,8 +90,16 @@ import org.joda.time.Interval;
  */
 @ReportedOn
 @Entity
-@javax.persistence.Entity
-@javax.persistence.Table(name = "Domain")
+@javax.persistence.Entity(name = "Domain")
+@javax.persistence.Table(
+    name = "Domain",
+    indexes = {
+      @javax.persistence.Index(columnList = "creationTime"),
+      @javax.persistence.Index(columnList = "currentSponsorClientId"),
+      @javax.persistence.Index(columnList = "deletionTime"),
+      @javax.persistence.Index(columnList = "fullyQualifiedDomainName"),
+      @javax.persistence.Index(columnList = "tld")
+    })
 @ExternalMessagingName("domain")
 public class DomainBase extends EppResource
     implements ForeignKeyedEppResource, ResourceWithTransferData {
@@ -105,7 +114,7 @@ public class DomainBase extends EppResource
           StatusValue.INACTIVE,
           StatusValue.PENDING_DELETE,
           StatusValue.SERVER_HOLD);
-  
+
   /**
    * Fully qualified domain name (puny-coded), which serves as the foreign key for this domain.
    *
@@ -115,22 +124,21 @@ public class DomainBase extends EppResource
    *
    * @invariant fullyQualifiedDomainName == fullyQualifiedDomainName.toLowerCase(Locale.ENGLISH)
    */
-  @Index
-  String fullyQualifiedDomainName;
+  @Index String fullyQualifiedDomainName;
 
   /** The top level domain this is under, dernormalized from {@link #fullyQualifiedDomainName}. */
   @Index
   String tld;
 
   /** References to hosts that are the nameservers for the domain. */
-  @Index @ElementCollection Set<Key<HostResource>> nsHosts;
+  @Index @ElementCollection @Transient Set<Key<HostResource>> nsHosts;
 
   /**
    * The union of the contacts visible via {@link #getContacts} and {@link #getRegistrant}.
    *
    * <p>These are stored in one field so that we can query across all contacts at once.
    */
-  @ElementCollection Set<DesignatedContact> allContacts;
+  @Transient Set<DesignatedContact> allContacts;
 
   /** Authorization info (aka transfer secret) of the domain. */
   @Embedded
@@ -146,7 +154,7 @@ public class DomainBase extends EppResource
    * <p>This is {@literal @}XmlTransient because it needs to be returned under the "extension" tag
    * of an info response rather than inside the "infData" tag.
    */
-  @ElementCollection Set<DelegationSignerData> dsData;
+  @Transient Set<DelegationSignerData> dsData;
 
   /**
    * The claims notice supplied when this application or domain was created, if there was one. It's
@@ -177,7 +185,8 @@ public class DomainBase extends EppResource
   String idnTableName;
 
   /** Fully qualified host names of this domain's active subordinate hosts. */
-  @ElementCollection Set<String> subordinateHosts;
+  @org.hibernate.annotations.Type(type = "google.registry.persistence.StringSetUserType")
+  Set<String> subordinateHosts;
 
   /** When this domain's registration will expire. */
   DateTime registrationExpirationTime;
@@ -189,7 +198,7 @@ public class DomainBase extends EppResource
    * refer to a {@link PollMessage} timed to when the domain is fully deleted. If the domain is
    * restored, the message should be deleted.
    */
-  Key<PollMessage.OneTime> deletePollMessage;
+  @Transient Key<PollMessage.OneTime> deletePollMessage;
 
   /**
    * The recurring billing event associated with this domain's autorenewals.
@@ -199,7 +208,7 @@ public class DomainBase extends EppResource
    * {@link #registrationExpirationTime} is changed the recurrence should be closed, a new one
    * should be created, and this field should be updated to point to the new one.
    */
-  Key<BillingEvent.Recurring> autorenewBillingEvent;
+  @Transient Key<BillingEvent.Recurring> autorenewBillingEvent;
 
   /**
    * The recurring poll message associated with this domain's autorenewals.
@@ -209,10 +218,10 @@ public class DomainBase extends EppResource
    * {@link #registrationExpirationTime} is changed the recurrence should be closed, a new one
    * should be created, and this field should be updated to point to the new one.
    */
-  Key<PollMessage.Autorenew> autorenewPollMessage;
+  @Transient Key<PollMessage.Autorenew> autorenewPollMessage;
 
   /** The unexpired grace periods for this domain (some of which may not be active yet). */
-  @ElementCollection Set<GracePeriod> gracePeriods;
+  @Transient @ElementCollection Set<GracePeriod> gracePeriods;
 
   /**
    * The id of the signed mark that was used to create this domain in sunrise.
@@ -223,31 +232,7 @@ public class DomainBase extends EppResource
   String smdId;
 
   /** Data about any pending or past transfers on this domain. */
-  @Embedded
-  @AttributeOverrides({
-    @AttributeOverride(
-        name = "transferRequestTrid",
-        column = @Column(name = "transfer_data_request_trid")),
-    @AttributeOverride(
-        name = "transferPeriod",
-        column = @Column(name = "transfer_data_transfer_period")),
-    @AttributeOverride(
-        name = "transferredRegistrationExpirationTime",
-        column = @Column(name = "transfer_data_registration_expiration_time")),
-    @AttributeOverride(
-        name = "serverApproveEntities",
-        column = @Column(name = "transfer_data_server_approve_entities")),
-    @AttributeOverride(
-        name = "serverApproveBillingEvent",
-        column = @Column(name = "transfer_data_server_approve_billing_event")),
-    @AttributeOverride(
-        name = "serverApproveAutorenewEvent",
-        column = @Column(name = "transfer_data_server_approve_autorenrew_event")),
-    @AttributeOverride(
-        name = "serverApproveAutorenewPollMessage",
-        column = @Column(name = "transfer_data_server_approve_autorenrew_poll_message")),
-  })
-  TransferData transferData;
+  @Transient TransferData transferData;
 
   /**
    * The time that this resource was last transferred.
@@ -285,7 +270,7 @@ public class DomainBase extends EppResource
   }
 
   @Override
-  public final TransferData getTransferData() {
+  public TransferData getTransferData() {
     return Optional.ofNullable(transferData).orElse(TransferData.EMPTY);
   }
 

core/src/main/java/google/registry/model/eppcommon/StatusValue.java

@@ -25,6 +25,7 @@ import google.registry.model.domain.DomainBase;
 import google.registry.model.host.HostResource;
 import google.registry.model.translators.EnumToAttributeAdapter.EppEnum;
 import google.registry.model.translators.StatusValueAdapter;
+import google.registry.persistence.EnumSetUserType;
 import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
 
 /**
@@ -164,4 +165,12 @@ public enum StatusValue implements EppEnum {
   public static StatusValue fromXmlName(String xmlName) {
     return StatusValue.valueOf(LOWER_CAMEL.to(UPPER_UNDERSCORE, nullToEmpty(xmlName)));
   }
+
+  /** Hibernate type for sets of {@link StatusValue}. */
+  public static class StatusValueSetType extends EnumSetUserType<StatusValue> {
+    @Override
+    protected Object convertToElem(Object value) {
+      return StatusValue.valueOf((String) value);
+    }
+  }
 }

core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java

@@ -0,0 +1,121 @@
+// Copyright 2017 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.model.domain;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.util.DateTimeUtils.START_OF_TIME;
+
+import com.google.common.collect.ImmutableSet;
+import com.googlecode.objectify.Key;
+import google.registry.model.EntityTestCase;
+import google.registry.model.contact.ContactResource;
+import google.registry.model.domain.DesignatedContact.Type;
+import google.registry.model.domain.launch.LaunchNotice;
+import google.registry.model.domain.secdns.DelegationSignerData;
+import google.registry.model.eppcommon.AuthInfo.PasswordAuth;
+import google.registry.model.eppcommon.StatusValue;
+import google.registry.persistence.transaction.JpaTestRules;
+import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule;
+import javax.persistence.EntityManager;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Verify that we can store/retrieve DomainBase objects from a SQL database. */
+@RunWith(JUnit4.class)
+public class DomainBaseSqlTest extends EntityTestCase {
+
+  @Rule
+  public final JpaIntegrationWithCoverageRule jpaRule =
+      new JpaTestRules.Builder().buildIntegrationWithCoverageRule();
+
+  DomainBase domain;
+  Key<ContactResource> contactKey;
+  Key<ContactResource> contact2Key;
+
+  @Before
+  public void setUp() {
+    contactKey = Key.create(ContactResource.class, "contact_id1");
+    contact2Key = Key.create(ContactResource.class, "contact_id2");
+
+    domain =
+        new DomainBase.Builder()
+            .setFullyQualifiedDomainName("example.com")
+            .setRepoId("4-COM")
+            .setCreationClientId("a registrar")
+            .setLastEppUpdateTime(clock.nowUtc())
+            .setLastEppUpdateClientId("AnotherRegistrar")
+            .setLastTransferTime(clock.nowUtc())
+            .setStatusValues(
+                ImmutableSet.of(
+                    StatusValue.CLIENT_DELETE_PROHIBITED,
+                    StatusValue.SERVER_DELETE_PROHIBITED,
+                    StatusValue.SERVER_TRANSFER_PROHIBITED,
+                    StatusValue.SERVER_UPDATE_PROHIBITED,
+                    StatusValue.SERVER_RENEW_PROHIBITED,
+                    StatusValue.SERVER_HOLD))
+            .setRegistrant(contactKey)
+            .setContacts(ImmutableSet.of(DesignatedContact.create(Type.ADMIN, contact2Key)))
+            .setSubordinateHosts(ImmutableSet.of("ns1.example.com"))
+            .setPersistedCurrentSponsorClientId("losing")
+            .setRegistrationExpirationTime(clock.nowUtc().plusYears(1))
+            .setAuthInfo(DomainAuthInfo.create(PasswordAuth.create("password")))
+            .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
+            .setLaunchNotice(
+                LaunchNotice.create("tcnid", "validatorId", START_OF_TIME, START_OF_TIME))
+            .setSmdId("smdid")
+            .build();
+  }
+
+  @Test
+  public void testDomainBasePersistence() {
+    jpaTm()
+        .transact(
+            () -> {
+              // Persist the domain.
+              EntityManager em = jpaTm().getEntityManager();
+              em.persist(domain);
+            });
+
+    jpaTm()
+        .transact(
+            () -> {
+              // Load the domain in its entirety.
+              EntityManager em = jpaTm().getEntityManager();
+              DomainBase result = em.find(DomainBase.class, "4-COM");
+
+              // Fix contacts, grace period and DS data, since we can't persist them yet.
+              result =
+                  result
+                      .asBuilder()
+                      .setRegistrant(contactKey)
+                      .setContacts(
+                          ImmutableSet.of(DesignatedContact.create(Type.ADMIN, contact2Key)))
+                      .setDsData(
+                          ImmutableSet.of(
+                              DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
+                      .build();
+
+              // Fix the original creation timestamp (this gets initialized on first write)
+              DomainBase org = domain.asBuilder().setCreationTime(result.getCreationTime()).build();
+
+              // Note that the equality comparison forces a lazy load of all fields.
+              assertThat(result).isEqualTo(org);
+            });
+  }
+}

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

@@ -15,6 +15,7 @@
 package google.registry.schema.integration;
 
 import com.google.common.truth.Expect;
+import google.registry.model.domain.DomainBaseSqlTest;
 import google.registry.model.registry.RegistryLockDaoTest;
 import google.registry.persistence.transaction.JpaEntityCoverage;
 import google.registry.schema.cursor.CursorDaoTest;
@@ -55,6 +56,7 @@ import org.junit.runners.Suite.SuiteClasses;
   CursorDaoTest.class,
   DomainLockUtilsTest.class,
   LockDomainCommandTest.class,
+  DomainBaseSqlTest.class,
   PremiumListDaoTest.class,
   RegistryLockDaoTest.class,
   RegistryLockGetActionTest.class,