From 84b9af5accb85891b5d960e49287c087d85e45fd Mon Sep 17 00:00:00 2001 From: gbrodman Date: Tue, 24 Nov 2020 14:33:06 -0500 Subject: [PATCH] Ensure that all relevant Keys can be converted to VKeys (#883) * Ensure that all relevant Keys can be converted to VKeys When replaying commit logs to SQL (specifically deletes) we will need to convert Datastore Keys to SQL VKeys in order to know what (if anything) to delete. The test added to EntityTest (and the associated code changes) mean that for every relevant object we'll be able to call VKeyTranslatorFactory.createVKey(Key) for all possible keys that we care about. Note that we do not care about entities that only exist in Datastore or entities that are non-replicated -- by their nature, deletes for those types of objects in Datastore are not relevant in SQL. * Responses to code review - changing comments / method names - using ModelUtils --- .../google/registry/model/ModelUtils.java | 2 +- .../registry/model/billing/BillingEvent.java | 11 +- .../google/registry/model/common/Cursor.java | 11 +- .../model/common/EntityGroupRoot.java | 11 +- .../model/contact/ContactHistory.java | 8 +- .../registry/model/domain/DomainHistory.java | 12 +- .../registry/model/host/HostHistory.java | 8 +- .../model/index/EppResourceIndex.java | 22 +-- .../model/index/EppResourceIndexBucket.java | 10 +- .../registry/model/index/ForeignKeyIndex.java | 24 +-- .../registry/model/ofy/CommitLogBucket.java | 11 +- .../model/ofy/CommitLogCheckpoint.java | 10 +- .../model/ofy/CommitLogCheckpointRoot.java | 11 +- .../registry/model/ofy/CommitLogManifest.java | 11 +- .../registry/model/ofy/CommitLogMutation.java | 11 +- .../registry/model/registrar/Registrar.java | 83 +++++------ .../model/registrar/RegistrarContact.java | 21 +-- .../model/registry/label/PremiumList.java | 18 +-- .../model/reporting/HistoryEntry.java | 31 ++++ .../registry/model/server/KmsSecret.java | 11 +- .../google/registry/model/server/Lock.java | 11 +- .../registry/model/server/ServerSecret.java | 16 +- .../registry/model/tmch/ClaimsListShard.java | 137 ++++++++---------- .../google/registry/model/tmch/TmchCrl.java | 3 +- .../schema/replay/DatastoreOnlyEntity.java | 24 +++ .../registry/beam/initsql/WriteToSqlTest.java | 2 +- .../registry/schema/replay/EntityTest.java | 59 +++++++- 27 files changed, 277 insertions(+), 312 deletions(-) create mode 100644 core/src/main/java/google/registry/schema/replay/DatastoreOnlyEntity.java diff --git a/core/src/main/java/google/registry/model/ModelUtils.java b/core/src/main/java/google/registry/model/ModelUtils.java index 14e62c0dc..ed4401d14 100644 --- a/core/src/main/java/google/registry/model/ModelUtils.java +++ b/core/src/main/java/google/registry/model/ModelUtils.java @@ -87,7 +87,7 @@ public class ModelUtils { }); /** Lists all instance fields on an object, including non-public and inherited fields. */ - static Map getAllFields(Class clazz) { + public static Map getAllFields(Class clazz) { return ALL_FIELDS_CACHE.getUnchecked(clazz); } diff --git a/core/src/main/java/google/registry/model/billing/BillingEvent.java b/core/src/main/java/google/registry/model/billing/BillingEvent.java index c873beed4..38867f9d1 100644 --- a/core/src/main/java/google/registry/model/billing/BillingEvent.java +++ b/core/src/main/java/google/registry/model/billing/BillingEvent.java @@ -24,7 +24,6 @@ import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; @@ -50,8 +49,7 @@ import google.registry.model.transfer.TransferData.TransferServerApproveEntity; import google.registry.persistence.VKey; import google.registry.persistence.WithLongVKey; import google.registry.schema.replay.DatastoreAndSqlEntity; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -683,7 +681,7 @@ public abstract class BillingEvent extends ImmutableObject @ReportedOn @Entity @WithLongVKey - public static class Modification extends BillingEvent implements DatastoreEntity { + public static class Modification extends BillingEvent implements DatastoreOnlyEntity { /** The change in cost that should be applied to the original billing event. */ Money cost; @@ -745,11 +743,6 @@ public abstract class BillingEvent extends ImmutableObject .build(); } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } - /** A builder for {@link Modification} since it is immutable. */ public static class Builder extends BillingEvent.Builder { diff --git a/core/src/main/java/google/registry/model/common/Cursor.java b/core/src/main/java/google/registry/model/common/Cursor.java index 2937d613e..58f8150bc 100644 --- a/core/src/main/java/google/registry/model/common/Cursor.java +++ b/core/src/main/java/google/registry/model/common/Cursor.java @@ -21,7 +21,6 @@ import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; @@ -29,8 +28,7 @@ import com.googlecode.objectify.annotation.Parent; import google.registry.model.ImmutableObject; import google.registry.model.UpdateAutoTimestamp; import google.registry.model.registry.Registry; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import java.util.List; import org.joda.time.DateTime; @@ -40,7 +38,7 @@ import org.joda.time.DateTime; * scoped on {@link EntityGroupRoot}. */ @Entity -public class Cursor extends ImmutableObject implements DatastoreEntity { +public class Cursor extends ImmutableObject implements DatastoreOnlyEntity { /** The types of cursors, used as the string id field for each cursor in Datastore. */ public enum CursorType { @@ -137,11 +135,6 @@ public class Cursor extends ImmutableObject implements DatastoreEntity { return CursorType.valueOf(String.join("_", id.subList(1, id.size()))); } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // Cursors are not converted since they are ephemeral - } - /** * Checks that the type of the scoped object (or null) matches the required type for the specified * cursor (or null, if the cursor is a global cursor). diff --git a/core/src/main/java/google/registry/model/common/EntityGroupRoot.java b/core/src/main/java/google/registry/model/common/EntityGroupRoot.java index 3c753ea89..a9999ca21 100644 --- a/core/src/main/java/google/registry/model/common/EntityGroupRoot.java +++ b/core/src/main/java/google/registry/model/common/EntityGroupRoot.java @@ -14,13 +14,11 @@ package google.registry.model.common; -import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; import google.registry.model.BackupGroupRoot; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; /** * The root key for the entity group which is known as the cross-tld entity group for historical @@ -37,7 +35,7 @@ import google.registry.schema.replay.SqlEntity; * entity group for the single namespace where global data applicable for all TLDs lived. */ @Entity -public class EntityGroupRoot extends BackupGroupRoot implements DatastoreEntity { +public class EntityGroupRoot extends BackupGroupRoot implements DatastoreOnlyEntity { @SuppressWarnings("unused") @Id @@ -47,9 +45,4 @@ public class EntityGroupRoot extends BackupGroupRoot implements DatastoreEntity public static Key getCrossTldKey() { return Key.create(EntityGroupRoot.class, "cross-tld"); } - - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } } 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 e9f4e11bc..79311fa77 100644 --- a/core/src/main/java/google/registry/model/contact/ContactHistory.java +++ b/core/src/main/java/google/registry/model/contact/ContactHistory.java @@ -95,9 +95,9 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { } /** Creates a {@link VKey} instance for this entity. */ + @SuppressWarnings("unchecked") public VKey createVKey() { - return VKey.create( - ContactHistory.class, new ContactHistoryId(getContactRepoId(), getId()), Key.create(this)); + return (VKey) createVKey(Key.create(this)); } @PostLoad @@ -116,7 +116,7 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { } /** Class to represent the composite primary key of {@link ContactHistory} entity. */ - static class ContactHistoryId extends ImmutableObject implements Serializable { + public static class ContactHistoryId extends ImmutableObject implements Serializable { private String contactRepoId; @@ -125,7 +125,7 @@ public class ContactHistory extends HistoryEntry implements SqlEntity { /** Hibernate requires this default constructor. */ private ContactHistoryId() {} - ContactHistoryId(String contactRepoId, long id) { + public ContactHistoryId(String contactRepoId, long id) { this.contactRepoId = contactRepoId; this.id = id; } 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 6eac53689..4003c8767 100644 --- a/core/src/main/java/google/registry/model/domain/DomainHistory.java +++ b/core/src/main/java/google/registry/model/domain/DomainHistory.java @@ -143,12 +143,8 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { @Nullable @Access(AccessType.PROPERTY) @AttributeOverrides({ - @AttributeOverride( - name = "unit", - column = @Column(name = "historyPeriodUnit")), - @AttributeOverride( - name = "value", - column = @Column(name = "historyPeriodValue")) + @AttributeOverride(name = "unit", column = @Column(name = "historyPeriodUnit")), + @AttributeOverride(name = "value", column = @Column(name = "historyPeriodValue")) }) public Period getPeriod() { return super.getPeriod(); @@ -231,9 +227,9 @@ public class DomainHistory extends HistoryEntry implements SqlEntity { } /** Creates a {@link VKey} instance for this entity. */ + @SuppressWarnings("unchecked") public VKey createVKey() { - return VKey.create( - DomainHistory.class, new DomainHistoryId(getDomainRepoId(), getId()), Key.create(this)); + return (VKey) createVKey(Key.create(this)); } @PostLoad 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 3c52666f1..984a4d958 100644 --- a/core/src/main/java/google/registry/model/host/HostHistory.java +++ b/core/src/main/java/google/registry/model/host/HostHistory.java @@ -96,9 +96,9 @@ public class HostHistory extends HistoryEntry implements SqlEntity { } /** Creates a {@link VKey} instance for this entity. */ + @SuppressWarnings("unchecked") public VKey createVKey() { - return VKey.create( - HostHistory.class, new HostHistoryId(getHostRepoId(), getId()), Key.create(this)); + return (VKey) createVKey(Key.create(this)); } @PostLoad @@ -117,7 +117,7 @@ public class HostHistory extends HistoryEntry implements SqlEntity { } /** Class to represent the composite primary key of {@link HostHistory} entity. */ - static class HostHistoryId extends ImmutableObject implements Serializable { + public static class HostHistoryId extends ImmutableObject implements Serializable { private String hostRepoId; @@ -126,7 +126,7 @@ public class HostHistory extends HistoryEntry implements SqlEntity { /** Hibernate requires this default constructor. */ private HostHistoryId() {} - HostHistoryId(String hostRepoId, long id) { + public HostHistoryId(String hostRepoId, long id) { this.hostRepoId = hostRepoId; this.id = id; } diff --git a/core/src/main/java/google/registry/model/index/EppResourceIndex.java b/core/src/main/java/google/registry/model/index/EppResourceIndex.java index b9f21a6a6..21f692ffc 100644 --- a/core/src/main/java/google/registry/model/index/EppResourceIndex.java +++ b/core/src/main/java/google/registry/model/index/EppResourceIndex.java @@ -17,7 +17,6 @@ package google.registry.model.index; import static google.registry.util.TypeUtils.instantiate; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; @@ -26,25 +25,21 @@ import com.googlecode.objectify.annotation.Parent; import google.registry.model.BackupGroupRoot; import google.registry.model.EppResource; import google.registry.model.annotations.ReportedOn; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; /** An index that allows for quick enumeration of all EppResource entities (e.g. via map reduce). */ @ReportedOn @Entity -public class EppResourceIndex extends BackupGroupRoot implements DatastoreEntity { +public class EppResourceIndex extends BackupGroupRoot implements DatastoreOnlyEntity { - @Id - String id; + @Id String id; - @Parent - Key bucket; + @Parent Key bucket; /** Although this field holds a {@link Key} it is named "reference" for historical reasons. */ Key reference; - @Index - String kind; + @Index String kind; public String getId() { return id; @@ -69,7 +64,7 @@ public class EppResourceIndex extends BackupGroupRoot implements DatastoreEntity EppResourceIndex instance = instantiate(EppResourceIndex.class); instance.reference = resourceKey; instance.kind = resourceKey.getKind(); - instance.id = resourceKey.getString(); // creates a web-safe key string + instance.id = resourceKey.getString(); // creates a web-safe key string instance.bucket = bucket; return instance; } @@ -77,9 +72,4 @@ public class EppResourceIndex extends BackupGroupRoot implements DatastoreEntity public static EppResourceIndex create(Key resourceKey) { return create(EppResourceIndexBucket.getBucketKey(resourceKey), resourceKey); } - - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not relevant in SQL - } } diff --git a/core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java b/core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java index c2fc76bde..31d9665b5 100644 --- a/core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java +++ b/core/src/main/java/google/registry/model/index/EppResourceIndexBucket.java @@ -24,23 +24,17 @@ import com.googlecode.objectify.annotation.Id; import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.annotations.VirtualEntity; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; /** A virtual entity to represent buckets to which EppResourceIndex objects are randomly added. */ @Entity @VirtualEntity -public class EppResourceIndexBucket extends ImmutableObject implements DatastoreEntity { +public class EppResourceIndexBucket extends ImmutableObject implements DatastoreOnlyEntity { @SuppressWarnings("unused") @Id private long bucketId; - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not relevant in SQL - } - /** * Deterministic function that returns a bucket id based on the resource's roid. * NB: At the moment, nothing depends on this being deterministic, so we have the ability to diff --git a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java index 2fd12ff2a..ed82b5036 100644 --- a/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java +++ b/core/src/main/java/google/registry/model/index/ForeignKeyIndex.java @@ -42,8 +42,7 @@ import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.host.HostResource; import google.registry.persistence.VKey; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import google.registry.util.NonFinalForTesting; import java.util.Map; import java.util.Optional; @@ -63,34 +62,19 @@ public abstract class ForeignKeyIndex extends BackupGroup @ReportedOn @Entity public static class ForeignKeyContactIndex extends ForeignKeyIndex - implements DatastoreEntity { - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not relevant in SQL - } - } + implements DatastoreOnlyEntity {} /** The {@link ForeignKeyIndex} type for {@link DomainBase} entities. */ @ReportedOn @Entity public static class ForeignKeyDomainIndex extends ForeignKeyIndex - implements DatastoreEntity { - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not relevant in SQL - } - } + implements DatastoreOnlyEntity {} /** The {@link ForeignKeyIndex} type for {@link HostResource} entities. */ @ReportedOn @Entity public static class ForeignKeyHostIndex extends ForeignKeyIndex - implements DatastoreEntity { - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not relevant in SQL - } - } + implements DatastoreOnlyEntity {} static final ImmutableMap, Class>> RESOURCE_CLASS_TO_FKI_CLASS = diff --git a/core/src/main/java/google/registry/model/ofy/CommitLogBucket.java b/core/src/main/java/google/registry/model/ofy/CommitLogBucket.java index 418188761..3a32b8fb6 100644 --- a/core/src/main/java/google/registry/model/ofy/CommitLogBucket.java +++ b/core/src/main/java/google/registry/model/ofy/CommitLogBucket.java @@ -22,7 +22,6 @@ import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.collect.ContiguousSet; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Range; @@ -34,8 +33,7 @@ import google.registry.model.Buildable; import google.registry.model.ImmutableObject; import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import google.registry.util.NonFinalForTesting; import java.util.Random; import java.util.function.Supplier; @@ -53,7 +51,7 @@ import org.joda.time.DateTime; */ @Entity @NotBackedUp(reason = Reason.COMMIT_LOGS) -public class CommitLogBucket extends ImmutableObject implements Buildable, DatastoreEntity { +public class CommitLogBucket extends ImmutableObject implements Buildable, DatastoreOnlyEntity { /** * Ranges from 1 to {@link RegistryConfig#getCommitLogBucketCount()}, inclusive; starts at 1 since @@ -72,11 +70,6 @@ public class CommitLogBucket extends ImmutableObject implements Buildable, Datas return lastWrittenTime; } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } - /** * Returns the key for the specified bucket ID. * diff --git a/core/src/main/java/google/registry/model/ofy/CommitLogCheckpoint.java b/core/src/main/java/google/registry/model/ofy/CommitLogCheckpoint.java index ec863b511..8454fe782 100644 --- a/core/src/main/java/google/registry/model/ofy/CommitLogCheckpoint.java +++ b/core/src/main/java/google/registry/model/ofy/CommitLogCheckpoint.java @@ -27,8 +27,7 @@ import com.googlecode.objectify.annotation.Parent; import google.registry.model.ImmutableObject; import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -46,7 +45,7 @@ import org.joda.time.DateTime; */ @Entity @NotBackedUp(reason = Reason.COMMIT_LOGS) -public class CommitLogCheckpoint extends ImmutableObject implements DatastoreEntity { +public class CommitLogCheckpoint extends ImmutableObject implements DatastoreOnlyEntity { /** Shared singleton parent entity for commit log checkpoints. */ @Parent @@ -73,11 +72,6 @@ public class CommitLogCheckpoint extends ImmutableObject implements DatastoreEnt return builder.build(); } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } - /** * Creates a CommitLogCheckpoint for the given wall time and bucket checkpoint times, specified as * a map from bucket ID to bucket commit timestamp. diff --git a/core/src/main/java/google/registry/model/ofy/CommitLogCheckpointRoot.java b/core/src/main/java/google/registry/model/ofy/CommitLogCheckpointRoot.java index c07dc16b0..11e4e4628 100644 --- a/core/src/main/java/google/registry/model/ofy/CommitLogCheckpointRoot.java +++ b/core/src/main/java/google/registry/model/ofy/CommitLogCheckpointRoot.java @@ -17,21 +17,19 @@ package google.registry.model.ofy; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.util.DateTimeUtils.START_OF_TIME; -import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; import google.registry.model.ImmutableObject; import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import org.joda.time.DateTime; /** Singleton parent entity for all commit log checkpoints. */ @Entity @NotBackedUp(reason = Reason.COMMIT_LOGS) -public class CommitLogCheckpointRoot extends ImmutableObject implements DatastoreEntity { +public class CommitLogCheckpointRoot extends ImmutableObject implements DatastoreOnlyEntity { public static final long SINGLETON_ID = 1; // There is always exactly one of these. @@ -50,11 +48,6 @@ public class CommitLogCheckpointRoot extends ImmutableObject implements Datastor return lastWrittenTime; } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } - public static CommitLogCheckpointRoot loadRoot() { CommitLogCheckpointRoot root = ofy().load().key(getKey()).now(); return root == null ? new CommitLogCheckpointRoot() : root; diff --git a/core/src/main/java/google/registry/model/ofy/CommitLogManifest.java b/core/src/main/java/google/registry/model/ofy/CommitLogManifest.java index 5bc07ed8f..4b2443d25 100644 --- a/core/src/main/java/google/registry/model/ofy/CommitLogManifest.java +++ b/core/src/main/java/google/registry/model/ofy/CommitLogManifest.java @@ -17,7 +17,6 @@ package google.registry.model.ofy; import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy; import static org.joda.time.DateTimeZone.UTC; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; @@ -26,8 +25,7 @@ import com.googlecode.objectify.annotation.Parent; import google.registry.model.ImmutableObject; import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import java.util.LinkedHashSet; import java.util.Set; import org.joda.time.DateTime; @@ -41,7 +39,7 @@ import org.joda.time.DateTime; */ @Entity @NotBackedUp(reason = Reason.COMMIT_LOGS) -public class CommitLogManifest extends ImmutableObject implements DatastoreEntity { +public class CommitLogManifest extends ImmutableObject implements DatastoreOnlyEntity { /** Commit log manifests are parented on a random bucket. */ @Parent @@ -70,11 +68,6 @@ public class CommitLogManifest extends ImmutableObject implements DatastoreEntit return nullToEmptyImmutableCopy(deletions); } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } - public static CommitLogManifest create( Key parent, DateTime commitTime, Set> deletions) { CommitLogManifest instance = new CommitLogManifest(); diff --git a/core/src/main/java/google/registry/model/ofy/CommitLogMutation.java b/core/src/main/java/google/registry/model/ofy/CommitLogMutation.java index 61942b073..95c59805d 100644 --- a/core/src/main/java/google/registry/model/ofy/CommitLogMutation.java +++ b/core/src/main/java/google/registry/model/ofy/CommitLogMutation.java @@ -21,7 +21,6 @@ import static google.registry.model.ofy.ObjectifyService.ofy; import com.google.appengine.api.datastore.KeyFactory; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; @@ -29,13 +28,12 @@ import com.googlecode.objectify.annotation.Parent; import google.registry.model.ImmutableObject; import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; /** Representation of a saved entity in a {@link CommitLogManifest} (not deletes). */ @Entity @NotBackedUp(reason = Reason.COMMIT_LOGS) -public class CommitLogMutation extends ImmutableObject implements DatastoreEntity { +public class CommitLogMutation extends ImmutableObject implements DatastoreOnlyEntity { /** The manifest this belongs to. */ @Parent @@ -61,11 +59,6 @@ public class CommitLogMutation extends ImmutableObject implements DatastoreEntit return createFromPbBytes(entityProtoBytes); } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } - /** * Returns a new mutation entity created from an @Entity ImmutableObject instance. * diff --git a/core/src/main/java/google/registry/model/registrar/Registrar.java b/core/src/main/java/google/registry/model/registrar/Registrar.java index f8a21cd1f..f00f4be1e 100644 --- a/core/src/main/java/google/registry/model/registrar/Registrar.java +++ b/core/src/main/java/google/registry/model/registrar/Registrar.java @@ -121,33 +121,34 @@ public class Registrar extends ImmutableObject REAL(Objects::nonNull), /** - * A registrar account used by a real third-party registrar undergoing operational testing - * and evaluation. Should only be created in sandbox, and should have null IANA/billing IDs. + * A registrar account used by a real third-party registrar undergoing operational testing and + * evaluation. Should only be created in sandbox, and should have null IANA/billing IDs. */ OTE(Objects::isNull), /** - * A registrar used for predelegation testing. Should have a null billing ID. The IANA ID - * should be either 9995 or 9996, which are reserved for predelegation testing. + * A registrar used for predelegation testing. Should have a null billing ID. The IANA ID should + * be either 9995 or 9996, which are reserved for predelegation testing. */ PDT(n -> ImmutableSet.of(9995L, 9996L).contains(n)), /** - * A registrar used for external monitoring by ICANN. Should have IANA ID 9997 and a null + * A registrar used for external monitoring by ICANN. Should have IANA ID 9997 and a null * billing ID. */ EXTERNAL_MONITORING(isEqual(9997L)), /** - * A registrar used for when the registry acts as a registrar. Must have either IANA ID - * 9998 (for billable transactions) or 9999 (for non-billable transactions). */ + * A registrar used for when the registry acts as a registrar. Must have either IANA ID 9998 + * (for billable transactions) or 9999 (for non-billable transactions). + */ // TODO(b/13786188): determine what billing ID for this should be, if any. INTERNAL(n -> ImmutableSet.of(9998L, 9999L).contains(n)), - /** A registrar used for internal monitoring. Should have null IANA/billing IDs. */ + /** A registrar used for internal monitoring. Should have null IANA/billing IDs. */ MONITORING(Objects::isNull), - /** A registrar used for internal testing. Should have null IANA/billing IDs. */ + /** A registrar used for internal testing. Should have null IANA/billing IDs. */ TEST(Objects::isNull); /** @@ -225,10 +226,7 @@ public class Registrar extends ImmutableObject */ private static final Supplier> CACHE_BY_CLIENT_ID = memoizeWithShortExpiration( - () -> - tm() - .doTransactionless( - () -> Maps.uniqueIndex(loadAll(), Registrar::getClientId))); + () -> tm().doTransactionless(() -> Maps.uniqueIndex(loadAll(), Registrar::getClientId))); @Parent @Transient Key parent = getCrossTldKey(); @@ -383,12 +381,10 @@ public class Registrar extends ImmutableObject @Index @Nullable Long ianaIdentifier; /** Identifier of registrar used in external billing system (e.g. Oracle). */ - @Nullable - Long billingIdentifier; + @Nullable Long billingIdentifier; /** Purchase Order number used for invoices in external billing system, if applicable. */ - @Nullable - String poNumber; + @Nullable String poNumber; /** * Map of currency-to-billing account for the registrar. @@ -498,9 +494,7 @@ public class Registrar extends ImmutableObject if (billingAccountMap == null) { return ImmutableMap.of(); } - return billingAccountMap - .entrySet() - .stream() + return billingAccountMap.entrySet().stream() .collect(toImmutableSortedMap(natural(), Map.Entry::getKey, v -> v.getValue().accountId)); } @@ -722,14 +716,18 @@ public class Registrar extends ImmutableObject /** Creates a {@link VKey} for this instance. */ public VKey createVKey() { - return VKey.create(Registrar.class, clientIdentifier, Key.create(this)); + return createVKey(Key.create(this)); } /** Creates a {@link VKey} for the given {@code registrarId}. */ public static VKey createVKey(String registrarId) { checkArgumentNotNull(registrarId, "registrarId must be specified"); - return VKey.create( - Registrar.class, registrarId, Key.create(getCrossTldKey(), Registrar.class, registrarId)); + return createVKey(Key.create(getCrossTldKey(), Registrar.class, registrarId)); + } + + /** Creates a {@link VKey} instance from a {@link Key} instance. */ + public static VKey createVKey(Key key) { + return VKey.create(Registrar.class, key.getName(), key); } /** A builder for constructing {@link Registrar}, since it is immutable. */ @@ -744,21 +742,22 @@ public class Registrar extends ImmutableObject // Client id must be [3,16] chars long. See "clIDType" in the base EPP schema of RFC 5730. // (Need to validate this here as there's no matching EPP XSD for validation.) checkArgument( - Range.closed(3, 16).contains(clientId.length()), + Range.closed(3, 16).contains(clientId.length()), "Client identifier must be 3-16 characters long."); getInstance().clientIdentifier = clientId; return this; } public Builder setIanaIdentifier(@Nullable Long ianaIdentifier) { - checkArgument(ianaIdentifier == null || ianaIdentifier > 0, - "IANA ID must be a positive number"); + checkArgument( + ianaIdentifier == null || ianaIdentifier > 0, "IANA ID must be a positive number"); getInstance().ianaIdentifier = ianaIdentifier; return this; } public Builder setBillingIdentifier(@Nullable Long billingIdentifier) { - checkArgument(billingIdentifier == null || billingIdentifier > 0, + checkArgument( + billingIdentifier == null || billingIdentifier > 0, "Billing ID must be a positive number"); getInstance().billingIdentifier = billingIdentifier; return this; @@ -774,9 +773,7 @@ public class Registrar extends ImmutableObject getInstance().billingAccountMap = null; } else { getInstance().billingAccountMap = - billingAccountMap - .entrySet() - .stream() + billingAccountMap.entrySet().stream() .collect(toImmutableMap(Map.Entry::getKey, BillingAccountEntry::new)); } return this; @@ -900,16 +897,12 @@ public class Registrar extends ImmutableObject } public Builder setPhoneNumber(String phoneNumber) { - getInstance().phoneNumber = (phoneNumber == null) - ? null - : checkValidPhoneNumber(phoneNumber); + getInstance().phoneNumber = (phoneNumber == null) ? null : checkValidPhoneNumber(phoneNumber); return this; } public Builder setFaxNumber(String faxNumber) { - getInstance().faxNumber = (faxNumber == null) - ? null - : checkValidPhoneNumber(faxNumber); + getInstance().faxNumber = (faxNumber == null) ? null : checkValidPhoneNumber(faxNumber); return this; } @@ -944,7 +937,8 @@ public class Registrar extends ImmutableObject } public Builder setDriveFolderId(@Nullable String driveFolderId) { - checkArgument(driveFolderId == null || !driveFolderId.contains("/"), + checkArgument( + driveFolderId == null || !driveFolderId.contains("/"), "Drive folder ID must not be a full URL"); getInstance().driveFolderId = driveFolderId; return this; @@ -962,9 +956,10 @@ public class Registrar extends ImmutableObject /** @throws IllegalArgumentException if provided passcode is not 5-digit numeric */ public Builder setPhonePasscode(String phonePasscode) { - checkArgument(phonePasscode == null - || PHONE_PASSCODE_PATTERN.matcher(phonePasscode).matches(), - "Not a valid telephone passcode (must be 5 digits long): %s", phonePasscode); + checkArgument( + phonePasscode == null || PHONE_PASSCODE_PATTERN.matcher(phonePasscode).matches(), + "Not a valid telephone passcode (must be 5 digits long): %s", + phonePasscode); getInstance().phonePasscode = phonePasscode; return this; } @@ -982,9 +977,11 @@ public class Registrar extends ImmutableObject checkArgument( getInstance().localizedAddress != null || getInstance().internationalizedAddress != null, "Must specify at least one of localized or internationalized address"); - checkArgument(getInstance().type.isValidIanaId(getInstance().ianaIdentifier), - String.format("Supplied IANA ID is not valid for %s registrar type: %s", - getInstance().type, getInstance().ianaIdentifier)); + checkArgument( + getInstance().type.isValidIanaId(getInstance().ianaIdentifier), + String.format( + "Supplied IANA ID is not valid for %s registrar type: %s", + getInstance().type, getInstance().ianaIdentifier)); return cloneEmptyToNull(super.build()); } } diff --git a/core/src/main/java/google/registry/model/registrar/RegistrarContact.java b/core/src/main/java/google/registry/model/registrar/RegistrarContact.java index ff1045e89..f27cc8fdb 100644 --- a/core/src/main/java/google/registry/model/registrar/RegistrarContact.java +++ b/core/src/main/java/google/registry/model/registrar/RegistrarContact.java @@ -119,9 +119,7 @@ public class RegistrarContact extends ImmutableObject String name; /** The email address of the contact. */ - @Id - @javax.persistence.Id - String emailAddress; + @Id @javax.persistence.Id String emailAddress; @Ignore @javax.persistence.Id String registrarId; @@ -147,8 +145,7 @@ public class RegistrarContact extends ImmutableObject * * @see com.google.appengine.api.users.User#getUserId() */ - @Index - String gaeUserId; + @Index String gaeUserId; /** * Whether this contact is publicly visible in WHOIS registrar query results as an Admin contact. @@ -202,8 +199,7 @@ public class RegistrarContact extends ImmutableObject */ public static void updateContacts( final Registrar registrar, final Set contacts) { - tm() - .transact( + tm().transact( () -> { ofy() .delete() @@ -364,8 +360,15 @@ public class RegistrarContact extends ImmutableObject } public VKey createVKey() { - return VKey.create( - RegistrarContact.class, new RegistrarPocId(emailAddress, registrarId), Key.create(this)); + return createVKey(Key.create(this)); + } + + /** Creates a {@link VKey} instance from a {@link Key} instance. */ + public static VKey createVKey(Key key) { + Key parent = key.getParent(); + String registrarId = parent.getName(); + String emailAddress = key.getName(); + return VKey.create(RegistrarContact.class, new RegistrarPocId(emailAddress, registrarId), key); } /** Class to represent the composite primary key for {@link RegistrarContact} entity. */ diff --git a/core/src/main/java/google/registry/model/registry/label/PremiumList.java b/core/src/main/java/google/registry/model/registry/label/PremiumList.java index 2dc55b41f..c3be24cc2 100644 --- a/core/src/main/java/google/registry/model/registry/label/PremiumList.java +++ b/core/src/main/java/google/registry/model/registry/label/PremiumList.java @@ -32,7 +32,6 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.CacheLoader.InvalidCacheLoadException; import com.google.common.cache.LoadingCache; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.hash.BloomFilter; import com.google.common.util.concurrent.UncheckedExecutionException; @@ -45,9 +44,8 @@ import google.registry.model.Buildable; import google.registry.model.ImmutableObject; import google.registry.model.annotations.ReportedOn; import google.registry.model.registry.Registry; -import google.registry.schema.replay.DatastoreEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import google.registry.schema.replay.NonReplicatedEntity; -import google.registry.schema.replay.SqlEntity; import google.registry.schema.tld.PremiumListDao; import google.registry.util.NonFinalForTesting; import java.io.ByteArrayOutputStream; @@ -114,7 +112,7 @@ public final class PremiumList extends BaseDomainLabelList parent; @@ -171,11 +169,6 @@ public final class PremiumList extends BaseDomainLabelList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } } /** @@ -332,7 +325,7 @@ public final class PremiumList extends BaseDomainLabelList - implements Buildable, DatastoreEntity { + implements Buildable, DatastoreOnlyEntity { @Parent Key parent; @@ -349,11 +342,6 @@ public final class PremiumList extends BaseDomainLabelList toSqlEntities() { - return ImmutableList.of(); - } - /** A builder for constructing {@link PremiumListEntry} objects, since they are immutable. */ public static class Builder extends DomainLabelEntry.Builder { diff --git a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java index b612c4c2d..c5e38912f 100644 --- a/core/src/main/java/google/registry/model/reporting/HistoryEntry.java +++ b/core/src/main/java/google/registry/model/reporting/HistoryEntry.java @@ -32,13 +32,17 @@ import google.registry.model.EppResource; import google.registry.model.ImmutableObject; import google.registry.model.annotations.ReportedOn; import google.registry.model.contact.ContactHistory; +import google.registry.model.contact.ContactHistory.ContactHistoryId; import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.domain.DomainHistory; +import google.registry.model.domain.DomainHistory.DomainHistoryId; import google.registry.model.domain.Period; import google.registry.model.eppcommon.Trid; import google.registry.model.host.HostHistory; +import google.registry.model.host.HostHistory.HostHistoryId; import google.registry.model.host.HostResource; +import google.registry.persistence.VKey; import google.registry.schema.replay.DatastoreEntity; import google.registry.schema.replay.SqlEntity; import java.util.Set; @@ -314,6 +318,33 @@ public class HistoryEntry extends ImmutableObject implements Buildable, Datastor return ImmutableList.of((SqlEntity) toChildHistoryEntity()); } + /** Creates a {@link VKey} instance from a {@link Key} instance. */ + public static VKey createVKey(Key key) { + String repoId = key.getParent().getName(); + long id = key.getId(); + Key parent = key.getParent(); + String parentKind = parent.getKind(); + if (parentKind.equals(getKind(DomainBase.class))) { + return VKey.create( + DomainHistory.class, + new DomainHistoryId(repoId, id), + Key.create(parent, DomainHistory.class, id)); + } else if (parentKind.equals(getKind(HostResource.class))) { + return VKey.create( + HostHistory.class, + new HostHistoryId(repoId, id), + Key.create(parent, HostHistory.class, id)); + } else if (parentKind.equals(getKind(ContactResource.class))) { + return VKey.create( + ContactHistory.class, + new ContactHistoryId(repoId, id), + Key.create(parent, ContactHistory.class, id)); + } else { + throw new IllegalStateException( + String.format("Unknown kind of HistoryEntry parent %s", parentKind)); + } + } + /** A builder for {@link HistoryEntry} since it is immutable */ public static class Builder> extends GenericBuilder { diff --git a/core/src/main/java/google/registry/model/server/KmsSecret.java b/core/src/main/java/google/registry/model/server/KmsSecret.java index 25ce85c0e..862097d6f 100644 --- a/core/src/main/java/google/registry/model/server/KmsSecret.java +++ b/core/src/main/java/google/registry/model/server/KmsSecret.java @@ -16,7 +16,6 @@ package google.registry.model.server; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; -import com.google.common.collect.ImmutableList; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; @@ -24,13 +23,12 @@ import com.googlecode.objectify.annotation.Parent; import google.registry.model.ImmutableObject; import google.registry.model.annotations.ReportedOn; import google.registry.model.common.EntityGroupRoot; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; /** Pointer to the latest {@link KmsSecretRevision}. */ @Entity @ReportedOn -public class KmsSecret extends ImmutableObject implements DatastoreEntity { +public class KmsSecret extends ImmutableObject implements DatastoreOnlyEntity { /** The unique name of this {@link KmsSecret}. */ @Id String name; @@ -48,11 +46,6 @@ public class KmsSecret extends ImmutableObject implements DatastoreEntity { return latestRevision; } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // not persisted in SQL - } - public static KmsSecret create(String name, KmsSecretRevision latestRevision) { KmsSecret instance = new KmsSecret(); instance.name = name; diff --git a/core/src/main/java/google/registry/model/server/Lock.java b/core/src/main/java/google/registry/model/server/Lock.java index 962a93319..a6c6d7a17 100644 --- a/core/src/main/java/google/registry/model/server/Lock.java +++ b/core/src/main/java/google/registry/model/server/Lock.java @@ -22,15 +22,13 @@ import static google.registry.util.DateTimeUtils.isAtOrAfter; import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; -import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; import google.registry.model.ImmutableObject; import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; import google.registry.util.RequestStatusChecker; import google.registry.util.RequestStatusCheckerImpl; import java.io.Serializable; @@ -50,7 +48,7 @@ import org.joda.time.Duration; */ @Entity @NotBackedUp(reason = Reason.TRANSIENT) -public class Lock extends ImmutableObject implements DatastoreEntity, Serializable { +public class Lock extends ImmutableObject implements DatastoreOnlyEntity, Serializable { private static final long serialVersionUID = 756397280691684645L; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -259,9 +257,4 @@ public class Lock extends ImmutableObject implements DatastoreEntity, Serializab } }); } - - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // Locks are not converted since they are ephemeral - } } diff --git a/core/src/main/java/google/registry/model/server/ServerSecret.java b/core/src/main/java/google/registry/model/server/ServerSecret.java index 290dddf28..e7d5ffe09 100644 --- a/core/src/main/java/google/registry/model/server/ServerSecret.java +++ b/core/src/main/java/google/registry/model/server/ServerSecret.java @@ -23,7 +23,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; -import com.google.common.collect.ImmutableList; import com.google.common.primitives.Longs; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; @@ -34,8 +33,7 @@ import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; import google.registry.model.common.CrossTldSingleton; import google.registry.persistence.VKey; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.NonReplicatedEntity; import java.nio.ByteBuffer; import java.util.UUID; import java.util.concurrent.ExecutionException; @@ -50,7 +48,7 @@ import javax.persistence.Transient; @Unindex @NotBackedUp(reason = Reason.AUTO_GENERATED) // TODO(b/27427316): Replace this with an entry in KMSKeyring -public class ServerSecret extends CrossTldSingleton implements DatastoreEntity, SqlEntity { +public class ServerSecret extends CrossTldSingleton implements NonReplicatedEntity { /** * Cache of the singleton ServerSecret instance that creates it if not present. @@ -139,16 +137,6 @@ public class ServerSecret extends CrossTldSingleton implements DatastoreEntity, .array(); } - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // dually-written - } - - @Override - public ImmutableList toDatastoreEntities() { - return ImmutableList.of(); // dually-written - } - @VisibleForTesting static void resetCache() { CACHE.invalidateAll(); diff --git a/core/src/main/java/google/registry/model/tmch/ClaimsListShard.java b/core/src/main/java/google/registry/model/tmch/ClaimsListShard.java index c1665e9a2..c17247f39 100644 --- a/core/src/main/java/google/registry/model/tmch/ClaimsListShard.java +++ b/core/src/main/java/google/registry/model/tmch/ClaimsListShard.java @@ -26,7 +26,6 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Supplier; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.MapDifference; import com.google.common.collect.MapDifference.ValueDifference; @@ -46,9 +45,8 @@ import google.registry.model.annotations.NotBackedUp; import google.registry.model.annotations.NotBackedUp.Reason; import google.registry.model.annotations.VirtualEntity; import google.registry.model.common.CrossTldSingleton; -import google.registry.schema.replay.DatastoreAndSqlEntity; -import google.registry.schema.replay.DatastoreEntity; -import google.registry.schema.replay.SqlEntity; +import google.registry.schema.replay.DatastoreOnlyEntity; +import google.registry.schema.replay.NonReplicatedEntity; import google.registry.util.CollectionUtils; import google.registry.util.Concurrent; import google.registry.util.Retrier; @@ -97,7 +95,7 @@ import org.joda.time.DateTime; @NotBackedUp(reason = Reason.EXTERNALLY_SOURCED) @javax.persistence.Entity(name = "ClaimsList") @Table -public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlEntity { +public class ClaimsListShard extends ImmutableObject implements NonReplicatedEntity { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -146,63 +144,62 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE private static final Retrier LOADER_RETRIER = new Retrier(new SystemSleeper(), 2); private static ClaimsListShard loadClaimsListShard() { - // Find the most recent revision. - Key revisionKey = getCurrentRevision(); + // Find the most recent revision. + Key revisionKey = getCurrentRevision(); - Map combinedLabelsToKeys = new HashMap<>(); - DateTime creationTime = START_OF_TIME; - if (revisionKey != null) { - // Grab all of the keys for the shards that belong to the current revision. - final List> shardKeys = - ofy().load().type(ClaimsListShard.class).ancestor(revisionKey).keys().list(); + Map combinedLabelsToKeys = new HashMap<>(); + DateTime creationTime = START_OF_TIME; + if (revisionKey != null) { + // Grab all of the keys for the shards that belong to the current revision. + final List> shardKeys = + ofy().load().type(ClaimsListShard.class).ancestor(revisionKey).keys().list(); - List shards; - try { - // Load all of the shards concurrently, each in a separate transaction. - shards = - Concurrent.transform( - shardKeys, - key -> - tm().transactNewReadOnly( - () -> { - ClaimsListShard claimsListShard = ofy().load().key(key).now(); - checkState( - claimsListShard != null, - "Key not found when loading claims list shards."); - return claimsListShard; - })); - } catch (UncheckedExecutionException e) { - // We retry on IllegalStateException. However, there's a checkState inside the - // Concurrent.transform, so if it's thrown it'll be wrapped in an - // UncheckedExecutionException. We want to unwrap it so it's caught by the retrier. - if (e.getCause() != null) { - throwIfUnchecked(e.getCause()); - } - throw e; - } - - // Combine the shards together and return the concatenated ClaimsList. - if (!shards.isEmpty()) { - creationTime = shards.get(0).creationTime; - for (ClaimsListShard shard : shards) { - combinedLabelsToKeys.putAll(shard.labelsToKeys); - checkState( - creationTime.equals(shard.creationTime), - "Inconsistent claims list shard creation times."); - } - } + List shards; + try { + // Load all of the shards concurrently, each in a separate transaction. + shards = + Concurrent.transform( + shardKeys, + key -> + tm().transactNewReadOnly( + () -> { + ClaimsListShard claimsListShard = ofy().load().key(key).now(); + checkState( + claimsListShard != null, + "Key not found when loading claims list shards."); + return claimsListShard; + })); + } catch (UncheckedExecutionException e) { + // We retry on IllegalStateException. However, there's a checkState inside the + // Concurrent.transform, so if it's thrown it'll be wrapped in an + // UncheckedExecutionException. We want to unwrap it so it's caught by the retrier. + if (e.getCause() != null) { + throwIfUnchecked(e.getCause()); } + throw e; + } - ClaimsListShard datastoreList = - create(creationTime, ImmutableMap.copyOf(combinedLabelsToKeys)); - // Also load the list from Cloud SQL, compare the two lists, and log if different. - try { - loadAndCompareCloudSqlList(datastoreList); - } catch (Throwable t) { - logger.atSevere().withCause(t).log("Error comparing claims lists."); + // Combine the shards together and return the concatenated ClaimsList. + if (!shards.isEmpty()) { + creationTime = shards.get(0).creationTime; + for (ClaimsListShard shard : shards) { + combinedLabelsToKeys.putAll(shard.labelsToKeys); + checkState( + creationTime.equals(shard.creationTime), + "Inconsistent claims list shard creation times."); } - return datastoreList; - }; + } + } + + ClaimsListShard datastoreList = create(creationTime, ImmutableMap.copyOf(combinedLabelsToKeys)); + // Also load the list from Cloud SQL, compare the two lists, and log if different. + try { + loadAndCompareCloudSqlList(datastoreList); + } catch (Throwable t) { + logger.atSevere().withCause(t).log("Error comparing claims lists."); + } + return datastoreList; + }; private static void loadAndCompareCloudSqlList(ClaimsListShard datastoreList) { Optional maybeCloudSqlList = ClaimsListDao.getLatestRevision(); @@ -304,8 +301,7 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE Concurrent.transform( CollectionUtils.partitionMap(labelsToKeys, shardSize), (final ImmutableMap labelsToKeysShard) -> - tm() - .transactNew( + tm().transactNew( () -> { ClaimsListShard shard = create(creationTime, labelsToKeysShard); shard.isShard = true; @@ -315,8 +311,7 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE })); // Persist the new revision, thus causing the newly created shards to go live. - tm() - .transactNew( + tm().transactNew( () -> { verify( (getCurrentRevision() == null && oldRevision == null) @@ -358,12 +353,10 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE /** Virtual parent entity for claims list shards of a specific revision. */ @Entity @VirtualEntity - public static class ClaimsListRevision extends ImmutableObject implements DatastoreEntity { - @Parent - Key parent; + public static class ClaimsListRevision extends ImmutableObject implements DatastoreOnlyEntity { + @Parent Key parent; - @Id - long versionId; + @Id long versionId; @VisibleForTesting public static Key createKey(ClaimsListSingleton singleton) { @@ -377,11 +370,6 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE public static Key createKey() { return createKey(new ClaimsListSingleton()); } - - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // ClaimsLists are dually written - } } /** @@ -390,7 +378,7 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE */ @Entity @NotBackedUp(reason = Reason.EXTERNALLY_SOURCED) - public static class ClaimsListSingleton extends CrossTldSingleton implements DatastoreEntity { + public static class ClaimsListSingleton extends CrossTldSingleton implements DatastoreOnlyEntity { Key activeRevision; static ClaimsListSingleton create(Key revision) { @@ -403,11 +391,6 @@ public class ClaimsListShard extends ImmutableObject implements DatastoreAndSqlE public void setActiveRevision(Key revision) { activeRevision = revision; } - - @Override - public ImmutableList toSqlEntities() { - return ImmutableList.of(); // ClaimsLists are dually written - } } /** diff --git a/core/src/main/java/google/registry/model/tmch/TmchCrl.java b/core/src/main/java/google/registry/model/tmch/TmchCrl.java index 07a2befa9..fb3a378a4 100644 --- a/core/src/main/java/google/registry/model/tmch/TmchCrl.java +++ b/core/src/main/java/google/registry/model/tmch/TmchCrl.java @@ -29,6 +29,7 @@ import google.registry.model.common.CrossTldSingleton; import google.registry.model.tmch.TmchCrl.TmchCrlId; import google.registry.persistence.VKey; import google.registry.schema.replay.DatastoreEntity; +import google.registry.schema.replay.NonReplicatedEntity; import google.registry.schema.replay.SqlEntity; import java.io.Serializable; import java.util.Optional; @@ -44,7 +45,7 @@ import org.joda.time.DateTime; @Immutable @NotBackedUp(reason = Reason.EXTERNALLY_SOURCED) @IdClass(TmchCrlId.class) -public final class TmchCrl extends CrossTldSingleton implements DatastoreEntity, SqlEntity { +public final class TmchCrl extends CrossTldSingleton implements NonReplicatedEntity { @Id String crl; diff --git a/core/src/main/java/google/registry/schema/replay/DatastoreOnlyEntity.java b/core/src/main/java/google/registry/schema/replay/DatastoreOnlyEntity.java new file mode 100644 index 000000000..f47a34345 --- /dev/null +++ b/core/src/main/java/google/registry/schema/replay/DatastoreOnlyEntity.java @@ -0,0 +1,24 @@ +// Copyright 2020 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.schema.replay; + +import com.google.common.collect.ImmutableList; + +public interface DatastoreOnlyEntity extends DatastoreEntity { + @Override + default ImmutableList toSqlEntities() { + return ImmutableList.of(); + } +} diff --git a/core/src/test/java/google/registry/beam/initsql/WriteToSqlTest.java b/core/src/test/java/google/registry/beam/initsql/WriteToSqlTest.java index e431f1f84..59e8e2bee 100644 --- a/core/src/test/java/google/registry/beam/initsql/WriteToSqlTest.java +++ b/core/src/test/java/google/registry/beam/initsql/WriteToSqlTest.java @@ -29,8 +29,8 @@ import google.registry.model.registrar.Registrar; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestExtension; import google.registry.testing.AppEngineExtension; -import google.registry.testing.DatastoreEntityExtension; import google.registry.testing.DatabaseHelper; +import google.registry.testing.DatastoreEntityExtension; import google.registry.testing.FakeClock; import google.registry.testing.InjectExtension; import java.io.Serializable; diff --git a/core/src/test/java/google/registry/schema/replay/EntityTest.java b/core/src/test/java/google/registry/schema/replay/EntityTest.java index 3f41a4301..06b433669 100644 --- a/core/src/test/java/google/registry/schema/replay/EntityTest.java +++ b/core/src/test/java/google/registry/schema/replay/EntityTest.java @@ -16,10 +16,16 @@ package google.registry.schema.replay; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import com.google.common.collect.ImmutableSet; +import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Embed; +import com.googlecode.objectify.annotation.Parent; +import google.registry.model.ModelUtils; import google.registry.model.common.GaeUserIdConverter; +import google.registry.persistence.VKey; +import google.registry.testing.DatastoreEntityExtension; import io.github.classgraph.ClassGraph; import io.github.classgraph.ClassInfo; import io.github.classgraph.ClassInfoList; @@ -28,13 +34,18 @@ import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.lang.reflect.Method; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; /** * Test to verify classes implement {@link SqlEntity} and {@link DatastoreEntity} when they should. */ public class EntityTest { + @RegisterExtension + final DatastoreEntityExtension datastoreEntityExtension = new DatastoreEntityExtension(); + private static final ImmutableSet> NON_CONVERTED_CLASSES = ImmutableSet.of(GaeUserIdConverter.class); @@ -59,6 +70,47 @@ public class EntityTest { } } + @Test + void testDatastoreEntityVKeyCreation() { + // For replication, we need to be able to convert from Key -> VKey for the relevant classes. + // This means that the relevant classes must have non-composite Objectify keys or must have a + // createVKey method + try (ScanResult scanResult = + new ClassGraph().enableAnnotationInfo().whitelistPackages("google.registry").scan()) { + ImmutableSet> datastoreEntityClasses = + getClasses(scanResult.getClassesImplementing(DatastoreEntity.class.getName())); + // some classes aren't converted so they aren't relevant + ImmutableSet> vkeyConversionNecessaryClasses = + datastoreEntityClasses.stream() + .filter(clazz -> !DatastoreOnlyEntity.class.isAssignableFrom(clazz)) + .filter(clazz -> !NonReplicatedEntity.class.isAssignableFrom(clazz)) + .collect(toImmutableSet()); + + ImmutableSet.Builder> failedClasses = new ImmutableSet.Builder<>(); + for (Class clazz : vkeyConversionNecessaryClasses) { + if (hasKeyWithParent(clazz)) { + try { + Method createVKeyMethod = clazz.getMethod("createVKey", Key.class); + if (!createVKeyMethod.getReturnType().equals(VKey.class)) { + failedClasses.add(clazz); + } + } catch (NoSuchMethodException e) { + failedClasses.add(clazz); + } + } + } + assertWithMessage( + "Some DatastoreEntity classes with parents were missing createVKey methods: ") + .that(failedClasses.build()) + .isEmpty(); + } + } + + private boolean hasKeyWithParent(Class clazz) { + return ModelUtils.getAllFields(clazz).values().stream() + .anyMatch(field -> field.getAnnotation(Parent.class) != null); + } + private ImmutableSet getAllClassesWithAnnotation( ScanResult scanResult, String annotation) { ImmutableSet.Builder result = new ImmutableSet.Builder<>(); @@ -70,17 +122,20 @@ public class EntityTest { return result.build(); } - private ImmutableSet getClassNames(ClassInfoList classInfoList) { + private ImmutableSet> getClasses(ClassInfoList classInfoList) { return classInfoList.stream() .filter(ClassInfo::isStandardClass) .map(ClassInfo::loadClass) .filter(clazz -> !clazz.isAnnotationPresent(EntityForTesting.class)) .filter(clazz -> !clazz.isAnnotationPresent(Embed.class)) .filter(clazz -> !NON_CONVERTED_CLASSES.contains(clazz)) - .map(Class::getName) .collect(toImmutableSet()); } + private ImmutableSet getClassNames(ClassInfoList classInfoList) { + return getClasses(classInfoList).stream().map(Class::getName).collect(toImmutableSet()); + } + /** Entities that are solely used for testing, to avoid scanning them in {@link EntityTest}. */ @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME)