Remove SMDRL completely from Datastore (#1104)

* Remove SMDRL completely from Datastore

* Remove some unnecessary stuff

* Change row count to 10000

* Remove implement EntityTestCase
This commit is contained in:
sarahcaseybot 2021-04-26 17:15:50 -04:00 committed by GitHub
parent 367a38c5b0
commit 2528ee05dd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 25 additions and 73 deletions

View file

@ -47,7 +47,6 @@ import google.registry.model.server.KmsSecret;
import google.registry.model.server.KmsSecretRevision; import google.registry.model.server.KmsSecretRevision;
import google.registry.model.server.Lock; import google.registry.model.server.Lock;
import google.registry.model.server.ServerSecret; import google.registry.model.server.ServerSecret;
import google.registry.model.smd.SignedMarkRevocationList;
import google.registry.model.tmch.ClaimsListShard; import google.registry.model.tmch.ClaimsListShard;
import google.registry.model.tmch.ClaimsListShard.ClaimsListRevision; import google.registry.model.tmch.ClaimsListShard.ClaimsListRevision;
import google.registry.model.tmch.ClaimsListShard.ClaimsListSingleton; import google.registry.model.tmch.ClaimsListShard.ClaimsListSingleton;
@ -105,7 +104,6 @@ public final class EntityClasses {
Registry.class, Registry.class,
ReservedList.class, ReservedList.class,
ServerSecret.class, ServerSecret.class,
SignedMarkRevocationList.class,
TmchCrl.class); TmchCrl.class);
private EntityClasses() {} private EntityClasses() {}

View file

@ -16,34 +16,24 @@ package google.registry.model.smd;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import static google.registry.model.CacheUtils.memoizeWithShortExpiration; import static google.registry.model.CacheUtils.memoizeWithShortExpiration;
import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
import static google.registry.util.DateTimeUtils.isBeforeOrAt; import static google.registry.util.DateTimeUtils.isBeforeOrAt;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Supplier; import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableMap; 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.ImmutableObject;
import google.registry.model.annotations.InCrossTld; import google.registry.schema.replay.DatastoreEntity;
import google.registry.model.annotations.NotBackedUp; import google.registry.schema.replay.SqlEntity;
import google.registry.model.annotations.NotBackedUp.Reason;
import google.registry.model.common.EntityGroupRoot;
import google.registry.schema.replay.NonReplicatedEntity;
import java.util.Map; import java.util.Map;
import java.util.Optional;
import javax.persistence.CollectionTable; import javax.persistence.CollectionTable;
import javax.persistence.Column; import javax.persistence.Column;
import javax.persistence.ElementCollection; import javax.persistence.ElementCollection;
import javax.persistence.Entity;
import javax.persistence.GeneratedValue; import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType; import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.JoinColumn; import javax.persistence.JoinColumn;
import javax.persistence.MapKeyColumn; import javax.persistence.MapKeyColumn;
import javax.persistence.Transient;
import org.joda.time.DateTime; 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 * 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. * file that's created, depending on the timestamp.
* *
* <p>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.
*
* <p>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 google.registry.tmch.SmdrlCsvParser
* @see <a href="http://tools.ietf.org/html/draft-lozano-tmch-func-spec-08#section-6.2">TMCH * @see <a href="http://tools.ietf.org/html/draft-lozano-tmch-func-spec-08#section-6.2">TMCH
* functional specifications - SMD Revocation List</a> * functional specifications - SMD Revocation List</a>
*/ */
@Entity @Entity
@javax.persistence.Entity public class SignedMarkRevocationList extends ImmutableObject implements SqlEntity {
@NotBackedUp(reason = Reason.EXTERNALLY_SOURCED)
@InCrossTld
public class SignedMarkRevocationList extends ImmutableObject implements NonReplicatedEntity {
@VisibleForTesting static final int SHARD_SIZE = 10000; @Id
/** Common ancestor for queries. */
@Parent @Transient Key<EntityGroupRoot> parent = getCrossTldKey();
/** ID for the sharded entity. */
@Id @Transient long id;
@Ignore
@javax.persistence.Id
@GeneratedValue(strategy = GenerationType.IDENTITY) @GeneratedValue(strategy = GenerationType.IDENTITY)
Long revisionId; Long revisionId;
@ -88,7 +58,6 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl
DateTime creationTime; DateTime creationTime;
/** A map from SMD IDs to revocation time. */ /** A map from SMD IDs to revocation time. */
@EmbedMap
@ElementCollection @ElementCollection
@CollectionTable( @CollectionTable(
name = "SignedMarkRevocationEntry", name = "SignedMarkRevocationEntry",
@ -97,17 +66,10 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl
@Column(name = "revocationTime", nullable = false) @Column(name = "revocationTime", nullable = false)
Map</*@MatchesPattern("[0-9]+-[0-9]+")*/ String, DateTime> revokes; Map</*@MatchesPattern("[0-9]+-[0-9]+")*/ String, DateTime> revokes;
/** Indicates that this is a shard rather than a "full" list. */ /** A cached supplier that fetches the {@link SignedMarkRevocationList} object. */
@Ignore @Transient boolean isShard;
/**
* A cached supplier that fetches the SMDRL shards from Datastore and recombines them into a
* single {@link SignedMarkRevocationList} object.
*/
private static final Supplier<SignedMarkRevocationList> CACHE = private static final Supplier<SignedMarkRevocationList> CACHE =
memoizeWithShortExpiration(SignedMarkRevocationListDao::load); memoizeWithShortExpiration(SignedMarkRevocationListDao::load);
/** Return a single logical instance that combines all Datastore shards. */
public static SignedMarkRevocationList get() { public static SignedMarkRevocationList get() {
return CACHE.get(); return CACHE.get();
} }
@ -137,20 +99,14 @@ public class SignedMarkRevocationList extends ImmutableObject implements NonRepl
return revokes.size(); 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() { public SignedMarkRevocationList save() {
SignedMarkRevocationListDao.save(this); SignedMarkRevocationListDao.save(this);
return this; return this;
} }
/** As a safety mechanism, fail if someone tries to save this class directly. */ @Override
@OnSave public Optional<DatastoreEntity> toDatastoreEntity() {
void disallowUnshardedSaves() { return Optional.empty(); // Not persisted in Datastore
if (!isShard) {
throw new UnshardedSaveException();
} }
}
/** Exception when trying to directly save a {@link SignedMarkRevocationList} without sharding. */
public static class UnshardedSaveException extends RuntimeException {}
} }

View file

@ -15,7 +15,6 @@
package google.registry.model.smd; package google.registry.model.smd;
import static com.google.common.truth.Truth.assertThat; 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 google.registry.util.DateTimeUtils.START_OF_TIME;
import static org.joda.time.Duration.standardDays; import static org.joda.time.Duration.standardDays;
import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrows;
@ -31,14 +30,13 @@ import org.junit.jupiter.api.extension.RegisterExtension;
public class SignedMarkRevocationListTest { public class SignedMarkRevocationListTest {
@RegisterExtension @RegisterExtension
public final AppEngineExtension appEngine = public final AppEngineExtension appEngine = AppEngineExtension.builder().withCloudSql().build();
AppEngineExtension.builder().withDatastoreAndCloudSql().build();
private final FakeClock clock = new FakeClock(DateTime.parse("2013-01-01T00:00:00Z")); private final FakeClock clock = new FakeClock(DateTime.parse("2013-01-01T00:00:00Z"));
@Test @Test
void testEmpty() { 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()) assertThat(SignedMarkRevocationList.get())
.isEqualTo(SignedMarkRevocationList.create(START_OF_TIME, ImmutableMap.of())); .isEqualTo(SignedMarkRevocationList.create(START_OF_TIME, ImmutableMap.of()));
} }
@ -65,7 +63,7 @@ public class SignedMarkRevocationListTest {
@Test @Test
void test_isSmdRevoked_garbage() { void test_isSmdRevoked_garbage() {
SignedMarkRevocationList smdrl = createSaveGetHelper(SHARD_SIZE + 1); SignedMarkRevocationList smdrl = createSaveGetHelper(100);
assertThat(smdrl.getCreationTime()).isEqualTo(clock.nowUtc()); assertThat(smdrl.getCreationTime()).isEqualTo(clock.nowUtc());
assertThat(smdrl.isSmdRevoked("rofl", clock.nowUtc())).isFalse(); assertThat(smdrl.isSmdRevoked("rofl", clock.nowUtc())).isFalse();
assertThat(smdrl.isSmdRevoked("31337", clock.nowUtc())).isFalse(); assertThat(smdrl.isSmdRevoked("31337", clock.nowUtc())).isFalse();
@ -84,7 +82,7 @@ public class SignedMarkRevocationListTest {
@Test @Test
void test_isSmdRevoked_present() { void test_isSmdRevoked_present() {
final int rows = SHARD_SIZE + 1; final int rows = 100;
SignedMarkRevocationList smdrl = createSaveGetHelper(rows); SignedMarkRevocationList smdrl = createSaveGetHelper(rows);
assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isTrue(); assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isTrue();
assertThat(smdrl.isSmdRevoked(Integer.toString(rows - 1), clock.nowUtc())).isTrue(); assertThat(smdrl.isSmdRevoked(Integer.toString(rows - 1), clock.nowUtc())).isTrue();
@ -93,7 +91,7 @@ public class SignedMarkRevocationListTest {
@Test @Test
void test_isSmdRevoked_future() { void test_isSmdRevoked_future() {
final int rows = SHARD_SIZE; final int rows = 100;
SignedMarkRevocationList smdrl = createSaveGetHelper(rows); SignedMarkRevocationList smdrl = createSaveGetHelper(rows);
clock.advanceOneMilli(); clock.advanceOneMilli();
assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isTrue(); assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isTrue();
@ -103,7 +101,7 @@ public class SignedMarkRevocationListTest {
@Test @Test
void test_isSmdRevoked_past() { void test_isSmdRevoked_past() {
final int rows = SHARD_SIZE; final int rows = 100;
SignedMarkRevocationList smdrl = createSaveGetHelper(rows); SignedMarkRevocationList smdrl = createSaveGetHelper(rows);
clock.setTo(clock.nowUtc().minusMillis(1)); clock.setTo(clock.nowUtc().minusMillis(1));
assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isFalse(); assertThat(smdrl.isSmdRevoked("0", clock.nowUtc())).isFalse();

View file

@ -172,6 +172,13 @@ public final class AppEngineExtension implements BeforeEachCallback, AfterEachCa
return this; 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. */ /** Disables insertion of canned data. */
public Builder withoutCannedData() { public Builder withoutCannedData() {
rule.withoutCannedData = true; rule.withoutCannedData = true;

View file

@ -12,5 +12,4 @@ RegistrarContact
Registry Registry
ReservedList ReservedList
ServerSecret ServerSecret
SignedMarkRevocationList
TmchCrl TmchCrl

View file

@ -826,12 +826,6 @@ class google.registry.model.server.ServerSecret {
long leastSignificant; long leastSignificant;
long mostSignificant; long mostSignificant;
} }
class google.registry.model.smd.SignedMarkRevocationList {
@Id long id;
@Parent com.googlecode.objectify.Key<google.registry.model.common.EntityGroupRoot> parent;
java.util.Map<java.lang.String, org.joda.time.DateTime> revokes;
org.joda.time.DateTime creationTime;
}
class google.registry.model.tmch.ClaimsListShard { class google.registry.model.tmch.ClaimsListShard {
@Id long id; @Id long id;
@Parent com.googlecode.objectify.Key<google.registry.model.tmch.ClaimsListShard$ClaimsListRevision> parent; @Parent com.googlecode.objectify.Key<google.registry.model.tmch.ClaimsListShard$ClaimsListRevision> parent;