Fix update timestamps for DomainContent types (#1517)

* Fix update timestamps for DomainContent types

We expect update timestamps to be updated whenever a containing entity is
modified and persisted, but unfortunately Hibernate doesn't seem to do this --
instead it appears to regard such an entity as unchanged.

To work around this, we explicitly reset the update timestamp whenever a
nested collection is modified in the Builder.

Note that this change only solves the problem for DomainContent.  All other
entitities containing UpdateAutoTimestamp will need to be audited and
instrumented with a similar change.

* Fix a handful of tests broken by this change

* Reformatted.

core/src/main/java/google/registry/model/BackupGroupRoot.java

@@ -60,4 +60,25 @@ public abstract class BackupGroupRoot extends ImmutableObject implements UnsafeS
   protected void copyUpdateTimestamp(BackupGroupRoot other) {
     this.updateTimestamp = PreconditionsUtils.checkArgumentNotNull(other, "other").updateTimestamp;
   }
+
+  /**
+   * Resets the {@link #updateTimestamp} to force Hibernate to persist it.
+   *
+   * <p>This method is for use in setters in derived builders that do not result in the derived
+   * object being persisted.
+   */
+  protected void resetUpdateTimestamp() {
+    this.updateTimestamp = UpdateAutoTimestamp.create(null);
+  }
+
+  /**
+   * Sets the {@link #updateTimestamp}.
+   *
+   * <p>This method is for use in the few places where we need to restore the update timestamp after
+   * mutating a collection in order to force the new timestamp to be persisted when it ordinarily
+   * wouldn't.
+   */
+  protected void setUpdateTimestamp(UpdateAutoTimestamp timestamp) {
+    updateTimestamp = timestamp;
+  }
 }

core/src/main/java/google/registry/model/EppResource.java

@@ -362,6 +362,16 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
       return thisCastToDerived();
     }
 
+    /**
+     * Set the update timestamp.
+     *
+     * <p>This is provided at EppResource since BackupGroupRoot doesn't have a Builder.
+     */
+    public B setUpdateTimestamp(UpdateAutoTimestamp updateTimestamp) {
+      getInstance().setUpdateTimestamp(updateTimestamp);
+      return thisCastToDerived();
+    }
+
     /** Build the resource, nullifying empty strings and sets and setting defaults. */
     @Override
     public T build() {

core/src/main/java/google/registry/model/bulkquery/BulkQueryEntities.java

@@ -74,6 +74,8 @@ public class BulkQueryEntities {
     builder.setGracePeriods(gracePeriods);
     builder.setDsData(delegationSignerData);
     builder.setNameservers(nsHosts);
+    // Restore the original update timestamp (this gets cleared when we set nameservers or DS data).
+    builder.setUpdateTimestamp(domainBaseLite.getUpdateTimestamp());
     return builder.build();
   }
 
@@ -100,6 +102,9 @@ public class BulkQueryEntities {
                   dsDataHistories.stream()
                       .map(DelegationSignerData::create)
                       .collect(toImmutableSet()))
+              // Restore the original update timestamp (this gets cleared when we set nameservers or
+              // DS data).
+              .setUpdateTimestamp(domainHistoryLite.domainContent.getUpdateTimestamp())
               .build();
       builder.setDomain(newDomainContent);
     }

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

@@ -895,6 +895,7 @@ public class DomainContent extends EppResource
 
     public B setDsData(ImmutableSet<DelegationSignerData> dsData) {
       getInstance().dsData = dsData;
+      getInstance().resetUpdateTimestamp();
       return thisCastToDerived();
     }
 
@@ -918,11 +919,13 @@ public class DomainContent extends EppResource
 
     public B setNameservers(VKey<HostResource> nameserver) {
       getInstance().nsHosts = ImmutableSet.of(nameserver);
+      getInstance().resetUpdateTimestamp();
       return thisCastToDerived();
     }
 
     public B setNameservers(ImmutableSet<VKey<HostResource>> nameservers) {
       getInstance().nsHosts = forceEmptyToNull(nameservers);
+      getInstance().resetUpdateTimestamp();
       return thisCastToDerived();
     }
 

core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java

@@ -195,7 +195,7 @@ public class DomainBaseUtilTest {
     domainTransformedByUtil = domainTransformedByUtil.asBuilder().build();
     assertAboutImmutableObjects()
         .that(domainTransformedByUtil)
-        .isEqualExceptFields(domainTransformedByOfy, "revisions");
+        .isEqualExceptFields(domainTransformedByOfy, "revisions", "updateTimestamp");
   }
 
   @Test
@@ -218,7 +218,7 @@ public class DomainBaseUtilTest {
     domainTransformedByUtil = domainTransformedByUtil.asBuilder().build();
     assertAboutImmutableObjects()
         .that(domainTransformedByUtil)
-        .isEqualExceptFields(domainWithoutFKeys, "revisions");
+        .isEqualExceptFields(domainWithoutFKeys, "revisions", "updateTimestamp");
   }
 
   @Test

core/src/test/java/google/registry/model/domain/DomainBaseSqlTest.java

@@ -671,4 +671,56 @@ public class DomainBaseSqlTest {
         .that(domain.getTransferData())
         .isEqualExceptFields(thatDomain.getTransferData(), "serverApproveEntities");
   }
+
+  @TestSqlOnly
+  void testUpdateTimeAfterNameserverUpdate() {
+    persistDomain();
+    DomainBase persisted = loadByKey(domain.createVKey());
+    DateTime originalUpdateTime = persisted.getUpdateTimestamp().getTimestamp();
+    fakeClock.advanceOneMilli();
+    DateTime transactionTime =
+        jpaTm()
+            .transact(
+                () -> {
+                  HostResource host2 =
+                      new HostResource.Builder()
+                          .setRepoId("host2")
+                          .setHostName("ns2.example.com")
+                          .setCreationRegistrarId("registrar1")
+                          .setPersistedCurrentSponsorRegistrarId("registrar2")
+                          .build();
+                  insertInDb(host2);
+                  domain = persisted.asBuilder().addNameserver(host2.createVKey()).build();
+                  updateInDb(domain);
+                  return jpaTm().getTransactionTime();
+                });
+    domain = loadByKey(domain.createVKey());
+    assertThat(domain.getUpdateTimestamp().getTimestamp()).isEqualTo(transactionTime);
+    assertThat(domain.getUpdateTimestamp().getTimestamp()).isNotEqualTo(originalUpdateTime);
+  }
+
+  @TestSqlOnly
+  void testUpdateTimeAfterDsDataUpdate() {
+    persistDomain();
+    DomainBase persisted = loadByKey(domain.createVKey());
+    DateTime originalUpdateTime = persisted.getUpdateTimestamp().getTimestamp();
+    fakeClock.advanceOneMilli();
+    DateTime transactionTime =
+        jpaTm()
+            .transact(
+                () -> {
+                  domain =
+                      persisted
+                          .asBuilder()
+                          .setDsData(
+                              ImmutableSet.of(
+                                  DelegationSignerData.create(1, 2, 3, new byte[] {0, 1, 2})))
+                          .build();
+                  updateInDb(domain);
+                  return jpaTm().getTransactionTime();
+                });
+    domain = loadByKey(domain.createVKey());
+    assertThat(domain.getUpdateTimestamp().getTimestamp()).isEqualTo(transactionTime);
+    assertThat(domain.getUpdateTimestamp().getTimestamp()).isNotEqualTo(originalUpdateTime);
+  }
 }