Correctly get the primary database value in PremiumListDualDao (#1052)

* Correctly get the primary database value in PremiumListDualDao

* Remove extra AppEngineExtension

* get rid of ofy call

* Remove extra duration skip in test
This commit is contained in:
sarahcaseybot 2021-04-05 17:44:30 +00:00 committed by GitHub
parent 7c3ef52026
commit eabf056f9b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 43 additions and 28 deletions

View file

@ -16,9 +16,10 @@ package google.registry.model.registry.label;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static google.registry.model.DatabaseMigrationUtils.suppressExceptionUnlessInTest;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
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;
@ -46,8 +47,7 @@ public class PremiumListDualDao {
* or absent if no such list exists.
*/
public static Optional<PremiumList> getLatestRevision(String premiumListName) {
// TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
if (tm().isOfy()) {
if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
return PremiumListDatastoreDao.getLatestRevision(premiumListName);
} else {
return PremiumListSqlDao.getLatestRevision(premiumListName);
@ -68,16 +68,14 @@ public class PremiumListDualDao {
}
String premiumListName = registry.getPremiumList().getName();
Optional<Money> primaryResult;
// TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
if (tm().isOfy()) {
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.
// TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
if (tm().isOfy()) {
if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
suppressExceptionUnlessInTest(
() -> {
Optional<Money> secondaryResult =
@ -120,8 +118,7 @@ public class PremiumListDualDao {
*/
public static PremiumList save(String name, List<String> inputData) {
PremiumList result;
// TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
if (tm().isOfy()) {
if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
result = PremiumListDatastoreDao.save(name, inputData);
suppressExceptionUnlessInTest(
() -> PremiumListSqlDao.save(name, inputData), "Error when saving premium list to SQL.");
@ -141,8 +138,7 @@ public class PremiumListDualDao {
* secondary database.
*/
public static void delete(PremiumList premiumList) {
// TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
if (tm().isOfy()) {
if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
PremiumListDatastoreDao.delete(premiumList);
suppressExceptionUnlessInTest(
() -> PremiumListSqlDao.delete(premiumList),
@ -159,8 +155,7 @@ public class PremiumListDualDao {
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
// TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
if (tm().isOfy()) {
if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
return PremiumListDatastoreDao.getLatestRevision(premiumListName).isPresent();
} else {
return PremiumListSqlDao.getLatestRevision(premiumListName).isPresent();
@ -179,8 +174,7 @@ public class PremiumListDualDao {
() ->
new IllegalArgumentException(
String.format("No premium list with name %s.", premiumListName)));
// TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check
if (tm().isOfy()) {
if (DatabaseMigrationUtils.isDatastore(TransitionId.DOMAIN_LABEL_LISTS)) {
return PremiumListDatastoreDao.loadPremiumListEntriesUncached(premiumList);
} else {
CurrencyUnit currencyUnit = premiumList.getCurrency();

View file

@ -15,35 +15,40 @@
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.AppEngineExtension;
import google.registry.testing.DualDatabaseTest;
import google.registry.testing.TestCacheExtension;
import google.registry.testing.TestOfyAndSql;
import google.registry.testing.TestOfyOnly;
import google.registry.testing.TestSqlOnly;
import org.joda.time.DateTime;
import org.joda.time.Duration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.extension.RegisterExtension;
/** Unit tests for {@link PremiumListDualDao}. */
@DualDatabaseTest
public class PremiumListDualDaoTest {
@RegisterExtension
public final AppEngineExtension appEngine =
AppEngineExtension.builder().withDatastoreAndCloudSql().build();
public class PremiumListDualDaoTest extends EntityTestCase {
// Set long persist times on caches so they can be tested (cache times default to 0 in tests).
@RegisterExtension
@ -56,9 +61,23 @@ public class PremiumListDualDaoTest {
@BeforeEach
void before() {
createTld("tld");
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));
}
@TestOfyOnly
@TestOfyAndSql
void testGetPremiumPrice_secondaryLoadMissingSql() {
PremiumListSqlDao.delete(PremiumListSqlDao.getLatestRevision("tld").get());
assertThat(
@ -71,8 +90,9 @@ public class PremiumListDualDaoTest {
+ "(Optional[USD 20.00]) and secondary SQL db (Optional.empty).");
}
@TestSqlOnly
@TestOfyAndSql
void testGetPremiumPrice_secondaryLoadMissingOfy() {
fakeClock.advanceBy(Duration.standardDays(5));
PremiumList premiumList = PremiumListDatastoreDao.getLatestRevision("tld").get();
PremiumListDatastoreDao.delete(premiumList);
assertThat(
@ -85,7 +105,7 @@ public class PremiumListDualDaoTest {
+ "and secondary Datastore db (Optional.empty).");
}
@TestOfyOnly
@TestOfyAndSql
void testGetPremiumPrice_secondaryDifferentSql() {
PremiumListSqlDao.save("tld", ImmutableList.of("brass,USD 50"));
assertThat(
@ -98,8 +118,9 @@ public class PremiumListDualDaoTest {
+ "(Optional[USD 20.00]) and secondary SQL db (Optional[USD 50.00]).");
}
@TestSqlOnly
@TestOfyAndSql
void testGetPremiumPrice_secondaryDifferentOfy() {
fakeClock.advanceBy(Duration.standardDays(5));
PremiumListDatastoreDao.save("tld", ImmutableList.of("brass,USD 50"));
assertThat(
assertThrows(

View file

@ -69,7 +69,7 @@ class ListPremiumListsActionTest extends ListActionTestCase {
Optional.empty(),
"^name\\s+labelsToPrices\\s*$",
"^-+\\s+-+\\s*$",
"^how\\s+\\{richer=5000\\.00\\}$",
"^how\\s+\\{richer=5000\\}\\s+$",
"^xn--q9jyb4c\\s+\\{rich=100\\.00\\}\\s+$");
}