diff --git a/core/src/main/java/google/registry/model/EntityClasses.java b/core/src/main/java/google/registry/model/EntityClasses.java index 009c96ffb..7dd316b55 100644 --- a/core/src/main/java/google/registry/model/EntityClasses.java +++ b/core/src/main/java/google/registry/model/EntityClasses.java @@ -47,7 +47,6 @@ import google.registry.model.server.KmsSecret; import google.registry.model.server.KmsSecretRevision; import google.registry.model.server.Lock; import google.registry.model.server.ServerSecret; -import google.registry.model.smd.SignedMarkRevocationList; import google.registry.model.tmch.ClaimsListShard; import google.registry.model.tmch.ClaimsListShard.ClaimsListRevision; import google.registry.model.tmch.ClaimsListShard.ClaimsListSingleton; @@ -105,7 +104,6 @@ public final class EntityClasses { Registry.class, ReservedList.class, ServerSecret.class, - SignedMarkRevocationList.class, TmchCrl.class); private EntityClasses() {} diff --git a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationList.java b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationList.java index 644c2bd2b..b8d8f542e 100644 --- a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationList.java +++ b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationList.java @@ -16,34 +16,24 @@ package google.registry.model.smd; import static com.google.common.base.Preconditions.checkNotNull; import static google.registry.model.CacheUtils.memoizeWithShortExpiration; -import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.util.DateTimeUtils.isBeforeOrAt; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableMap; -import com.googlecode.objectify.Key; -import com.googlecode.objectify.annotation.EmbedMap; -import com.googlecode.objectify.annotation.Entity; -import com.googlecode.objectify.annotation.Id; -import com.googlecode.objectify.annotation.Ignore; -import com.googlecode.objectify.annotation.OnSave; -import com.googlecode.objectify.annotation.Parent; import google.registry.model.ImmutableObject; -import google.registry.model.annotations.InCrossTld; -import google.registry.model.annotations.NotBackedUp; -import google.registry.model.annotations.NotBackedUp.Reason; -import google.registry.model.common.EntityGroupRoot; -import google.registry.schema.replay.NonReplicatedEntity; +import google.registry.schema.replay.DatastoreEntity; +import google.registry.schema.replay.SqlEntity; import java.util.Map; +import java.util.Optional; import javax.persistence.CollectionTable; import javax.persistence.Column; import javax.persistence.ElementCollection; +import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; +import javax.persistence.Id; import javax.persistence.JoinColumn; import javax.persistence.MapKeyColumn; -import javax.persistence.Transient; import org.joda.time.DateTime; /** @@ -53,34 +43,14 @@ import org.joda.time.DateTime; * all the {@link SignedMark SignedMarks} that have been revoked. A new list is created for each new * file that's created, depending on the timestamp. * - *

We'll be putting the entire table into a single entity for the sake of performance. But in - * order to avoid exceeding the one megabyte max entity size limit, we'll also be sharding that - * entity into multiple entities, each entity containing {@value #SHARD_SIZE} rows. - * - *

TODO: We can remove the sharding once we have converted entirely to Cloud SQL storage during - * the Registry 3.0 migration. Then, the entire table will be stored conceptually as one entity (in - * fact in SignedMarkRevocationList and SignedMarkRevocationEntry tables). - * * @see google.registry.tmch.SmdrlCsvParser * @see TMCH * functional specifications - SMD Revocation List */ @Entity -@javax.persistence.Entity -@NotBackedUp(reason = Reason.EXTERNALLY_SOURCED) -@InCrossTld -public class SignedMarkRevocationList extends ImmutableObject implements NonReplicatedEntity { +public class SignedMarkRevocationList extends ImmutableObject implements SqlEntity { - @VisibleForTesting static final int SHARD_SIZE = 10000; - - /** Common ancestor for queries. */ - @Parent @Transient Key parent = getCrossTldKey(); - - /** ID for the sharded entity. */ - @Id @Transient long id; - - @Ignore - @javax.persistence.Id + @Id @GeneratedValue(strategy = GenerationType.IDENTITY) Long revisionId; @@ -88,7 +58,6 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl DateTime creationTime; /** A map from SMD IDs to revocation time. */ - @EmbedMap @ElementCollection @CollectionTable( name = "SignedMarkRevocationEntry", @@ -97,17 +66,10 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl @Column(name = "revocationTime", nullable = false) Map revokes; - /** Indicates that this is a shard rather than a "full" list. */ - @Ignore @Transient boolean isShard; - - /** - * A cached supplier that fetches the SMDRL shards from Datastore and recombines them into a - * single {@link SignedMarkRevocationList} object. - */ + /** A cached supplier that fetches the {@link SignedMarkRevocationList} object. */ private static final Supplier CACHE = memoizeWithShortExpiration(SignedMarkRevocationListDao::load); - /** Return a single logical instance that combines all Datastore shards. */ public static SignedMarkRevocationList get() { return CACHE.get(); } @@ -137,20 +99,14 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl return revokes.size(); } - /** Save this list to Datastore in sharded form and to Cloud SQL. Returns {@code this}. */ + /** Save this list to Cloud SQL. Returns {@code this}. */ public SignedMarkRevocationList save() { SignedMarkRevocationListDao.save(this); return this; } - /** As a safety mechanism, fail if someone tries to save this class directly. */ - @OnSave - void disallowUnshardedSaves() { - if (!isShard) { - throw new UnshardedSaveException(); - } + @Override + public Optional toDatastoreEntity() { + return Optional.empty(); // Not persisted in Datastore } - - /** Exception when trying to directly save a {@link SignedMarkRevocationList} without sharding. */ - public static class UnshardedSaveException extends RuntimeException {} } diff --git a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java index 9671d8666..07b160d9a 100644 --- a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java +++ b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java @@ -15,7 +15,6 @@ package google.registry.model.smd; import static com.google.common.truth.Truth.assertThat; -import static google.registry.model.smd.SignedMarkRevocationList.SHARD_SIZE; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.time.Duration.standardDays; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -31,14 +30,13 @@ import org.junit.jupiter.api.extension.RegisterExtension; public class SignedMarkRevocationListTest { @RegisterExtension - public final AppEngineExtension appEngine = - AppEngineExtension.builder().withDatastoreAndCloudSql().build(); + public final AppEngineExtension appEngine = AppEngineExtension.builder().withCloudSql().build(); private final FakeClock clock = new FakeClock(DateTime.parse("2013-01-01T00:00:00Z")); @Test void testEmpty() { - // When Datastore is empty, it should give us an empty thing. + // When Cloud SQL is empty, it should give us an empty thing. assertThat(SignedMarkRevocationList.get()) .isEqualTo(SignedMarkRevocationList.create(START_OF_TIME, ImmutableMap.of())); } @@ -65,7 +63,7 @@ public class SignedMarkRevocationListTest { @Test void test_isSmdRevoked_garbage() { - SignedMarkRevocationList smdrl = createSaveGetHelper(SHARD_SIZE + 1); + SignedMarkRevocationList smdrl = createSaveGetHelper(100); assertThat(smdrl.getCreationTime()).isEqualTo(clock.nowUtc()); assertThat(smdrl.isSmdRevoked("rofl", clock.nowUtc())).isFalse(); assertThat(smdrl.isSmdRevoked("31337", clock.nowUtc())).isFalse(); @@ -84,7 +82,7 @@ public class SignedMarkRevocationListTest { @Test void test_isSmdRevoked_present() { - final int rows = SHARD_SIZE + 1; + final int rows = 100; SignedMarkRevocationList smdrl = createSaveGetHelper(rows); assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isTrue(); assertThat(smdrl.isSmdRevoked(Integer.toString(rows - 1), clock.nowUtc())).isTrue(); @@ -93,7 +91,7 @@ public class SignedMarkRevocationListTest { @Test void test_isSmdRevoked_future() { - final int rows = SHARD_SIZE; + final int rows = 100; SignedMarkRevocationList smdrl = createSaveGetHelper(rows); clock.advanceOneMilli(); assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isTrue(); @@ -103,7 +101,7 @@ public class SignedMarkRevocationListTest { @Test void test_isSmdRevoked_past() { - final int rows = SHARD_SIZE; + final int rows = 100; SignedMarkRevocationList smdrl = createSaveGetHelper(rows); clock.setTo(clock.nowUtc().minusMillis(1)); assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isFalse(); diff --git a/core/src/test/java/google/registry/testing/AppEngineExtension.java b/core/src/test/java/google/registry/testing/AppEngineExtension.java index 49bad0839..7933b580c 100644 --- a/core/src/test/java/google/registry/testing/AppEngineExtension.java +++ b/core/src/test/java/google/registry/testing/AppEngineExtension.java @@ -172,6 +172,13 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa return this; } + /** Turns on Cloud SQL only, for use by test data generators. */ + public Builder withCloudSql() { + rule.withCloudSql = true; + rule.withDatastore = false; + return this; + } + /** Disables insertion of canned data. */ public Builder withoutCannedData() { rule.withoutCannedData = true; diff --git a/core/src/test/resources/google/registry/export/crosstld_kinds.txt b/core/src/test/resources/google/registry/export/crosstld_kinds.txt index 8ec4978f4..50a6fa3be 100644 --- a/core/src/test/resources/google/registry/export/crosstld_kinds.txt +++ b/core/src/test/resources/google/registry/export/crosstld_kinds.txt @@ -12,5 +12,4 @@ RegistrarContact Registry ReservedList ServerSecret -SignedMarkRevocationList TmchCrl diff --git a/core/src/test/resources/google/registry/model/schema.txt b/core/src/test/resources/google/registry/model/schema.txt index ace79d5eb..3d6ddc5f6 100644 --- a/core/src/test/resources/google/registry/model/schema.txt +++ b/core/src/test/resources/google/registry/model/schema.txt @@ -826,12 +826,6 @@ class google.registry.model.server.ServerSecret { long leastSignificant; long mostSignificant; } -class google.registry.model.smd.SignedMarkRevocationList { - @Id long id; - @Parent com.googlecode.objectify.Key parent; - java.util.Map revokes; - org.joda.time.DateTime creationTime; -} class google.registry.model.tmch.ClaimsListShard { @Id long id; @Parent com.googlecode.objectify.Key parent;