diff --git a/core/src/main/java/google/registry/model/registry/label/PremiumListDualDao.java b/core/src/main/java/google/registry/model/registry/label/PremiumListDualDao.java index 7e3f81e06..dcf0d4fe9 100644 --- a/core/src/main/java/google/registry/model/registry/label/PremiumListDualDao.java +++ b/core/src/main/java/google/registry/model/registry/label/PremiumListDualDao.java @@ -14,12 +14,11 @@ package google.registry.model.registry.label; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static google.registry.model.DatabaseMigrationUtils.suppressExceptionUnlessInTest; import com.google.common.collect.Streams; -import google.registry.model.DatabaseMigrationUtils; -import google.registry.model.common.DatabaseTransitionSchedule.TransitionId; import google.registry.model.registry.Registry; import google.registry.model.registry.label.PremiumList.PremiumListEntry; import google.registry.schema.tld.PremiumListSqlDao; @@ -32,27 +31,20 @@ import org.joda.money.Money; /** * DAO for {@link PremiumList} objects that handles the branching paths for SQL and Datastore. * - *

For write actions, this class will perform the action against the primary database then, after - * that success or failure, against the secondary database. If the secondary database fails, an - * error is logged (but not thrown). + *

For write actions, this class will perform the action against Cloud SQL then, after that + * success or failure, against Datastore. If Datastore fails, an error is logged (but not thrown). * *

For read actions, when retrieving a price, we will log if the primary and secondary databases - * have different values (or if the retrieval from the second database fails). - * - *

TODO (gbrodman): Change the isOfy() calls to the runtime selection of DBs when available + * have different values (or if the retrieval from Datastore fails). */ public class PremiumListDualDao { /** - * Retrieves from the appropriate DB and returns the most recent premium list with the given name, - * or absent if no such list exists. + * Retrieves from Cloud SQL and returns the most recent premium list with the given name, or + * absent if no such list exists. */ public static Optional getLatestRevision(String premiumListName) { - if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - return PremiumListDatastoreDao.getLatestRevision(premiumListName); - } else { return PremiumListSqlDao.getLatestRevision(premiumListName); - } } /** @@ -61,7 +53,7 @@ public class PremiumListDualDao { *

Returns absent if the label is not premium or there is no premium list for this registry. * *

Retrieves the price from both primary and secondary databases, and logs in the event of a - * failure in the secondary (but does not throw an exception). + * failure in Datastore (but does not throw an exception). */ public static Optional getPremiumPrice(String label, Registry registry) { if (registry.getPremiumList() == null) { @@ -69,98 +61,56 @@ public class PremiumListDualDao { } String premiumListName = registry.getPremiumList().getName(); Optional primaryResult; - if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - primaryResult = - PremiumListDatastoreDao.getPremiumPrice(premiumListName, label, registry.getTldStr()); - } else { primaryResult = PremiumListSqlDao.getPremiumPrice(premiumListName, label); - } - // Also load the value from the secondary DB, compare the two results, and log if different. - if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - suppressExceptionUnlessInTest( - () -> { - Optional secondaryResult = - PremiumListSqlDao.getPremiumPrice(premiumListName, label); - if (!primaryResult.equals(secondaryResult)) { - throw new IllegalStateException( - String.format( - "Unequal prices for domain %s.%s from primary Datastore DB (%s) and " - + "secondary SQL db (%s).", - label, registry.getTldStr(), primaryResult, secondaryResult)); - } - }, - String.format( - "Error loading price of domain %s.%s from Cloud SQL.", label, registry.getTldStr())); - } else { - suppressExceptionUnlessInTest( - () -> { - Optional secondaryResult = - PremiumListDatastoreDao.getPremiumPrice( - premiumListName, label, registry.getTldStr()); - if (!primaryResult.equals(secondaryResult)) { - throw new IllegalStateException( - String.format( - "Unequal prices for domain %s.%s from primary SQL DB (%s) and secondary " - + "Datastore db (%s).", - label, registry.getTldStr(), primaryResult, secondaryResult)); - } - }, - String.format( - "Error loading price of domain %s.%s from Datastore.", label, registry.getTldStr())); - } + // Also load the value from Datastore, compare the two results, and log if different. + suppressExceptionUnlessInTest( + () -> { + Optional secondaryResult = + PremiumListDatastoreDao.getPremiumPrice(premiumListName, label, registry.getTldStr()); + checkState( + primaryResult.equals(secondaryResult), + "Unequal prices for domain %s.%s from primary SQL DB (%s) and secondary Datastore db" + + " (%s).", + label, + registry.getTldStr(), + primaryResult, + secondaryResult); + }, + String.format( + "Error loading price of domain %s.%s from Datastore.", label, registry.getTldStr())); return primaryResult; } /** * Saves the given list data to both primary and secondary databases. * - *

Logs but doesn't throw an exception in the event of a failure when writing to the secondary - * database. + *

Logs but doesn't throw an exception in the event of a failure when writing to Datastore. */ public static PremiumList save(String name, List inputData) { - PremiumList result; - if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - result = PremiumListDatastoreDao.save(name, inputData); - suppressExceptionUnlessInTest( - () -> PremiumListSqlDao.save(name, inputData), "Error when saving premium list to SQL."); - } else { - result = PremiumListSqlDao.save(name, inputData); + PremiumList result = PremiumListSqlDao.save(name, inputData); suppressExceptionUnlessInTest( () -> PremiumListDatastoreDao.save(name, inputData), "Error when saving premium list to Datastore."); - } return result; } /** * Deletes the premium list. * - *

Logs but doesn't throw an exception in the event of a failure when deleting from the - * secondary database. + *

Logs but doesn't throw an exception in the event of a failure when deleting from Datastore. */ public static void delete(PremiumList premiumList) { - if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - PremiumListDatastoreDao.delete(premiumList); - suppressExceptionUnlessInTest( - () -> PremiumListSqlDao.delete(premiumList), - "Error when deleting premium list from SQL."); - } else { PremiumListSqlDao.delete(premiumList); suppressExceptionUnlessInTest( () -> PremiumListDatastoreDao.delete(premiumList), "Error when deleting premium list from Datastore."); - } } /** Returns whether or not there exists a premium list with the given name. */ public static boolean exists(String premiumListName) { // It may seem like overkill, but loading the list has ways been the way we check existence and // given that we usually load the list around the time we check existence, we'll hit the cache - if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - return PremiumListDatastoreDao.getLatestRevision(premiumListName).isPresent(); - } else { - return PremiumListSqlDao.getLatestRevision(premiumListName).isPresent(); - } + return PremiumListSqlDao.getLatestRevision(premiumListName).isPresent(); } /** @@ -175,9 +125,6 @@ public class PremiumListDualDao { () -> new IllegalArgumentException( String.format("No premium list with name %s.", premiumListName))); - if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - return PremiumListDatastoreDao.loadPremiumListEntriesUncached(premiumList); - } else { CurrencyUnit currencyUnit = premiumList.getCurrency(); return Streams.stream(PremiumListSqlDao.loadPremiumListEntriesUncached(premiumList)) .map( @@ -188,7 +135,6 @@ public class PremiumListDualDao { .build()) .collect(toImmutableList()); } - } private PremiumListDualDao() {} } diff --git a/core/src/main/java/google/registry/model/registry/label/ReservedListDualDatabaseDao.java b/core/src/main/java/google/registry/model/registry/label/ReservedListDualDatabaseDao.java index f24c83c80..15b207aa1 100644 --- a/core/src/main/java/google/registry/model/registry/label/ReservedListDualDatabaseDao.java +++ b/core/src/main/java/google/registry/model/registry/label/ReservedListDualDatabaseDao.java @@ -15,13 +15,11 @@ package google.registry.model.registry.label; import static com.google.common.collect.ImmutableMap.toImmutableMap; -import static google.registry.model.DatabaseMigrationUtils.isDatastore; import com.google.common.collect.MapDifference; import com.google.common.collect.MapDifference.ValueDifference; import com.google.common.collect.Maps; import google.registry.model.DatabaseMigrationUtils; -import google.registry.model.common.DatabaseTransitionSchedule.TransitionId; import google.registry.model.registry.label.ReservedList.ReservedListEntry; import java.util.Map; import java.util.Optional; @@ -38,32 +36,18 @@ public class ReservedListDualDatabaseDao { /** Persist a new reserved list to the database. */ public static void save(ReservedList reservedList) { - if (isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - ReservedListDatastoreDao.save(reservedList); - DatabaseMigrationUtils.suppressExceptionUnlessInTest( - () -> ReservedListSqlDao.save(reservedList), - "Error saving the reserved list to Cloud SQL."); - } else { ReservedListSqlDao.save(reservedList); DatabaseMigrationUtils.suppressExceptionUnlessInTest( () -> ReservedListDatastoreDao.save(reservedList), "Error saving the reserved list to Datastore."); - } } /** Delete a reserved list from both databases. */ public static void delete(ReservedList reservedList) { - if (isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) { - ReservedListDatastoreDao.delete(reservedList); - DatabaseMigrationUtils.suppressExceptionUnlessInTest( - () -> ReservedListSqlDao.delete(reservedList), - "Error deleting the reserved list from Cloud SQL."); - } else { ReservedListSqlDao.delete(reservedList); DatabaseMigrationUtils.suppressExceptionUnlessInTest( () -> ReservedListDatastoreDao.delete(reservedList), "Error deleting the reserved list from Datastore."); - } } /** @@ -72,9 +56,7 @@ public class ReservedListDualDatabaseDao { */ public static Optional getLatestRevision(String reservedListName) { Optional maybePrimaryList = - isDatastore(TransitionId.DOMAIN_LABEL_LISTS) - ? ReservedListDatastoreDao.getLatestRevision(reservedListName) - : ReservedListSqlDao.getLatestRevision(reservedListName); + ReservedListSqlDao.getLatestRevision(reservedListName); DatabaseMigrationUtils.suppressExceptionUnlessInTest( () -> maybePrimaryList.ifPresent(primaryList -> loadAndCompare(primaryList)), "Error comparing reserved lists."); @@ -83,14 +65,9 @@ public class ReservedListDualDatabaseDao { private static void loadAndCompare(ReservedList primaryList) { Optional maybeSecondaryList = - isDatastore(TransitionId.DOMAIN_LABEL_LISTS) - ? ReservedListSqlDao.getLatestRevision(primaryList.getName()) - : ReservedListDatastoreDao.getLatestRevision(primaryList.getName()); + ReservedListDatastoreDao.getLatestRevision(primaryList.getName()); if (!maybeSecondaryList.isPresent()) { - throw new IllegalStateException( - String.format( - "Reserved list in the secondary database (%s) is empty.", - isDatastore(TransitionId.DOMAIN_LABEL_LISTS) ? "Cloud SQL" : "Datastore")); + throw new IllegalStateException("Reserved list in Datastore is empty."); } Map labelsToReservations = primaryList.reservedListMap.entrySet().parallelStream() @@ -110,12 +87,10 @@ public class ReservedListDualDatabaseDao { if (diff.entriesDiffering().size() > 10) { throw new IllegalStateException( String.format( - "Unequal reserved lists detected, %s list with revision" + "Unequal reserved lists detected, Datastore list with revision" + " id %d has %d different records than the current" - + " primary database list.", - isDatastore(TransitionId.DOMAIN_LABEL_LISTS) ? "Cloud SQL" : "Datastore", - secondaryList.getRevisionId(), - diff.entriesDiffering().size())); + + " Cloud SQL list.", + secondaryList.getRevisionId(), diff.entriesDiffering().size())); } StringBuilder diffMessage = new StringBuilder("Unequal reserved lists detected:\n"); diff.entriesDiffering().entrySet().stream() @@ -125,12 +100,9 @@ public class ReservedListDualDatabaseDao { ValueDifference valueDiff = entry.getValue(); diffMessage.append( String.format( - "Domain label %s has entry %s in %s and entry" - + " %s in the secondary database.\n", - label, - valueDiff.leftValue(), - isDatastore(TransitionId.DOMAIN_LABEL_LISTS) ? "Datastore" : "Cloud SQL", - valueDiff.rightValue())); + "Domain label %s has entry %s in Cloud SQL and entry" + + " %s in the Datastore.\n", + label, valueDiff.leftValue(), valueDiff.rightValue())); }); diff.entriesOnlyOnLeft().entrySet().stream() .forEach( @@ -138,9 +110,7 @@ public class ReservedListDualDatabaseDao { String label = entry.getKey(); diffMessage.append( String.format( - "Domain label %s has entry in %s, but not in the secondary database.\n", - label, - isDatastore(TransitionId.DOMAIN_LABEL_LISTS) ? "Datastore" : "Cloud SQL")); + "Domain label %s has entry in Cloud SQL, but not in Datastore.\n", label)); }); diff.entriesOnlyOnRight().entrySet().stream() .forEach( @@ -148,9 +118,7 @@ public class ReservedListDualDatabaseDao { String label = entry.getKey(); diffMessage.append( String.format( - "Domain label %s has entry in %s, but not in the primary database.\n", - label, - isDatastore(TransitionId.DOMAIN_LABEL_LISTS) ? "Cloud SQL" : "Datastore")); + "Domain label %s has entry in Datastore, but not in Cloud SQL.\n", label)); }); throw new IllegalStateException(diffMessage.toString()); } diff --git a/core/src/main/java/google/registry/tools/CreateReservedListCommand.java b/core/src/main/java/google/registry/tools/CreateReservedListCommand.java index 4bb5a01bb..17078fb02 100644 --- a/core/src/main/java/google/registry/tools/CreateReservedListCommand.java +++ b/core/src/main/java/google/registry/tools/CreateReservedListCommand.java @@ -30,8 +30,8 @@ import java.nio.file.Files; import java.util.List; import org.joda.time.DateTime; -/** Command to create a {@link ReservedList} on Datastore. */ -@Parameters(separators = " =", commandDescription = "Create a ReservedList in Datastore.") +/** Command to create a {@link ReservedList}. */ +@Parameters(separators = " =", commandDescription = "Create a ReservedList.") final class CreateReservedListCommand extends CreateOrUpdateReservedListCommand { @VisibleForTesting diff --git a/core/src/main/java/google/registry/tools/DeletePremiumListCommand.java b/core/src/main/java/google/registry/tools/DeletePremiumListCommand.java index 66c9f86c1..e45313167 100644 --- a/core/src/main/java/google/registry/tools/DeletePremiumListCommand.java +++ b/core/src/main/java/google/registry/tools/DeletePremiumListCommand.java @@ -25,10 +25,10 @@ import google.registry.model.registry.label.PremiumListDualDao; import javax.annotation.Nullable; /** - * Command to delete a {@link PremiumList} in Datastore. This command will fail if the premium list - * is currently in use on a tld. + * Command to delete a {@link PremiumList}. This command will fail if the premium list is currently + * in use on a tld. */ -@Parameters(separators = " =", commandDescription = "Delete a PremiumList from Datastore.") +@Parameters(separators = " =", commandDescription = "Delete a PremiumList.") final class DeletePremiumListCommand extends ConfirmingCommand implements CommandWithRemoteApi { @Nullable PremiumList premiumList; @@ -55,7 +55,7 @@ final class DeletePremiumListCommand extends ConfirmingCommand implements Comman @Override protected String prompt() { - return "You are about to delete the premium list: \n" + premiumList; + return "You are about to delete the premium list: \n" + premiumList.getName(); } @Override diff --git a/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java b/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java index f99a041b6..3e426c75b 100644 --- a/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java +++ b/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java @@ -22,13 +22,11 @@ import com.google.common.base.Strings; import com.googlecode.objectify.Key; import google.registry.model.registry.label.ReservedList; import google.registry.persistence.VKey; -import google.registry.util.SystemClock; import java.nio.file.Files; import java.util.List; -import org.joda.time.DateTime; -/** Command to safely update {@link ReservedList} on Datastore. */ -@Parameters(separators = " =", commandDescription = "Update a ReservedList in Datastore.") +/** Command to safely update {@link ReservedList}. */ +@Parameters(separators = " =", commandDescription = "Update a ReservedList.") final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand { @Override @@ -44,12 +42,10 @@ final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand boolean shouldPublish = this.shouldPublish == null ? existingReservedList.getShouldPublish() : this.shouldPublish; List allLines = Files.readAllLines(input, UTF_8); - DateTime now = new SystemClock().nowUtc(); ReservedList.Builder updated = existingReservedList .asBuilder() .setReservedListMapFromLines(allLines) - .setLastUpdateTime(now) .setShouldPublish(shouldPublish); reservedList = updated.build(); // only call stageEntityChange if there are changes in entries diff --git a/core/src/main/java/google/registry/tools/server/ListPremiumListsAction.java b/core/src/main/java/google/registry/tools/server/ListPremiumListsAction.java index 662355c84..7c8baccd8 100644 --- a/core/src/main/java/google/registry/tools/server/ListPremiumListsAction.java +++ b/core/src/main/java/google/registry/tools/server/ListPremiumListsAction.java @@ -15,8 +15,7 @@ package google.registry.tools.server; import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.request.Action.Method.GET; import static google.registry.request.Action.Method.POST; @@ -51,14 +50,15 @@ public final class ListPremiumListsAction extends ListObjectsAction @Override public ImmutableSet loadObjects() { - return transactIfJpaTm( - () -> - tm().loadAllOf(PremiumList.class).stream() - .map(PremiumList::getName) - .map(PremiumListDualDao::getLatestRevision) - .filter(Optional::isPresent) - .map(Optional::get) - .peek(list -> Hibernate.initialize(list.getLabelsToPrices())) - .collect(toImmutableSortedSet(Comparator.comparing(PremiumList::getName)))); + return jpaTm() + .transact( + () -> + jpaTm().loadAllOf(PremiumList.class).stream() + .map(PremiumList::getName) + .map(PremiumListDualDao::getLatestRevision) + .filter(Optional::isPresent) + .map(Optional::get) + .peek(list -> Hibernate.initialize(list.getLabelsToPrices())) + .collect(toImmutableSortedSet(Comparator.comparing(PremiumList::getName)))); } } diff --git a/core/src/test/java/google/registry/model/registry/label/PremiumListDualDaoTest.java b/core/src/test/java/google/registry/model/registry/label/PremiumListDualDaoTest.java index 2995d05dc..2369a9126 100644 --- a/core/src/test/java/google/registry/model/registry/label/PremiumListDualDaoTest.java +++ b/core/src/test/java/google/registry/model/registry/label/PremiumListDualDaoTest.java @@ -15,40 +15,27 @@ package google.registry.model.registry.label; import static com.google.common.truth.Truth.assertThat; -import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.newRegistry; import static google.registry.testing.DatabaseHelper.persistResource; -import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.time.Duration.standardDays; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedMap; import com.google.common.truth.Truth8; import google.registry.dns.writer.VoidDnsWriter; 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.model.pricing.StaticPremiumListPricingEngine; import google.registry.model.registry.Registry; -import google.registry.schema.tld.PremiumListSqlDao; -import google.registry.testing.DualDatabaseTest; import google.registry.testing.TestCacheExtension; -import google.registry.testing.TestOfyAndSql; -import org.joda.time.DateTime; import org.joda.time.Duration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; /** Unit tests for {@link PremiumListDualDao}. */ -@DualDatabaseTest public class PremiumListDualDaoTest extends EntityTestCase { // Set long persist times on caches so they can be tested (cache times default to 0 in tests). @@ -63,19 +50,6 @@ public class PremiumListDualDaoTest extends EntityTestCase { void before() { createTld("tld"); fakeClock.setAutoIncrementStep(Duration.millis(1)); - fakeClock.setTo(DateTime.parse("1984-12-21T00:00:00.000Z")); - DatabaseTransitionSchedule schedule = - DatabaseTransitionSchedule.create( - TransitionId.DOMAIN_LABEL_LISTS, - TimedTransitionProperty.fromValueMap( - ImmutableSortedMap.of( - START_OF_TIME, - PrimaryDatabase.DATASTORE, - fakeClock.nowUtc().plusDays(1), - PrimaryDatabase.CLOUD_SQL), - PrimaryDatabaseTransition.class)); - - tm().transactNew(() -> ofyTm().putWithoutBackup(schedule)); } @AfterEach @@ -83,22 +57,8 @@ public class PremiumListDualDaoTest extends EntityTestCase { fakeClock.setAutoIncrementStep(Duration.ZERO); } - @TestOfyAndSql - void testGetPremiumPrice_secondaryLoadMissingSql() { - PremiumListSqlDao.delete(PremiumListSqlDao.getLatestRevision("tld").get()); - assertThat( - assertThrows( - IllegalStateException.class, - () -> PremiumListDualDao.getPremiumPrice("brass", Registry.get("tld")))) - .hasMessageThat() - .isEqualTo( - "Unequal prices for domain brass.tld from primary Datastore DB " - + "(Optional[USD 20.00]) and secondary SQL db (Optional.empty)."); - } - - @TestOfyAndSql + @Test void testGetPremiumPrice_secondaryLoadMissingOfy() { - fakeClock.advanceBy(Duration.standardDays(5)); PremiumList premiumList = PremiumListDatastoreDao.getLatestRevision("tld").get(); PremiumListDatastoreDao.delete(premiumList); assertThat( @@ -111,22 +71,8 @@ public class PremiumListDualDaoTest extends EntityTestCase { + "and secondary Datastore db (Optional.empty)."); } - @TestOfyAndSql - void testGetPremiumPrice_secondaryDifferentSql() { - PremiumListSqlDao.save("tld", ImmutableList.of("brass,USD 50")); - assertThat( - assertThrows( - IllegalStateException.class, - () -> PremiumListDualDao.getPremiumPrice("brass", Registry.get("tld")))) - .hasMessageThat() - .isEqualTo( - "Unequal prices for domain brass.tld from primary Datastore DB " - + "(Optional[USD 20.00]) and secondary SQL db (Optional[USD 50.00])."); - } - - @TestOfyAndSql + @Test void testGetPremiumPrice_secondaryDifferentOfy() { - fakeClock.advanceBy(Duration.standardDays(5)); PremiumListDatastoreDao.save("tld", ImmutableList.of("brass,USD 50")); assertThat( assertThrows( @@ -138,7 +84,7 @@ public class PremiumListDualDaoTest extends EntityTestCase { + "(Optional[USD 20.00]) and secondary Datastore db (Optional[USD 50.00])."); } - @TestOfyAndSql + @Test void testGetPremiumPrice_returnsNoPriceWhenNoPremiumListConfigured() { createTld("ghost"); persistResource( @@ -151,14 +97,14 @@ public class PremiumListDualDaoTest extends EntityTestCase { Truth8.assertThat(PremiumListDualDao.getPremiumPrice("blah", Registry.get("ghost"))).isEmpty(); } - @TestOfyAndSql + @Test void testGetPremiumPrice_emptyWhenPremiumListDeleted() { PremiumList toDelete = PremiumListDualDao.getLatestRevision("tld").get(); PremiumListDualDao.delete(toDelete); Truth8.assertThat(PremiumListDualDao.getPremiumPrice("blah", Registry.get("tld"))).isEmpty(); } - @TestOfyAndSql + @Test void getPremiumPrice_returnsNoneWhenNoPremiumListConfigured() { persistResource(newRegistry("foobar", "FOOBAR").asBuilder().setPremiumList(null).build()); Truth8.assertThat(PremiumListDualDao.getPremiumPrice("rich", Registry.get("foobar"))).isEmpty(); diff --git a/core/src/test/java/google/registry/model/registry/label/PremiumListTest.java b/core/src/test/java/google/registry/model/registry/label/PremiumListTest.java index 5bc0a3252..ab7618bf2 100644 --- a/core/src/test/java/google/registry/model/registry/label/PremiumListTest.java +++ b/core/src/test/java/google/registry/model/registry/label/PremiumListTest.java @@ -16,7 +16,6 @@ package google.registry.model.registry.label; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.persistPremiumList; import static google.registry.testing.DatabaseHelper.persistReservedList; @@ -24,9 +23,9 @@ import static google.registry.testing.DatabaseHelper.persistResource; import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableList; +import com.google.common.hash.BloomFilter; import google.registry.model.registry.Registry; import google.registry.model.registry.label.PremiumList.PremiumListEntry; -import google.registry.model.registry.label.PremiumList.PremiumListRevision; import google.registry.testing.AppEngineExtension; import org.joda.money.Money; import org.junit.jupiter.api.BeforeEach; @@ -68,13 +67,13 @@ public class PremiumListTest { } @Test - void testProbablePremiumLabels() { + void testBloomFilter() { PremiumList pl = PremiumListDualDao.getLatestRevision("tld").get(); - PremiumListRevision revision = ofy().load().key(pl.getRevisionKey()).now(); - assertThat(revision.getProbablePremiumLabels().mightContain("notpremium")).isFalse(); + BloomFilter bloomFilter = pl.getBloomFilter(); + assertThat(bloomFilter.mightContain("notpremium")).isFalse(); for (String label : ImmutableList.of("rich", "lol", "johnny-be-goode", "icann")) { assertWithMessage(label + " should be a probable premium") - .that(revision.getProbablePremiumLabels().mightContain(label)) + .that(bloomFilter.mightContain(label)) .isTrue(); } } diff --git a/core/src/test/java/google/registry/model/registry/label/ReservedListDualDatabaseDaoTest.java b/core/src/test/java/google/registry/model/registry/label/ReservedListDualDatabaseDaoTest.java index 6d89b0008..edcd035b0 100644 --- a/core/src/test/java/google/registry/model/registry/label/ReservedListDualDatabaseDaoTest.java +++ b/core/src/test/java/google/registry/model/registry/label/ReservedListDualDatabaseDaoTest.java @@ -15,36 +15,17 @@ package google.registry.model.registry.label; import static com.google.common.truth.Truth.assertThat; -import static google.registry.model.ofy.ObjectifyService.ofy; -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.model.registry.label.ReservedList.ReservedListEntry; -import google.registry.testing.DualDatabaseTest; -import google.registry.testing.SystemPropertyExtension; -import google.registry.testing.TestOfyAndSql; import java.util.Optional; -import org.joda.time.DateTime; -import org.joda.time.Duration; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.Test; -@DualDatabaseTest public class ReservedListDualDatabaseDaoTest extends EntityTestCase { - @RegisterExtension - final SystemPropertyExtension systemPropertyExtension = new SystemPropertyExtension(); - private ImmutableMap reservations; private ReservedList reservedList; @@ -65,41 +46,18 @@ public class ReservedListDualDatabaseDaoTest extends EntityTestCase { .setShouldPublish(false) .setReservedListMap(reservations) .build(); - - fakeClock.setTo(DateTime.parse("1984-12-21T00:00:00.000Z")); - DatabaseTransitionSchedule schedule = - DatabaseTransitionSchedule.create( - TransitionId.DOMAIN_LABEL_LISTS, - 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() { + @Test + void testSave_success() { ReservedListDualDatabaseDao.save(reservedList); Optional savedList = ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()); assertThat(savedList.get()).isEqualTo(reservedList); } - @TestOfyAndSql - void testSave_cloudSqlPrimary_success() { - fakeClock.advanceBy(Duration.standardDays(5)); - ReservedListDualDatabaseDao.save(reservedList); - Optional savedList = - ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()); - assertThat(savedList.get()).isEqualTo(reservedList); - } - - @TestOfyAndSql - void testDelete_datastorePrimary_success() { + @Test + void testDelete_success() { ReservedListDualDatabaseDao.save(reservedList); assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()).isPresent()) .isTrue(); @@ -108,19 +66,8 @@ public class ReservedListDualDatabaseDaoTest extends EntityTestCase { .isFalse(); } - @TestOfyAndSql - void testDelete_cloudSqlPrimary_success() { - fakeClock.advanceBy(Duration.standardDays(5)); - ReservedListDualDatabaseDao.save(reservedList); - assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()).isPresent()) - .isTrue(); - ReservedListDualDatabaseDao.delete(reservedList); - assertThat(ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()).isPresent()) - .isFalse(); - } - - @TestOfyAndSql - void testSaveAndLoad_datastorePrimary_emptyList() { + @Test + void testSaveAndLoad_emptyList() { ReservedList list = new ReservedList.Builder() .setName("empty") @@ -132,22 +79,8 @@ public class ReservedListDualDatabaseDaoTest extends EntityTestCase { assertThat(savedList.get()).isEqualTo(list); } - @TestOfyAndSql - void testSaveAndLoad_cloudSqlPrimary_emptyList() { - fakeClock.advanceBy(Duration.standardDays(5)); - ReservedList list = - new ReservedList.Builder() - .setName("empty") - .setLastUpdateTime(fakeClock.nowUtc()) - .setReservedListMap(ImmutableMap.of()) - .build(); - ReservedListDualDatabaseDao.save(list); - Optional savedList = ReservedListDualDatabaseDao.getLatestRevision("empty"); - assertThat(savedList.get()).isEqualTo(list); - } - - @TestOfyAndSql - void testSave_datastorePrimary_multipleVersions() { + @Test + void testSave_multipleVersions() { ReservedListDualDatabaseDao.save(reservedList); assertThat( ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()) @@ -173,36 +106,8 @@ public class ReservedListDualDatabaseDaoTest extends EntityTestCase { .isEqualTo(newReservations); } - @TestOfyAndSql - void testSave_cloudSqlPrimary_multipleVersions() { - fakeClock.advanceBy(Duration.standardDays(5)); - ReservedListDualDatabaseDao.save(reservedList); - assertThat( - ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()) - .get() - .getReservedListEntries()) - .isEqualTo(reservations); - ImmutableMap newReservations = - ImmutableMap.of( - "food", - ReservedListEntry.create("food", ReservationType.RESERVED_FOR_SPECIFIC_USE, null)); - ReservedList secondList = - new ReservedList.Builder() - .setName("testlist2") - .setLastUpdateTime(fakeClock.nowUtc()) - .setShouldPublish(false) - .setReservedListMap(newReservations) - .build(); - ReservedListDualDatabaseDao.save(secondList); - assertThat( - ReservedListDualDatabaseDao.getLatestRevision(secondList.getName()) - .get() - .getReservedListEntries()) - .isEqualTo(newReservations); - } - - @TestOfyAndSql - void testLoad_datastorePrimary_unequalLists() { + @Test + void testLoad_unequalLists() { ReservedListDualDatabaseDao.save(reservedList); ReservedList secondList = new ReservedList.Builder() @@ -222,78 +127,16 @@ public class ReservedListDualDatabaseDaoTest extends EntityTestCase { () -> ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName())); assertThat(thrown) .hasMessageThat() - .contains("Domain label music has entry in Datastore, but not in the secondary database."); + .contains("Domain label music has entry in Datastore, but not in Cloud SQL."); } - @TestOfyAndSql - void testLoad_cloudSqlPrimary_unequalLists() { - fakeClock.advanceBy(Duration.standardDays(5)); - ReservedListDualDatabaseDao.save(reservedList); - ReservedList secondList = - new ReservedList.Builder() - .setName(reservedList.name) - .setLastUpdateTime(fakeClock.nowUtc()) - .setShouldPublish(false) - .setReservedListMap( - ImmutableMap.of( - "food", - ReservedListEntry.create( - "food", ReservationType.RESERVED_FOR_SPECIFIC_USE, null))) - .build(); - ReservedListSqlDao.save(secondList); - IllegalStateException thrown = - assertThrows( - IllegalStateException.class, - () -> ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName())); - assertThat(thrown) - .hasMessageThat() - .contains("Domain label music has entry in Datastore, but not in the primary database."); - } - - @TestOfyAndSql - void testLoad_cloudSqlPrimary_unequalLists_succeedsInProduction() { - RegistryEnvironment.PRODUCTION.setup(systemPropertyExtension); - fakeClock.advanceBy(Duration.standardDays(5)); - ReservedListDualDatabaseDao.save(reservedList); - ReservedList secondList = - new ReservedList.Builder() - .setName(reservedList.name) - .setLastUpdateTime(fakeClock.nowUtc()) - .setShouldPublish(false) - .setReservedListMap( - ImmutableMap.of( - "food", - ReservedListEntry.create( - "food", ReservationType.RESERVED_FOR_SPECIFIC_USE, null))) - .build(); - ReservedListSqlDao.save(secondList); - Optional savedList = - ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName()); - assertThat(savedList.get()).isEqualTo(secondList); - } - - @TestOfyAndSql - void testLoad_DatastorePrimary_noListInCloudSql() { - ReservedListDatastoreDao.save(reservedList); - IllegalStateException thrown = - assertThrows( - IllegalStateException.class, - () -> ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName())); - assertThat(thrown) - .hasMessageThat() - .contains("Reserved list in the secondary database (Cloud SQL) is empty."); - } - - @TestOfyAndSql - void testLoad_cloudSqlPrimary_noListInDatastore() { - fakeClock.advanceBy(Duration.standardDays(5)); + @Test + void testLoad_noListInDatastore() { ReservedListSqlDao.save(reservedList); IllegalStateException thrown = assertThrows( IllegalStateException.class, () -> ReservedListDualDatabaseDao.getLatestRevision(reservedList.getName())); - assertThat(thrown) - .hasMessageThat() - .contains("Reserved list in the secondary database (Datastore) is empty."); + assertThat(thrown).hasMessageThat().contains("Reserved list in Datastore is empty."); } } diff --git a/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java b/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java index ad1523939..7d994fec1 100644 --- a/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java @@ -21,15 +21,10 @@ import static google.registry.testing.DatabaseHelper.createTlds; import static google.registry.testing.DatabaseHelper.persistReservedList; import static google.registry.testing.DatabaseHelper.persistResource; import static google.registry.tools.CreateReservedListCommand.INVALID_FORMAT_ERROR_MESSAGE; -import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; -import com.google.common.collect.ImmutableMap; import google.registry.model.registry.Registry; import google.registry.model.registry.label.ReservedList; -import google.registry.model.registry.label.ReservedList.ReservedListEntry; -import google.registry.model.registry.label.ReservedListSqlDao; -import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -58,16 +53,6 @@ class CreateReservedListCommandTest assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent(); } - @Test - void testSuccess_timestampsSetCorrectly() throws Exception { - DateTime before = DateTime.now(UTC); - runCommandForced("--input=" + reservedTermsPath); - assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isPresent(); - ReservedList rl = ReservedList.get("xn--q9jyb4c_common-reserved").get(); - assertThat(rl.getCreationTime()).isAtLeast(before); - assertThat(rl.getLastUpdateTime()).isEqualTo(rl.getCreationTime()); - } - @Test void testSuccess_shouldPublishDefaultsToTrue() throws Exception { runCommandForced("--input=" + reservedTermsPath); @@ -187,21 +172,6 @@ class CreateReservedListCommandTest verifyXnq9jyb4cInCloudSql(); } - @Test - void testSaveToCloudSql_noExceptionThrownWhenSaveFail() throws Exception { - // Note that, during the dual-write phase, we want to make sure that no exception will be - // thrown if saving reserved list to Cloud SQL fails. - ReservedListSqlDao.save( - createCloudSqlReservedList( - "xn--q9jyb4c_common-reserved", - fakeClock.nowUtc(), - true, - ImmutableMap.of( - "testdomain", ReservedListEntry.create("testdomain", FULLY_BLOCKED, "")))); - runCommandForced("--name=xn--q9jyb4c_common-reserved", "--input=" + reservedTermsPath); - verifyXnq9jyb4cInDatastore(); - } - private void runNameTestExpectedFailure(String name, String expectedErrorMsg) { IllegalArgumentException thrown = assertThrows( diff --git a/core/src/test/java/google/registry/tools/DeletePremiumListCommandTest.java b/core/src/test/java/google/registry/tools/DeletePremiumListCommandTest.java index 1dc37669d..3c0662af8 100644 --- a/core/src/test/java/google/registry/tools/DeletePremiumListCommandTest.java +++ b/core/src/test/java/google/registry/tools/DeletePremiumListCommandTest.java @@ -16,7 +16,6 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; -import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadPremiumListEntries; import static google.registry.testing.DatabaseHelper.persistPremiumList; @@ -25,7 +24,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import google.registry.model.registry.Registry; import google.registry.model.registry.label.PremiumList; -import google.registry.model.registry.label.PremiumList.PremiumListEntry; import google.registry.model.registry.label.PremiumListDualDao; import org.junit.jupiter.api.Test; @@ -38,13 +36,6 @@ class DeletePremiumListCommandTest extends CommandTestCase