Make the contacts prohibited feature flag for min reg data set more lenient (#2787)

It will now only throw errors on domain updates if a new contact/registrant has
been specified where none was previously present. This means that domain updates
on unrelated fields (e.g. nameserver changes) will succeed even if there is
existing contact data that the update is not removing.

This is a follow-up to #2781.

BUG=http://b/434958659

core/build.gradle

@@ -17,7 +17,7 @@ import java.util.Optional
 
 plugins {
   id 'java-library'
-  id "org.flywaydb.flyway" version "9.22.3"
+  id "org.flywaydb.flyway" version "11.0.1"
   id 'maven-publish'
 }
 

core/src/main/java/google/registry/flows/domain/DomainFlowUtils.java

@@ -481,10 +481,10 @@ public class DomainFlowUtils {
   }
 
   /**
-   * Enforces the presence/absence of contact data depending on the minimum data set migration
-   * schedule.
+   * Enforces the presence/absence of contact data on domain creates depending on the minimum data
+   * set migration schedule.
    */
-  static void validateContactDataPresence(
+  static void validateCreateContactData(
       Optional<VKey<Contact>> registrant, Set<DesignatedContact> contacts)
       throws RequiredParameterMissingException, ParameterValuePolicyErrorException {
     // TODO(b/353347632): Change these flag checks to a registry config check once minimum data set
@@ -514,6 +514,45 @@ public class DomainFlowUtils {
     }
   }
 
+  /**
+   * Enforces the presence/absence of contact data on domain updates depending on the minimum data
+   * set migration schedule.
+   */
+  static void validateUpdateContactData(
+      Optional<VKey<Contact>> existingRegistrant,
+      Optional<VKey<Contact>> newRegistrant,
+      Set<DesignatedContact> existingContacts,
+      Set<DesignatedContact> newContacts)
+      throws RequiredParameterMissingException, ParameterValuePolicyErrorException {
+    // TODO(b/353347632): Change these flag checks to a registry config check once minimum data set
+    //                    migration is completed.
+    if (FeatureFlag.isActiveNow(MINIMUM_DATASET_CONTACTS_PROHIBITED)) {
+      // Throw if the update specifies a new registrant that is different from the existing one.
+      if (newRegistrant.isPresent() && !newRegistrant.equals(existingRegistrant)) {
+        throw new RegistrantProhibitedException();
+      }
+      // Throw if the update specifies any new contacts that weren't already present on the domain.
+      if (!Sets.difference(newContacts, existingContacts).isEmpty()) {
+        throw new ContactsProhibitedException();
+      }
+    } else if (!FeatureFlag.isActiveNow(MINIMUM_DATASET_CONTACTS_OPTIONAL)) {
+      // Throw if the update empties out a registrant that had been present.
+      if (newRegistrant.isEmpty() && existingRegistrant.isPresent()) {
+        throw new MissingRegistrantException();
+      }
+      // Throw if the update contains no admin contact when one had been present.
+      if (existingContacts.stream().anyMatch(c -> c.getType().equals(Type.ADMIN))
+          && newContacts.stream().noneMatch(c -> c.getType().equals(Type.ADMIN))) {
+        throw new MissingAdminContactException();
+      }
+      // Throw if the update contains no tech contact when one had been present.
+      if (existingContacts.stream().anyMatch(c -> c.getType().equals(Type.TECH))
+          && newContacts.stream().noneMatch(c -> c.getType().equals(Type.TECH))) {
+        throw new MissingTechnicalContactException();
+      }
+    }
+  }
+
   static void validateRegistrantAllowedOnTld(String tld, Optional<String> registrantContactId)
       throws RegistrantNotAllowedException {
     ImmutableSet<String> allowedRegistrants = Tld.get(tld).getAllowedRegistrantContactIds();
@@ -1054,7 +1093,7 @@ public class DomainFlowUtils {
     String tldStr = tld.getTldStr();
     validateRegistrantAllowedOnTld(tldStr, command.getRegistrantContactId());
     validateNoDuplicateContacts(command.getContacts());
-    validateContactDataPresence(command.getRegistrant(), command.getContacts());
+    validateCreateContactData(command.getRegistrant(), command.getContacts());
     ImmutableSet<String> hostNames = command.getNameserverHostNames();
     validateNameserversCountForTld(tldStr, domainName, hostNames.size());
     validateNameserversAllowedOnTld(tldStr, hostNames);

core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java

@@ -30,7 +30,6 @@ import static google.registry.flows.ResourceFlowUtils.verifyResourceOwnership;
 import static google.registry.flows.domain.DomainFlowUtils.checkAllowedAccessToTld;
 import static google.registry.flows.domain.DomainFlowUtils.cloneAndLinkReferences;
 import static google.registry.flows.domain.DomainFlowUtils.updateDsData;
-import static google.registry.flows.domain.DomainFlowUtils.validateContactDataPresence;
 import static google.registry.flows.domain.DomainFlowUtils.validateContactsHaveTypes;
 import static google.registry.flows.domain.DomainFlowUtils.validateDsData;
 import static google.registry.flows.domain.DomainFlowUtils.validateFeesAckedIfPresent;
@@ -38,6 +37,7 @@ import static google.registry.flows.domain.DomainFlowUtils.validateNameserversAl
 import static google.registry.flows.domain.DomainFlowUtils.validateNameserversCountForTld;
 import static google.registry.flows.domain.DomainFlowUtils.validateNoDuplicateContacts;
 import static google.registry.flows.domain.DomainFlowUtils.validateRegistrantAllowedOnTld;
+import static google.registry.flows.domain.DomainFlowUtils.validateUpdateContactData;
 import static google.registry.flows.domain.DomainFlowUtils.verifyClientUpdateNotProhibited;
 import static google.registry.flows.domain.DomainFlowUtils.verifyNotInPendingDelete;
 import static google.registry.model.common.FeatureFlag.FeatureName.MINIMUM_DATASET_CONTACTS_OPTIONAL;
@@ -186,7 +186,7 @@ public final class DomainUpdateFlow implements MutatingFlow {
     Domain newDomain = performUpdate(command, existingDomain, now);
     DomainHistory domainHistory =
         historyBuilder.setType(DOMAIN_UPDATE).setDomain(newDomain).build();
-    validateNewState(newDomain);
+    validateNewState(existingDomain, newDomain);
     if (requiresDnsUpdate(existingDomain, newDomain)) {
       requestDomainDnsRefresh(targetId);
     }
@@ -328,8 +328,13 @@ public final class DomainUpdateFlow implements MutatingFlow {
    * compliant with the additions or amendments, otherwise existing data can become invalid and
    * cause Domain update failure.
    */
-  private static void validateNewState(Domain newDomain) throws EppException {
-    validateContactDataPresence(newDomain.getRegistrant(), newDomain.getContacts());
+  private static void validateNewState(Domain existingDomain, Domain newDomain)
+      throws EppException {
+    validateUpdateContactData(
+        existingDomain.getRegistrant(),
+        newDomain.getRegistrant(),
+        existingDomain.getContacts(),
+        newDomain.getContacts());
     validateDsData(newDomain.getDsData());
     validateNameserversCountForTld(
         newDomain.getTld(),

core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java

@@ -346,18 +346,18 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
   }
 
   @Test
-  void testFailure_minimumDatasetPhase2_nonRegistrantContactsStillExist() throws Exception {
+  void testFailure_minimumDatasetPhase2_whenAddingNewContacts() throws Exception {
     persistResource(
         new FeatureFlag.Builder()
             .setFeatureName(MINIMUM_DATASET_CONTACTS_PROHIBITED)
             .setStatusMap(
                 ImmutableSortedMap.of(START_OF_TIME, INACTIVE, clock.nowUtc().minusDays(5), ACTIVE))
             .build());
+    // This EPP adds a new technical contact mak21 that wasn't already present.
     setEppInput("domain_update_empty_registrant.xml");
     persistReferencedEntities();
     persistDomain();
-    // Fails because after the update the domain would still have some contacts on it even though
-    // the registrant has been removed.
+    // Fails because the update adds some new contacts, although the registrant has been removed.
     ContactsProhibitedException thrown =
         assertThrows(ContactsProhibitedException.class, this::runFlow);
     assertAboutEppExceptions().that(thrown).marshalsToXml();
@@ -1574,14 +1574,13 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
   }
 
   @Test
-  void testFailure_minimumDatasetPhase2_registrantStillExists() throws Exception {
+  void testFailure_minimumDatasetPhase2_addingNewRegistrantFails() throws Exception {
     persistResource(
         new FeatureFlag.Builder()
             .setFeatureName(MINIMUM_DATASET_CONTACTS_PROHIBITED)
             .setStatusMap(
                 ImmutableSortedMap.of(START_OF_TIME, INACTIVE, clock.nowUtc().minusDays(5), ACTIVE))
             .build());
-    setEppInput("domain_update_remove_admin.xml");
     persistReferencedEntities();
     persistResource(
         DatabaseHelper.newDomain(getUniqueIdFromCommand())
@@ -1590,7 +1589,10 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
                 ImmutableSet.of(
                     DesignatedContact.create(Type.ADMIN, sh8013Contact.createVKey()),
                     DesignatedContact.create(Type.TECH, sh8013Contact.createVKey())))
+            .setRegistrant(Optional.empty())
             .build());
+    // This EPP sets the registrant to sh8013, whereas in our test setup it is absent.
+    setEppInput("domain_update_registrant.xml");
     RegistrantProhibitedException thrown =
         assertThrows(RegistrantProhibitedException.class, this::runFlow);
     assertAboutEppExceptions().that(thrown).marshalsToXml();
@@ -1657,6 +1659,32 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
     assertThat(updatedDomain.getContacts()).isEmpty();
   }
 
+  @Test
+  void testSuccess_minimumDatasetPhase2_removeOneContact() throws Exception {
+    persistResource(
+        new FeatureFlag.Builder()
+            .setFeatureName(MINIMUM_DATASET_CONTACTS_PROHIBITED)
+            .setStatusMap(
+                ImmutableSortedMap.of(START_OF_TIME, INACTIVE, clock.nowUtc().minusDays(5), ACTIVE))
+            .build());
+    setEppInput("domain_update_remove_admin.xml");
+    persistReferencedEntities();
+    persistResource(
+        DatabaseHelper.newDomain(getUniqueIdFromCommand())
+            .asBuilder()
+            .setContacts(
+                ImmutableSet.of(
+                    DesignatedContact.create(Type.ADMIN, sh8013Contact.createVKey()),
+                    DesignatedContact.create(Type.TECH, sh8013Contact.createVKey())))
+            .build());
+    assertThat(reloadResourceByForeignKey().getRegistrant()).isPresent();
+    assertThat(reloadResourceByForeignKey().getContacts()).hasSize(2);
+    runFlowAssertResponse(loadFile("generic_success_response.xml"));
+    Domain updatedDomain = reloadResourceByForeignKey();
+    assertThat(updatedDomain.getRegistrant()).isPresent();
+    assertThat(updatedDomain.getContacts()).hasSize(1);
+  }
+
   @Test
   void testFailure_addPendingDeleteContact() throws Exception {
     persistReferencedEntities();

db/build.gradle

@@ -23,7 +23,7 @@ buildscript {
 }
 
 plugins {
-  id "org.flywaydb.flyway" version "9.22.3"
+  id "org.flywaydb.flyway" version "11.0.1"
   id 'maven-publish'
 }
 

gradle.properties

@@ -1,4 +1,4 @@
-f# This file defines properties used by the gradle build. It must be kept in
+# This file defines properties used by the gradle build. It must be kept in
 # sync with config/nom_build.py.
 #
 # To regenerate, run ./nom_build --generate-gradle-properties