Add JPA annotations to ContactResource and generate schema (#547)

* Add JPA annotations to ContactResource and generate schema

* Resolve comments

* Resolve comments

* Manually add foreign key constraints

* Run with junit5

* Rebase on HEAD

* Fix DomainBaseSqlTest

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

@@ -70,9 +70,11 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
 
   /** The ID of the registrar that is currently sponsoring this resource. */
   @Index
+  @Column(nullable = false)
   String currentSponsorClientId;
 
   /** The ID of the registrar that created this resource. */
+  @Column(nullable = false)
   String creationClientId;
 
   /**
@@ -88,7 +90,9 @@ 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 CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null);
+  @Column(nullable = false)
+  @Index
+  CreateAutoTimestamp creationTime = CreateAutoTimestamp.create(null);
 
   /**
    * The time when this resource was or will be deleted.

core/src/main/java/google/registry/model/contact/ContactAddress.java

@@ -16,13 +16,14 @@ package google.registry.model.contact;
 
 import com.googlecode.objectify.annotation.Embed;
 import google.registry.model.eppcommon.Address;
+import javax.persistence.Embeddable;
 
 /**
  * EPP Contact Address
  *
- * <p>This class is embedded inside the {@link PostalInfo} of an EPP contact to hold its
+ * <p>This class is embedded inside the {@link PostalInfo} of an EPP contact to hold its address.
- * address. The fields are all defined in parent class {@link Address}, but the subclass is still
+ * The fields are all defined in parent class {@link Address}, but the subclass is still necessary
- * necessary to pick up the contact namespace.
+ * to pick up the contact namespace.
  *
  * <p>This does not implement {@code Overlayable} because it is intended to be bulk replaced on
  * update.
@@ -30,6 +31,7 @@ import google.registry.model.eppcommon.Address;
  * @see PostalInfo
  */
 @Embed
+@Embeddable
 public class ContactAddress extends Address {
 
   /** Builder for {@link ContactAddress}. */

core/src/main/java/google/registry/model/contact/ContactAuthInfo.java

@@ -20,6 +20,7 @@ import javax.xml.bind.annotation.XmlType;
 
 /** A version of authInfo specifically for contacts. */
 @Embed
+@javax.persistence.Embeddable
 @XmlType(namespace = "urn:ietf:params:xml:ns:contact-1.0")
 public class ContactAuthInfo extends AuthInfo {
   public static ContactAuthInfo create(PasswordAuth pw) {

core/src/main/java/google/registry/model/contact/ContactPhoneNumber.java

@@ -16,6 +16,7 @@ package google.registry.model.contact;
 
 import com.googlecode.objectify.annotation.Embed;
 import google.registry.model.eppcommon.PhoneNumber;
+import javax.persistence.Embeddable;
 
 /**
  * EPP Contact Phone Number
@@ -27,6 +28,7 @@ import google.registry.model.eppcommon.PhoneNumber;
  * @see ContactResource
  */
 @Embed
+@Embeddable
 public class ContactPhoneNumber extends PhoneNumber {
 
   /** Builder for {@link ContactPhoneNumber}. */

core/src/main/java/google/registry/model/contact/ContactResource.java

@@ -33,6 +33,11 @@ import google.registry.model.transfer.TransferData;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Stream;
+import javax.persistence.AttributeOverride;
+import javax.persistence.AttributeOverrides;
+import javax.persistence.Column;
+import javax.persistence.Embedded;
+import javax.persistence.Transient;
 import javax.xml.bind.annotation.XmlElement;
 import org.joda.time.DateTime;
 
@@ -43,9 +48,19 @@ import org.joda.time.DateTime;
  */
 @ReportedOn
 @Entity
+@javax.persistence.Entity(name = "Contact")
+@javax.persistence.Table(
+    name = "Contact",
+    indexes = {
+      @javax.persistence.Index(columnList = "creationTime"),
+      @javax.persistence.Index(columnList = "currentSponsorClientId"),
+      @javax.persistence.Index(columnList = "deletionTime"),
+      @javax.persistence.Index(columnList = "contactId", unique = true),
+      @javax.persistence.Index(columnList = "searchName")
+    })
 @ExternalMessagingName("contact")
-public class ContactResource extends EppResource implements
+public class ContactResource extends EppResource
-    ForeignKeyedEppResource, ResourceWithTransferData {
+    implements ForeignKeyedEppResource, ResourceWithTransferData {
 
   /**
    * Unique identifier for this contact.
@@ -61,13 +76,55 @@ public class ContactResource extends EppResource implements
    * US-ASCII character set. Personal info; cleared by {@link Builder#wipeOut}.
    */
   @IgnoreSave(IfNull.class)
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "name", column = @Column(name = "addr_local_name")),
+    @AttributeOverride(name = "org", column = @Column(name = "addr_local_org")),
+    @AttributeOverride(name = "type", column = @Column(name = "addr_local_type")),
+    @AttributeOverride(
+        name = "address.streetLine1",
+        column = @Column(name = "addr_local_street_line1")),
+    @AttributeOverride(
+        name = "address.streetLine2",
+        column = @Column(name = "addr_local_street_line2")),
+    @AttributeOverride(
+        name = "address.streetLine3",
+        column = @Column(name = "addr_local_street_line3")),
+    @AttributeOverride(name = "address.city", column = @Column(name = "addr_local_city")),
+    @AttributeOverride(name = "address.state", column = @Column(name = "addr_local_state")),
+    @AttributeOverride(name = "address.zip", column = @Column(name = "addr_local_zip")),
+    @AttributeOverride(
+        name = "address.countryCode",
+        column = @Column(name = "addr_local_country_code"))
+  })
   PostalInfo localizedPostalInfo;
 
   /**
-   * Internationalized postal info for the contact. Personal info; cleared by
+   * Internationalized postal info for the contact. Personal info; cleared by {@link
-   * {@link Builder#wipeOut}.
+   * Builder#wipeOut}.
    */
   @IgnoreSave(IfNull.class)
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "name", column = @Column(name = "addr_i18n_name")),
+    @AttributeOverride(name = "org", column = @Column(name = "addr_i18n_org")),
+    @AttributeOverride(name = "type", column = @Column(name = "addr_i18n_type")),
+    @AttributeOverride(
+        name = "address.streetLine1",
+        column = @Column(name = "addr_i18n_street_line1")),
+    @AttributeOverride(
+        name = "address.streetLine2",
+        column = @Column(name = "addr_i18n_street_line2")),
+    @AttributeOverride(
+        name = "address.streetLine3",
+        column = @Column(name = "addr_i18n_street_line3")),
+    @AttributeOverride(name = "address.city", column = @Column(name = "addr_i18n_city")),
+    @AttributeOverride(name = "address.state", column = @Column(name = "addr_i18n_state")),
+    @AttributeOverride(name = "address.zip", column = @Column(name = "addr_i18n_zip")),
+    @AttributeOverride(
+        name = "address.countryCode",
+        column = @Column(name = "addr_i18n_country_code"))
+  })
   PostalInfo internationalizedPostalInfo;
 
   /**
@@ -80,10 +137,20 @@ public class ContactResource extends EppResource implements
 
   /** Contact’s voice number. Personal info; cleared by {@link Builder#wipeOut}. */
   @IgnoreSave(IfNull.class)
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "phoneNumber", column = @Column(name = "voice_phone_number")),
+    @AttributeOverride(name = "extension", column = @Column(name = "voice_phone_extension")),
+  })
   ContactPhoneNumber voice;
 
   /** Contact’s fax number. Personal info; cleared by {@link Builder#wipeOut}. */
   @IgnoreSave(IfNull.class)
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "phoneNumber", column = @Column(name = "fax_phone_number")),
+    @AttributeOverride(name = "extension", column = @Column(name = "fax_phone_extension")),
+  })
   ContactPhoneNumber fax;
 
   /** Contact’s email address. Personal info; cleared by {@link Builder#wipeOut}. */
@@ -91,10 +158,16 @@ public class ContactResource extends EppResource implements
   String email;
 
   /** Authorization info (aka transfer secret) of the contact. */
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "pw.value", column = @Column(name = "auth_info_value")),
+    @AttributeOverride(name = "pw.repoId", column = @Column(name = "auth_info_repo_id")),
+  })
   ContactAuthInfo authInfo;
 
   /** Data about any pending or past transfers on this contact. */
-  TransferData transferData;
+  // TODO(b/153363295): Figure out how to persist transfer data
+  @Transient TransferData transferData;
 
   /**
    * The time that this resource was last transferred.
@@ -107,6 +180,16 @@ public class ContactResource extends EppResource implements
   // the wipeOut() function, so that data is not kept around for deleted contacts.
 
   /** Disclosure policy. */
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "name", column = @Column(name = "disclose_types_name")),
+    @AttributeOverride(name = "org", column = @Column(name = "disclose_types_org")),
+    @AttributeOverride(name = "addr", column = @Column(name = "disclose_types_addr")),
+    @AttributeOverride(name = "flag", column = @Column(name = "disclose_mode_flag")),
+    @AttributeOverride(name = "voice.marked", column = @Column(name = "disclose_show_voice")),
+    @AttributeOverride(name = "fax.marked", column = @Column(name = "disclose_show_fax")),
+    @AttributeOverride(name = "email.marked", column = @Column(name = "disclose_show_email"))
+  })
   Disclose disclose;
 
   public String getContactId() {

core/src/main/java/google/registry/model/contact/Disclose.java

@@ -22,11 +22,14 @@ import google.registry.model.Buildable;
 import google.registry.model.ImmutableObject;
 import google.registry.model.eppcommon.PresenceMarker;
 import java.util.List;
+import javax.persistence.Embeddable;
+import javax.persistence.Embedded;
 import javax.xml.bind.annotation.XmlAttribute;
 import javax.xml.bind.annotation.XmlType;
 
 /** The "discloseType" from {@link "http://tools.ietf.org/html/rfc5733"}. */
 @Embed
+@Embeddable
 @XmlType(propOrder = {"name", "org", "addr", "voice", "fax", "email"})
 public class Disclose extends ImmutableObject {
 
@@ -36,11 +39,11 @@ public class Disclose extends ImmutableObject {
 
   List<PostalInfoChoice> addr;
 
-  PresenceMarker voice;
+  @Embedded PresenceMarker voice;
 
-  PresenceMarker fax;
+  @Embedded PresenceMarker fax;
 
-  PresenceMarker email;
+  @Embedded PresenceMarker email;
 
   @XmlAttribute
   Boolean flag;

core/src/main/java/google/registry/model/contact/PostalInfo.java

@@ -21,6 +21,9 @@ import google.registry.model.Buildable;
 import google.registry.model.Buildable.Overlayable;
 import google.registry.model.ImmutableObject;
 import java.util.Optional;
+import javax.persistence.Embeddable;
+import javax.persistence.EnumType;
+import javax.persistence.Enumerated;
 import javax.xml.bind.annotation.XmlAttribute;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlEnumValue;
@@ -29,10 +32,11 @@ import javax.xml.bind.annotation.adapters.NormalizedStringAdapter;
 import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
 
 /**
- * Implementation of both "postalInfoType" and "chgPostalInfoType" from
+ * Implementation of both "postalInfoType" and "chgPostalInfoType" from {@link
- * {@link "http://tools.ietf.org/html/rfc5733"}.
+ * "http://tools.ietf.org/html/rfc5733"}.
  */
 @Embed
+@Embeddable
 @XmlType(propOrder = {"name", "org", "address", "type"})
 public class PostalInfo extends ImmutableObject implements Overlayable<PostalInfo> {
 
@@ -53,6 +57,7 @@ public class PostalInfo extends ImmutableObject implements Overlayable<PostalInf
   @XmlElement(name = "addr")
   ContactAddress address;
 
+  @Enumerated(EnumType.STRING)
   @XmlAttribute
   Type type;
 

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

@@ -18,6 +18,8 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import google.registry.model.Buildable;
 import google.registry.model.ImmutableObject;
+import javax.persistence.Embeddable;
+import javax.persistence.MappedSuperclass;
 import javax.xml.bind.annotation.XmlAttribute;
 import javax.xml.bind.annotation.XmlTransient;
 import javax.xml.bind.annotation.XmlValue;
@@ -31,17 +33,21 @@ import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
  * "e164Type" type from {@link "http://tools.ietf.org/html/draft-lozano-tmch-smd"}.
  *
  * <blockquote>
+ *
  * <p>"Contact telephone number structure is derived from structures defined in [ITU.E164.2005].
  * Telephone numbers described in this mapping are character strings that MUST begin with a plus
  * sign ("+", ASCII value 0x002B), followed by a country code defined in [ITU.E164.2005], followed
  * by a dot (".", ASCII value 0x002E), followed by a sequence of digits representing the telephone
  * number. An optional "x" attribute is provided to note telephone extension information."
+ *
  * </blockquote>
  *
  * @see google.registry.model.contact.ContactPhoneNumber
  * @see google.registry.model.mark.MarkPhoneNumber
  */
 @XmlTransient
+@Embeddable
+@MappedSuperclass
 public class PhoneNumber extends ImmutableObject {
 
   @XmlValue

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

@@ -17,6 +17,7 @@ package google.registry.model.eppcommon;
 import com.googlecode.objectify.annotation.Embed;
 import google.registry.model.ImmutableObject;
 import java.io.Serializable;
+import javax.persistence.Embeddable;
 import javax.xml.bind.annotation.XmlTransient;
 
 /**
@@ -26,6 +27,7 @@ import javax.xml.bind.annotation.XmlTransient;
  * {@code <foo></foo>}, and will unmarshal always to {@code <foo/>}.
  */
 @Embed
+@Embeddable
 public class PresenceMarker extends ImmutableObject implements Serializable {
   @XmlTransient
   boolean marked = true;

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

@@ -18,6 +18,7 @@ import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.Arrays;
 import java.util.Objects;
 import java.util.stream.Stream;
@@ -31,6 +32,7 @@ import javax.persistence.PostUpdate;
 import javax.persistence.PrePersist;
 import javax.persistence.PreRemove;
 import javax.persistence.PreUpdate;
+import javax.persistence.Transient;
 
 /**
  * A listener class to invoke entity callbacks in cases where Hibernate doesn't invoke the callback
@@ -167,10 +169,12 @@ public class EntityCallbacksListener {
 
     private Stream<Object> findEmbeddedProperties(Object object, Class<?> clazz) {
       return Arrays.stream(clazz.getDeclaredFields())
+          .filter(field -> !field.isAnnotationPresent(Transient.class))
           .filter(
               field ->
                   field.isAnnotationPresent(Embedded.class)
                       || field.getType().isAnnotationPresent(Embeddable.class))
+          .filter(field -> !Modifier.isStatic(field.getModifiers()))
           .map(field -> getFieldObject(field, object))
           .filter(Objects::nonNull);
     }

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

@@ -0,0 +1,36 @@
+// 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 google.registry.model.contact.Disclose.PostalInfoChoice;
+import google.registry.model.contact.PostalInfo;
+import java.util.List;
+import javax.persistence.AttributeConverter;
+import javax.persistence.Converter;
+
+/** JPA {@link AttributeConverter} for storing/retrieving {@link List < PostalInfoChoice >}. */
+@Converter(autoApply = true)
+public class PostalInfoChoiceListConverter extends StringListConverterBase<PostalInfoChoice> {
+
+  @Override
+  String toString(PostalInfoChoice element) {
+    return element.getType().name();
+  }
+
+  @Override
+  PostalInfoChoice fromString(String value) {
+    return PostalInfoChoice.create(PostalInfo.Type.valueOf(value));
+  }
+}

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

@@ -19,6 +19,7 @@
       *  Move tests to another (sub)project. This is not a big problem, but feels unnatural.
       *  Use Hibernate's ServiceRegistry for bootstrapping (not JPA-compliant)
     -->
+    <class>google.registry.model.contact.ContactResource</class>
     <class>google.registry.model.domain.DomainBase</class>
     <class>google.registry.model.host.HostResource</class>
     <class>google.registry.model.registrar.Registrar</class>
@@ -41,6 +42,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.PostalInfoChoiceListConverter</class>
     <class>google.registry.persistence.converter.RegistrarPocSetConverter</class>
     <class>google.registry.persistence.converter.StatusValueSetConverter</class>
     <class>google.registry.persistence.converter.StringListConverter</class>

core/src/test/java/google/registry/model/contact/ContactResourceTest.java

@@ -17,10 +17,13 @@ package google.registry.model.contact;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
+import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
 import static google.registry.testing.DatastoreHelper.cloneAndSetAutoTimestamps;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.persistResource;
+import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation;
+import static google.registry.testing.SqlHelper.saveRegistrar;
 import static google.registry.util.DateTimeUtils.END_OF_TIME;
 import static org.junit.Assert.assertThrows;
 
@@ -37,28 +40,31 @@ import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.eppcommon.Trid;
 import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferStatus;
-import org.junit.Before;
+import google.registry.persistence.VKey;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link ContactResource}. */
 public class ContactResourceTest extends EntityTestCase {
+  ContactResource originalContact;
   ContactResource contactResource;
 
-  @Before
+  public ContactResourceTest() {
+    super(true);
+  }
+
+  @BeforeEach
   public void setUp() {
     createTld("foobar");
-    // Set up a new persisted ContactResource entity.
+    originalContact =
-    contactResource =
-        persistResource(
-            cloneAndSetAutoTimestamps(
         new ContactResource.Builder()
             .setContactId("contact_id")
             .setRepoId("1-FOOBAR")
-                    .setCreationClientId("a registrar")
+            .setCreationClientId("registrar1")
             .setLastEppUpdateTime(fakeClock.nowUtc())
-                    .setLastEppUpdateClientId("another registrar")
+            .setLastEppUpdateClientId("registrar2")
             .setLastTransferTime(fakeClock.nowUtc())
-                    .setPersistedCurrentSponsorClientId("a third registrar")
+            .setPersistedCurrentSponsorClientId("registrar3")
             .setLocalizedPostalInfo(
                 new PostalInfo.Builder()
                     .setType(Type.LOCALIZED)
@@ -83,8 +89,7 @@ public class ContactResourceTest extends EntityTestCase {
                             .setCountryCode("US")
                             .build())
                     .build())
-                    .setVoiceNumber(
+            .setVoiceNumber(new ContactPhoneNumber.Builder().setPhoneNumber("867-5309").build())
-                        new ContactPhoneNumber.Builder().setPhoneNumber("867-5309").build())
             .setFaxNumber(
                 new ContactPhoneNumber.Builder()
                     .setPhoneNumber("867-5309")
@@ -98,12 +103,9 @@ public class ContactResourceTest extends EntityTestCase {
                     .setEmail(new PresenceMarker())
                     .setFax(new PresenceMarker())
                     .setFlag(true)
-                            .setAddrs(
+                    .setAddrs(ImmutableList.of(PostalInfoChoice.create(Type.INTERNATIONALIZED)))
-                                ImmutableList.of(PostalInfoChoice.create(Type.INTERNATIONALIZED)))
+                    .setNames(ImmutableList.of(PostalInfoChoice.create(Type.INTERNATIONALIZED)))
-                            .setNames(
+                    .setOrgs(ImmutableList.of(PostalInfoChoice.create(Type.INTERNATIONALIZED)))
-                                ImmutableList.of(PostalInfoChoice.create(Type.INTERNATIONALIZED)))
-                            .setOrgs(
-                                ImmutableList.of(PostalInfoChoice.create(Type.INTERNATIONALIZED)))
                     .build())
             .setStatusValues(ImmutableSet.of(StatusValue.OK))
             .setTransferData(
@@ -117,7 +119,40 @@ public class ContactResourceTest extends EntityTestCase {
                     .setTransferStatus(TransferStatus.SERVER_APPROVED)
                     .setTransferRequestTrid(Trid.create("client-trid", "server-trid"))
                     .build())
-                    .build()));
+            .build();
+    // Set up a new persisted ContactResource entity.
+    contactResource = persistResource(cloneAndSetAutoTimestamps(originalContact));
+  }
+
+  @Test
+  public void testCloudSqlPersistence_failWhenViolateForeignKeyConstraint() {
+    assertThrowForeignKeyViolation(() -> jpaTm().transact(() -> jpaTm().saveNew(originalContact)));
+  }
+
+  @Test
+  public void testCloudSqlPersistence_succeed() {
+    saveRegistrar("registrar1");
+    saveRegistrar("registrar2");
+    saveRegistrar("registrar3");
+    jpaTm().transact(() -> jpaTm().saveNew(originalContact));
+    ContactResource persisted =
+        jpaTm()
+            .transact(
+                () ->
+                    jpaTm()
+                        .load(VKey.createSql(ContactResource.class, originalContact.getRepoId())))
+            .get();
+    // TODO(b/153378849): Remove the hard code for postal info after resolving the issue that
+    // @PostLoad doesn't work in Address
+    ContactResource fixed =
+        originalContact
+            .asBuilder()
+            .setCreationTime(persisted.getCreationTime())
+            .setInternationalizedPostalInfo(persisted.getInternationalizedPostalInfo())
+            .setLocalizedPostalInfo(persisted.getLocalizedPostalInfo())
+            .setTransferData(null)
+            .build();
+    assertThat(persisted).isEqualTo(fixed);
   }
 
   @Test

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

@@ -16,9 +16,10 @@ 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.testing.SqlHelper.assertThrowForeignKeyViolation;
+import static google.registry.testing.SqlHelper.saveRegistrar;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.joda.time.DateTimeZone.UTC;
-import static org.junit.Assert.assertThrows;
 
 import com.google.common.collect.ImmutableSet;
 import com.googlecode.objectify.Key;
@@ -34,9 +35,7 @@ import google.registry.persistence.transaction.JpaTestRules;
 import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageExtension;
 import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
-import java.sql.SQLException;
 import javax.persistence.EntityManager;
-import javax.persistence.RollbackException;
 import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Order;
@@ -73,9 +72,9 @@ public class DomainBaseSqlTest {
         new DomainBase.Builder()
             .setFullyQualifiedDomainName("example.com")
             .setRepoId("4-COM")
-            .setCreationClientId("a registrar")
+            .setCreationClientId("registrar1")
             .setLastEppUpdateTime(fakeClock.nowUtc())
-            .setLastEppUpdateClientId("AnotherRegistrar")
+            .setLastEppUpdateClientId("registrar2")
             .setLastTransferTime(fakeClock.nowUtc())
             .setNameservers(host1VKey)
             .setStatusValues(
@@ -89,7 +88,7 @@ public class DomainBaseSqlTest {
             .setRegistrant(contactKey)
             .setContacts(ImmutableSet.of(DesignatedContact.create(Type.ADMIN, contact2Key)))
             .setSubordinateHosts(ImmutableSet.of("ns1.example.com"))
-            .setPersistedCurrentSponsorClientId("losing")
+            .setPersistedCurrentSponsorClientId("registrar3")
             .setRegistrationExpirationTime(fakeClock.nowUtc().plusYears(1))
             .setAuthInfo(DomainAuthInfo.create(PasswordAuth.create("password")))
             .setDsData(ImmutableSet.of(DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
@@ -102,11 +101,17 @@ public class DomainBaseSqlTest {
         new HostResource.Builder()
             .setRepoId("host1")
             .setFullyQualifiedHostName("ns1.example.com")
+            .setCreationClientId("registrar1")
+            .setPersistedCurrentSponsorClientId("registrar2")
             .build();
   }
 
   @Test
   public void testDomainBasePersistence() {
+    saveRegistrar("registrar1");
+    saveRegistrar("registrar2");
+    saveRegistrar("registrar3");
+
     jpaTm()
         .transact(
             () -> {
@@ -147,9 +152,7 @@ public class DomainBaseSqlTest {
 
   @Test
   public void testForeignKeyConstraints() {
-    Exception e =
+    assertThrowForeignKeyViolation(
-        assertThrows(
-            RollbackException.class,
         () -> {
           jpaTm()
               .transact(
@@ -159,16 +162,5 @@ public class DomainBaseSqlTest {
                     em.persist(domain);
                   });
         });
-    assertThat(e)
-        .hasCauseThat() // ConstraintViolationException
-        .hasCauseThat() // ConstraintViolationException
-        .hasCauseThat()
-        .isInstanceOf(SQLException.class);
-    assertThat(e)
-        .hasCauseThat() // ConstraintViolationException
-        .hasCauseThat() // ConstraintViolationException
-        .hasCauseThat()
-        .hasMessageThat()
-        .contains("\"DomainHost\" violates foreign key constraint \"fk_domainhost_host");
   }
 }

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

@@ -16,6 +16,7 @@ package google.registry.schema.integration;
 
 import static com.google.common.truth.Truth.assert_;
 
+import google.registry.model.contact.ContactResourceTest;
 import google.registry.model.domain.DomainBaseSqlTest;
 import google.registry.model.registry.RegistryLockDaoTest;
 import google.registry.persistence.transaction.JpaEntityCoverage;
@@ -68,6 +69,7 @@ import org.junit.runner.RunWith;
   // BeforeSuiteTest must be the first entry. See class javadoc for details.
   BeforeSuiteTest.class,
   ClaimsListDaoTest.class,
+  ContactResourceTest.class,
   CursorDaoTest.class,
   DomainBaseSqlTest.class,
   LockDaoTest.class,

core/src/test/java/google/registry/testing/SqlHelper.java

@@ -14,12 +14,19 @@
 
 package google.registry.testing;
 
+import static com.google.common.truth.Truth.assertThat;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
+import static google.registry.testing.AppEngineRule.makeRegistrar1;
+import static org.junit.Assert.assertThrows;
 
+import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import google.registry.model.registry.RegistryLockDao;
 import google.registry.schema.domain.RegistryLock;
+import java.sql.SQLException;
 import java.util.Optional;
+import javax.persistence.RollbackException;
+import org.junit.function.ThrowingRunnable;
 
 /** Static utils for setting up and retrieving test resources from the SQL database. */
 public class SqlHelper {
@@ -52,5 +59,19 @@ public class SqlHelper {
     return jpaTm().transact(() -> RegistryLockDao.getByRevisionId(revisionId));
   }
 
+  public static void saveRegistrar(String clientId) {
+    jpaTm()
+        .transact(
+            () -> jpaTm().saveNew(makeRegistrar1().asBuilder().setClientId(clientId).build()));
+  }
+
+  public static void assertThrowForeignKeyViolation(ThrowingRunnable runnable) {
+    RollbackException thrown = assertThrows(RollbackException.class, runnable);
+    assertThat(Throwables.getRootCause(thrown)).isInstanceOf(SQLException.class);
+    assertThat(Throwables.getRootCause(thrown))
+        .hasMessageThat()
+        .contains("violates foreign key constraint");
+  }
+
   private SqlHelper() {}
 }

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

@@ -0,0 +1,107 @@
+-- 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.
+
+create table "Contact" (
+   repo_id text not null,
+    creation_client_id text not null,
+    creation_time timestamptz not null,
+    current_sponsor_client_id text not null,
+    deletion_time timestamptz,
+    last_epp_update_client_id text,
+    last_epp_update_time timestamptz,
+    statuses text[],
+    auth_info_repo_id text,
+    auth_info_value text,
+    contact_id text,
+    disclose_types_addr text[],
+    disclose_show_email boolean,
+    disclose_show_fax boolean,
+    disclose_mode_flag boolean,
+    disclose_types_name text[],
+    disclose_types_org text[],
+    disclose_show_voice boolean,
+    email text,
+    fax_phone_extension text,
+    fax_phone_number text,
+    addr_i18n_city text,
+    addr_i18n_country_code text,
+    addr_i18n_state text,
+    addr_i18n_street_line1 text,
+    addr_i18n_street_line2 text,
+    addr_i18n_street_line3 text,
+    addr_i18n_zip text,
+    addr_i18n_name text,
+    addr_i18n_org text,
+    addr_i18n_type text,
+    last_transfer_time timestamptz,
+    addr_local_city text,
+    addr_local_country_code text,
+    addr_local_state text,
+    addr_local_street_line1 text,
+    addr_local_street_line2 text,
+    addr_local_street_line3 text,
+    addr_local_zip text,
+    addr_local_name text,
+    addr_local_org text,
+    addr_local_type text,
+    search_name text,
+    voice_phone_extension text,
+    voice_phone_number text,
+    primary key (repo_id)
+);
+
+create index IDX3y752kr9uh4kh6uig54vemx0l on "Contact" (creation_time);
+create index IDXbn8t4wp85fgxjl8q4ctlscx55 on "Contact" (current_sponsor_client_id);
+create index IDXn1f711wicdnooa2mqb7g1m55o on "Contact" (deletion_time);
+create index IDX1p3esngcwwu6hstyua6itn6ff on "Contact" (search_name);
+
+alter table if exists "Contact"
+    add constraint UKoqd7n4hbx86hvlgkilq75olas unique (contact_id);
+
+alter table "Domain" alter column creation_time set not null;
+alter table "Domain" alter column creation_client_id set not null;
+alter table "Domain" alter column current_sponsor_client_id set not null;
+
+drop index IDX8ffrqm27qtj20jac056j7yq07;
+create index IDXkjt9yaq92876dstimd93hwckh on "Domain" (current_sponsor_client_id);
+
+alter table if exists "Contact"
+   add constraint FK1sfyj7o7954prbn1exk7lpnoe
+   foreign key (creation_client_id)
+   references "Registrar";
+
+alter table if exists "Contact"
+   add constraint FK93c185fx7chn68uv7nl6uv2s0
+   foreign key (current_sponsor_client_id)
+   references "Registrar";
+
+alter table if exists "Contact"
+   add constraint FKmb7tdiv85863134w1wogtxrb2
+   foreign key (last_epp_update_client_id)
+   references "Registrar";
+
+alter table if exists "Domain"
+   add constraint FK2jc69qyg2tv9hhnmif6oa1cx1
+   foreign key (creation_client_id)
+   references "Registrar";
+
+alter table if exists "Domain"
+   add constraint FK2u3srsfbei272093m3b3xwj23
+   foreign key (current_sponsor_client_id)
+   references "Registrar";
+
+alter table if exists "Domain"
+   add constraint FKjc0r9r5y1lfbt4gpbqw4wsuvq
+   foreign key (last_epp_update_client_id)
+   references "Registrar";

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

@@ -26,6 +26,55 @@
         primary key (revision_id)
     );
 
+    create table "Contact" (
+       repo_id text not null,
+        creation_client_id text not null,
+        creation_time timestamptz not null,
+        current_sponsor_client_id text not null,
+        deletion_time timestamptz,
+        last_epp_update_client_id text,
+        last_epp_update_time timestamptz,
+        statuses text[],
+        auth_info_repo_id text,
+        auth_info_value text,
+        contact_id text,
+        disclose_types_addr text[],
+        disclose_show_email boolean,
+        disclose_show_fax boolean,
+        disclose_mode_flag boolean,
+        disclose_types_name text[],
+        disclose_types_org text[],
+        disclose_show_voice boolean,
+        email text,
+        fax_phone_extension text,
+        fax_phone_number text,
+        addr_i18n_city text,
+        addr_i18n_country_code text,
+        addr_i18n_state text,
+        addr_i18n_street_line1 text,
+        addr_i18n_street_line2 text,
+        addr_i18n_street_line3 text,
+        addr_i18n_zip text,
+        addr_i18n_name text,
+        addr_i18n_org text,
+        addr_i18n_type text,
+        last_transfer_time timestamptz,
+        addr_local_city text,
+        addr_local_country_code text,
+        addr_local_state text,
+        addr_local_street_line1 text,
+        addr_local_street_line2 text,
+        addr_local_street_line3 text,
+        addr_local_zip text,
+        addr_local_name text,
+        addr_local_org text,
+        addr_local_type text,
+        search_name text,
+        voice_phone_extension text,
+        voice_phone_number text,
+        primary key (repo_id)
+    );
+
     create table "Cursor" (
        scope text not null,
         type text not null,
@@ -44,9 +93,9 @@
 
     create table "Domain" (
        repo_id text not null,
-        creation_client_id text,
+        creation_client_id text not null,
-        creation_time timestamptz,
+        creation_time timestamptz not null,
-        current_sponsor_client_id text,
+        current_sponsor_client_id text not null,
         deletion_time timestamptz,
         last_epp_update_client_id text,
         last_epp_update_time timestamptz,
@@ -84,9 +133,9 @@
 
     create table "HostResource" (
        repo_id text not null,
-        creation_client_id text,
+        creation_client_id text not null,
-        creation_time timestamptz,
+        creation_time timestamptz not null,
-        current_sponsor_client_id text,
+        current_sponsor_client_id text not null,
         deletion_time timestamptz,
         last_epp_update_client_id text,
         last_epp_update_time timestamptz,
@@ -227,6 +276,13 @@
         should_publish boolean not null,
         primary key (revision_id)
     );
+create index IDX3y752kr9uh4kh6uig54vemx0l on "Contact" (creation_time);
+create index IDXbn8t4wp85fgxjl8q4ctlscx55 on "Contact" (current_sponsor_client_id);
+create index IDXn1f711wicdnooa2mqb7g1m55o on "Contact" (deletion_time);
+create index IDX1p3esngcwwu6hstyua6itn6ff on "Contact" (search_name);
+
+    alter table if exists "Contact" 
+       add constraint UKoqd7n4hbx86hvlgkilq75olas unique (contact_id);
 create index IDX8nr0ke9mrrx4ewj6pd2ag4rmr on "Domain" (creation_time);
 create index IDX8ffrqm27qtj20jac056j7yq07 on "Domain" (current_sponsor_client_id);
 create index IDX5mnf0wn20tno4b9do88j61klr on "Domain" (deletion_time);

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

@@ -75,6 +75,59 @@ CREATE SEQUENCE public."ClaimsList_revision_id_seq"
 ALTER SEQUENCE public."ClaimsList_revision_id_seq" OWNED BY public."ClaimsList".revision_id;
 
 
+--
+-- Name: Contact; Type: TABLE; Schema: public; Owner: -
+--
+
+CREATE TABLE public."Contact" (
+    repo_id text NOT NULL,
+    creation_client_id text NOT NULL,
+    creation_time timestamp with time zone NOT NULL,
+    current_sponsor_client_id text NOT NULL,
+    deletion_time timestamp with time zone,
+    last_epp_update_client_id text,
+    last_epp_update_time timestamp with time zone,
+    statuses text[],
+    auth_info_repo_id text,
+    auth_info_value text,
+    contact_id text,
+    disclose_types_addr text[],
+    disclose_show_email boolean,
+    disclose_show_fax boolean,
+    disclose_mode_flag boolean,
+    disclose_types_name text[],
+    disclose_types_org text[],
+    disclose_show_voice boolean,
+    email text,
+    fax_phone_extension text,
+    fax_phone_number text,
+    addr_i18n_city text,
+    addr_i18n_country_code text,
+    addr_i18n_state text,
+    addr_i18n_street_line1 text,
+    addr_i18n_street_line2 text,
+    addr_i18n_street_line3 text,
+    addr_i18n_zip text,
+    addr_i18n_name text,
+    addr_i18n_org text,
+    addr_i18n_type text,
+    last_transfer_time timestamp with time zone,
+    addr_local_city text,
+    addr_local_country_code text,
+    addr_local_state text,
+    addr_local_street_line1 text,
+    addr_local_street_line2 text,
+    addr_local_street_line3 text,
+    addr_local_zip text,
+    addr_local_name text,
+    addr_local_org text,
+    addr_local_type text,
+    search_name text,
+    voice_phone_extension text,
+    voice_phone_number text
+);
+
+
 --
 -- Name: Cursor; Type: TABLE; Schema: public; Owner: -
 --
@@ -93,9 +146,9 @@ CREATE TABLE public."Cursor" (
 
 CREATE TABLE public."Domain" (
     repo_id text NOT NULL,
-    creation_client_id text,
+    creation_client_id text NOT NULL,
-    creation_time timestamp with time zone,
+    creation_time timestamp with time zone NOT NULL,
-    current_sponsor_client_id text,
+    current_sponsor_client_id text NOT NULL,
     deletion_time timestamp with time zone,
     last_epp_update_client_id text,
     last_epp_update_time timestamp with time zone,
@@ -414,6 +467,14 @@ ALTER TABLE ONLY public."ClaimsList"
     ADD CONSTRAINT "ClaimsList_pkey" PRIMARY KEY (revision_id);
 
 
+--
+-- Name: Contact Contact_pkey; Type: CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public."Contact"
+    ADD CONSTRAINT "Contact_pkey" PRIMARY KEY (repo_id);
+
+
 --
 -- Name: Cursor Cursor_pkey; Type: CONSTRAINT; Schema: public; Owner: -
 --
@@ -510,6 +571,21 @@ ALTER TABLE ONLY public."RegistryLock"
     ADD CONSTRAINT idx_registry_lock_repo_id_revision_id UNIQUE (repo_id, revision_id);
 
 
+--
+-- Name: Contact ukoqd7n4hbx86hvlgkilq75olas; Type: CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public."Contact"
+    ADD CONSTRAINT ukoqd7n4hbx86hvlgkilq75olas UNIQUE (contact_id);
+
+
+--
+-- Name: idx1p3esngcwwu6hstyua6itn6ff; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX idx1p3esngcwwu6hstyua6itn6ff ON public."Contact" USING btree (search_name);
+
+
 --
 -- Name: idx1rcgkdd777bpvj0r94sltwd5y; Type: INDEX; Schema: public; Owner: -
 --
@@ -517,6 +593,13 @@ ALTER TABLE ONLY public."RegistryLock"
 CREATE INDEX idx1rcgkdd777bpvj0r94sltwd5y ON public."Domain" USING btree (fully_qualified_domain_name);
 
 
+--
+-- Name: idx3y752kr9uh4kh6uig54vemx0l; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX idx3y752kr9uh4kh6uig54vemx0l ON public."Contact" USING btree (creation_time);
+
+
 --
 -- Name: idx5mnf0wn20tno4b9do88j61klr; Type: INDEX; Schema: public; Owner: -
 --
@@ -524,13 +607,6 @@ CREATE INDEX idx1rcgkdd777bpvj0r94sltwd5y ON public."Domain" USING btree (fully_
 CREATE INDEX idx5mnf0wn20tno4b9do88j61klr ON public."Domain" USING btree (deletion_time);
 
 
---
--- Name: idx8ffrqm27qtj20jac056j7yq07; Type: INDEX; Schema: public; Owner: -
---
-
-CREATE INDEX idx8ffrqm27qtj20jac056j7yq07 ON public."Domain" USING btree (current_sponsor_client_id);
-
-
 --
 -- Name: idx8nr0ke9mrrx4ewj6pd2ag4rmr; Type: INDEX; Schema: public; Owner: -
 --
@@ -552,6 +628,27 @@ CREATE INDEX idx_registry_lock_registrar_id ON public."RegistryLock" USING btree
 CREATE INDEX idx_registry_lock_verification_code ON public."RegistryLock" USING btree (verification_code);
 
 
+--
+-- Name: idxbn8t4wp85fgxjl8q4ctlscx55; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX idxbn8t4wp85fgxjl8q4ctlscx55 ON public."Contact" USING btree (current_sponsor_client_id);
+
+
+--
+-- Name: idxkjt9yaq92876dstimd93hwckh; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX idxkjt9yaq92876dstimd93hwckh ON public."Domain" USING btree (current_sponsor_client_id);
+
+
+--
+-- Name: idxn1f711wicdnooa2mqb7g1m55o; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX idxn1f711wicdnooa2mqb7g1m55o ON public."Contact" USING btree (deletion_time);
+
+
 --
 -- Name: idxrwl38wwkli1j7gkvtywi9jokq; Type: INDEX; Schema: public; Owner: -
 --
@@ -594,6 +691,22 @@ CREATE INDEX registrarpoc_gae_user_id_idx ON public."RegistrarPoc" USING btree (
 CREATE INDEX reservedlist_name_idx ON public."ReservedList" USING btree (name);
 
 
+--
+-- Name: Contact fk1sfyj7o7954prbn1exk7lpnoe; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public."Contact"
+    ADD CONSTRAINT fk1sfyj7o7954prbn1exk7lpnoe FOREIGN KEY (creation_client_id) REFERENCES public."Registrar"(client_id);
+
+
+--
+-- Name: Domain fk2jc69qyg2tv9hhnmif6oa1cx1; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public."Domain"
+    ADD CONSTRAINT fk2jc69qyg2tv9hhnmif6oa1cx1 FOREIGN KEY (creation_client_id) REFERENCES public."Registrar"(client_id);
+
+
 --
 -- Name: RegistryLock fk2lhcwpxlnqijr96irylrh1707; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -602,6 +715,14 @@ ALTER TABLE ONLY public."RegistryLock"
     ADD CONSTRAINT fk2lhcwpxlnqijr96irylrh1707 FOREIGN KEY (relock_revision_id) REFERENCES public."RegistryLock"(revision_id);
 
 
+--
+-- Name: Domain fk2u3srsfbei272093m3b3xwj23; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public."Domain"
+    ADD CONSTRAINT fk2u3srsfbei272093m3b3xwj23 FOREIGN KEY (current_sponsor_client_id) REFERENCES public."Registrar"(client_id);
+
+
 --
 -- Name: ClaimsEntry fk6sc6at5hedffc0nhdcab6ivuq; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -618,6 +739,14 @@ 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: -
+--
+
+ALTER TABLE ONLY public."Contact"
+    ADD CONSTRAINT fk93c185fx7chn68uv7nl6uv2s0 FOREIGN KEY (current_sponsor_client_id) REFERENCES public."Registrar"(client_id);
+
+
 --
 -- Name: DomainHost fk_domainhost_host_valid; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
@@ -642,6 +771,22 @@ ALTER TABLE ONLY public."ReservedEntry"
     ADD CONSTRAINT fkgq03rk0bt1hb915dnyvd3vnfc FOREIGN KEY (revision_id) REFERENCES public."ReservedList"(revision_id);
 
 
+--
+-- Name: Domain fkjc0r9r5y1lfbt4gpbqw4wsuvq; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public."Domain"
+    ADD CONSTRAINT fkjc0r9r5y1lfbt4gpbqw4wsuvq FOREIGN KEY (last_epp_update_client_id) REFERENCES public."Registrar"(client_id);
+
+
+--
+-- Name: Contact fkmb7tdiv85863134w1wogtxrb2; Type: FK CONSTRAINT; Schema: public; Owner: -
+--
+
+ALTER TABLE ONLY public."Contact"
+    ADD CONSTRAINT fkmb7tdiv85863134w1wogtxrb2 FOREIGN KEY (last_epp_update_client_id) REFERENCES public."Registrar"(client_id);
+
+
 --
 -- Name: PremiumEntry fko0gw90lpo1tuee56l0nb6y6g5; Type: FK CONSTRAINT; Schema: public; Owner: -
 --