Make cross database comparison recursive (#942)

* Make cross database comparison recursive

Cross-database comparison was previously just a shallow check: fields marked
with DoNotCompare on nested objects were still compared.  This causes problems
in some cases where there are nested immutable objects.

This change introduces recursive comparison.  It also provides a
hasCorrectHashCode() method that verifies that an object has not been mutated
since the hash code was calculated, which has been a problem in certain cases.

Finally, this also fixes the problem of objects that are mutated in multiple
transactions: we were previously comparing against the value in datastore, but
this doesn't work in these cases because the object in datastore may have
changed since the transaction that we are verifying.  Instead, check against
the value that we would have persisted in the original transaction.

* Changes requested in review

* Converted check method interfaces

Per review discussion, converted check method interface so that they
consistently return a ComparisonResult object which encapsulates a success
indicator and an optional error message.

* Another round of changes on ImmutableObjectSubject

* Final changes for review

Removed unnecessary null check, minor reformatting.

(this also removes an obsolete nullness assertion from an earlier commit that
should have been fixed in the rebase)

* Try removing that nullness check import again....

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

@@ -14,11 +14,14 @@
 
 package google.registry.model;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.truth.Truth.assertAbout;
 import static com.google.common.truth.Truth.assertThat;
-import static google.registry.testing.truth.TruthUtils.assertNullnessParity;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import com.google.common.truth.Correspondence;
 import com.google.common.truth.Correspondence.BinaryPredicate;
 import com.google.common.truth.FailureMetadata;
@@ -26,10 +29,15 @@ import com.google.common.truth.SimpleSubjectBuilder;
 import com.google.common.truth.Subject;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Field;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collector;
 import javax.annotation.Nullable;
 
 /** Truth subject for asserting things about ImmutableObjects that are not built in. */
@@ -63,15 +71,297 @@ public final class ImmutableObjectSubject extends Subject {
    * <p>This is used to verify that entities stored in both cloud SQL and Datastore are identical.
    */
   public void isEqualAcrossDatabases(@Nullable ImmutableObject expected) {
-    assertNullnessParity(actual, expected);
-    if (actual != null) {
-      Map<Field, Object> actualFields = filterFields(actual, ImmutableObject.DoNotCompare.class);
-      Map<Field, Object> expectedFields =
-          filterFields(expected, ImmutableObject.DoNotCompare.class);
-      assertThat(actualFields).containsExactlyEntriesIn(expectedFields);
+    ComparisonResult result =
+        checkObjectAcrossDatabases(
+            actual, expected, actual != null ? actual.getClass().getName() : "null");
+    if (result.isFailure()) {
+      throw new AssertionError(result.getMessage());
     }
   }
 
+  // The following "check" methods implement a recursive check of immutable object equality across
+  // databases.  All of them function in both assertive and predicate modes: if "path" is
+  // provided (not null) then they throw AssertionError's with detailed error messages.  If
+  // it is null, they return true for equal objects and false for inequal ones.
+  //
+  // The reason for this dual-mode behavior is that all of these methods can either be used in the
+  // context of a test assertion (in which case we want a detailed error message describing exactly
+  // the location in a complex object where a difference was discovered) or in the context of a
+  // membership check in a set (in which case we don't care about the specific location of the first
+  // difference, we just want to be able to determine if the object "is equal to" another object as
+  // efficiently as possible -- see checkSetAcrossDatabase()).
+
+  @VisibleForTesting
+  static ComparisonResult checkObjectAcrossDatabases(
+      @Nullable Object actual, @Nullable Object expected, @Nullable String path) {
+    if (Objects.equals(actual, expected)) {
+      return ComparisonResult.createSuccess();
+    }
+
+    // They're different, do a more detailed comparison.
+
+    // Check for null first (we can assume both variables are not null at this point).
+    if (actual == null) {
+      return ComparisonResult.createFailure(path, "expected ", expected, "got null.");
+    } else if (expected == null) {
+      return ComparisonResult.createFailure(path, "expected null, got ", actual);
+
+      // For immutable objects, we have to recurse since the contained
+      // object could have DoNotCompare fields, too.
+    } else if (expected instanceof ImmutableObject) {
+      // We only verify that actual is an ImmutableObject so we get a good error message instead
+      // of a context-less ClassCastException.
+      if (!(actual instanceof ImmutableObject)) {
+        return ComparisonResult.createFailure(path, actual, " is not an immutable object.");
+      }
+
+      return checkImmutableAcrossDatabases(
+          (ImmutableObject) actual, (ImmutableObject) expected, path);
+    } else if (expected instanceof Map) {
+      if (!(actual instanceof Map)) {
+        return ComparisonResult.createFailure(path, actual, " is not a Map.");
+      }
+
+      // This would likely be more efficient if we could assume that keys can be compared across
+      // databases using .equals(), however we cannot guarantee key equality so the simplest and
+      // most correct way to accomplish this is by reusing the set comparison.
+      return checkSetAcrossDatabases(
+          ((Map<?, ?>) actual).entrySet(), ((Map<?, ?>) expected).entrySet(), path, "Map");
+    } else if (expected instanceof Set) {
+      if (!(actual instanceof Set)) {
+        return ComparisonResult.createFailure(path, actual, " is not a Set.");
+      }
+
+      return checkSetAcrossDatabases((Set<?>) actual, (Set<?>) expected, path, "Set");
+    } else if (expected instanceof Collection) {
+      if (!(actual instanceof Collection)) {
+        return ComparisonResult.createFailure(path, actual, " is not a Collection.");
+      }
+
+      return checkListAcrossDatabases((Collection<?>) actual, (Collection<?>) expected, path);
+      // Give Map.Entry special treatment to facilitate the use of Set comparison for verification
+      // of Map.
+    } else if (expected instanceof Map.Entry) {
+      if (!(actual instanceof Map.Entry)) {
+        return ComparisonResult.createFailure(path, actual, " is not a Map.Entry.");
+      }
+
+      // Check both the key and value.  We can always ignore the path here, this should only be
+      // called from within a set comparison.
+      ComparisonResult result;
+      if ((result =
+              checkObjectAcrossDatabases(
+                  ((Map.Entry<?, ?>) actual).getKey(), ((Map.Entry<?, ?>) expected).getKey(), null))
+          .isFailure()) {
+        return result;
+      }
+      if ((result =
+              checkObjectAcrossDatabases(
+                  ((Map.Entry<?, ?>) actual).getValue(),
+                  ((Map.Entry<?, ?>) expected).getValue(),
+                  null))
+          .isFailure()) {
+        return result;
+      }
+    } else {
+      // Since we know that the objects are not equal and since any other types can not be expected
+      // to contain DoNotCompare elements, this condition is always a failure.
+      return ComparisonResult.createFailure(path, actual, " is not equal to ", expected);
+    }
+
+    return ComparisonResult.createSuccess();
+  }
+
+  private static ComparisonResult checkSetAcrossDatabases(
+      Set<?> actual, Set<?> expected, String path, String type) {
+    // Unfortunately, we can't just check to see whether one set "contains" all of the elements of
+    // the other, as the cross database checks don't require strict equality.  Instead we have to do
+    // an N^2 comparison to search for an equivalent element.
+
+    // Objects in expected that aren't in actual.  We use "identity sets" here and below because we
+    // want to keep track of the _objects themselves_ rather than rely upon any overridable notion
+    // of equality.
+    Set<Object> missing = path != null ? Sets.newIdentityHashSet() : null;
+
+    // Objects from actual that have matching elements in expected.
+    Set<Object> found = Sets.newIdentityHashSet();
+
+    // Build missing and found.
+    for (Object expectedElem : expected) {
+      boolean gotMatch = false;
+      for (Object actualElem : actual) {
+        if (!checkObjectAcrossDatabases(actualElem, expectedElem, null).isFailure()) {
+          gotMatch = true;
+
+          // Add the element to the set of expected elements that were "found" in actual.  If the
+          // element matches multiple elements in "expected," we have a basic problem with this
+          // kind of set that we'll want to know about.
+          if (!found.add(actualElem)) {
+            return ComparisonResult.createFailure(
+                path, "element ", actualElem, " matches multiple elements in ", expected);
+          }
+
+          break;
+        }
+      }
+
+      if (!gotMatch) {
+        if (path == null) {
+          return ComparisonResult.createFailure();
+        }
+        missing.add(expectedElem);
+      }
+    }
+
+    if (path != null) {
+      // Provide a detailed message consisting of any missing or unexpected items.
+
+      // Build a set of all objects in actual that don't have counterparts in expected.
+      Set<Object> unexpected =
+          actual.stream()
+              .filter(actualElem -> !found.contains(actualElem))
+              .collect(
+                  Collector.of(
+                      Sets::newIdentityHashSet,
+                      Set::add,
+                      (result, values) -> {
+                        result.addAll(values);
+                        return result;
+                      }));
+
+      if (!missing.isEmpty() || !unexpected.isEmpty()) {
+        String message = type + " does not contain the expected contents.";
+        if (!missing.isEmpty()) {
+          message += "  It is missing: " + formatItems(missing.iterator());
+        }
+
+        if (!unexpected.isEmpty()) {
+          message += "  It contains additional elements: " + formatItems(unexpected.iterator());
+        }
+
+        return ComparisonResult.createFailure(path, message);
+      }
+
+      // We just need to check if there were any objects in "actual" that were not in "expected"
+      // (where "found" is a proxy for "expected").
+    } else if (actual.stream().anyMatch(Predicate.not(found::contains))) {
+      return ComparisonResult.createFailure();
+    }
+
+    return ComparisonResult.createSuccess();
+  }
+
+  private static ComparisonResult checkListAcrossDatabases(
+      Collection<?> actual, Collection<?> expected, @Nullable String path) {
+    Iterator<?> actualIter = actual.iterator();
+    Iterator<?> expectedIter = expected.iterator();
+    int index = 0;
+    while (actualIter.hasNext() && expectedIter.hasNext()) {
+      Object actualItem = actualIter.next();
+      Object expectedItem = expectedIter.next();
+      ComparisonResult result =
+          checkObjectAcrossDatabases(
+              actualItem, expectedItem, path != null ? path + "[" + index + "]" : null);
+      if (result.isFailure()) {
+        return result;
+      }
+      ++index;
+    }
+
+    if (actualIter.hasNext()) {
+      return ComparisonResult.createFailure(
+          path, "has additional items: ", formatItems(actualIter));
+    }
+
+    if (expectedIter.hasNext()) {
+      return ComparisonResult.createFailure(path, "missing items: ", formatItems(expectedIter));
+    }
+
+    return ComparisonResult.createSuccess();
+  }
+
+  /** Recursive helper for isEqualAcrossDatabases. */
+  private static ComparisonResult checkImmutableAcrossDatabases(
+      ImmutableObject actual, ImmutableObject expected, String path) {
+    Map<Field, Object> actualFields = filterFields(actual, ImmutableObject.DoNotCompare.class);
+    Map<Field, Object> expectedFields = filterFields(expected, ImmutableObject.DoNotCompare.class);
+
+    for (Map.Entry<Field, Object> entry : expectedFields.entrySet()) {
+      if (!actualFields.containsKey(entry.getKey())) {
+        return ComparisonResult.createFailure(path, "is missing field ", entry.getKey().getName());
+      }
+
+      // Verify that the field values are the same.
+      Object expectedFieldValue = entry.getValue();
+      Object actualFieldValue = actualFields.get(entry.getKey());
+      ComparisonResult result =
+          checkObjectAcrossDatabases(
+              actualFieldValue,
+              expectedFieldValue,
+              path != null ? path + "." + entry.getKey().getName() : null);
+      if (result.isFailure()) {
+        return result;
+      }
+    }
+
+    // Check for fields in actual that are not in expected.
+    for (Map.Entry<Field, Object> entry : actualFields.entrySet()) {
+      if (!expectedFields.containsKey(entry.getKey())) {
+        return ComparisonResult.createFailure(
+            path, "has additional field ", entry.getKey().getName());
+      }
+    }
+
+    return ComparisonResult.createSuccess();
+  }
+
+  private static String formatItems(Iterator<?> iter) {
+    return Joiner.on(", ").join(iter);
+  }
+
+  /** Encapsulates success/failure result in recursive comparison with optional error string. */
+  static class ComparisonResult {
+    boolean succeeded;
+    String message;
+
+    private ComparisonResult(boolean succeeded, @Nullable String message) {
+      this.succeeded = succeeded;
+      this.message = message;
+    }
+
+    static ComparisonResult createFailure() {
+      return new ComparisonResult(false, null);
+    }
+
+    static ComparisonResult createFailure(@Nullable String path, Object... message) {
+      return new ComparisonResult(false, "At " + path + ": " + Joiner.on("").join(message));
+    }
+
+    static ComparisonResult createSuccess() {
+      return new ComparisonResult(true, null);
+    }
+
+    String getMessage() {
+      checkNotNull(message);
+      return message;
+    }
+
+    boolean isFailure() {
+      return !succeeded;
+    }
+  }
+
+  /**
+   * Checks that the hash value reported by {@code actual} is correct.
+   *
+   * <p>This is used in the replay tests to ensure that hibernate hasn't modified any fields that
+   * are not marked as @Insignificant while loading.
+   */
+  public void hasCorrectHashValue() {
+    assertThat(Arrays.hashCode(actual.getSignificantFields().values().toArray()))
+        .isEqualTo(actual.hashCode());
+  }
+
   public static Correspondence<ImmutableObject, ImmutableObject> immutableObjectCorrespondence(
       String... ignoredFields) {
     return Correspondence.from(

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

@@ -0,0 +1,481 @@
+// Copyright 2021 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 static google.registry.model.ImmutableObjectSubject.ComparisonResult;
+import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
+import static google.registry.model.ImmutableObjectSubject.checkObjectAcrossDatabases;
+import static org.junit.Assert.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.regex.Pattern;
+import javax.annotation.Nullable;
+import org.junit.jupiter.api.Test;
+
+public class ImmutableObjectSubjectTest {
+
+  // Unique id to assign to the "ignored" field so that it always gets a unique value.
+  private static int uniqueId = 0;
+
+  @Test
+  void testCrossDatabase_nulls() {
+    assertAboutImmutableObjects().that(null).isEqualAcrossDatabases(null);
+    assertAboutImmutableObjects()
+        .that(makeTestAtom(null))
+        .isEqualAcrossDatabases(makeTestAtom(null));
+
+    assertThat(checkObjectAcrossDatabases(null, makeTestAtom("foo"), null).isFailure()).isTrue();
+    assertThat(checkObjectAcrossDatabases(null, makeTestAtom("foo"), null).isFailure()).isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_equalObjects() {
+    TestImmutableObject actual = makeTestObj();
+    assertAboutImmutableObjects().that(actual).isEqualAcrossDatabases(actual);
+    assertAboutImmutableObjects().that(actual).isEqualAcrossDatabases(makeTestObj());
+    assertThat(checkObjectAcrossDatabases(makeTestObj(), makeTestObj(), null).isFailure())
+        .isFalse();
+  }
+
+  @Test
+  void testCrossDatabase_simpleFieldFailure() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withStringField("bar")));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$TestImmutableObject.stringField:");
+    assertThat(
+            checkObjectAcrossDatabases(makeTestObj(), makeTestObj().withStringField(null), null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_nestedImmutableFailure() {
+    // Repeat the null checks to verify that the attribute path is preserved.
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withNested(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$TestImmutableObject.nested:"
+                + " expected null, got TestImmutableObject");
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj().withNested(null))
+                    .isEqualAcrossDatabases(makeTestObj()));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$TestImmutableObject.nested:"
+                + " expected TestImmutableObject");
+    assertThat(e).hasMessageThat().contains("got null.");
+
+    // Test with a field difference.
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj().withNested(makeTestObj().withNested(null))));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.nested.stringField:");
+    assertThat(
+            checkObjectAcrossDatabases(makeTestObj(), makeTestObj().withNested(null), null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_listFailure() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withList(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$" + "TestImmutableObject.list:");
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj().withList(ImmutableList.of(makeTestAtom("wack")))));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.list[0].stringField:");
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj()
+                            .withList(
+                                ImmutableList.of(
+                                    makeTestAtom("baz"),
+                                    makeTestAtom("bot"),
+                                    makeTestAtom("boq")))));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.list: missing items");
+    // Make sure multiple additional items get formatted nicely.
+    assertThat(e).hasMessageThat().contains("}, TestImmutableObject");
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withList(ImmutableList.of())));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.list: has additional items");
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj()
+                        .withList(ImmutableList.of(makeTestAtom("baz"), makeTestAtom("gauze"))),
+                    null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(), makeTestObj().withList(ImmutableList.of()), null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj().withList(ImmutableList.of(makeTestAtom("gauze"))),
+                    null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_setFailure() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withSet(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.set: expected null, got ");
+
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj().withSet(ImmutableSet.of(makeTestAtom("jim")))));
+    assertThat(e)
+        .hasMessageThat()
+        .containsMatch(
+            Pattern.compile(
+                "Set does not contain the expected contents.  "
+                    + "It is missing: .*jim.*  It contains additional elements: .*bob",
+                Pattern.DOTALL));
+
+    // Trickery here to verify that multiple items that both match existing items in the set trigger
+    // an error: we can add two of the same items because equality for purposes of the set includes
+    // the DoNotCompare field.
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj()
+                            .withSet(ImmutableSet.of(makeTestAtom("bob"), makeTestAtom("bob")))));
+    assertThat(e)
+        .hasMessageThat()
+        .containsMatch(
+            Pattern.compile(
+                "At google.registry.model.ImmutableObjectSubjectTest\\$TestImmutableObject.set: "
+                    + "element .*bob.* matches multiple elements in .*bob.*bob",
+                Pattern.DOTALL));
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(
+                        makeTestObj()
+                            .withSet(ImmutableSet.of(makeTestAtom("bob"), makeTestAtom("bob"))))
+                    .isEqualAcrossDatabases(makeTestObj()));
+    assertThat(e)
+        .hasMessageThat()
+        .containsMatch(
+            Pattern.compile(
+                "At google.registry.model.ImmutableObjectSubjectTest\\$TestImmutableObject.set: "
+                    + "Set does not contain the expected contents.  It contains additional "
+                    + "elements: .*bob",
+                Pattern.DOTALL));
+
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj()
+                        .withSet(ImmutableSet.of(makeTestAtom("bob"), makeTestAtom("robert"))),
+                    null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(), makeTestObj().withSet(ImmutableSet.of()), null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj()
+                        .withSet(ImmutableSet.of(makeTestAtom("bob"), makeTestAtom("bob"))),
+                    null)
+                .isFailure())
+        .isTrue();
+    // We don't test the case where actual's set contains multiple items matching the single item in
+    // the expected set: that path is the same as the "additional contents" path.
+  }
+
+  @Test
+  void testCrossDatabase_mapFailure() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(makeTestObj().withMap(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$"
+                + "TestImmutableObject.map: expected null, got ");
+
+    e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(makeTestObj())
+                    .isEqualAcrossDatabases(
+                        makeTestObj()
+                            .withMap(ImmutableMap.of(makeTestAtom("difk"), makeTestAtom("difv")))));
+    assertThat(e)
+        .hasMessageThat()
+        .containsMatch(
+            Pattern.compile(
+                "Map does not contain the expected contents.  "
+                    + "It is missing: .*difk.*difv.*  It contains additional elements: .*key.*val",
+                Pattern.DOTALL));
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(),
+                    makeTestObj()
+                        .withMap(
+                            ImmutableMap.of(
+                                makeTestAtom("key"), makeTestAtom("val"),
+                                makeTestAtom("otherk"), makeTestAtom("otherv"))),
+                    null)
+                .isFailure())
+        .isTrue();
+    assertThat(
+            checkObjectAcrossDatabases(
+                    makeTestObj(), makeTestObj().withMap(ImmutableMap.of()), null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testCrossDatabase_typeChecks() {
+    ComparisonResult result = checkObjectAcrossDatabases("blech", makeTestObj(), "xxx");
+    assertThat(result.getMessage()).isEqualTo("At xxx: blech is not an immutable object.");
+    assertThat(result.isFailure()).isTrue();
+    assertThat(checkObjectAcrossDatabases("blech", makeTestObj(), null).isFailure()).isTrue();
+
+    result = checkObjectAcrossDatabases("blech", ImmutableMap.of(), "xxx");
+    assertThat(result.getMessage()).isEqualTo("At xxx: blech is not a Map.");
+    assertThat(result.isFailure()).isTrue();
+    assertThat(checkObjectAcrossDatabases("blech", ImmutableMap.of(), null).isFailure()).isTrue();
+
+    result = checkObjectAcrossDatabases("blech", ImmutableList.of(), "xxx");
+    assertThat(result.getMessage()).isEqualTo("At xxx: blech is not a Collection.");
+    assertThat(result.isFailure()).isTrue();
+    assertThat(checkObjectAcrossDatabases("blech", ImmutableList.of(), null).isFailure()).isTrue();
+
+    for (ImmutableMap.Entry<String, String> entry : ImmutableMap.of("foo", "bar").entrySet()) {
+      result = checkObjectAcrossDatabases("blech", entry, "xxx");
+      assertThat(result.getMessage()).isEqualTo("At xxx: blech is not a Map.Entry.");
+      assertThat(result.isFailure()).isTrue();
+      assertThat(checkObjectAcrossDatabases("blech", entry, "xxx").isFailure()).isTrue();
+    }
+  }
+
+  @Test
+  void testCrossDatabase_checkAdditionalFields() {
+    AssertionError e =
+        assertThrows(
+            AssertionError.class,
+            () ->
+                assertAboutImmutableObjects()
+                    .that(DerivedImmutableObject.create())
+                    .isEqualAcrossDatabases(makeTestAtom(null)));
+    assertThat(e)
+        .hasMessageThat()
+        .contains(
+            "At google.registry.model.ImmutableObjectSubjectTest$DerivedImmutableObject: "
+                + "has additional field extraField");
+
+    assertThat(
+            checkObjectAcrossDatabases(DerivedImmutableObject.create(), makeTestAtom(null), null)
+                .isFailure())
+        .isTrue();
+  }
+
+  @Test
+  void testHasCorrectHashValue() {
+    TestImmutableObject object = makeTestObj();
+    assertAboutImmutableObjects().that(object).hasCorrectHashValue();
+
+    object.stringField = "changed value!";
+    assertThrows(
+        AssertionError.class,
+        () -> assertAboutImmutableObjects().that(object).hasCorrectHashValue());
+  }
+
+  /** Make a test object with all fields set up. */
+  TestImmutableObject makeTestObj() {
+    return TestImmutableObject.create(
+        "foo",
+        makeTestAtom("bar"),
+        ImmutableList.of(makeTestAtom("baz")),
+        ImmutableSet.of(makeTestAtom("bob")),
+        ImmutableMap.of(makeTestAtom("key"), makeTestAtom("val")));
+  }
+
+  /** Make a test object without the collection fields. */
+  TestImmutableObject makeTestAtom(String stringField) {
+    return TestImmutableObject.create(stringField, null, null, null, null);
+  }
+
+  static class TestImmutableObject extends ImmutableObject {
+    String stringField;
+    TestImmutableObject nested;
+    ImmutableList<TestImmutableObject> list;
+    ImmutableSet<TestImmutableObject> set;
+    ImmutableMap<TestImmutableObject, TestImmutableObject> map;
+
+    @ImmutableObject.DoNotCompare int ignored;
+
+    static TestImmutableObject create(
+        String stringField,
+        TestImmutableObject nested,
+        ImmutableList<TestImmutableObject> list,
+        ImmutableSet<TestImmutableObject> set,
+        ImmutableMap<TestImmutableObject, TestImmutableObject> map) {
+      TestImmutableObject instance = new TestImmutableObject();
+      instance.stringField = stringField;
+      instance.nested = nested;
+      instance.list = list;
+      instance.set = set;
+      instance.map = map;
+      instance.ignored = ++uniqueId;
+      return instance;
+    }
+
+    TestImmutableObject withStringField(@Nullable String stringField) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.stringField = stringField;
+      return result;
+    }
+
+    TestImmutableObject withNested(@Nullable TestImmutableObject nested) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.nested = nested;
+      return result;
+    }
+
+    TestImmutableObject withList(@Nullable ImmutableList<TestImmutableObject> list) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.list = list;
+      return result;
+    }
+
+    TestImmutableObject withSet(@Nullable ImmutableSet<TestImmutableObject> set) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.set = set;
+      return result;
+    }
+
+    TestImmutableObject withMap(
+        @Nullable ImmutableMap<TestImmutableObject, TestImmutableObject> map) {
+      TestImmutableObject result = ImmutableObject.clone(this);
+      result.map = map;
+      return result;
+    }
+  }
+
+  static class DerivedImmutableObject extends TestImmutableObject {
+    String extraField;
+
+    static DerivedImmutableObject create() {
+      return new DerivedImmutableObject();
+    }
+  }
+}

core/src/test/java/google/registry/testing/ReplayExtension.java

@@ -17,7 +17,6 @@ package google.registry.testing;
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
-import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -26,6 +25,7 @@ import google.registry.model.ImmutableObject;
 import google.registry.model.ofy.ReplayQueue;
 import google.registry.model.ofy.TransactionInfo;
 import google.registry.persistence.VKey;
+import google.registry.schema.replay.DatastoreEntity;
 import java.util.Optional;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
@@ -106,16 +106,20 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback {
           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());
-        Optional<?> ofyValue = ofyTm().transact(() -> ofyTm().loadByKeyIfPresent(vkey));
         Optional<?> jpaValue = jpaTm().transact(() -> jpaTm().loadByKeyIfPresent(vkey));
         if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) {
           assertThat(jpaValue.isPresent()).isFalse();
-          assertThat(ofyValue.isPresent()).isFalse();
         } else {
+          ImmutableObject immutJpaObject = (ImmutableObject) jpaValue.get();
+          assertAboutImmutableObjects().that(immutJpaObject).hasCorrectHashValue();
           assertAboutImmutableObjects()
-              .that((ImmutableObject) jpaValue.get())
-              .isEqualAcrossDatabases((ImmutableObject) ofyValue.get());
+              .that(immutJpaObject)
+              .isEqualAcrossDatabases(
+                  (ImmutableObject) ((DatastoreEntity) entry.getValue()).toSqlEntity().get());
         }
       }
     }