Revert "Do not enqueue DNS updates when flow doesn't affect nameservers (#1785)" (#1808)

This reverts commit 775f672f2a.

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()) {

core/src/main/java/google/registry/tools/GetEppResourceCommand.java

@@ -15,30 +15,31 @@
 package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
 import google.registry.model.EppResource;
+import google.registry.util.Clock;
 import java.util.Optional;
+import javax.inject.Inject;
 import org.joda.time.DateTime;
 
 /** Abstract command to print one or more resources to stdout. */
 @Parameters(separators = " =")
 abstract class GetEppResourceCommand implements CommandWithRemoteApi {
 
-  private final DateTime now = DateTime.now(UTC);
-
   @Parameter(
       names = "--read_timestamp",
       description = "Timestamp to use when reading. May not be in the past.")
-  protected DateTime readTimestamp = now;
+  protected DateTime readTimestamp;
 
   @Parameter(
       names = "--expand",
       description = "Fully expand the requested resource. NOTE: Output may be lengthy.")
   boolean expand;
 
+  @Inject Clock clock;
+
   /** Runs the command's own logic that calls {@link #printResource}. */
   abstract void runAndPrint();
 
@@ -59,7 +60,11 @@ abstract class GetEppResourceCommand implements CommandWithRemoteApi {
 
   @Override
   public void run() {
-    checkArgument(!readTimestamp.isBefore(now), "--read_timestamp may not be in the past");
+    if (readTimestamp == null) {
+      readTimestamp = clock.nowUtc();
+    }
+    checkArgument(
+        !readTimestamp.isBefore(clock.nowUtc()), "--read_timestamp may not be in the past");
     runAndPrint();
   }
 }

core/src/main/java/google/registry/tools/UniformRapidSuspensionCommand.java

@@ -21,7 +21,6 @@ import static google.registry.model.EppResourceUtils.checkResourcesExist;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
-import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
@@ -38,11 +37,13 @@ import google.registry.model.eppcommon.StatusValue;
 import google.registry.model.host.Host;
 import google.registry.tools.soy.DomainRenewSoyInfo;
 import google.registry.tools.soy.UniformRapidSuspensionSoyInfo;
+import google.registry.util.Clock;
 import google.registry.util.DomainNameUtils;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import javax.inject.Inject;
 import javax.xml.bind.annotation.adapters.HexBinaryAdapter;
 import org.joda.time.DateTime;
 import org.joda.time.format.DateTimeFormat;
@@ -119,10 +120,12 @@ final class UniformRapidSuspensionCommand extends MutatingEppToolCommand {
   /** Set of status values to remove. */
   ImmutableSet<String> removeStatuses;
 
+  @Inject Clock clock;
+
   @Override
   protected void initMutatingEppToolCommand() {
     superuser = true;
-    DateTime now = DateTime.now(UTC);
+    DateTime now = clock.nowUtc();
     ImmutableList<String> newCanonicalHosts =
         newHosts.stream().map(DomainNameUtils::canonicalizeHostname).collect(toImmutableList());
     ImmutableSet<String> newHostsSet = ImmutableSet.copyOf(newCanonicalHosts);

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();
-  }
 }

core/src/test/java/google/registry/tools/CommandTestCase.java

@@ -19,7 +19,6 @@ import static com.google.common.collect.Iterables.toArray;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.joda.time.DateTimeZone.UTC;
 
 import com.beust.jcommander.JCommander;
 import com.google.common.base.Joiner;
@@ -63,7 +62,7 @@ public abstract class CommandTestCase<C extends Command> {
 
   protected C command;
 
-  protected final FakeClock fakeClock = new FakeClock(DateTime.now(UTC));
+  protected final FakeClock fakeClock = new FakeClock(DateTime.parse("2022-09-01T00:00:00.000Z"));
 
   @RegisterExtension
   public final AppEngineExtension appEngine =

core/src/test/java/google/registry/tools/GetContactCommandTest.java

@@ -19,22 +19,19 @@ import static google.registry.testing.DatabaseHelper.newContact;
 import static google.registry.testing.DatabaseHelper.persistActiveContact;
 import static google.registry.testing.DatabaseHelper.persistDeletedContact;
 import static google.registry.testing.DatabaseHelper.persistResource;
-import static org.joda.time.DateTimeZone.UTC;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.beust.jcommander.ParameterException;
-import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link GetContactCommand}. */
 class GetContactCommandTest extends CommandTestCase<GetContactCommand> {
 
-  private DateTime now = DateTime.now(UTC);
-
   @BeforeEach
   void beforeEach() {
     createTld("tld");
+    command.clock = fakeClock;
   }
 
   @Test
@@ -67,7 +64,7 @@ class GetContactCommandTest extends CommandTestCase<GetContactCommand> {
 
   @Test
   void testSuccess_deletedContact() throws Exception {
-    persistDeletedContact("sh8013", now.minusDays(1));
+    persistDeletedContact("sh8013", fakeClock.nowUtc().minusDays(1));
     runCommand("sh8013");
     assertInStdout("Contact 'sh8013' does not exist or is deleted");
   }
@@ -85,8 +82,9 @@ class GetContactCommandTest extends CommandTestCase<GetContactCommand> {
 
   @Test
   void testSuccess_contactDeletedInFuture() throws Exception {
-    persistResource(newContact("sh8013").asBuilder().setDeletionTime(now.plusDays(1)).build());
-    runCommand("sh8013", "--read_timestamp=" + now.plusMonths(1));
+    persistResource(
+        newContact("sh8013").asBuilder().setDeletionTime(fakeClock.nowUtc().plusDays(1)).build());
+    runCommand("sh8013", "--read_timestamp=" + fakeClock.nowUtc().plusMonths(1));
     assertInStdout("Contact 'sh8013' does not exist or is deleted");
   }
 }

core/src/test/java/google/registry/tools/GetDomainCommandTest.java

@@ -31,6 +31,7 @@ class GetDomainCommandTest extends CommandTestCase<GetDomainCommand> {
   @BeforeEach
   void beforeEach() {
     createTld("tld");
+    command.clock = fakeClock;
   }
 
   @Test

core/src/test/java/google/registry/tools/GetHostCommandTest.java

@@ -19,22 +19,19 @@ import static google.registry.testing.DatabaseHelper.newHost;
 import static google.registry.testing.DatabaseHelper.persistActiveHost;
 import static google.registry.testing.DatabaseHelper.persistDeletedHost;
 import static google.registry.testing.DatabaseHelper.persistResource;
-import static org.joda.time.DateTimeZone.UTC;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.beust.jcommander.ParameterException;
-import org.joda.time.DateTime;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 /** Unit tests for {@link GetHostCommand}. */
 class GetHostCommandTest extends CommandTestCase<GetHostCommand> {
 
-  private DateTime now = DateTime.now(UTC);
-
   @BeforeEach
   void beforeEach() {
     createTld("tld");
+    command.clock = fakeClock;
   }
 
   @Test
@@ -77,7 +74,7 @@ class GetHostCommandTest extends CommandTestCase<GetHostCommand> {
 
   @Test
   void testSuccess_deletedHost() throws Exception {
-    persistDeletedHost("ns1.example.tld", now.minusDays(1));
+    persistDeletedHost("ns1.example.tld", fakeClock.nowUtc().minusDays(1));
     runCommand("ns1.example.tld");
     assertInStdout("Host 'ns1.example.tld' does not exist or is deleted");
   }
@@ -91,8 +88,11 @@ class GetHostCommandTest extends CommandTestCase<GetHostCommand> {
   @Test
   void testSuccess_hostDeletedInFuture() throws Exception {
     persistResource(
-        newHost("ns1.example.tld").asBuilder().setDeletionTime(now.plusDays(1)).build());
-    runCommand("ns1.example.tld", "--read_timestamp=" + now.plusMonths(1));
+        newHost("ns1.example.tld")
+            .asBuilder()
+            .setDeletionTime(fakeClock.nowUtc().plusDays(1))
+            .build());
+    runCommand("ns1.example.tld", "--read_timestamp=" + fakeClock.nowUtc().plusMonths(1));
     assertInStdout("Host 'ns1.example.tld' does not exist or is deleted");
   }
 

core/src/test/java/google/registry/tools/UniformRapidSuspensionCommandTest.java

@@ -48,6 +48,7 @@ class UniformRapidSuspensionCommandTest
 
   @BeforeEach
   void beforeEach() {
+    command.clock = fakeClock;
     // Since the command's history client ID must be CharlestonRoad, resave TheRegistrar that way.
     persistResource(
         loadRegistrar("TheRegistrar").asBuilder().setRegistrarId("CharlestonRoad").build());

core/src/test/resources/google/registry/flows/domain/domain_update_no_ds_change.xml

@@ -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>