From bb7d0b6b894c56fe614adbfe02adcfd95bea7aaf Mon Sep 17 00:00:00 2001 From: Michael Muller Date: Tue, 22 Feb 2022 10:49:57 -0500 Subject: [PATCH] Do full database comparison during replay tests (#1524) * Fix entity delete replication, compare db @ replay Replay tests currently only verify that the contents of a transaction are can be successfully replicated to the other database. They do not verify that the contents of both databases are equivalent. As a result, we miss any changes omitted from the transaction (as was the case with entity deletions). This change adds a final database comparison to ReplayExtension so we can safely say that the databases are in the same state. This comparison is introduced in part as a unit test for the one-line fix for replication of an "entity delete" operation (where we delete using an entity object instead of the object's key) which so far has only affected PollMessage deletion. The fix is also included in this commit within JpaTransactionManagerImpl. * Exclude tests and entities with failing comparisons * Get all tests to pass and fix more timestamp Fix most of the unit tests that were broken by this change. - Fix timestamp updates after grace period changes in DomainContent and for TLD changes in Registrar. - Reenable full database comparison for most DomainCreateFlowTest's. - Make some test entities NonReplicated so they don't break when used with jpaTm().delete() - Diable checking of a few more entity types that are failing comparisons. - Add some formatting fixes. * Remove unnecessary "NoDatabaseCompare" I turns out that after other fixes/elisions we no longer need these for any tests in DomainCreateFlowTest. * Changes for review * Remove old "compare" flag. * Reformatted. --- .../registry/model/domain/DomainContent.java | 3 + .../registry/model/ofy/CommitLoggedWork.java | 2 +- .../registry/model/registrar/Registrar.java | 12 + .../JpaTransactionManagerImpl.java | 20 +- .../registrar/RegistrarSettingsAction.java | 7 + .../flows/domain/DomainCreateFlowTest.java | 1 + .../flows/domain/DomainUpdateFlowTest.java | 4 + .../EntityCallbacksListenerTest.java | 3 +- .../JpaEntityCoverageExtension.java | 2 +- .../transaction/TransactionManagerTest.java | 3 +- .../registry/testing/ReplayExtension.java | 238 +++++++++++++----- .../RegistrarSettingsActionTest.java | 19 +- 12 files changed, 244 insertions(+), 70 deletions(-) diff --git a/core/src/main/java/google/registry/model/domain/DomainContent.java b/core/src/main/java/google/registry/model/domain/DomainContent.java index fb3748613..a038d433f 100644 --- a/core/src/main/java/google/registry/model/domain/DomainContent.java +++ b/core/src/main/java/google/registry/model/domain/DomainContent.java @@ -1035,17 +1035,20 @@ public class DomainContent extends EppResource public B setGracePeriods(ImmutableSet gracePeriods) { getInstance().gracePeriods = gracePeriods; + getInstance().resetUpdateTimestamp(); return thisCastToDerived(); } public B addGracePeriod(GracePeriod gracePeriod) { getInstance().gracePeriods = union(getInstance().getGracePeriods(), gracePeriod); + getInstance().resetUpdateTimestamp(); return thisCastToDerived(); } public B removeGracePeriod(GracePeriod gracePeriod) { getInstance().gracePeriods = CollectionUtils.difference(getInstance().getGracePeriods(), gracePeriod); + getInstance().resetUpdateTimestamp(); return thisCastToDerived(); } diff --git a/core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java b/core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java index d3efc043a..e3a1e1319 100644 --- a/core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java +++ b/core/src/main/java/google/registry/model/ofy/CommitLoggedWork.java @@ -41,7 +41,7 @@ import org.joda.time.DateTime; /** Wrapper for {@link Supplier} that associates a time with each attempt. */ @DeleteAfterMigration -class CommitLoggedWork implements Runnable { +public class CommitLoggedWork implements Runnable { private final Supplier work; private final Clock clock; diff --git a/core/src/main/java/google/registry/model/registrar/Registrar.java b/core/src/main/java/google/registry/model/registrar/Registrar.java index 75cf0dd33..b85ceceff 100644 --- a/core/src/main/java/google/registry/model/registrar/Registrar.java +++ b/core/src/main/java/google/registry/model/registrar/Registrar.java @@ -46,6 +46,7 @@ import static google.registry.util.X509Utils.loadCertificate; import static java.util.Comparator.comparing; import static java.util.function.Predicate.isEqual; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; @@ -824,6 +825,7 @@ public class Registrar extends ImmutableObject public Builder setAllowedTlds(Set allowedTlds) { getInstance().allowedTlds = ImmutableSortedSet.copyOf(assertTldsExist(allowedTlds)); + getInstance().lastUpdateTime = UpdateAutoTimestamp.create(null); return this; } @@ -991,6 +993,16 @@ public class Registrar extends ImmutableObject return this; } + /** + * This lets tests set the update timestamp in cases where setting fields resets the timestamp + * and breaks the verification that an object has not been updated since it was copied. + */ + @VisibleForTesting + public Builder setLastUpdateTime(DateTime timestamp) { + getInstance().lastUpdateTime = UpdateAutoTimestamp.create(timestamp); + return this; + } + /** Build the registrar, nullifying empty fields. */ @Override public Registrar build() { diff --git a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java index 6463a90d5..24c39d537 100644 --- a/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java +++ b/core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java @@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Streams; import com.google.common.flogger.FluentLogger; +import com.googlecode.objectify.Key; import google.registry.model.ImmutableObject; import google.registry.model.common.DatabaseMigrationStateSchedule; import google.registry.model.common.DatabaseMigrationStateSchedule.ReplayDirection; @@ -604,6 +605,13 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { managedEntity = getEntityManager().merge(entity); } getEntityManager().remove(managedEntity); + + // We check shouldReplicate() in TransactionInfo.addDelete(), but we have to check it here as + // well prior to attempting to create a datastore key because a non-replicated entity may not + // have one. + if (shouldReplicate(entity.getClass())) { + transactionInfo.get().addDelete(VKey.from(Key.create(entity))); + } return managedEntity; } @@ -827,6 +835,12 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { replaySqlToDatastoreOverrideForTest.set(Optional.empty()); } + /** Returns true if the entity class should be replicated from SQL to datastore. */ + private static boolean shouldReplicate(Class entityClass) { + return !NonReplicatedEntity.class.isAssignableFrom(entityClass) + && !SqlOnlyEntity.class.isAssignableFrom(entityClass); + } + private static class TransactionInfo { ReadOnlyCheckingEntityManager entityManager; boolean inTransaction = false; @@ -883,12 +897,6 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager { } } - /** Returns true if the entity class should be replicated from SQL to datastore. */ - private boolean shouldReplicate(Class entityClass) { - return !NonReplicatedEntity.class.isAssignableFrom(entityClass) - && !SqlOnlyEntity.class.isAssignableFrom(entityClass); - } - private void recordTransaction() { if (contentsBuilder != null) { Transaction persistedTxn = contentsBuilder.build(); diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 4b962e14e..33d4cc4c5 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -453,6 +453,13 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA Map diffs = DiffUtils.deepDiff( originalRegistrar.toDiffableFieldMap(), updatedRegistrar.toDiffableFieldMap(), true); + + // It's expected that the update timestamp will be changed, as it gets reset whenever we change + // nested collections. If it's the only change, just return the original registrar. + if (diffs.keySet().equals(ImmutableSet.of("lastUpdateTime"))) { + return originalRegistrar; + } + throw new ForbiddenException( String.format("Unauthorized: only %s can change fields %s", allowedRole, diffs.keySet())); } diff --git a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java index 97744b53c..3738c7558 100644 --- a/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/core/src/test/java/google/registry/flows/domain/DomainCreateFlowTest.java @@ -553,6 +553,7 @@ class DomainCreateFlowTest extends ResourceFlowTestCase> ALL_JPA_ENTITIES = + public static final ImmutableSet> ALL_JPA_ENTITIES = PersistenceXmlUtility.getManagedClasses().stream() .filter(e -> !IGNORE_ENTITIES.contains(e.getSimpleName())) .filter(e -> e.isAnnotationPresent(Entity.class)) diff --git a/core/src/test/java/google/registry/persistence/transaction/TransactionManagerTest.java b/core/src/test/java/google/registry/persistence/transaction/TransactionManagerTest.java index 7e04ca37c..f79c4fc91 100644 --- a/core/src/test/java/google/registry/persistence/transaction/TransactionManagerTest.java +++ b/core/src/test/java/google/registry/persistence/transaction/TransactionManagerTest.java @@ -30,6 +30,7 @@ import com.googlecode.objectify.annotation.Id; import google.registry.model.ImmutableObject; import google.registry.model.ofy.DatastoreTransactionManager; import google.registry.model.ofy.Ofy; +import google.registry.model.replay.NonReplicatedEntity; import google.registry.persistence.VKey; import google.registry.persistence.transaction.TransactionManagerFactory.ReadOnlyModeException; import google.registry.testing.AppEngineExtension; @@ -448,7 +449,7 @@ public class TransactionManagerTest { @Entity(name = "TxnMgrTestEntity") @javax.persistence.Entity(name = "TestEntity") - private static class TestEntity extends TestEntityBase { + private static class TestEntity extends TestEntityBase implements NonReplicatedEntity { private String data; diff --git a/core/src/test/java/google/registry/testing/ReplayExtension.java b/core/src/test/java/google/registry/testing/ReplayExtension.java index af090a1fa..53aeb8cbb 100644 --- a/core/src/test/java/google/registry/testing/ReplayExtension.java +++ b/core/src/test/java/google/registry/testing/ReplayExtension.java @@ -14,17 +14,25 @@ package google.registry.testing; +import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; +import static google.registry.model.ofy.ObjectifyService.auditedOfy; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static org.junit.Assert.fail; import com.google.common.base.Ascii; import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Streams; +import com.google.common.flogger.FluentLogger; import com.googlecode.objectify.Key; +import google.registry.model.EntityClasses; import google.registry.model.ImmutableObject; import google.registry.model.domain.DomainBase; import google.registry.model.ofy.CommitLogBucket; @@ -34,6 +42,7 @@ import google.registry.model.replay.DatastoreEntity; import google.registry.model.replay.ReplicateToDatastoreAction; import google.registry.model.replay.SqlEntity; import google.registry.persistence.VKey; +import google.registry.persistence.transaction.JpaEntityCoverageExtension; import google.registry.persistence.transaction.JpaTransactionManagerImpl; import google.registry.persistence.transaction.Transaction; import google.registry.persistence.transaction.Transaction.Delete; @@ -42,10 +51,15 @@ import google.registry.persistence.transaction.Transaction.Update; import google.registry.persistence.transaction.TransactionEntity; import google.registry.util.RequestStatusChecker; import java.io.IOException; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Optional; import javax.annotation.Nullable; +import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.ExtensionContext; @@ -58,30 +72,74 @@ import org.mockito.Mockito; * that extension are also replayed. If AppEngineExtension is not used, * JpaTransactionManagerExtension must be, and this extension should be ordered _after_ * JpaTransactionManagerExtension so that writes to SQL work. - * - *

If the "compare" flag is set in the constructor, this will also compare all touched objects in - * both databases after performing the replay. */ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + private static ImmutableSet NON_REPLICATED_TYPES = + ImmutableSet.of( + "PremiumList", + "PremiumListRevision", + "PremiumListEntry", + "ReservedList", + "RdeRevision", + "ServerSecret", + "SignedMarkRevocationList", + "ClaimsListShard", + "TmchCrl", + "EppResourceIndex", + "ForeignKeyIndex", + "ForeignKeyHostIndex", + "ForeignKeyContactIndex", + "ForeignKeyDomainIndex"); + + // Entity classes to be ignored during the final database comparison. Note that this is just a + // mash-up of Datastore and SQL classes, and used for filtering both sets. We could split them + // out, but there is plenty of overlap and no name collisions so it doesn't matter very much. + private static ImmutableSet IGNORED_ENTITIES = + Streams.concat( + ImmutableSet.of( + // These entities *should* be comparable, but this isn't working yet so exclude + // them so we can tackle them independently. + "GracePeriod", + "GracePeriodHistory", + "HistoryEntry", + "DomainHistory", + "ContactHistory", + "HostHistory", + "DomainDsDataHistory", + "DelegationSignerData", + "DomainTransactionRecord", + + // These entities are legitimately not comparable. + "ClaimsEntry", + "ClaimsList", + "CommitLogBucket", + "CommitLogManifest", + "CommitLogMutation", + "PremiumEntry", + "ReservedListEntry") + .stream(), + NON_REPLICATED_TYPES.stream()) + .collect(toImmutableSet()); + FakeClock clock; - boolean compare; boolean replayed = false; boolean inOfyContext; InjectExtension injectExtension = new InjectExtension(); @Nullable ReplicateToDatastoreAction sqlToDsReplicator; List expectedUpdates = new ArrayList<>(); boolean enableDomainTimestampChecks; + boolean enableDatabaseCompare = true; - private ReplayExtension( - FakeClock clock, boolean compare, @Nullable ReplicateToDatastoreAction sqlToDsReplicator) { + private ReplayExtension(FakeClock clock, @Nullable ReplicateToDatastoreAction sqlToDsReplicator) { this.clock = clock; - this.compare = compare; this.sqlToDsReplicator = sqlToDsReplicator; } public static ReplayExtension createWithCompare(FakeClock clock) { - return new ReplayExtension(clock, true, null); + return new ReplayExtension(clock, null); } // This allows us to disable the replay tests from an environment variable in specific @@ -104,7 +162,6 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback { if (replayTestsEnabled()) { return new ReplayExtension( clock, - true, new ReplicateToDatastoreAction( clock, Mockito.mock(RequestStatusChecker.class), new FakeResponse())); } else { @@ -138,6 +195,11 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback { @Override public void beforeEach(ExtensionContext context) { + Optional elem = context.getTestMethod(); + if (elem.isPresent() && elem.get().isAnnotationPresent(NoDatabaseCompare.class)) { + enableDatabaseCompare = false; + } + // Use a single bucket to expose timestamp inversion problems. This typically happens when // a test with this extension rolls back the fake clock in the setup method, creating inverted // timestamp with the canned data preloaded by AppengineExtension. The solution is to move @@ -170,23 +232,6 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback { } } - private static ImmutableSet NON_REPLICATED_TYPES = - ImmutableSet.of( - "PremiumList", - "PremiumListRevision", - "PremiumListEntry", - "ReservedList", - "RdeRevision", - "ServerSecret", - "SignedMarkRevocationList", - "ClaimsListShard", - "TmchCrl", - "EppResourceIndex", - "ForeignKeyIndex", - "ForeignKeyHostIndex", - "ForeignKeyContactIndex", - "ForeignKeyDomainIndex"); - public void replay() { if (!replayed) { if (inOfyContext) { @@ -211,34 +256,32 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback { ImmutableMap, Object> changes = ReplayQueue.replay(); // Compare JPA to OFY, if requested. - if (compare) { - for (ImmutableMap.Entry, Object> entry : changes.entrySet()) { - // Don't verify non-replicated types. - if (NON_REPLICATED_TYPES.contains(entry.getKey().getKind())) { - continue; - } - - // Since the object may have changed in datastore by the time we're doing the replay, we - // have to compare the current value in SQL (which we just mutated) against the value that - // we originally would have persisted (that being the object in the entry). - VKey vkey = VKey.from(entry.getKey()); - jpaTm() - .transact( - () -> { - Optional jpaValue = jpaTm().loadByKeyIfPresent(vkey); - if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) { - assertThat(jpaValue.isPresent()).isFalse(); - } else { - ImmutableObject immutJpaObject = (ImmutableObject) jpaValue.get(); - assertAboutImmutableObjects().that(immutJpaObject).hasCorrectHashValue(); - assertAboutImmutableObjects() - .that(immutJpaObject) - .isEqualAcrossDatabases( - (ImmutableObject) - ((DatastoreEntity) entry.getValue()).toSqlEntity().get()); - } - }); + for (ImmutableMap.Entry, Object> entry : changes.entrySet()) { + // Don't verify non-replicated types. + if (NON_REPLICATED_TYPES.contains(entry.getKey().getKind())) { + continue; } + + // Since the object may have changed in datastore by the time we're doing the replay, we + // have to compare the current value in SQL (which we just mutated) against the value that + // we originally would have persisted (that being the object in the entry). + VKey vkey = VKey.from(entry.getKey()); + jpaTm() + .transact( + () -> { + Optional jpaValue = jpaTm().loadByKeyIfPresent(vkey); + if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) { + assertThat(jpaValue.isPresent()).isFalse(); + } else { + ImmutableObject immutJpaObject = (ImmutableObject) jpaValue.get(); + assertAboutImmutableObjects().that(immutJpaObject).hasCorrectHashValue(); + assertAboutImmutableObjects() + .that(immutJpaObject) + .isEqualAcrossDatabases( + (ImmutableObject) + ((DatastoreEntity) entry.getValue()).toSqlEntity().get()); + } + }); } } @@ -252,12 +295,15 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback { transactionBatch = sqlToDsReplicator.getTransactionBatchAtSnapshot(); for (TransactionEntity txn : transactionBatch) { ReplicateToDatastoreAction.applyTransaction(txn); - if (compare) { - ofyTm().transact(() -> compareSqlTransaction(txn)); - } + ofyTm().transact(() -> compareSqlTransaction(txn)); clock.advanceOneMilli(); } } while (!transactionBatch.isEmpty()); + + // Now that everything has been replayed, compare the databases. + if (enableDatabaseCompare) { + compareDatabases(); + } } /** Verifies that the replaying the SQL transaction created the same entities in Datastore. */ @@ -305,4 +351,84 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback { } } } + + /** Compares the final state of both databases after replay is complete. */ + private void compareDatabases() { + boolean gotDiffs = false; + + // Build a map containing all of the SQL entities indexed by their key. + HashMap sqlEntities = new HashMap<>(); + for (Class cls : JpaEntityCoverageExtension.ALL_JPA_ENTITIES) { + if (IGNORED_ENTITIES.contains(cls.getSimpleName())) { + continue; + } + + jpaTm() + .transact( + () -> jpaTm().loadAllOfStream(cls).forEach(e -> sqlEntities.put(getSqlKey(e), e))); + } + + for (Class cls : EntityClasses.ALL_CLASSES) { + if (IGNORED_ENTITIES.contains(cls.getSimpleName())) { + continue; + } + + for (ImmutableObject entity : auditedOfy().load().type(cls).list()) { + // Find the entity in SQL and verify that it's the same. + Key ofyKey = Key.create(entity); + Object sqlKey = VKey.from(ofyKey).getSqlKey(); + ImmutableObject sqlEntity = (ImmutableObject) sqlEntities.get(sqlKey); + Optional expectedSqlEntity = ((DatastoreEntity) entity).toSqlEntity(); + if (expectedSqlEntity.isPresent()) { + // Check for null just so we get a better error message. + if (sqlEntity == null) { + logger.atSevere().log("Entity %s is in Datastore but not in SQL.", ofyKey); + gotDiffs = true; + } else { + try { + assertAboutImmutableObjects() + .that((ImmutableObject) expectedSqlEntity.get()) + .isEqualAcrossDatabases(sqlEntity); + } catch (AssertionError e) { + // Show the message but swallow the stack trace (we'll get that from the fail() at + // the end of the comparison). + logger.atSevere().log("For entity %s: %s", ofyKey, e.getMessage()); + gotDiffs = true; + } + } + } else { + logger.atInfo().log("Datastore entity has no sql representation for %s", ofyKey); + } + sqlEntities.remove(sqlKey); + } + } + + // Report any objects in the SQL set that we didn't remove while iterating over the Datastore + // objects. + if (!sqlEntities.isEmpty()) { + for (Object item : sqlEntities.values()) { + logger.atSevere().log( + "Entity of %s found in SQL but not in datastore: %s", item.getClass().getName(), item); + } + gotDiffs = true; + } + + if (gotDiffs) { + fail("There were differences between the final SQL and Datastore contents."); + } + } + + private static Object getSqlKey(Object entity) { + return jpaTm() + .getEntityManager() + .getEntityManagerFactory() + .getPersistenceUnitUtil() + .getIdentifier(entity); + } + + /** Annotation to use for test methods where we don't want to do a database comparison yet. */ + @Target({METHOD}) + @Retention(RUNTIME) + @TestTemplate + public @interface NoDatabaseCompare {} } diff --git a/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java index 639697b18..0e8b86a20 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java @@ -267,9 +267,16 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase { Map response = action.handleJsonRequest( ImmutableMap.of( - "op", "update", - "id", CLIENT_ID, - "args", setter.apply(registrar.asBuilder(), newValue).build().toJsonMap())); + "op", + "update", + "id", + CLIENT_ID, + "args", + setter + .apply(registrar.asBuilder(), newValue) + .setLastUpdateTime(registrar.getLastUpdateTime()) + .build() + .toJsonMap())); Registrar updatedRegistrar = loadRegistrar(CLIENT_ID); persistResource(registrar); @@ -320,7 +327,11 @@ class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase { "id", CLIENT_ID, "args", - setter.apply(registrar.asBuilder(), newValue).build().toJsonMap())); + setter + .apply(registrar.asBuilder(), newValue) + .setLastUpdateTime(registrar.getLastUpdateTime()) + .build() + .toJsonMap())); Registrar updatedRegistrar = loadRegistrar(CLIENT_ID); persistResource(registrar);