diff --git a/core/src/test/java/google/registry/model/ImmutableObjectSubject.java b/core/src/test/java/google/registry/model/ImmutableObjectSubject.java index 046cf0e21..87693fb8f 100644 --- a/core/src/test/java/google/registry/model/ImmutableObjectSubject.java +++ b/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 { *

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 actualFields = filterFields(actual, ImmutableObject.DoNotCompare.class); - Map 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 missing = path != null ? Sets.newIdentityHashSet() : null; + + // Objects from actual that have matching elements in expected. + Set 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 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 actualFields = filterFields(actual, ImmutableObject.DoNotCompare.class); + Map expectedFields = filterFields(expected, ImmutableObject.DoNotCompare.class); + + for (Map.Entry 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 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. + * + *

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 immutableObjectCorrespondence( String... ignoredFields) { return Correspondence.from( diff --git a/core/src/test/java/google/registry/model/ImmutableObjectSubjectTest.java b/core/src/test/java/google/registry/model/ImmutableObjectSubjectTest.java new file mode 100644 index 000000000..d1d5b22dc --- /dev/null +++ b/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 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 list; + ImmutableSet set; + ImmutableMap map; + + @ImmutableObject.DoNotCompare int ignored; + + static TestImmutableObject create( + String stringField, + TestImmutableObject nested, + ImmutableList list, + ImmutableSet set, + ImmutableMap 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 list) { + TestImmutableObject result = ImmutableObject.clone(this); + result.list = list; + return result; + } + + TestImmutableObject withSet(@Nullable ImmutableSet set) { + TestImmutableObject result = ImmutableObject.clone(this); + result.set = set; + return result; + } + + TestImmutableObject withMap( + @Nullable ImmutableMap map) { + TestImmutableObject result = ImmutableObject.clone(this); + result.map = map; + return result; + } + } + + static class DerivedImmutableObject extends TestImmutableObject { + String extraField; + + static DerivedImmutableObject create() { + return new DerivedImmutableObject(); + } + } +} diff --git a/core/src/test/java/google/registry/testing/ReplayExtension.java b/core/src/test/java/google/registry/testing/ReplayExtension.java index 8ad8c9544..754d399de 100644 --- a/core/src/test/java/google/registry/testing/ReplayExtension.java +++ b/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()); } } }