Make the contacts prohibited feature flag for min reg data set more lenient

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

@@ -481,10 +481,10 @@ static void validateNoDuplicateContacts(Set<DesignatedContact> contacts)
   }
 
   /**
-   * 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 @@ static void validateContactDataPresence(
     }
   }
 
+  /**
+   * 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 @@ static void validateCreateCommandContactsAndNameservers(
     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,14 +30,14 @@
 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;
 import static google.registry.flows.domain.DomainFlowUtils.validateNameserversAllowedOnTld;
 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 EppResponse run() throws EppException {
     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 @@ private Optional<VKey<Contact>> determineUpdatedRegistrant(Change change, Domain
    * 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 @@ void testSuccess_minimumDatasetPhase1_emptyRegistrant() throws Exception {
   }
 
   @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 @@ void testSuccess_minimumDatasetPhase1_removeAdmin() throws Exception {
   }
 
   @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 @@ void testFailure_minimumDatasetPhase2_registrantStillExists() throws Exception {
                 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 @@ void testSuccess_minimumDatasetPhase2_removeAllContacts() throws Exception {
     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();