Add lastUpdateTime column to epp resources (#683)

* Add lastUpdateTime column to epp resources

Property was inadvertently left out.

Renamed getter and setter to match the property name.

Added a test helper to compare EppResources while ignoring
lastUpdateTime, which changes every time an instance is persisted.

core/src/main/java/google/registry/batch/DeleteContactsAndHostsAction.java

@@ -532,7 +532,7 @@ public class DeleteContactsAndHostsAction implements Runnable {
           resource.getClass().getSimpleName());
       return new AutoValue_DeleteContactsAndHostsAction_DeletionRequest.Builder()
           .setKey(resourceKey)
-          .setLastUpdateTime(resource.getUpdateAutoTimestamp().getTimestamp())
+          .setLastUpdateTime(resource.getUpdateTimestamp().getTimestamp())
           .setRequestingClientId(
               checkNotNull(
                   params.get(PARAM_REQUESTING_CLIENT_ID), "Requesting client id not specified"))

core/src/main/java/google/registry/batch/RefreshDnsOnHostRenameAction.java

@@ -319,13 +319,13 @@ public class RefreshDnsOnHostRenameAction implements Runnable {
       HostResource host =
           checkNotNull(ofy().load().key(hostKey).now(), "Host to refresh doesn't exist");
       boolean isHostDeleted =
-          isDeleted(host, latestOf(now, host.getUpdateAutoTimestamp().getTimestamp()));
+          isDeleted(host, latestOf(now, host.getUpdateTimestamp().getTimestamp()));
       if (isHostDeleted) {
         logger.atInfo().log("Host %s is already deleted, not refreshing DNS.", hostKey);
       }
       return new AutoValue_RefreshDnsOnHostRenameAction_DnsRefreshRequest.Builder()
           .setHostKey(hostKey)
-          .setLastUpdateTime(host.getUpdateAutoTimestamp().getTimestamp())
+          .setLastUpdateTime(host.getUpdateTimestamp().getTimestamp())
           .setRequestedTime(
               DateTime.parse(
                   checkNotNull(params.get(PARAM_REQUESTED_TIME), "Requested time not specified")))

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

@@ -14,16 +14,21 @@
 
 package google.registry.model;
 
+import com.google.common.annotations.VisibleForTesting;
+import javax.persistence.Access;
+import javax.persistence.AccessType;
+import javax.persistence.MappedSuperclass;
 import javax.xml.bind.annotation.XmlTransient;
 
 /**
  * Base class for entities that are the root of a Registry 2.0 entity group that gets enrolled in
  * commit logs for backup purposes.
  *
- * <p>The commit log system needs to preserve the ordering of closely timed mutations to entities
+ * <p>The commit log system needs to preserve the ordering of closely timed mutations to entities in
- * in a single entity group. We require an {@link UpdateAutoTimestamp} field on the root of a group
+ * a single entity group. We require an {@link UpdateAutoTimestamp} field on the root of a group so
- * so that we can enforce strictly increasing timestamps.
+ * that we can enforce strictly increasing timestamps.
  */
+@MappedSuperclass
 public abstract class BackupGroupRoot extends ImmutableObject {
   /**
    * An automatically managed timestamp of when this object was last written to Datastore.
@@ -32,10 +37,14 @@ public abstract class BackupGroupRoot extends ImmutableObject {
    * that this is updated on every save, rather than only in response to an {@code <update>} command
    */
   @XmlTransient
+  // Prevents subclasses from unexpectedly accessing as property (e.g., HostResource), which would
+  // require an unnecessary non-private setter method.
+  @Access(AccessType.FIELD)
+  @VisibleForTesting
   UpdateAutoTimestamp updateTimestamp = UpdateAutoTimestamp.create(null);
 
   /** Get the {@link UpdateAutoTimestamp} for this entity. */
-  public final UpdateAutoTimestamp getUpdateAutoTimestamp() {
+  public UpdateAutoTimestamp getUpdateTimestamp() {
     return updateTimestamp;
   }
 }

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

@@ -155,7 +155,7 @@ public final class EppResourceUtils {
     // time for writes.
     return Optional.of(
         cloneProjectedAtTime(
-            resource, latestOf(now, resource.getUpdateAutoTimestamp().getTimestamp())));
+            resource, latestOf(now, resource.getUpdateTimestamp().getTimestamp())));
   }
 
   /**
@@ -298,7 +298,7 @@ public final class EppResourceUtils {
     // and returns it projected forward to exactly the desired timestamp, or null if the resource is
     // deleted at that timestamp.
     final Result<T> loadResult =
-        isAtOrAfter(timestamp, resource.getUpdateAutoTimestamp().getTimestamp())
+        isAtOrAfter(timestamp, resource.getUpdateTimestamp().getTimestamp())
             ? new ResultNow<>(resource)
             : loadMostRecentRevisionAtTime(resource, timestamp);
     return () -> {

core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java

@@ -168,7 +168,7 @@ class CommitLoggedWork<R> implements Runnable {
       DateTime transactionTime, Set<Entry<Key<BackupGroupRoot>, BackupGroupRoot>> bgrEntries) {
     ImmutableMap.Builder<Key<BackupGroupRoot>, DateTime> builder = new ImmutableMap.Builder<>();
     for (Entry<Key<BackupGroupRoot>, BackupGroupRoot> entry : bgrEntries) {
-      DateTime updateTime = entry.getValue().getUpdateAutoTimestamp().getTimestamp();
+      DateTime updateTime = entry.getValue().getUpdateTimestamp().getTimestamp();
       if (!updateTime.isBefore(transactionTime)) {
         builder.put(entry.getKey(), updateTime);
       }

core/src/test/java/google/registry/batch/ResaveAllEppResourcesActionTest.java

@@ -50,12 +50,12 @@ public class ResaveAllEppResourcesActionTest
   @Test
   public void test_mapreduceSuccessfullyResavesEntity() throws Exception {
     ContactResource contact = persistActiveContact("test123");
-    DateTime creationTime = contact.getUpdateAutoTimestamp().getTimestamp();
+    DateTime creationTime = contact.getUpdateTimestamp().getTimestamp();
-    assertThat(ofy().load().entity(contact).now().getUpdateAutoTimestamp().getTimestamp())
+    assertThat(ofy().load().entity(contact).now().getUpdateTimestamp().getTimestamp())
         .isEqualTo(creationTime);
     ofy().clearSessionCache();
     runMapreduce();
-    assertThat(ofy().load().entity(contact).now().getUpdateAutoTimestamp().getTimestamp())
+    assertThat(ofy().load().entity(contact).now().getUpdateTimestamp().getTimestamp())
         .isGreaterThan(creationTime);
   }
 

core/src/test/java/google/registry/model/EppResourceTestUtils.java

@@ -0,0 +1,35 @@
+// Copyright 2020 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package google.registry.model;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import java.util.Objects;
+
+/** Test helpers for {@link EppResource}. */
+public final class EppResourceTestUtils {
+
+  private EppResourceTestUtils() {}
+
+  public static <E extends EppResource> void assertEqualsIgnoreLastUpdateTime(
+      E actual, E expected) {
+    if (Objects.equals(actual, expected)) {
+      return;
+    }
+    actual = (E) actual.asBuilder().build();
+    actual.updateTimestamp = expected.getUpdateTimestamp();
+    assertThat(actual).isEqualTo(expected);
+  }
+}

core/src/test/java/google/registry/model/EppResourceUtilsTest.java

@@ -166,7 +166,9 @@ public class EppResourceUtilsTest {
     // Even though there is no revision, make a best effort guess to use the oldest revision.
     assertThat(
             loadAtPointInTime(host, clock.nowUtc().minus(Duration.standardDays(32)))
-          .now().getUpdateAutoTimestamp().getTimestamp())
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
         .isEqualTo(host.getRevisions().firstKey());
   }
 }

core/src/test/java/google/registry/model/contact/ContactResourceTest.java

@@ -16,6 +16,7 @@ package google.registry.model.contact;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth8.assertThat;
+import static google.registry.model.EppResourceTestUtils.assertEqualsIgnoreLastUpdateTime;
 import static google.registry.model.EppResourceUtils.loadByForeignKey;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.testing.ContactResourceSubject.assertAboutContacts;
@@ -154,7 +155,7 @@ public class ContactResourceTest extends EntityTestCase {
                     .setServerApproveEntities(null)
                     .build())
             .build();
-    assertThat(persisted).isEqualTo(fixed);
+    assertEqualsIgnoreLastUpdateTime(persisted, fixed);
   }
 
   @Test

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

@@ -14,7 +14,7 @@
 
 package google.registry.model.domain;
 
-import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.EppResourceTestUtils.assertEqualsIgnoreLastUpdateTime;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 import static google.registry.testing.SqlHelper.assertThrowForeignKeyViolation;
 import static google.registry.testing.SqlHelper.saveRegistrar;
@@ -154,7 +154,7 @@ public class DomainBaseSqlTest {
               DomainBase org = domain.asBuilder().setCreationTime(result.getCreationTime()).build();
 
               // Note that the equality comparison forces a lazy load of all fields.
-              assertThat(result).isEqualTo(org);
+              assertEqualsIgnoreLastUpdateTime(result, org);
             });
   }
 

core/src/test/java/google/registry/model/ofy/OfyCommitLogTest.java

@@ -247,8 +247,14 @@ public class OfyCommitLogTest {
   void testSavingRootAndChild_updatesTimestampOnBackupGroupRoot() {
     tm().transact(() -> ofy().save().entity(Root.create(1, getCrossTldKey())));
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
     clock.advanceOneMilli();
     tm()
         .transact(
@@ -257,43 +263,79 @@ public class OfyCommitLogTest {
               ofy().save().entity(new Child());
             });
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
   }
 
   @Test
   void testSavingOnlyChild_updatesTimestampOnBackupGroupRoot() {
     tm().transact(() -> ofy().save().entity(Root.create(1, getCrossTldKey())));
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
     clock.advanceOneMilli();
     tm().transact(() -> ofy().save().entity(new Child()));
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
   }
 
   @Test
   void testDeletingChild_updatesTimestampOnBackupGroupRoot() {
     tm().transact(() -> ofy().save().entity(Root.create(1, getCrossTldKey())));
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
     clock.advanceOneMilli();
     // The fact that the child was never persisted is irrelevant.
     tm().transact(() -> ofy().delete().entity(new Child()));
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
   }
 
   @Test
   void testReadingRoot_doesntUpdateTimestamp() {
     tm().transact(() -> ofy().save().entity(Root.create(1, getCrossTldKey())));
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
     clock.advanceOneMilli();
     tm()
         .transact(
@@ -304,16 +346,28 @@ public class OfyCommitLogTest {
               ofy().load().entity(Root.create(1, getCrossTldKey()));
             });
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc().minusMillis(1));
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc().minusMillis(1));
   }
 
   @Test
   void testReadingChild_doesntUpdateTimestampOnBackupGroupRoot() {
     tm().transact(() -> ofy().save().entity(Root.create(1, getCrossTldKey())));
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
     clock.advanceOneMilli();
     tm()
         .transact(
@@ -324,8 +378,14 @@ public class OfyCommitLogTest {
               ofy().load().entity(new Child()); // All Child objects are under Root(1).
             });
     ofy().clearSessionCache();
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc().minusMillis(1));
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc().minusMillis(1));
   }
 
   @Test
@@ -340,8 +400,14 @@ public class OfyCommitLogTest {
             });
     ofy().clearSessionCache();
     for (int i = 1; i <= 3; i++) {
-      assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, i)).now()
+      assertThat(
-          .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+              ofy()
+                  .load()
+                  .key(Key.create(getCrossTldKey(), Root.class, i))
+                  .now()
+                  .getUpdateTimestamp()
+                  .getTimestamp())
+          .isEqualTo(clock.nowUtc());
     }
     clock.advanceOneMilli();
     // Mutate one root, and a child of a second, ignoring the third.
@@ -353,14 +419,32 @@ public class OfyCommitLogTest {
             });
     ofy().clearSessionCache();
     // Child was touched.
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 1)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 1))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
     // Directly touched.
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 2)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc());
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 2))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc());
     // Wasn't touched.
-    assertThat(ofy().load().key(Key.create(getCrossTldKey(), Root.class, 3)).now()
+    assertThat(
-        .getUpdateAutoTimestamp().getTimestamp()).isEqualTo(clock.nowUtc().minusMillis(1));
+            ofy()
+                .load()
+                .key(Key.create(getCrossTldKey(), Root.class, 3))
+                .now()
+                .getUpdateTimestamp()
+                .getTimestamp())
+        .isEqualTo(clock.nowUtc().minusMillis(1));
   }
 
   @Entity

core/src/test/java/google/registry/model/ofy/OfyTest.java

@@ -79,8 +79,8 @@ public class OfyTest {
   }
 
   private void doBackupGroupRootTimestampInversionTest(Runnable runnable) {
-    DateTime groupTimestamp = ofy().load().key(someObject.getParent()).now()
+    DateTime groupTimestamp =
-        .getUpdateAutoTimestamp().getTimestamp();
+        ofy().load().key(someObject.getParent()).now().getUpdateTimestamp().getTimestamp();
     // Set the clock in Ofy to the same time as the backup group root's save time.
     Ofy ofy = new Ofy(new FakeClock(groupTimestamp));
     TimestampInversionException thrown =

db/src/main/resources/sql/flyway/V39__add_updatetime_column.sql

@@ -0,0 +1,23 @@
+-- Copyright 2020 The Nomulus Authors. All Rights Reserved.
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+ALTER TABLE "Contact" ADD COLUMN update_timestamp timestamptz;
+
+ALTER TABLE "ContactHistory" ADD COLUMN update_timestamp timestamptz;
+
+ALTER TABLE "Domain" ADD COLUMN update_timestamp timestamptz;
+
+ALTER TABLE "HostHistory" ADD COLUMN update_timestamp timestamptz;
+
+ALTER TABLE "HostResource" ADD COLUMN update_timestamp timestamptz;

db/src/main/resources/sql/schema/db-schema.sql.generated

@@ -77,6 +77,7 @@ create sequence history_id_sequence start 1 increment 1;
 
     create table "Contact" (
        repo_id text not null,
+        update_timestamp timestamptz,
         creation_registrar_id text not null,
         creation_time timestamptz not null,
         current_sponsor_registrar_id text not null,
@@ -197,6 +198,7 @@ create sequence history_id_sequence start 1 increment 1;
         last_epp_update_registrar_id text,
         last_epp_update_time timestamptz,
         statuses text[],
+        update_timestamp timestamptz,
         contact_repo_id text not null,
         primary key (history_revision_id)
     );
@@ -219,6 +221,7 @@ create sequence history_id_sequence start 1 increment 1;
 
     create table "Domain" (
        repo_id text not null,
+        update_timestamp timestamptz,
         creation_registrar_id text not null,
         creation_time timestamptz not null,
         current_sponsor_registrar_id text not null,
@@ -300,12 +303,14 @@ create sequence history_id_sequence start 1 increment 1;
         last_epp_update_registrar_id text,
         last_epp_update_time timestamptz,
         statuses text[],
+        update_timestamp timestamptz,
         host_repo_id text not null,
         primary key (history_revision_id)
     );
 
     create table "HostResource" (
        repo_id text not null,
+        update_timestamp timestamptz,
         creation_registrar_id text not null,
         creation_time timestamptz not null,
         current_sponsor_registrar_id text not null,

db/src/main/resources/sql/schema/nomulus.golden.sql

@@ -250,7 +250,8 @@ CREATE TABLE public."Contact" (
     transfer_losing_registrar_id text,
     transfer_pending_expiration_time timestamp with time zone,
     transfer_request_time timestamp with time zone,
-    transfer_status text
+    transfer_status text,
+    update_timestamp timestamp with time zone
 );
 
 
@@ -334,7 +335,8 @@ CREATE TABLE public."ContactHistory" (
     last_epp_update_registrar_id text,
     last_epp_update_time timestamp with time zone,
     statuses text[],
-    contact_repo_id text NOT NULL
+    contact_repo_id text NOT NULL,
+    update_timestamp timestamp with time zone
 );
 
 
@@ -395,7 +397,8 @@ CREATE TABLE public."Domain" (
     transfer_losing_registrar_id text,
     transfer_pending_expiration_time timestamp with time zone,
     transfer_request_time timestamp with time zone,
-    transfer_status text
+    transfer_status text,
+    update_timestamp timestamp with time zone
 );
 
 
@@ -436,7 +439,8 @@ CREATE TABLE public."HostHistory" (
     last_epp_update_registrar_id text,
     last_epp_update_time timestamp with time zone,
     statuses text[],
-    host_repo_id text NOT NULL
+    host_repo_id text NOT NULL,
+    update_timestamp timestamp with time zone
 );
 
 
@@ -457,7 +461,8 @@ CREATE TABLE public."HostResource" (
     last_superordinate_change timestamp with time zone,
     last_transfer_time timestamp with time zone,
     superordinate_domain text,
-    inet_addresses text[]
+    inet_addresses text[],
+    update_timestamp timestamp with time zone
 );