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 e0989246b..5011c6c8e 100644 --- a/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java +++ b/core/src/main/java/google/registry/model/smd/SignedMarkRevocationListDao.java @@ -19,7 +19,6 @@ 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.DatabaseTransitionSchedule.PrimaryDatabase.DATASTORE; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.model.ofy.ObjectifyService.allocateId; import static google.registry.model.ofy.ObjectifyService.ofy; @@ -35,9 +34,6 @@ 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.model.DatabaseMigrationUtils; -import google.registry.model.common.DatabaseTransitionSchedule.PrimaryDatabase; -import google.registry.model.common.DatabaseTransitionSchedule.TransitionId; import google.registry.util.CollectionUtils; import java.util.Map; import java.util.Optional; @@ -50,42 +46,24 @@ public class SignedMarkRevocationListDao { /** * Loads the {@link SignedMarkRevocationList}. * - *

Loads the list from the specified primary database, and attempts to load from the secondary - * database. If the load the secondary database fails, or the list from the secondary database - * does not match the list from the primary database, the error will be logged but no exception - * will be thrown. + *

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. */ static SignedMarkRevocationList load() { - PrimaryDatabase primaryDatabase = - tm().transactNew( - () -> - DatabaseMigrationUtils.getPrimaryDatabase( - TransitionId.SIGNED_MARK_REVOCATION_LIST)); - Optional primaryList = - primaryDatabase.equals(DATASTORE) ? loadFromDatastore() : loadFromCloudSql(); + Optional primaryList = loadFromCloudSql(); if (!primaryList.isPresent()) { - throw new IllegalStateException( - String.format( - "SignedMarkRevocationList not found in the primary database (%s).", - primaryDatabase.name())); + return SignedMarkRevocationList.create(START_OF_TIME, ImmutableMap.of()); } suppressExceptionUnlessInTest( - () -> loadAndCompare(primaryDatabase, primaryList.get()), - String.format( - "Error loading and comparing the SignedMarkRevocationList from the secondary database" - + " (%s).", - primaryDatabase.equals(DATASTORE) ? "Cloud SQL" : "Datastore")); + () -> loadAndCompare(primaryList.get()), + "Error loading and comparing the SignedMarkRevocationList from Datastore"); return primaryList.get(); } - /** - * Loads the list from the secondary database and compares it to the list from the primary - * database. - */ - private static void loadAndCompare( - PrimaryDatabase primaryDatabase, SignedMarkRevocationList primaryList) { - Optional secondaryList = - primaryDatabase.equals(DATASTORE) ? loadFromCloudSql() : loadFromDatastore(); + /** 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); @@ -93,11 +71,9 @@ public class SignedMarkRevocationListDao { if (diff.entriesDiffering().size() > 10) { String message = String.format( - "Unequal SignedMarkRevocationList detected, %s list with revision id" - + " %d has %d different records than the current primary database list.", - primaryDatabase.equals(DATASTORE) ? "Cloud SQL" : "Datastore", - secondaryList.get().revisionId, - diff.entriesDiffering().size()); + "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 = @@ -107,21 +83,15 @@ public class SignedMarkRevocationListDao { (label, valueDiff) -> diffMessage.append( String.format( - "SMD %s has key %s in %s and key %s in secondary database.\n", - label, - valueDiff.leftValue(), - primaryDatabase.name(), - valueDiff.rightValue()))); + "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( - String.format( - "SignedMarkRevocationList in %s is empty while it is not empty in the primary" - + " database.", - primaryDatabase.equals(DATASTORE) ? "Cloud SQL" : "Datastore")); + "SignedMarkRevocationList in Datastore is empty while it is not empty in Cloud SQL."); } } } @@ -172,29 +142,16 @@ public class SignedMarkRevocationListDao { /** * Save the given {@link SignedMarkRevocationList} * - *

Saves the list to the specified primary database, and attempts to save to the secondary - * database. If the save to the secondary database fails, the error will be logged but no - * exception will be thrown. + *

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. */ static void save(SignedMarkRevocationList signedMarkRevocationList) { - PrimaryDatabase primaryDatabase = - tm().transactNew( - () -> - DatabaseMigrationUtils.getPrimaryDatabase( - TransitionId.SIGNED_MARK_REVOCATION_LIST)); - if (primaryDatabase.equals(DATASTORE)) { - saveToDatastore(signedMarkRevocationList.revokes, signedMarkRevocationList.creationTime); - suppressExceptionUnlessInTest( - () -> SignedMarkRevocationListDao.saveToCloudSql(signedMarkRevocationList), - "Error inserting signed mark revocations into secondary database (Cloud SQL)."); - } else { SignedMarkRevocationListDao.saveToCloudSql(signedMarkRevocationList); suppressExceptionUnlessInTest( () -> saveToDatastore( signedMarkRevocationList.revokes, signedMarkRevocationList.creationTime), "Error inserting signed mark revocations into secondary database (Datastore)."); - } } private static void saveToCloudSql(SignedMarkRevocationList signedMarkRevocationList) { 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 5026e6886..b6bd5af4a 100644 --- a/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java +++ b/core/src/test/java/google/registry/model/smd/SignedMarkRevocationListDaoTest.java @@ -16,21 +16,12 @@ 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.persistence.transaction.TransactionManagerFactory.jpaTm; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSortedMap; import google.registry.config.RegistryEnvironment; import google.registry.model.EntityTestCase; -import google.registry.model.common.DatabaseTransitionSchedule; -import google.registry.model.common.DatabaseTransitionSchedule.PrimaryDatabase; -import google.registry.model.common.DatabaseTransitionSchedule.PrimaryDatabaseTransition; -import google.registry.model.common.DatabaseTransitionSchedule.TransitionId; -import google.registry.model.common.TimedTransitionProperty; import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageExtension; import google.registry.testing.DatastoreEntityExtension; @@ -38,8 +29,6 @@ import google.registry.testing.DualDatabaseTest; import google.registry.testing.SystemPropertyExtension; import google.registry.testing.TestOfyAndSql; import org.joda.time.DateTime; -import org.joda.time.Duration; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.extension.RegisterExtension; @@ -58,36 +47,8 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { @Order(value = Integer.MAX_VALUE) final SystemPropertyExtension systemPropertyExtension = new SystemPropertyExtension(); - @BeforeEach - void setup() { - fakeClock.setTo(DateTime.parse("1984-12-21T00:00:00.000Z")); - DatabaseTransitionSchedule schedule = - DatabaseTransitionSchedule.create( - TransitionId.SIGNED_MARK_REVOCATION_LIST, - TimedTransitionProperty.fromValueMap( - ImmutableSortedMap.of( - START_OF_TIME, - PrimaryDatabase.DATASTORE, - fakeClock.nowUtc().plusDays(1), - PrimaryDatabase.CLOUD_SQL), - PrimaryDatabaseTransition.class)); - - tm().transactNew(() -> ofy().saveWithoutBackup().entity(schedule).now()); - } - - @TestOfyAndSql - void testSave_datastorePrimary_success() { - SignedMarkRevocationList list = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); - SignedMarkRevocationListDao.save(list); - SignedMarkRevocationList fromDb = SignedMarkRevocationListDao.load(); - assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list, "revisionId"); - } - @TestOfyAndSql void testSave_cloudSqlPrimary_success() { - fakeClock.advanceBy(Duration.standardDays(5)); SignedMarkRevocationList list = SignedMarkRevocationList.create( fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); @@ -96,18 +57,8 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list); } - @TestOfyAndSql - void testSaveAndLoad_datastorePrimary_emptyList() { - SignedMarkRevocationList list = - SignedMarkRevocationList.create(START_OF_TIME, ImmutableMap.of()); - SignedMarkRevocationListDao.save(list); - SignedMarkRevocationList fromDb = SignedMarkRevocationListDao.load(); - assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list, "revisionId"); - } - @TestOfyAndSql void testSaveAndLoad_cloudSqlPrimary_emptyList() { - fakeClock.advanceBy(Duration.standardDays(5)); SignedMarkRevocationList list = SignedMarkRevocationList.create(fakeClock.nowUtc(), ImmutableMap.of()); SignedMarkRevocationListDao.save(list); @@ -115,26 +66,8 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list, "revisionId"); } - @TestOfyAndSql - void testSave_datastorePrimary_multipleVersions() { - SignedMarkRevocationList list = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); - SignedMarkRevocationListDao.save(list); - assertThat(SignedMarkRevocationListDao.load().isSmdRevoked("mark", fakeClock.nowUtc())) - .isTrue(); - - // Now remove the revocation - SignedMarkRevocationList secondList = - SignedMarkRevocationList.create(fakeClock.nowUtc(), ImmutableMap.of()); - SignedMarkRevocationListDao.save(secondList); - assertThat(SignedMarkRevocationListDao.load().isSmdRevoked("mark", fakeClock.nowUtc())) - .isFalse(); - } - @TestOfyAndSql void testSave_cloudSqlPrimary_multipleVersions() { - fakeClock.advanceBy(Duration.standardDays(5)); SignedMarkRevocationList list = SignedMarkRevocationList.create( fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); @@ -150,28 +83,9 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { .isFalse(); } - @TestOfyAndSql - void testLoad_datastorePrimary_unequalLists() { - 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-20T23:00:00.000Z in DATASTORE and key" - + " 1984-12-20T21:00:00.000Z in secondary database."); - } - @TestOfyAndSql void testLoad_cloudSqlPrimary_unequalLists() { - fakeClock.advanceBy(Duration.standardDays(5)); + fakeClock.setTo(DateTime.parse("1984-12-26T00:00:00.000Z")); SignedMarkRevocationList list = SignedMarkRevocationList.create( fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); @@ -185,14 +99,13 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { 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 secondary database."); + "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); - fakeClock.advanceBy(Duration.standardDays(5)); SignedMarkRevocationList list = SignedMarkRevocationList.create( fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); @@ -205,25 +118,8 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { assertAboutImmutableObjects().that(fromDb).isEqualExceptFields(list2, "revisionId"); } - @TestOfyAndSql - void testLoad_datastorePrimary_noListInCloudSql() { - SignedMarkRevocationList list = - SignedMarkRevocationList.create( - fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); - SignedMarkRevocationListDao.save(list); - jpaTm().transact(() -> jpaTm().delete(list)); - RuntimeException thrown = - assertThrows(RuntimeException.class, SignedMarkRevocationListDao::load); - assertThat(thrown) - .hasMessageThat() - .contains( - "SignedMarkRevocationList in Cloud SQL is empty while it is not empty in the" - + " primary database."); - } - @TestOfyAndSql void testLoad_cloudSqlPrimary_noListInDatastore() { - fakeClock.advanceBy(Duration.standardDays(5)); SignedMarkRevocationList list = SignedMarkRevocationList.create( fakeClock.nowUtc(), ImmutableMap.of("mark", fakeClock.nowUtc().minusHours(1))); @@ -233,7 +129,6 @@ public class SignedMarkRevocationListDaoTest extends EntityTestCase { assertThat(thrown) .hasMessageThat() .contains( - "SignedMarkRevocationList in Datastore is empty while it is not empty in the" - + " primary database."); + "SignedMarkRevocationList in Datastore is empty while it is not empty in Cloud SQL."); } }