From fe3472d31d0e73dbafdb3c3afa1d6a5086969ba3 Mon Sep 17 00:00:00 2001
From: Pavlo Tkach <3469726+ptkach@users.noreply.github.com>
Date: Wed, 5 Oct 2022 14:13:52 -0400
Subject: [PATCH] Revert "Do not enqueue DNS updates when flow doesn't affect
 nameservers (#1785)" (#1808)

This reverts commit 775f672f2a1235832996ee8c2c98da7edd99a043.
---
 .../flows/domain/DomainUpdateFlow.java         | 16 +++-------------
 .../flows/domain/DomainUpdateFlowTest.java     | 18 +++++++-----------
 .../domain/domain_update_no_ds_change.xml      | 18 ------------------
 3 files changed, 10 insertions(+), 42 deletions(-)
 delete mode 100644 core/src/test/resources/google/registry/flows/domain/domain_update_no_ds_change.xml

diff --git a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java
index 95f709cc3..af92e2416 100644
--- a/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java
+++ b/core/src/main/java/google/registry/flows/domain/DomainUpdateFlow.java
@@ -87,7 +87,6 @@ import google.registry.model.poll.PendingActionNotificationResponse.DomainPendin
 import google.registry.model.poll.PollMessage;
 import google.registry.model.reporting.IcannReportingTypes.ActivityReportField;
 import google.registry.model.tld.Registry;
-import java.util.Objects;
 import java.util.Optional;
 import javax.inject.Inject;
 import org.joda.time.DateTime;
@@ -182,10 +181,7 @@ public final class DomainUpdateFlow implements TransactionalFlow {
     DomainHistory domainHistory =
         historyBuilder.setType(DOMAIN_UPDATE).setDomain(newDomain).build();
     validateNewState(newDomain);
-    if (!Objects.equals(newDomain.getDsData(), existingDomain.getDsData())
-        || !Objects.equals(newDomain.getNsHosts(), existingDomain.getNsHosts())) {
-      dnsQueue.addDomainRefreshTask(targetId);
-    }
+    dnsQueue.addDomainRefreshTask(targetId);
     ImmutableSet.Builder<ImmutableObject> entitiesToSave = new ImmutableSet.Builder<>();
     entitiesToSave.add(newDomain, domainHistory);
     Optional<BillingEvent.OneTime> statusUpdateBillingEvent =
@@ -271,18 +267,12 @@ public final class DomainUpdateFlow implements TransactionalFlow {
             .setLastEppUpdateRegistrarId(registrarId)
             .addStatusValues(add.getStatusValues())
             .removeStatusValues(remove.getStatusValues())
+            .addNameservers(add.getNameservers().stream().collect(toImmutableSet()))
+            .removeNameservers(remove.getNameservers().stream().collect(toImmutableSet()))
             .removeContacts(remove.getContacts())
             .addContacts(add.getContacts())
             .setRegistrant(firstNonNull(change.getRegistrant(), domain.getRegistrant()))
             .setAuthInfo(firstNonNull(change.getAuthInfo(), domain.getAuthInfo()));
-
-    if (!add.getNameservers().isEmpty()) {
-      domainBuilder.addNameservers(add.getNameservers().stream().collect(toImmutableSet()));
-    }
-    if (!remove.getNameservers().isEmpty()) {
-      domainBuilder.removeNameservers(remove.getNameservers().stream().collect(toImmutableSet()));
-    }
-
     Optional<DomainUpdateSuperuserExtension> superuserExt =
         eppInput.getSingleExtension(DomainUpdateSuperuserExtension.class);
     if (superuserExt.isPresent()) {
diff --git a/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java
index 7fefa2ae8..232796280 100644
--- a/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java
+++ b/core/src/test/java/google/registry/flows/domain/DomainUpdateFlowTest.java
@@ -47,7 +47,6 @@ import static google.registry.testing.DomainSubject.assertAboutDomains;
 import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
 import static google.registry.testing.HistoryEntrySubject.assertAboutHistoryEntries;
 import static google.registry.testing.TaskQueueHelper.assertDnsTasksEnqueued;
-import static google.registry.testing.TaskQueueHelper.assertNoDnsTasksEnqueued;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static org.joda.money.CurrencyUnit.USD;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -497,11 +496,13 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
             expectedDsData.stream()
                 .map(ds -> ds.cloneWithDomainRepoId(resource.getRepoId()))
                 .collect(toImmutableSet()));
-    if (dnsTaskEnqueued) {
-      assertDnsTasksEnqueued("example.tld");
-    } else {
-      assertNoDnsTasksEnqueued();
-    }
+
+    // TODO: REENABLE AFTER PROPER FIX FOR DNS PUBLISHING TASKS IS FOUND
+    // if (dnsTaskEnqueued) {
+    //   assertDnsTasksEnqueued("example.tld");
+    // } else {
+    //   assertNoDnsTasksEnqueued();
+    // }
   }
 
   @Test
@@ -1746,9 +1747,4 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
     assertAboutDomains().that(reloadResourceByForeignKey()).hasNoAutorenewEndTime();
   }
 
-  @Test
-  void testDnsTaskIsNotTriggeredWhenNoDSChangeSubmitted() {
-    setEppInput("domain_update_no_ds_change.xml");
-    assertNoDnsTasksEnqueued();
-  }
 }
diff --git a/core/src/test/resources/google/registry/flows/domain/domain_update_no_ds_change.xml b/core/src/test/resources/google/registry/flows/domain/domain_update_no_ds_change.xml
deleted file mode 100644
index 0981ffbc7..000000000
--- a/core/src/test/resources/google/registry/flows/domain/domain_update_no_ds_change.xml
+++ /dev/null
@@ -1,18 +0,0 @@
-<epp xmlns="urn:ietf:params:xml:ns:epp-1.0">
-  <command>
-    <update>
-      <domain:update
-       xmlns:domain="urn:ietf:params:xml:ns:domain-1.0">
-        <domain:name>example.tld</domain:name>
-        <domain:add>
-          <domain:contact type="admin">mak21</domain:contact>
-          <domain:contact type="billing">mak21</domain:contact>
-          <domain:contact type="tech">mak21</domain:contact>
-          <domain:status s="serverHold"
-              lang="en">Server hold.</domain:status>
-        </domain:add>
-      </domain:update>
-    </update>
-    <clTRID>ABC-12345</clTRID>
-  </command>
-</epp>