Make all contacts nullable in the data model (#2490)

This doesn't yet allow them to be absent in EPP flows, but it should make the
code not break if they happen to be null in the database. This is a follow-up to
PR #2477, which ends up being a bit easier because whereas the registrant is
used in more parts of the codebase, the other contact types (admin, technical,
billing) are really only used in RDE, WHOIS, and RDAP, and because they were
already being used as a collection anyway, the handling for if that collection
contains fewer elements or is empty happened to already be mostly correct.

core/src/main/java/google/registry/beam/rde/RdePipeline.java

@@ -466,13 +466,10 @@ public class RdePipeline implements Serializable {
                               // Contacts and hosts are only deposited in RDE, not BRDA.
                               if (pendingDeposit.mode() == RdeMode.FULL) {
                                 HashSet<Serializable> contacts = new HashSet<>();
-                                contacts.add(domain.getAdminContact().getKey());
-                                contacts.add(domain.getTechContact().getKey());
-                                domain.getRegistrant().ifPresent(r -> contacts.add(r.getKey()));
-                                // Billing contact is not mandatory.
-                                if (domain.getBillingContact() != null) {
-                                  contacts.add(domain.getBillingContact().getKey());
-                                }
+                                domain.getAdminContact().ifPresent(c -> contacts.add(c.getKey()));
+                                domain.getTechContact().ifPresent(c -> contacts.add(c.getKey()));
+                                domain.getRegistrant().ifPresent(c -> contacts.add(c.getKey()));
+                                domain.getBillingContact().ifPresent(c -> contacts.add(c.getKey()));
                                 referencedContactCounter.inc(contacts.size());
                                 contacts.forEach(
                                     contactRepoId ->

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

@@ -46,6 +46,7 @@ import google.registry.model.EppResource;
 import google.registry.model.EppResource.ResourceWithTransferData;
 import google.registry.model.billing.BillingRecurrence;
 import google.registry.model.contact.Contact;
+import google.registry.model.domain.DesignatedContact.Type;
 import google.registry.model.domain.launch.LaunchNotice;
 import google.registry.model.domain.rgp.GracePeriodStatus;
 import google.registry.model.domain.secdns.DomainDsData;
@@ -131,10 +132,10 @@ public class DomainBase extends EppResource
   @Expose @Transient Set<VKey<Host>> nsHosts;
 
   /** Contacts. */
-  @Expose VKey<Contact> adminContact;
+  @Expose @Nullable VKey<Contact> adminContact;
 
-  @Expose VKey<Contact> billingContact;
-  @Expose VKey<Contact> techContact;
+  @Expose @Nullable VKey<Contact> billingContact;
+  @Expose @Nullable VKey<Contact> techContact;
   @Expose @Nullable VKey<Contact> registrantContact;
 
   /** Authorization info (aka transfer secret) of the domain. */
@@ -589,24 +590,32 @@ public class DomainBase extends EppResource
     return Optional.ofNullable(registrantContact);
   }
 
-  public VKey<Contact> getAdminContact() {
-    return adminContact;
+  public Optional<VKey<Contact>> getAdminContact() {
+    return Optional.ofNullable(adminContact);
   }
 
-  public VKey<Contact> getBillingContact() {
-    return billingContact;
+  public Optional<VKey<Contact>> getBillingContact() {
+    return Optional.ofNullable(billingContact);
   }
 
-  public VKey<Contact> getTechContact() {
-    return techContact;
+  public Optional<VKey<Contact>> getTechContact() {
+    return Optional.ofNullable(techContact);
   }
 
-  /** Associated contacts for the domain (other than registrant). */
+  /**
+   * Associated contacts for the domain (other than registrant).
+   *
+   * <p>Note: This can be an empty set if no contacts are present for the domain.
+   */
   public ImmutableSet<DesignatedContact> getContacts() {
     return getAllContacts(false);
   }
 
-  /** Gets all associated contacts for the domain, including the registrant. */
+  /**
+   * Gets all associated contacts for the domain, including the registrant.
+   *
+   * <p>Note: This can be an empty set if no contacts are present for the domain.
+   */
   public ImmutableSet<DesignatedContact> getAllContacts() {
     return getAllContacts(true);
   }
@@ -615,7 +624,11 @@ public class DomainBase extends EppResource
     return authInfo;
   }
 
-  /** Returns all referenced contacts from this domain. */
+  /**
+   * Returns all referenced contacts from this domain.
+   *
+   * <p>Note: This can be an empty set if no contacts are present for the domain.
+   */
   public ImmutableSet<VKey<Contact>> getReferencedContacts() {
     return nullToEmptyImmutableCopy(getAllContacts(true)).stream()
         .map(DesignatedContact::getContactKey)
@@ -625,18 +638,12 @@ public class DomainBase extends EppResource
 
   private ImmutableSet<DesignatedContact> getAllContacts(boolean includeRegistrant) {
     ImmutableSet.Builder<DesignatedContact> builder = new ImmutableSet.Builder<>();
-    if (includeRegistrant && registrantContact != null) {
-      builder.add(DesignatedContact.create(DesignatedContact.Type.REGISTRANT, registrantContact));
-    }
-    if (adminContact != null) {
-      builder.add(DesignatedContact.create(DesignatedContact.Type.ADMIN, adminContact));
-    }
-    if (billingContact != null) {
-      builder.add(DesignatedContact.create(DesignatedContact.Type.BILLING, billingContact));
-    }
-    if (techContact != null) {
-      builder.add(DesignatedContact.create(DesignatedContact.Type.TECH, techContact));
+    if (includeRegistrant) {
+      getRegistrant().ifPresent(c -> builder.add(DesignatedContact.create(Type.REGISTRANT, c)));
     }
+    getAdminContact().ifPresent(c -> builder.add(DesignatedContact.create(Type.ADMIN, c)));
+    getBillingContact().ifPresent(c -> builder.add(DesignatedContact.create(Type.BILLING, c)));
+    getTechContact().ifPresent(c -> builder.add(DesignatedContact.create(Type.TECH, c)));
     return builder.build();
   }
 
@@ -652,11 +659,13 @@ public class DomainBase extends EppResource
    */
   void setContactFields(Set<DesignatedContact> contacts, boolean includeRegistrant) {
     // Set the individual contact fields.
-    billingContact = techContact = adminContact = null;
+    billingContact = null;
+    techContact = null;
+    adminContact = null;
     if (includeRegistrant) {
       registrantContact = null;
     }
-    HashSet<DesignatedContact.Type> contactsDiscovered = new HashSet<>();
+    HashSet<Type> contactsDiscovered = new HashSet<>();
     for (DesignatedContact contact : contacts) {
       checkArgument(
           !contactsDiscovered.contains(contact.getType()),
@@ -687,7 +696,7 @@ public class DomainBase extends EppResource
 
   /** Predicate to determine if a given {@link DesignatedContact} is the registrant. */
   static final Predicate<DesignatedContact> IS_REGISTRANT =
-      (DesignatedContact contact) -> DesignatedContact.Type.REGISTRANT.equals(contact.type);
+      (DesignatedContact contact) -> Type.REGISTRANT.equals(contact.type);
 
   /** An override of {@link EppResource#asBuilder} with tighter typing. */
   @Override

core/src/main/java/google/registry/rdap/RdapJsonFormatter.java

@@ -381,8 +381,11 @@ public class RdapJsonFormatter {
                 () ->
                     ImmutableSet.copyOf(replicaTm().loadByKeys(domain.getNameservers()).values()));
     // Load the registrant and other contacts and add them to the data.
+    ImmutableSet<VKey<Contact>> contacts = domain.getReferencedContacts();
     ImmutableMap<VKey<? extends Contact>, Contact> loadedContacts =
-        replicaTm().transact(() -> replicaTm().loadByKeysIfPresent(domain.getReferencedContacts()));
+        contacts.isEmpty()
+            ? ImmutableMap.of()
+            : replicaTm().transact(() -> replicaTm().loadByKeysIfPresent(contacts));
 
     // RDAP Response Profile 2.7.1, 2.7.3 - we MUST have the contacts. 2.7.4 discusses redaction of
     // fields we don't want to show (as opposed to not having contacts at all) because of GDPR etc.
@@ -544,7 +547,8 @@ public class RdapJsonFormatter {
     // TODO(mcilwain): Once the RDAP profile is fully updated for minimum registration data set,
     //                 we will want to not include non-existent contacts at all, rather than
     //                 pretending they exist and just showing REDACTED info. This is especially
-    //                 important for authorized flows, where you wouldn't expect to see redaction.
+    //                 important for authorized flows, where you wouldn't expect to see redaction
+    //                 (although no one actually has access to authorized flows yet).
     boolean isAuthorized =
         contact.isPresent()
             && rdapAuthorization.isAuthorizedForRegistrar(