Add object comparison to replay tests (#925)

* Add object comparison to replay tests

Allow optional object comparison in the replay test extension and enable it
for the DomainCreateFlow test.

To faciliate this, add two new field annotations to ImmutableObject:
DoNotCompare, to be used for fields that are not relevant to either database,
and Insignificant, to be used for fields that are mutated after they have been
accessed and therefore violate immutability (there is currently only one of
these, however we might discover more in the course of adding more comparisons
to the replay test.

* Revert commented out premium price error log

* Added static create methods for ReplayExtension
This commit is contained in:
Michael Muller 2021-01-15 14:20:55 -05:00 committed by GitHub
parent 554e675303
commit 9e6f99face
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 262 additions and 58 deletions

View file

@ -148,7 +148,7 @@ public abstract class EppResource extends BackupGroupRoot implements Buildable {
*
* @see google.registry.model.translators.CommitLogRevisionsTranslatorFactory
*/
@Transient
@Transient @DoNotCompare
ImmutableSortedMap<DateTime, Key<CommitLogManifest>> revisions = ImmutableSortedMap.of();
public String getRepoId() {

View file

@ -54,9 +54,37 @@ public abstract class ImmutableObject implements Cloneable {
@Target(FIELD)
public @interface DoNotHydrate {}
@Ignore
@XmlTransient
Integer hashCode;
/**
* 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.
*/
@Documented
@Retention(RUNTIME)
@Target(FIELD)
public @interface EmptySetToNull {}
/**
* Indicates that the field does not take part in the immutability contract.
*
* <p>Certain fields currently get modified by hibernate and there is nothing we can do about it.
* As well as violating immutability, this breaks hashing and equality comparisons, so we mark
* these fields with this annotation to exclude them from most operations.
*/
@Documented
@Retention(RUNTIME)
@Target(FIELD)
public @interface Insignificant {}
@Ignore @XmlTransient protected Integer hashCode;
private boolean equalsImmutableObject(ImmutableObject other) {
return getClass().equals(other.getClass())
@ -71,7 +99,14 @@ public abstract class ImmutableObject implements Cloneable {
* <p>Isolated into a method so that derived classes can override it.
*/
protected Map<Field, Object> getSignificantFields() {
return ModelUtils.getFieldValues(this);
// Can't use streams or ImmutableMap because we can have null values.
Map<Field, Object> result = new LinkedHashMap();
for (Map.Entry<Field, Object> entry : ModelUtils.getFieldValues(this).entrySet()) {
if (!entry.getKey().isAnnotationPresent(Insignificant.class)) {
result.put(entry.getKey(), entry.getValue());
}
}
return result;
}
@Override

View file

@ -50,6 +50,7 @@ import com.googlecode.objectify.condition.IfNull;
import google.registry.flows.ResourceFlowUtils;
import google.registry.model.EppResource;
import google.registry.model.EppResource.ResourceWithTransferData;
import google.registry.model.ImmutableObject.EmptySetToNull;
import google.registry.model.billing.BillingEvent;
import google.registry.model.common.EntityGroupRoot;
import google.registry.model.contact.ContactResource;
@ -132,7 +133,7 @@ public class DomainContent extends EppResource
@Index String tld;
/** References to hosts that are the nameservers for the domain. */
@Index @Transient Set<VKey<HostResource>> nsHosts;
@EmptySetToNull @Index @Transient Set<VKey<HostResource>> nsHosts;
/**
* The union of the contacts visible via {@link #getContacts} and {@link #getRegistrant}.
@ -319,6 +320,11 @@ public class DomainContent extends EppResource
autorenewPollMessageHistoryId = getHistoryId(autorenewPollMessage);
autorenewBillingEventHistoryId = getHistoryId(autorenewBillingEvent);
deletePollMessageHistoryId = getHistoryId(deletePollMessage);
// Fix PollMessage VKeys.
autorenewPollMessage = PollMessage.Autorenew.convertVKey(autorenewPollMessage);
deletePollMessage = PollMessage.OneTime.convertVKey(deletePollMessage);
dsData =
nullToEmptyImmutableCopy(dsData).stream()
.map(dsData -> dsData.cloneWithDomainRepoId(getRepoId()))

View file

@ -104,6 +104,7 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
"domain_history_history_revision_id,domain_history_domain_repo_id,host_repo_id",
unique = true),
})
@ImmutableObject.EmptySetToNull
@Column(name = "host_repo_id")
Set<VKey<HostResource>> nsHosts;
@ -180,7 +181,9 @@ public class DomainHistory extends HistoryEntry implements SqlEntity {
* #getDomainTransactionRecords()}.
*/
@Access(AccessType.PROPERTY)
@OneToMany(cascade = {CascadeType.ALL})
@OneToMany(
cascade = {CascadeType.ALL},
fetch = FetchType.EAGER)
@JoinColumn(name = "historyRevisionId", referencedColumnName = "historyRevisionId")
@JoinColumn(name = "domainRepoId", referencedColumnName = "domainRepoId")
@SuppressWarnings("unused")

View file

@ -14,7 +14,18 @@
package google.registry.model.ofy;
import static google.registry.model.ofy.EntityWritePriorities.getEntityPriority;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import com.google.common.collect.ImmutableMap;
import com.googlecode.objectify.Key;
import google.registry.config.RegistryEnvironment;
import google.registry.model.UpdateAutoTimestamp;
import google.registry.persistence.VKey;
import google.registry.schema.replay.DatastoreEntity;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentLinkedQueue;
/**
@ -24,23 +35,79 @@ import java.util.concurrent.ConcurrentLinkedQueue;
*/
public class ReplayQueue {
static ConcurrentLinkedQueue<TransactionInfo> queue =
new ConcurrentLinkedQueue<TransactionInfo>();
static ConcurrentLinkedQueue<ImmutableMap<Key<?>, Object>> queue =
new ConcurrentLinkedQueue<ImmutableMap<Key<?>, Object>>();
static void addInTests(TransactionInfo info) {
if (RegistryEnvironment.get() == RegistryEnvironment.UNITTEST) {
queue.add(info);
// Transform the entities to be persisted to the set of values as they were actually
// persisted.
ImmutableMap.Builder<Key<?>, Object> builder = new ImmutableMap.Builder<Key<?>, Object>();
for (ImmutableMap.Entry<Key<?>, Object> entry : info.getChanges().entrySet()) {
if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) {
builder.put(entry.getKey(), entry.getValue());
} else {
// The value is an entity object that has not yet been persisted, and thus some of the
// special transformations that we do (notably the auto-timestamp transformations) have
// not been applied. Converting the object to an entity and then back again performs
// those transformations so that we persist the same values to SQL that we have in
// Datastore.
builder.put(entry.getKey(), ofy().toPojo(ofy().toEntity(entry.getValue())));
}
}
queue.add(builder.build());
}
}
public static void replay() {
TransactionInfo info;
while ((info = queue.poll()) != null) {
info.saveToJpa();
/** Replay all transactions, return the set of keys that were replayed. */
public static ImmutableMap<Key<?>, Object> replay() {
// We can't use an ImmutableMap.Builder here, we need to be able to overwrite existing values
// and the builder doesn't support that.
Map<Key<?>, Object> result = new HashMap<Key<?>, Object>();
ImmutableMap<Key<?>, Object> changes;
while ((changes = queue.poll()) != null) {
saveToJpa(changes);
result.putAll(changes);
}
return ImmutableMap.copyOf(result);
}
public static void clear() {
queue.clear();
}
/** Returns the priority of the entity type in the map entry. */
private static int getPriority(ImmutableMap.Entry<Key<?>, Object> entry) {
return getEntityPriority(
entry.getKey().getKind(), entry.getValue().equals(TransactionInfo.Delete.SENTINEL));
}
private static int compareByPriority(
ImmutableMap.Entry<Key<?>, Object> a, ImmutableMap.Entry<Key<?>, Object> b) {
return getPriority(a) - getPriority(b);
}
private static void saveToJpa(ImmutableMap<Key<?>, Object> changes) {
try (UpdateAutoTimestamp.DisableAutoUpdateResource disabler =
UpdateAutoTimestamp.disableAutoUpdate()) {
// Sort the changes into an order that will work for insertion into the database.
jpaTm()
.transact(
() -> {
changes.entrySet().stream()
.sorted(ReplayQueue::compareByPriority)
.forEach(
entry -> {
if (entry.getValue().equals(TransactionInfo.Delete.SENTINEL)) {
jpaTm().delete(VKey.from(entry.getKey()));
} else {
((DatastoreEntity) entry.getValue())
.toSqlEntity()
.ifPresent(jpaTm()::put);
}
});
});
}
}
}

View file

@ -20,24 +20,20 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Maps.filterValues;
import static com.google.common.collect.Maps.toMap;
import static google.registry.model.ofy.CommitLogBucket.getArbitraryBucketId;
import static google.registry.model.ofy.EntityWritePriorities.getEntityPriority;
import static google.registry.model.ofy.ObjectifyService.ofy;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.googlecode.objectify.Key;
import google.registry.persistence.VKey;
import google.registry.schema.replay.DatastoreEntity;
import java.util.Map;
import org.joda.time.DateTime;
/** Metadata for an {@link Ofy} transaction that saves commit logs. */
class TransactionInfo {
public class TransactionInfo {
@VisibleForTesting
enum Delete {
public enum Delete {
SENTINEL
}
@ -87,6 +83,10 @@ class TransactionInfo {
return ImmutableSet.copyOf(changesBuilder.build().keySet());
}
ImmutableMap<Key<?>, Object> getChanges() {
return changesBuilder.build();
}
ImmutableSet<Key<?>> getDeletes() {
return ImmutableSet.copyOf(
filterValues(changesBuilder.build(), Delete.SENTINEL::equals).keySet());
@ -100,35 +100,4 @@ class TransactionInfo {
.filter(not(Delete.SENTINEL::equals))
.collect(toImmutableSet());
}
/** Returns the weight of the entity type in the map entry. */
@VisibleForTesting
static int getWeight(ImmutableMap.Entry<Key<?>, Object> entry) {
return getEntityPriority(entry.getKey().getKind(), entry.getValue().equals(Delete.SENTINEL));
}
private static int compareByWeight(
ImmutableMap.Entry<Key<?>, Object> a, ImmutableMap.Entry<Key<?>, Object> b) {
return getWeight(a) - getWeight(b);
}
void saveToJpa() {
// Sort the changes into an order that will work for insertion into the database.
jpaTm()
.transact(
() -> {
changesBuilder.build().entrySet().stream()
.sorted(TransactionInfo::compareByWeight)
.forEach(
entry -> {
if (entry.getValue().equals(Delete.SENTINEL)) {
jpaTm().delete(VKey.from(entry.getKey()));
} else {
((DatastoreEntity) entry.getValue())
.toSqlEntity()
.ifPresent(jpaTm()::put);
}
});
});
}
}

View file

@ -52,6 +52,7 @@ import google.registry.persistence.WithLongVKey;
import google.registry.schema.replay.DatastoreAndSqlEntity;
import java.util.List;
import java.util.Optional;
import javax.annotation.Nullable;
import javax.persistence.AttributeOverride;
import javax.persistence.AttributeOverrides;
import javax.persistence.Column;
@ -185,6 +186,7 @@ public abstract class PollMessage extends ImmutableObject
@Override
public abstract VKey<? extends PollMessage> createVKey();
/** Static VKey factory method for use by VKeyTranslatorFactory. */
public static VKey<PollMessage> createVKey(Key<PollMessage> key) {
return VKey.create(PollMessage.class, key.getId(), key);
}
@ -289,7 +291,7 @@ public abstract class PollMessage extends ImmutableObject
@Transient List<ContactTransferResponse> contactTransferResponses;
@Transient
@Transient @ImmutableObject.DoNotCompare
List<DomainPendingActionNotificationResponse> domainPendingActionNotificationResponses;
@Transient List<DomainTransferResponse> domainTransferResponses;
@ -355,6 +357,11 @@ public abstract class PollMessage extends ImmutableObject
return VKey.create(OneTime.class, getId(), Key.create(this));
}
/** Converts an unspecialized VKey&lt;PollMessage&gt; to a VKey of the derived class. */
public static @Nullable VKey<OneTime> convertVKey(@Nullable VKey<OneTime> key) {
return key == null ? null : VKey.create(OneTime.class, key.getSqlKey(), key.getOfyKey());
}
@Override
public Builder asBuilder() {
return new Builder(clone(this));
@ -456,6 +463,11 @@ public abstract class PollMessage extends ImmutableObject
return VKey.create(Autorenew.class, getId(), Key.create(this));
}
/** Converts an unspecialized VKey&lt;PollMessage&gt; to a VKey of the derived class. */
public static @Nullable VKey<Autorenew> convertVKey(VKey<Autorenew> key) {
return key == null ? null : VKey.create(Autorenew.class, key.getSqlKey(), key.getOfyKey());
}
@Override
public ImmutableList<ResponseData> getResponseData() {
// Note that the event time is when the auto-renew occured, so the expiration time in the

View file

@ -49,7 +49,9 @@ public class DomainTransactionRecord extends ImmutableObject
@Id
@Ignore
@ImmutableObject.DoNotCompare
@GeneratedValue(strategy = GenerationType.IDENTITY)
@ImmutableObject.Insignificant
Long id;
/** The TLD this record operates on. */

View file

@ -198,6 +198,7 @@ public class HistoryEntry extends ImmutableObject implements Buildable, Datastor
* transaction counts (such as contact or host mutations).
*/
@Transient // domain-specific
@ImmutableObject.EmptySetToNull
protected Set<DomainTransactionRecord> domainTransactionRecords;
public long getId() {

View file

@ -184,7 +184,7 @@ class DomainCreateFlowTest extends ResourceFlowTestCase<DomainCreateFlow, Domain
@Order(value = Order.DEFAULT - 2)
@RegisterExtension
final ReplayExtension replayExtension = new ReplayExtension(clock);
final ReplayExtension replayExtension = ReplayExtension.createWithCompare(clock);
DomainCreateFlowTest() {
setEppInput("domain_create.xml", ImmutableMap.of("DOMAIN", "example.tld"));

View file

@ -112,7 +112,7 @@ class DomainDeleteFlowTest extends ResourceFlowTestCase<DomainDeleteFlow, Domain
@Order(value = Order.DEFAULT - 2)
@RegisterExtension
final ReplayExtension replayExtension = new ReplayExtension(clock);
final ReplayExtension replayExtension = ReplayExtension.createWithoutCompare(clock);
private DomainBase domain;
private HistoryEntry earlierHistoryEntry;

View file

@ -108,7 +108,7 @@ class DomainRenewFlowTest extends ResourceFlowTestCase<DomainRenewFlow, DomainBa
@Order(value = Order.DEFAULT - 2)
@RegisterExtension
final ReplayExtension replayExtension = new ReplayExtension(clock);
final ReplayExtension replayExtension = ReplayExtension.createWithoutCompare(clock);
@BeforeEach
void initDomainTest() {

View file

@ -117,7 +117,7 @@ class DomainUpdateFlowTest extends ResourceFlowTestCase<DomainUpdateFlow, Domain
@Order(value = Order.DEFAULT - 2)
@RegisterExtension
final ReplayExtension replayExtension = new ReplayExtension(clock);
final ReplayExtension replayExtension = ReplayExtension.createWithoutCompare(clock);
@BeforeEach
void initDomainTest() {

View file

@ -23,10 +23,12 @@ 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.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import javax.annotation.Nullable;
/** Truth subject for asserting things about ImmutableObjects that are not built in. */
@ -53,6 +55,26 @@ 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}.
*
* <p>This is used to verify that entities stored in both cloud SQL and Datastore are identical.
*/
public void isEqualAcrossDatabases(@Nullable ImmutableObject expected) {
if (actual == null) {
assertThat(expected).isNull();
} else {
assertThat(expected).isNotNull();
}
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);
}
}
public static Correspondence<ImmutableObject, ImmutableObject> immutableObjectCorrespondence(
String... ignoredFields) {
return Correspondence.from(
@ -99,4 +121,26 @@ public final class ImmutableObjectSubject extends Subject {
}
return result;
}
/** Filter out fields with the given annotation. */
public static Map<Field, Object> filterFields(
ImmutableObject original, Class<? extends Annotation> annotation) {
Map<Field, Object> originalFields = ModelUtils.getFieldValues(original);
// don't use ImmutableMap or a stream->collect model since we can have nulls
Map<Field, Object> result = new LinkedHashMap<>();
for (Map.Entry<Field, Object> entry : originalFields.entrySet()) {
if (!entry.getKey().isAnnotationPresent(annotation)) {
// 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;
}
}

View file

@ -14,7 +14,19 @@
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;
import com.googlecode.objectify.Key;
import google.registry.model.ImmutableObject;
import google.registry.model.ofy.ReplayQueue;
import google.registry.model.ofy.TransactionInfo;
import google.registry.persistence.VKey;
import java.util.Optional;
import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext;
@ -26,13 +38,26 @@ import org.junit.jupiter.api.extension.ExtensionContext;
* that extension are also replayed. If AppEngineExtension is not used,
* JpaTransactionManagerExtension must be, and this extension should be ordered _after_
* JpaTransactionManagerExtension so that writes to SQL work.
*
* <p>If the "compare" flag is set in the constructor, this will also compare all touched objects in
* both databases after performing the replay.
*/
public class ReplayExtension implements BeforeEachCallback, AfterEachCallback {
FakeClock clock;
boolean compare;
public ReplayExtension(FakeClock clock) {
private ReplayExtension(FakeClock clock, boolean compare) {
this.clock = clock;
this.compare = compare;
}
public static ReplayExtension createWithCompare(FakeClock clock) {
return new ReplayExtension(clock, true);
}
public static ReplayExtension createWithoutCompare(FakeClock clock) {
return new ReplayExtension(clock, false);
}
@Override
@ -51,8 +76,48 @@ public class ReplayExtension implements BeforeEachCallback, AfterEachCallback {
replayToSql();
}
private static ImmutableSet<String> NON_REPLICATED_TYPES =
ImmutableSet.of(
"PremiumList",
"PremiumListRevision",
"PremiumListEntry",
"ReservedList",
"RdeRevision",
"KmsSecretRevision",
"ServerSecret",
"SignedMarkRevocationList",
"ClaimsListShard",
"TmchCrl",
"EppResourceIndex",
"ForeignKeyIndex",
"ForeignKeyHostIndex",
"ForeignKeyContactIndex",
"ForeignKeyDomainIndex");
public void replayToSql() {
DatabaseHelper.setAlwaysSaveWithBackup(false);
ReplayQueue.replay();
ImmutableMap<Key<?>, Object> changes = ReplayQueue.replay();
// Compare JPA to OFY, if requested.
if (compare) {
for (ImmutableMap.Entry<Key<?>, Object> entry : changes.entrySet()) {
// Don't verify non-replicated types.
if (NON_REPLICATED_TYPES.contains(entry.getKey().getKind())) {
continue;
}
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 {
assertAboutImmutableObjects()
.that((ImmutableObject) jpaValue.get())
.isEqualAcrossDatabases((ImmutableObject) ofyValue.get());
}
}
}
}
}