diff --git a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java index 5011c6c8e..4e132f0a9 100644 --- a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java +++ b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java @@ -14,178 +14,44 @@ package google.registry.model.smd; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; -import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.Iterables.isEmpty; -import static google.registry.model.DatabaseMigrationUtils.suppressExceptionUnlessInTest; -import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; -import static google.registry.model.ofy.ObjectifyService.allocateId; -import static google.registry.model.ofy.ObjectifyService.ofy; -import static google.registry.model.smd.SignedMarkRevocationList.SHARD_SIZE; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; -import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import static google.registry.util.CollectionUtils.isNullOrEmpty; import static google.registry.util.DateTimeUtils.START_OF_TIME; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Iterables; -import com.google.common.collect.MapDifference; -import com.google.common.collect.Maps; import com.google.common.flogger.FluentLogger; -import google.registry.util.CollectionUtils; -import java.util.Map; import java.util.Optional; -import org.joda.time.DateTime; public class SignedMarkRevocationListDao { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - /** - * Loads the {@link SignedMarkRevocationList}. - * - *

Loads the list from Cloud SQL, and attempts to load from Datastore. If the load from - * Datastore fails, or the list from Datastore does not match the list from Cloud SQL, the error - * will be logged but no exception will be thrown. - */ + /** Loads the {@link SignedMarkRevocationList}. */ static SignedMarkRevocationList load() { - Optional primaryList = loadFromCloudSql(); - if (!primaryList.isPresent()) { - return SignedMarkRevocationList.create(START_OF_TIME, ImmutableMap.of()); - } - suppressExceptionUnlessInTest( - () -> loadAndCompare(primaryList.get()), - "Error loading and comparing the SignedMarkRevocationList from Datastore"); - return primaryList.get(); + Optional smdrl = + jpaTm() + .transact( + () -> { + Long revisionId = + jpaTm() + .query("SELECT MAX(revisionId) FROM SignedMarkRevocationList", Long.class) + .getSingleResult(); + return jpaTm() + .query( + "FROM SignedMarkRevocationList smrl LEFT JOIN FETCH smrl.revokes " + + "WHERE smrl.revisionId = :revisionId", + SignedMarkRevocationList.class) + .setParameter("revisionId", revisionId) + .getResultStream() + .findFirst(); + }); + return smdrl.orElseGet(() -> SignedMarkRevocationList.create(START_OF_TIME, ImmutableMap.of())); } - /** Loads the list from Datastore and compares it to the list from Cloud SQL. */ - private static void loadAndCompare(SignedMarkRevocationList primaryList) { - Optional secondaryList = loadFromDatastore(); - if (secondaryList.isPresent() && !isNullOrEmpty(secondaryList.get().revokes)) { - MapDifference diff = - Maps.difference(primaryList.revokes, secondaryList.get().revokes); - if (!diff.areEqual()) { - if (diff.entriesDiffering().size() > 10) { - String message = - String.format( - "Unequal SignedMarkRevocationList detected, Datastore list with revision id" - + " %d has %d different records than the current Cloud SQL list.", - secondaryList.get().revisionId, diff.entriesDiffering().size()); - throw new IllegalStateException(message); - } else { - StringBuilder diffMessage = - new StringBuilder("Unequal SignedMarkRevocationList detected:\n"); - diff.entriesDiffering() - .forEach( - (label, valueDiff) -> - diffMessage.append( - String.format( - "SMD %s has key %s in Cloud SQL and key %s in Datastore.\n", - label, valueDiff.leftValue(), valueDiff.rightValue()))); - throw new IllegalStateException(diffMessage.toString()); - } - } - } else { - if (primaryList.size() != 0) { - throw new IllegalStateException( - "SignedMarkRevocationList in Datastore is empty while it is not empty in Cloud SQL."); - } - } - } - - /** Loads the shards from Datastore and combines them into one list. */ - private static Optional loadFromDatastore() { - return ofyTm() - .transactNewReadOnly( - () -> { - Iterable shards = - ofy().load().type(SignedMarkRevocationList.class).ancestor(getCrossTldKey()); - DateTime creationTime = - isEmpty(shards) - ? START_OF_TIME - : checkNotNull(Iterables.get(shards, 0).creationTime, "creationTime"); - ImmutableMap.Builder revokes = new ImmutableMap.Builder<>(); - for (SignedMarkRevocationList shard : shards) { - revokes.putAll(shard.revokes); - checkState( - creationTime.equals(shard.creationTime), - "Inconsistent creation times in Datastore shard: %s vs. %s", - creationTime, - shard.creationTime); - } - return Optional.of(SignedMarkRevocationList.create(creationTime, revokes.build())); - }); - } - - private static Optional loadFromCloudSql() { - return jpaTm() - .transact( - () -> { - Long revisionId = - jpaTm() - .query("SELECT MAX(revisionId) FROM SignedMarkRevocationList", Long.class) - .getSingleResult(); - return jpaTm() - .query( - "FROM SignedMarkRevocationList smrl LEFT JOIN FETCH smrl.revokes " - + "WHERE smrl.revisionId = :revisionId", - SignedMarkRevocationList.class) - .setParameter("revisionId", revisionId) - .getResultStream() - .findFirst(); - }); - } - - /** - * Save the given {@link SignedMarkRevocationList} - * - *

Saves the list to Cloud SQL, and attempts to save to Datastore. If the save to Datastore - * fails, the error will be logged but no exception will be thrown. - */ + /** Save the given {@link SignedMarkRevocationList} */ static void save(SignedMarkRevocationList signedMarkRevocationList) { - SignedMarkRevocationListDao.saveToCloudSql(signedMarkRevocationList); - suppressExceptionUnlessInTest( - () -> - saveToDatastore( - signedMarkRevocationList.revokes, signedMarkRevocationList.creationTime), - "Error inserting signed mark revocations into secondary database (Datastore)."); - } - - private static void saveToCloudSql(SignedMarkRevocationList signedMarkRevocationList) { jpaTm().transact(() -> jpaTm().insert(signedMarkRevocationList)); logger.atInfo().log( "Inserted %,d signed mark revocations into Cloud SQL.", signedMarkRevocationList.revokes.size()); } - - private static void saveToDatastore(Map revokes, DateTime creationTime) { - tm().transact( - () -> { - ofy() - .deleteWithoutBackup() - .keys( - ofy() - .load() - .type(SignedMarkRevocationList.class) - .ancestor(getCrossTldKey()) - .keys()); - ofy() - .saveWithoutBackup() - .entities( - CollectionUtils.partitionMap(revokes, SHARD_SIZE).stream() - .map( - shardRevokes -> { - SignedMarkRevocationList shard = - SignedMarkRevocationList.create(creationTime, shardRevokes); - shard.id = allocateId(); - shard.isShard = - true; // Avoid the exception in disallowUnshardedSaves(). - return shard; - }) - .collect(toImmutableList())); - }); - } } diff --git a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java index b6bd5af4a..d89b28d02 100644 --- a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java +++ b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java @@ -16,39 +16,22 @@ package google.registry.model.smd; 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 org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableMap; -import google.registry.config.RegistryEnvironment; import google.registry.model.EntityTestCase; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageExtension; -import google.registry.testing.DatastoreEntityExtension; -import google.registry.testing.DualDatabaseTest; -import google.registry.testing.SystemPropertyExtension; -import google.registry.testing.TestOfyAndSql; -import org.joda.time.DateTime; -import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -@DualDatabaseTest public class SignedMarkRevocationListDaoTest extends EntityTestCase { @RegisterExtension final JpaIntegrationWithCoverageExtension jpa = new JpaTestRules.Builder().withClock(fakeClock).buildIntegrationWithCoverageExtension(); - @RegisterExtension - @Order(value = 1) - final DatastoreEntityExtension datastoreEntityExtension = new DatastoreEntityExtension(); - - @RegisterExtension - @Order(value = Integer.MAX_VALUE) - final SystemPropertyExtension systemPropertyExtension = new SystemPropertyExtension(); - - @TestOfyAndSql - void testSave_cloudSqlPrimary_success() { + @Test + void testSave_success() { SignedMarkRevocationList list = SignedMarkRevocationList.create( fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); @@ -57,8 +40,8 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list); } - @TestOfyAndSql - void testSaveAndLoad_cloudSqlPrimary_emptyList() { + @Test + void testSaveAndLoad_emptyList() { SignedMarkRevocationList list = SignedMarkRevocationList.create(fakeClock.nowUtc(), ImmutableMap.of()); SignedMarkRevocationListDao.save(list); @@ -66,8 +49,8 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list, "revisionId"); } - @TestOfyAndSql - void testSave_cloudSqlPrimary_multipleVersions() { + @Test + void testSave_multipleVersions() { SignedMarkRevocationList list = SignedMarkRevocationList.create( fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); @@ -83,52 +66,4 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { .isFalse(); } - @TestOfyAndSql - void testLoad_cloudSqlPrimary_unequalLists() { - fakeClock.setTo(DateTime.parse("1984-12-26T00:00:00.000Z")); - SignedMarkRevocationList list = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); - SignedMarkRevocationListDao.save(list); - SignedMarkRevocationList list2 = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(3))); - jpaTm().transact(() -> jpaTm().put(list2)); - RuntimeException thrown = - assertThrows(RuntimeException.class, SignedMarkRevocationListDao::load); - assertThat(thrown) - .hasMessageThat() - .contains( - "SMD mark has key 1984-12-25T21:00:00.000Z in Cloud SQL and key" - + " 1984-12-25T23:00:00.000Z in Datastore."); - } - - @TestOfyAndSql - void testLoad_cloudSqlPrimary_unequalLists_succeedsInProduction() { - RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension); - SignedMarkRevocationList list = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); - SignedMarkRevocationListDao.save(list); - SignedMarkRevocationList list2 = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(3))); - jpaTm().transact(() -> jpaTm().put(list2)); - SignedMarkRevocationList fromDb = SignedMarkRevocationListDao.load(); - assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list2, "revisionId"); - } - - @TestOfyAndSql - void testLoad_cloudSqlPrimary_noListInDatastore() { - SignedMarkRevocationList list = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); - jpaTm().transact(() -> jpaTm().put(list)); - RuntimeException thrown = - assertThrows(RuntimeException.class, SignedMarkRevocationListDao::load); - assertThat(thrown) - .hasMessageThat() - .contains( - "SignedMarkRevocationList in Datastore is empty while it is not empty in Cloud SQL."); - } } 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 1206cfb95..9671d8666 100644 --- a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java +++ b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListTest.java @@ -15,11 +15,7 @@ package google.registry.model.smd; import static com.google.common.truth.Truth.assertThat; -import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.smd.SignedMarkRevocationList.SHARD_SIZE; -import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.time.Duration.standardDays; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -40,24 +36,6 @@ public class SignedMarkRevocationListTest { private final FakeClock clock = new FakeClock(DateTime.parse("2013-01-01T00:00:00Z")); - @Test - void testUnshardedSaveFails() { - // Our @Entity's @OnSave method will notice that this shouldn't be saved. - assertThrows( - SignedMarkRevocationList.UnshardedSaveException.class, - () -> - tm() - .transact( - () -> { - SignedMarkRevocationList smdrl = - SignedMarkRevocationList.create( - tm().getTransactionTime(), - ImmutableMap.of("a", tm().getTransactionTime())); - smdrl.id = 1; // Without an id this won't save anyways. - ofy().saveWithoutBackup().entity(smdrl).now(); - })); - } - @Test void testEmpty() { // When Datastore is empty, it should give us an empty thing. @@ -65,41 +43,6 @@ public class SignedMarkRevocationListTest { .isEqualTo(SignedMarkRevocationList.create(START_OF_TIME, ImmutableMap.of())); } - @Test - void testSharding2() { - final int rows = SHARD_SIZE + 1; - // Create a SignedMarkRevocationList that will need 2 shards to save. - ImmutableMap.Builder revokes = new ImmutableMap.Builder<>(); - for (int i = 0; i < rows; i++) { - revokes.put(Integer.toString(i), clock.nowUtc()); - } - // Save it with sharding, and make sure that reloading it works. - SignedMarkRevocationList unsharded = - SignedMarkRevocationList.create(clock.nowUtc(), revokes.build()).save(); - assertAboutImmutableObjects() - .that(SignedMarkRevocationList.get()) - .isEqualExceptFields(unsharded, "revisionId"); - assertThat(ofy().load().type(SignedMarkRevocationList.class).count()).isEqualTo(2); - } - - @Test - void testSharding4() { - final int rows = SHARD_SIZE * 3 + 1; - // Create a SignedMarkRevocationList that will need 4 shards to save. - ImmutableMap.Builder revokes = new ImmutableMap.Builder<>(); - for (int i = 0; i < rows; i++) { - revokes.put(Integer.toString(i), clock.nowUtc()); - } - // Save it with sharding, and make sure that reloading it works. - SignedMarkRevocationList unsharded = SignedMarkRevocationList - .create(clock.nowUtc(), revokes.build()) - .save(); - assertAboutImmutableObjects() - .that(SignedMarkRevocationList.get()) - .isEqualExceptFields(unsharded, "revisionId"); - assertThat(ofy().load().type(SignedMarkRevocationList.class).count()).isEqualTo(4); - } - private SignedMarkRevocationList createSaveGetHelper(int rows) { ImmutableMap.Builder revokes = new ImmutableMap.Builder<>(); for (int i = 0; i < rows; i++) { @@ -139,22 +82,6 @@ public class SignedMarkRevocationListTest { .isEqualTo(DateTime.parse("2000-01-01T00:00:00Z")); } - @Test - void test_getCreationTime_unequalListsInDatabases() { - clock.setTo(DateTime.parse("2000-01-01T00:00:00Z")); - createSaveGetHelper(1); - ImmutableMap.Builder revokes = new ImmutableMap.Builder<>(); - for (int i = 0; i < 3; i++) { - revokes.put(Integer.toString(i), clock.nowUtc()); - } - jpaTm() - .transact( - () -> jpaTm().insert(SignedMarkRevocationList.create(clock.nowUtc(), revokes.build()))); - RuntimeException thrown = - assertThrows(RuntimeException.class, () -> SignedMarkRevocationList.get()); - assertThat(thrown).hasMessageThat().contains("Unequal SignedMarkRevocationList detected:"); - } - @Test void test_isSmdRevoked_present() { final int rows = SHARD_SIZE + 1;