diff --git a/core/src/main/java/google/registry/model/ImmutableObject.java b/core/src/main/java/google/registry/model/ImmutableObject.java index 66b78cef0..c179033cd 100644 --- a/core/src/main/java/google/registry/model/ImmutableObject.java +++ b/core/src/main/java/google/registry/model/ImmutableObject.java @@ -14,17 +14,13 @@ package google.registry.model; -import static com.google.common.collect.Iterables.transform; import static com.google.common.collect.Maps.transformValues; -import static google.registry.model.ofy.ObjectifyService.auditedOfy; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static java.lang.annotation.ElementType.FIELD; import static java.lang.annotation.RetentionPolicy.RUNTIME; import static java.util.stream.Collectors.toCollection; import static java.util.stream.Collectors.toList; import com.google.common.base.Joiner; -import com.google.common.collect.Maps; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Ignore; import google.registry.persistence.VKey; @@ -56,15 +52,6 @@ public abstract class ImmutableObject implements Cloneable { @Target(FIELD) public @interface DoNotHydrate {} - /** - * Indicates that the field should be ignored when comparing an object in the datastore to the - * corresponding object in Cloud SQL. - */ - @Documented - @Retention(RUNTIME) - @Target(FIELD) - public @interface DoNotCompare {} - /** * Indicates that the field stores a null value to indicate an empty set. This is also used in * object comparison. @@ -105,7 +92,7 @@ public abstract class ImmutableObject implements Cloneable { */ protected Map getSignificantFields() { // Can't use streams or ImmutableMap because we can have null values. - Map result = new LinkedHashMap(); + Map result = new LinkedHashMap<>(); for (Map.Entry entry : ModelUtils.getFieldValues(this).entrySet()) { if (!entry.getKey().isAnnotationPresent(Insignificant.class)) { result.put(entry.getKey(), entry.getValue()); @@ -190,15 +177,15 @@ public abstract class ImmutableObject implements Cloneable { /** Helper function to recursively hydrate an ImmutableObject. */ private static Object hydrate(Object value) { if (value instanceof Key) { - if (tm().isOfy()) { - return hydrate(auditedOfy().load().key((Key) value).now()); - } return value; - } else if (value instanceof Map) { + } + if (value instanceof Map) { return transformValues((Map) value, ImmutableObject::hydrate); - } else if (value instanceof Collection) { - return transform((Collection) value, ImmutableObject::hydrate); - } else if (value instanceof ImmutableObject) { + } + if (value instanceof Collection) { + return ((Collection) value).stream().map(ImmutableObject::hydrate); + } + if (value instanceof ImmutableObject) { return ((ImmutableObject) value).toHydratedString(); } return value; @@ -220,7 +207,7 @@ public abstract class ImmutableObject implements Cloneable { } return result; } else if (o instanceof Map) { - return Maps.transformValues((Map) o, ImmutableObject::toMapRecursive); + return transformValues((Map) o, ImmutableObject::toMapRecursive); } else if (o instanceof Set) { return ((Set) o) .stream() diff --git a/core/src/main/java/google/registry/model/contact/ContactHistory.java b/core/src/main/java/google/registry/model/contact/ContactHistory.java index e02d24aec..bb2d86c8f 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -60,7 +60,7 @@ public class ContactHistory extends HistoryEntry implements UnsafeSerializable { // Store ContactBase instead of Contact so we don't pick up its @Id // Nullable for the sake of pre-Registry-3.0 history objects - @DoNotCompare @Nullable ContactBase contactBase; + @Nullable ContactBase contactBase; @Id @Access(AccessType.PROPERTY) diff --git a/core/src/main/java/google/registry/model/domain/DomainHistory.java b/core/src/main/java/google/registry/model/domain/DomainHistory.java index e494bccb8..aa9117ebf 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -83,7 +83,7 @@ public class DomainHistory extends HistoryEntry { // Store DomainBase instead of Domain so we don't pick up its @Id // Nullable for the sake of pre-Registry-3.0 history objects - @DoNotCompare @Nullable DomainBase domainBase; + @Nullable DomainBase domainBase; @Id @Access(AccessType.PROPERTY) @@ -102,7 +102,6 @@ public class DomainHistory extends HistoryEntry { // We could have reused domainBase.nsHosts here, but Hibernate throws a weird exception after // we change to use a composite primary key. // TODO(b/166776754): Investigate if we can reuse domainBase.nsHosts for storing host keys. - @DoNotCompare @ElementCollection @JoinTable( name = "DomainHistoryHost", @@ -116,7 +115,6 @@ public class DomainHistory extends HistoryEntry { @Column(name = "host_repo_id") Set> nsHosts; - @DoNotCompare @OneToMany( cascade = {CascadeType.ALL}, fetch = FetchType.EAGER, @@ -137,7 +135,6 @@ public class DomainHistory extends HistoryEntry { @Ignore Set dsDataHistories = new HashSet<>(); - @DoNotCompare @OneToMany( cascade = {CascadeType.ALL}, fetch = FetchType.EAGER, diff --git a/core/src/main/java/google/registry/model/host/HostHistory.java b/core/src/main/java/google/registry/model/host/HostHistory.java index 29d9ee69f..147b7e07f 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -61,7 +61,7 @@ public class HostHistory extends HistoryEntry implements UnsafeSerializable { // Store HostBase instead of Host so we don't pick up its @Id // Nullable for the sake of pre-Registry-3.0 history objects - @DoNotCompare @Nullable HostBase hostBase; + @Nullable HostBase hostBase; @Id @Access(AccessType.PROPERTY) diff --git a/core/src/test/java/google/registry/model/ImmutableObjectSubject.java b/core/src/test/java/google/registry/model/ImmutableObjectSubject.java index bf9048de8..f93104401 100644 --- a/core/src/test/java/google/registry/model/ImmutableObjectSubject.java +++ b/core/src/test/java/google/registry/model/ImmutableObjectSubject.java @@ -18,26 +18,19 @@ 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 com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -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; 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.stream.Collector; import javax.annotation.Nullable; /** Truth subject for asserting things about ImmutableObjects that are not built in. */ @@ -45,7 +38,7 @@ public final class ImmutableObjectSubject extends Subject { @Nullable private final ImmutableObject actual; - protected ImmutableObjectSubject( + private ImmutableObjectSubject( FailureMetadata failureMetadata, @Nullable ImmutableObject actual) { super(failureMetadata, actual); this.actual = actual; @@ -73,261 +66,6 @@ public final class ImmutableObjectSubject extends Subject { } } - /** - * Checks that {@code expected} has the same contents as {@code actual} except for fields that are - * marked with {@link ImmutableObject.DoNotCompare}. - * - *

This is used to verify that entities stored in both cloud SQL and Datastore are identical. - */ - public void isEqualAcrossDatabases(@Nullable ImmutableObject expected) { - 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 (!found.containsAll(actual)) { - 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; @@ -412,7 +150,6 @@ public final class ImmutableObjectSubject extends Subject { // don't use ImmutableMap or a stream->collect model since we can have nulls Map result = new LinkedHashMap<>(); for (Map.Entry entry : originalFields.entrySet()) { - // TODO(b/203685960): filter by @DoNotCompare instead. if (entry.getKey().isAnnotationPresent(ImmutableObject.Insignificant.class)) { continue; } @@ -422,28 +159,4 @@ public final class ImmutableObjectSubject extends Subject { } return result; } - - /** Filter out fields with the given annotation. */ - private static Map filterFields( - ImmutableObject original, Class annotation) { - Map originalFields = ModelUtils.getFieldValues(original); - // don't use ImmutableMap or a stream->collect model since we can have nulls - Map result = new LinkedHashMap<>(); - for (Map.Entry entry : originalFields.entrySet()) { - // TODO(b/203685960): filter by @DoNotCompare instead. - if (!entry.getKey().isAnnotationPresent(annotation) - && !entry.getKey().isAnnotationPresent(ImmutableObject.Insignificant.class)) { - - // Perform any necessary substitutions. - if (entry.getKey().isAnnotationPresent(ImmutableObject.EmptySetToNull.class) - && entry.getValue() != null - && ((Set) entry.getValue()).isEmpty()) { - result.put(entry.getKey(), null); - } else { - result.put(entry.getKey(), entry.getValue()); - } - } - } - return result; - } } diff --git a/core/src/test/java/google/registry/model/ImmutableObjectSubjectTest.java b/core/src/test/java/google/registry/model/ImmutableObjectSubjectTest.java index d1d5b22dc..854ec5ce6 100644 --- a/core/src/test/java/google/registry/model/ImmutableObjectSubjectTest.java +++ b/core/src/test/java/google/registry/model/ImmutableObjectSubjectTest.java @@ -14,380 +14,17 @@ 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(); @@ -421,8 +58,6 @@ public class ImmutableObjectSubjectTest { ImmutableSet set; ImmutableMap map; - @ImmutableObject.DoNotCompare int ignored; - static TestImmutableObject create( String stringField, TestImmutableObject nested, @@ -435,7 +70,6 @@ public class ImmutableObjectSubjectTest { instance.list = list; instance.set = set; instance.map = map; - instance.ignored = ++uniqueId; return instance; }