From a07fbb27c51fb5ed098a9df6f9dfeeb188b7d9b7 Mon Sep 17 00:00:00 2001 From: gbrodman Date: Mon, 22 Feb 2021 21:19:48 -0500 Subject: [PATCH] Refactor PremiumList storage and retrieval for dual-database setup (#950) * Refactor PremiumList storage and retrieval for dual-database setup Previously, the storage and retrieval code was scattered across various places haphazardly and there was no good way to set up dual database access. This reorganizes the code so that retrieval is simpler and it allows for dual-write and dual-read. This includes the following changes: - Move all static / object retrieval code out of PremiumList -- the class should solely consist of its data and methods on its data and it shouldn't have to worry about complicated caching or retrieval - Split all PremiumList retrieval methods into PremiumListDatastoreDao and PremiumListSqlDao that handle retrieval of the premium list entry objects from the corresponding databases (since the way the actual data itself is stored is not the same between the two - Create a dual-DAO for PremiumList retrieval that branches between SQL/Datastore depending on which is appropriate -- it will read from and write to both but only log errors for the secondary DB - Cache the mapping from name to premium list in the dual-DAO. This is a common code path regardless of database so we can cache it at a high level - Cache the ways to go from premium list -> premium entries in the Datastore and SQL DAOs. These caches are specific to the corresponding DB and should thus be stored in the corresponding DAO. - Moves the database-choosing code from the actions to the lower-level dual-DAO. This is because we will often wish to access this premium list data in flows and all accesses should use the proper DB-selecting code --- .../export/ExportPremiumTermsAction.java | 10 +- .../registry/model/OteAccountBuilder.java | 3 +- .../StaticPremiumListPricingEngine.java | 4 +- .../model/registry/label/PremiumList.java | 133 +------ .../label/PremiumListDatastoreDao.java | 365 ++++++++++++++++++ .../registry/label/PremiumListDualDao.java | 199 ++++++++++ .../registry/label/PremiumListUtils.java | 255 ------------ .../registry/schema/tld/PremiumEntry.java | 15 + .../registry/schema/tld/PremiumListCache.java | 106 ----- .../registry/schema/tld/PremiumListDao.java | 175 --------- .../schema/tld/PremiumListSqlDao.java | 238 ++++++++++++ .../registry/schema/tld/PremiumListUtils.java | 8 +- .../tools/CreateOrUpdateTldCommand.java | 4 +- .../tools/DeletePremiumListCommand.java | 16 +- .../CreateOrUpdatePremiumListAction.java | 19 +- .../tools/server/CreatePremiumListAction.java | 38 +- .../tools/server/UpdatePremiumListAction.java | 27 +- .../ReplayCommitLogsToSqlActionTest.java | 10 +- .../beam/initsql/BackupTestStoreTest.java | 13 + .../beam/initsql/CommitLogTransformsTest.java | 16 +- .../beam/initsql/DomainBaseUtilTest.java | 2 +- ....java => ExportLoadingTransformsTest.java} | 15 +- .../initsql/LoadDatastoreSnapshotTest.java | 13 + .../export/ExportPremiumTermsActionTest.java | 28 +- .../registry/model/registry/RegistryTest.java | 3 +- ....java => PremiumListDatastoreDaoTest.java} | 186 ++++----- .../label/PremiumListDualDaoTest.java | 139 +++++++ .../model/registry/label/PremiumListTest.java | 4 +- .../integration/SqlIntegrationTestSuite.java | 4 +- ...aoTest.java => PremiumListSqlDaoTest.java} | 126 +++--- .../schema/tld/PremiumListUtilsTest.java | 6 +- .../registry/testing/DatabaseHelper.java | 65 ++-- .../registry/testing/TestCacheExtension.java | 12 +- .../tools/CreateDomainCommandTest.java | 4 +- .../tools/DeletePremiumListCommandTest.java | 5 +- .../server/CreatePremiumListActionTest.java | 16 +- .../server/UpdatePremiumListActionTest.java | 34 +- 37 files changed, 1270 insertions(+), 1046 deletions(-) create mode 100644 core/src/main/java/google/registry/model/registry/label/PremiumListDatastoreDao.java create mode 100644 core/src/main/java/google/registry/model/registry/label/PremiumListDualDao.java delete mode 100644 core/src/main/java/google/registry/model/registry/label/PremiumListUtils.java delete mode 100644 core/src/main/java/google/registry/schema/tld/PremiumListCache.java delete mode 100644 core/src/main/java/google/registry/schema/tld/PremiumListDao.java create mode 100644 core/src/main/java/google/registry/schema/tld/PremiumListSqlDao.java rename core/src/test/java/google/registry/beam/initsql/{ExportloadingTransformsTest.java => ExportLoadingTransformsTest.java} (92%) rename core/src/test/java/google/registry/model/registry/label/{PremiumListUtilsTest.java => PremiumListDatastoreDaoTest.java} (63%) create mode 100644 core/src/test/java/google/registry/model/registry/label/PremiumListDualDaoTest.java rename core/src/test/java/google/registry/schema/tld/{PremiumListDaoTest.java => PremiumListSqlDaoTest.java} (65%) diff --git a/core/src/main/java/google/registry/export/ExportPremiumTermsAction.java b/core/src/main/java/google/registry/export/ExportPremiumTermsAction.java index 5178728ed..74f2a5edc 100644 --- a/core/src/main/java/google/registry/export/ExportPremiumTermsAction.java +++ b/core/src/main/java/google/registry/export/ExportPremiumTermsAction.java @@ -17,7 +17,6 @@ package google.registry.export; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; -import static google.registry.model.registry.label.PremiumListUtils.loadPremiumListEntries; import static google.registry.request.Action.Method.POST; import static java.nio.charset.StandardCharsets.UTF_8; import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR; @@ -32,7 +31,7 @@ import com.google.common.flogger.FluentLogger; import com.google.common.net.MediaType; import google.registry.config.RegistryConfig.Config; import google.registry.model.registry.Registry; -import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.RequestParameters; @@ -137,10 +136,11 @@ public class ExportPremiumTermsAction implements Runnable { } private String getFormattedPremiumTerms(Registry registry) { - Optional premiumList = PremiumList.getCached(registry.getPremiumList().getName()); - checkState(premiumList.isPresent(), "Could not load premium list for " + tld); + String premiumListName = registry.getPremiumList().getName(); + checkState( + PremiumListDualDao.exists(premiumListName), "Could not load premium list for " + tld); SortedSet premiumTerms = - Streams.stream(loadPremiumListEntries(premiumList.get())) + Streams.stream(PremiumListDualDao.loadAllPremiumListEntries(premiumListName)) .map(entry -> Joiner.on(",").join(entry.getLabel(), entry.getValue())) .collect(ImmutableSortedSet.toImmutableSortedSet(String::compareTo)); diff --git a/core/src/main/java/google/registry/model/OteAccountBuilder.java b/core/src/main/java/google/registry/model/OteAccountBuilder.java index f34ee455c..56f236788 100644 --- a/core/src/main/java/google/registry/model/OteAccountBuilder.java +++ b/core/src/main/java/google/registry/model/OteAccountBuilder.java @@ -40,6 +40,7 @@ import google.registry.model.registrar.RegistrarContact; import google.registry.model.registry.Registry; import google.registry.model.registry.Registry.TldState; import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.util.CidrAddressBlock; import java.util.Collection; import java.util.Optional; @@ -288,7 +289,7 @@ public final class OteAccountBuilder { boolean isEarlyAccess, int roidSuffix) { String tldNameAlphaNumerical = tldName.replaceAll("[^a-z0-9]", ""); - Optional premiumList = PremiumList.getUncached(DEFAULT_PREMIUM_LIST); + Optional premiumList = PremiumListDualDao.getLatestRevision(DEFAULT_PREMIUM_LIST); checkState(premiumList.isPresent(), "Couldn't find premium list %s.", DEFAULT_PREMIUM_LIST); Registry.Builder builder = new Registry.Builder() diff --git a/core/src/main/java/google/registry/model/pricing/StaticPremiumListPricingEngine.java b/core/src/main/java/google/registry/model/pricing/StaticPremiumListPricingEngine.java index 6d3ed38bd..c94d73492 100644 --- a/core/src/main/java/google/registry/model/pricing/StaticPremiumListPricingEngine.java +++ b/core/src/main/java/google/registry/model/pricing/StaticPremiumListPricingEngine.java @@ -15,11 +15,11 @@ package google.registry.model.pricing; import static com.google.common.base.Preconditions.checkNotNull; -import static google.registry.model.registry.label.PremiumListUtils.getPremiumPrice; import static google.registry.util.DomainNameUtils.getTldFromDomainName; import com.google.common.net.InternetDomainName; import google.registry.model.registry.Registry; +import google.registry.model.registry.label.PremiumListDualDao; import java.util.Optional; import javax.inject.Inject; import org.joda.money.Money; @@ -38,7 +38,7 @@ public final class StaticPremiumListPricingEngine implements PremiumPricingEngin String tld = getTldFromDomainName(fullyQualifiedDomainName); String label = InternetDomainName.from(fullyQualifiedDomainName).parts().get(0); Registry registry = Registry.get(checkNotNull(tld, "tld")); - Optional premiumPrice = getPremiumPrice(label, registry); + Optional premiumPrice = PremiumListDualDao.getPremiumPrice(label, registry); return DomainPrices.create( premiumPrice.isPresent(), premiumPrice.orElse(registry.getStandardCreateCost()), diff --git a/core/src/main/java/google/registry/model/registry/label/PremiumList.java b/core/src/main/java/google/registry/model/registry/label/PremiumList.java index c3be24cc2..285c72e81 100644 --- a/core/src/main/java/google/registry/model/registry/label/PremiumList.java +++ b/core/src/main/java/google/registry/model/registry/label/PremiumList.java @@ -18,23 +18,12 @@ import static com.google.common.base.Charsets.US_ASCII; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.hash.Funnels.stringFunnel; import static com.google.common.hash.Funnels.unencodedCharsFunnel; -import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration; -import static google.registry.config.RegistryConfig.getSingletonCachePersistDuration; -import static google.registry.config.RegistryConfig.getStaticPremiumListMaxCachedEntries; -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.persistence.transaction.TransactionManagerFactory.tm; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Splitter; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.CacheLoader.InvalidCacheLoadException; -import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableMap; import com.google.common.hash.BloomFilter; -import com.google.common.util.concurrent.UncheckedExecutionException; import com.googlecode.objectify.Key; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; @@ -46,18 +35,14 @@ import google.registry.model.annotations.ReportedOn; import google.registry.model.registry.Registry; import google.registry.schema.replay.DatastoreOnlyEntity; import google.registry.schema.replay.NonReplicatedEntity; -import google.registry.schema.tld.PremiumListDao; -import google.registry.util.NonFinalForTesting; +import google.registry.schema.tld.PremiumListSqlDao; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.math.BigDecimal; import java.util.List; import java.util.Map; -import java.util.NoSuchElementException; import java.util.Objects; -import java.util.Optional; import java.util.Set; -import java.util.concurrent.ExecutionException; import javax.annotation.Nullable; import javax.persistence.CollectionTable; import javax.persistence.Column; @@ -72,7 +57,6 @@ import javax.persistence.Transient; import org.hibernate.LazyInitializationException; import org.joda.money.CurrencyUnit; import org.joda.money.Money; -import org.joda.time.Duration; /** * A premium list entity that is used to check domain label prices. @@ -171,124 +155,11 @@ public final class PremiumList extends BaseDomainLabelListThis is cached for a shorter duration because we need to periodically reload this entity to - * check if a new revision has been published, and if so, then use that. - */ - @NonFinalForTesting - static LoadingCache cachePremiumLists = - createCachePremiumLists(getDomainLabelListCacheDuration()); - - @VisibleForTesting - public static void setPremiumListCacheForTest(Optional expiry) { - Duration effectiveExpiry = expiry.orElse(getDomainLabelListCacheDuration()); - cachePremiumLists = createCachePremiumLists(effectiveExpiry); - } - - @VisibleForTesting - static LoadingCache createCachePremiumLists(Duration cachePersistDuration) { - return CacheBuilder.newBuilder() - .expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis())) - .build( - new CacheLoader() { - @Override - public PremiumList load(final String name) { - return tm().doTransactionless(() -> loadPremiumList(name)); - } - }); - } - - private static PremiumList loadPremiumList(String name) { - return tm().isOfy() - ? ofy().load().type(PremiumList.class).parent(getCrossTldKey()).id(name).now() - : PremiumListDao.getLatestRevision(name).orElseThrow(NoSuchElementException::new); - } - - /** - * In-memory cache for {@link PremiumListRevision}s, used for retrieving Bloom filters quickly. - * - *

This is cached for a long duration (essentially indefinitely) because a given {@link - * PremiumListRevision} is immutable and cannot ever be changed once created, so its cache need - * not ever expire. - */ - static final LoadingCache, PremiumListRevision> - cachePremiumListRevisions = - CacheBuilder.newBuilder() - .expireAfterWrite( - java.time.Duration.ofMillis(getSingletonCachePersistDuration().getMillis())) - .build( - new CacheLoader, PremiumListRevision>() { - @Override - public PremiumListRevision load(final Key revisionKey) { - return tm().doTransactionless(() -> ofy().load().key(revisionKey).now()); - } - }); - - /** - * In-memory cache for {@link PremiumListEntry}s for a given label and {@link PremiumListRevision} - * - *

Because the PremiumList itself makes up part of the PremiumListRevision's key, this is - * specific to a given premium list. Premium list entries might not be present, as indicated by - * the Optional wrapper, and we want to cache that as well. - * - *

This is cached for a long duration (essentially indefinitely) because a given {@link - * PremiumListRevision} and its child {@link PremiumListEntry}s are immutable and cannot ever be - * changed once created, so the cache need not ever expire. - * - *

A maximum size is set here on the cache because it can potentially grow too big to fit in - * memory if there are a large number of distinct premium list entries being queried (both those - * that exist, as well as those that might exist according to the Bloom filter, must be cached). - * The entries judged least likely to be accessed again will be evicted first. - */ - @NonFinalForTesting - static LoadingCache, Optional> cachePremiumListEntries = - createCachePremiumListEntries(getSingletonCachePersistDuration()); - - @VisibleForTesting - public static void setPremiumListEntriesCacheForTest(Optional expiry) { - Duration effectiveExpiry = expiry.orElse(getSingletonCachePersistDuration()); - cachePremiumListEntries = createCachePremiumListEntries(effectiveExpiry); - } - - @VisibleForTesting - static LoadingCache, Optional> - createCachePremiumListEntries(Duration cachePersistDuration) { - return CacheBuilder.newBuilder() - .expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis())) - .maximumSize(getStaticPremiumListMaxCachedEntries()) - .build( - new CacheLoader, Optional>() { - @Override - public Optional load(final Key entryKey) { - return tm().doTransactionless( - () -> Optional.ofNullable(ofy().load().key(entryKey).now())); - } - }); - } - @VisibleForTesting public Key getRevisionKey() { return revisionKey; } - /** Returns the PremiumList with the specified name, from cache. */ - public static Optional getCached(String name) { - try { - return Optional.of(cachePremiumLists.get(name)); - } catch (InvalidCacheLoadException e) { - return Optional.empty(); - } catch (ExecutionException e) { - throw new UncheckedExecutionException("Could not retrieve premium list named " + name, e); - } - } - - /** Returns the PremiumList with the specified name, uncached. */ - public static Optional getUncached(String name) { - return Optional.ofNullable(loadPremiumList(name)); - } - /** Returns the {@link CurrencyUnit} used for this list. */ public CurrencyUnit getCurrency() { return currency; @@ -300,7 +171,7 @@ public final class PremiumList extends BaseDomainLabelListNote that this is lazily loaded and thus will throw a {@link LazyInitializationException} if * used outside the transaction in which the given entity was loaded. You generally should not be * using this anyway as it's inefficient to load all of the PremiumEntry rows if you don't need - * them. To check prices, use {@link PremiumListDao#getPremiumPrice} instead. + * them. To check prices, use {@link PremiumListSqlDao#getPremiumPrice} instead. */ @Nullable public ImmutableMap getLabelsToPrices() { diff --git a/core/src/main/java/google/registry/model/registry/label/PremiumListDatastoreDao.java b/core/src/main/java/google/registry/model/registry/label/PremiumListDatastoreDao.java new file mode 100644 index 000000000..ea4845c11 --- /dev/null +++ b/core/src/main/java/google/registry/model/registry/label/PremiumListDatastoreDao.java @@ -0,0 +1,365 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.model.registry.label; + +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.common.collect.Iterables.partition; +import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration; +import static google.registry.config.RegistryConfig.getSingletonCachePersistDuration; +import static google.registry.config.RegistryConfig.getStaticPremiumListMaxCachedEntries; +import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; +import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.BLOOM_FILTER_NEGATIVE; +import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.CACHED_NEGATIVE; +import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.CACHED_POSITIVE; +import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.UNCACHED_NEGATIVE; +import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.UNCACHED_POSITIVE; +import static google.registry.persistence.transaction.TransactionManagerFactory.ofyTm; +import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static org.joda.time.DateTimeZone.UTC; + +import com.google.auto.value.AutoValue; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.CacheLoader.InvalidCacheLoadException; +import com.google.common.cache.LoadingCache; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Streams; +import com.googlecode.objectify.Key; +import google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome; +import google.registry.model.registry.label.PremiumList.PremiumListEntry; +import google.registry.model.registry.label.PremiumList.PremiumListRevision; +import google.registry.persistence.VKey; +import google.registry.util.NonFinalForTesting; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.ExecutionException; +import org.joda.money.Money; +import org.joda.time.DateTime; +import org.joda.time.Duration; + +/** + * DAO for {@link PremiumList} objects stored in Datastore. + * + *

This class handles both the mapping from string to Datastore-level PremiumList objects as well + * as the mapping from PremiumList objects to the contents of those premium lists in the Datastore + * world. Specifically, this deals with retrieving the most recent revision for a given list and + * retrieving (or writing/deleting) all entries associated with that particular revision. The {@link + * PremiumList} object itself, in the Datastore world, does not store the premium pricing data. + */ +public class PremiumListDatastoreDao { + + /** The number of premium list entry entities that are created and deleted per batch. */ + private static final int TRANSACTION_BATCH_SIZE = 200; + + /** + * In-memory cache for premium lists. + * + *

This is cached for a shorter duration because we need to periodically reload this entity to + * check if a new revision has been published, and if so, then use that. + * + *

We also cache the absence of premium lists with a given name to avoid unnecessary pointless + * lookups. Note that this cache is only applicable to PremiumList objects stored in Datastore. + */ + @NonFinalForTesting + static LoadingCache> premiumListCache = + createPremiumListCache(getDomainLabelListCacheDuration()); + + @VisibleForTesting + public static void setPremiumListCacheForTest(Optional expiry) { + Duration effectiveExpiry = expiry.orElse(getSingletonCachePersistDuration()); + premiumListCache = createPremiumListCache(effectiveExpiry); + } + + @VisibleForTesting + public static LoadingCache> createPremiumListCache( + Duration cachePersistDuration) { + return CacheBuilder.newBuilder() + .expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis())) + .build( + new CacheLoader>() { + @Override + public Optional load(final String name) { + return tm().doTransactionless(() -> getLatestRevisionUncached(name)); + } + }); + } + + /** + * In-memory cache for {@link PremiumListRevision}s, used for retrieving Bloom filters quickly. + * + *

This is cached for a long duration (essentially indefinitely) because a given {@link + * PremiumListRevision} is immutable and cannot ever be changed once created, so its cache need + * not ever expire. + */ + static final LoadingCache, PremiumListRevision> + premiumListRevisionsCache = + CacheBuilder.newBuilder() + .expireAfterWrite( + java.time.Duration.ofMillis(getSingletonCachePersistDuration().getMillis())) + .build( + new CacheLoader, PremiumListRevision>() { + @Override + public PremiumListRevision load(final Key revisionKey) { + return ofyTm().doTransactionless(() -> ofy().load().key(revisionKey).now()); + } + }); + + /** + * In-memory cache for {@link PremiumListEntry}s for a given label and {@link PremiumListRevision} + * + *

Because the PremiumList itself makes up part of the PremiumListRevision's key, this is + * specific to a given premium list. Premium list entries might not be present, as indicated by + * the Optional wrapper, and we want to cache that as well. + * + *

This is cached for a long duration (essentially indefinitely) because a given {@link + * PremiumListRevision} and its child {@link PremiumListEntry}s are immutable and cannot ever be + * changed once created, so the cache need not ever expire. + * + *

A maximum size is set here on the cache because it can potentially grow too big to fit in + * memory if there are a large number of distinct premium list entries being queried (both those + * that exist, as well as those that might exist according to the Bloom filter, must be cached). + * The entries judged least likely to be accessed again will be evicted first. + */ + @NonFinalForTesting + static LoadingCache, Optional> premiumListEntriesCache = + createPremiumListEntriesCache(getSingletonCachePersistDuration()); + + @VisibleForTesting + public static void setPremiumListEntriesCacheForTest(Optional expiry) { + Duration effectiveExpiry = expiry.orElse(getSingletonCachePersistDuration()); + premiumListEntriesCache = createPremiumListEntriesCache(effectiveExpiry); + } + + @VisibleForTesting + static LoadingCache, Optional> + createPremiumListEntriesCache(Duration cachePersistDuration) { + return CacheBuilder.newBuilder() + .expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis())) + .maximumSize(getStaticPremiumListMaxCachedEntries()) + .build( + new CacheLoader, Optional>() { + @Override + public Optional load(final Key entryKey) { + return ofyTm() + .doTransactionless(() -> Optional.ofNullable(ofy().load().key(entryKey).now())); + } + }); + } + + public static Optional getLatestRevision(String name) { + return premiumListCache.getUnchecked(name); + } + + /** + * Returns the premium price for the specified list, label, and TLD, or absent if the label is not + * premium. + */ + public static Optional getPremiumPrice(String premiumListName, String label, String tld) { + DateTime startTime = DateTime.now(UTC); + Optional maybePremumList = getLatestRevision(premiumListName); + if (!maybePremumList.isPresent()) { + return Optional.empty(); + } + PremiumList premiumList = maybePremumList.get(); + // If we're dealing with a list from SQL, reload from Datastore if necessary + if (premiumList.getRevisionKey() == null) { + Optional fromDatastore = getLatestRevision(premiumList.getName()); + if (fromDatastore.isPresent()) { + premiumList = fromDatastore.get(); + } else { + return Optional.empty(); + } + } + PremiumListRevision revision; + try { + revision = premiumListRevisionsCache.get(premiumList.getRevisionKey()); + } catch (InvalidCacheLoadException | ExecutionException e) { + throw new RuntimeException( + "Could not load premium list revision " + premiumList.getRevisionKey(), e); + } + checkState( + revision.getProbablePremiumLabels() != null, + "Probable premium labels Bloom filter is null on revision '%s'", + premiumList.getRevisionKey()); + + CheckResults checkResults = checkStatus(revision, label); + DomainLabelMetrics.recordPremiumListCheckOutcome( + tld, + premiumList.getName(), + checkResults.checkOutcome(), + DateTime.now(UTC).getMillis() - startTime.getMillis()); + + return checkResults.premiumPrice(); + } + + /** + * Persists a new or updated PremiumList object and its descendant entities to Datastore. + * + *

The flow here is: save the new premium list entries parented on that revision entity, + * save/update the PremiumList, and then delete the old premium list entries associated with the + * old revision. + * + *

This is the only valid way to save these kinds of entities! + */ + public static PremiumList save(String name, List inputData) { + PremiumList premiumList = new PremiumList.Builder().setName(name).build(); + ImmutableMap premiumListEntries = premiumList.parse(inputData); + final Optional oldPremiumList = getLatestRevisionUncached(premiumList.getName()); + + // Create the new revision (with its Bloom filter) and parent the entries on it. + final PremiumListRevision newRevision = + PremiumListRevision.create(premiumList, premiumListEntries.keySet()); + final Key newRevisionKey = Key.create(newRevision); + ImmutableSet parentedEntries = + parentPremiumListEntriesOnRevision(premiumListEntries.values(), newRevisionKey); + + // Save the new child entities in a series of transactions. + for (final List batch : partition(parentedEntries, TRANSACTION_BATCH_SIZE)) { + ofyTm().transactNew(() -> ofy().save().entities(batch)); + } + + // Save the new PremiumList and revision itself. + return ofyTm() + .transactNew( + () -> { + DateTime now = ofyTm().getTransactionTime(); + // Assert that the premium list hasn't been changed since we started this process. + Key key = + Key.create(getCrossTldKey(), PremiumList.class, premiumList.getName()); + Optional existing = + ofyTm().loadByKeyIfPresent(VKey.createOfy(PremiumList.class, key)); + checkOfyFieldsEqual(existing, oldPremiumList); + PremiumList newList = + premiumList + .asBuilder() + .setLastUpdateTime(now) + .setCreationTime( + oldPremiumList.isPresent() ? oldPremiumList.get().creationTime : now) + .setRevision(newRevisionKey) + .build(); + ofy().save().entities(newList, newRevision); + premiumListCache.invalidate(premiumList.getName()); + return newList; + }); + } + + public static void delete(PremiumList premiumList) { + ofyTm().transactNew(() -> ofy().delete().entity(premiumList)); + if (premiumList.getRevisionKey() == null) { + return; + } + for (final List> batch : + partition( + ofy().load().type(PremiumListEntry.class).ancestor(premiumList.revisionKey).keys(), + TRANSACTION_BATCH_SIZE)) { + ofyTm().transactNew(() -> ofy().delete().keys(batch)); + batch.forEach(premiumListEntriesCache::invalidate); + } + ofyTm().transactNew(() -> ofy().delete().key(premiumList.getRevisionKey())); + premiumListCache.invalidate(premiumList.getName()); + premiumListRevisionsCache.invalidate(premiumList.getRevisionKey()); + } + + /** Re-parents the given {@link PremiumListEntry}s on the given {@link PremiumListRevision}. */ + @VisibleForTesting + public static ImmutableSet parentPremiumListEntriesOnRevision( + Iterable entries, final Key revisionKey) { + return Streams.stream(entries) + .map((PremiumListEntry entry) -> entry.asBuilder().setParent(revisionKey).build()) + .collect(toImmutableSet()); + } + + /** + * Returns all {@link PremiumListEntry PremiumListEntries} in the given {@code premiumList}. + * + *

This is an expensive operation and should only be used when the entire list is required. + */ + public static Iterable loadPremiumListEntriesUncached(PremiumList premiumList) { + return ofy().load().type(PremiumListEntry.class).ancestor(premiumList.revisionKey).iterable(); + } + + private static Optional getLatestRevisionUncached(String name) { + return Optional.ofNullable( + ofy().load().key(Key.create(getCrossTldKey(), PremiumList.class, name)).now()); + } + + private static void checkOfyFieldsEqual( + Optional oneOptional, Optional twoOptional) { + if (!oneOptional.isPresent()) { + checkState(!twoOptional.isPresent(), "Premium list concurrently deleted"); + return; + } else { + checkState(twoOptional.isPresent(), "Premium list concurrently deleted"); + } + PremiumList one = oneOptional.get(); + PremiumList two = twoOptional.get(); + checkState( + Objects.equals(one.revisionKey, two.revisionKey), + "Premium list revision key concurrently edited"); + checkState(Objects.equals(one.name, two.name), "Premium list name concurrently edited"); + checkState(Objects.equals(one.parent, two.parent), "Premium list parent concurrently edited"); + checkState( + Objects.equals(one.creationTime, two.creationTime), + "Premium list creation time concurrently edited"); + } + + private static CheckResults checkStatus(PremiumListRevision premiumListRevision, String label) { + if (!premiumListRevision.getProbablePremiumLabels().mightContain(label)) { + return CheckResults.create(BLOOM_FILTER_NEGATIVE, Optional.empty()); + } + + Key entryKey = + Key.create(Key.create(premiumListRevision), PremiumListEntry.class, label); + try { + // getIfPresent() returns null if the key is not in the cache + Optional entry = premiumListEntriesCache.getIfPresent(entryKey); + if (entry != null) { + if (entry.isPresent()) { + return CheckResults.create(CACHED_POSITIVE, Optional.of(entry.get().getValue())); + } else { + return CheckResults.create(CACHED_NEGATIVE, Optional.empty()); + } + } + + entry = premiumListEntriesCache.get(entryKey); + if (entry.isPresent()) { + return CheckResults.create(UNCACHED_POSITIVE, Optional.of(entry.get().getValue())); + } else { + return CheckResults.create(UNCACHED_NEGATIVE, Optional.empty()); + } + } catch (InvalidCacheLoadException | ExecutionException e) { + throw new RuntimeException("Could not load premium list entry " + entryKey, e); + } + } + + /** Value type class used by {@link #checkStatus} to return the results of a premiumness check. */ + @AutoValue + abstract static class CheckResults { + static CheckResults create(PremiumListCheckOutcome checkOutcome, Optional premiumPrice) { + return new AutoValue_PremiumListDatastoreDao_CheckResults(checkOutcome, premiumPrice); + } + + abstract PremiumListCheckOutcome checkOutcome(); + + abstract Optional premiumPrice(); + } + + private PremiumListDatastoreDao() {} +} 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 new file mode 100644 index 000000000..6073dc703 --- /dev/null +++ b/core/src/main/java/google/registry/model/registry/label/PremiumListDualDao.java @@ -0,0 +1,199 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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.registry.Registry; +import google.registry.model.registry.label.PremiumList.PremiumListEntry; +import google.registry.schema.tld.PremiumListSqlDao; +import java.util.List; +import java.util.Optional; +import org.joda.money.CurrencyUnit; +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 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 + */ +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. + */ + public static Optional getLatestRevision(String premiumListName) { + // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check + if (tm().isOfy()) { + return PremiumListDatastoreDao.getLatestRevision(premiumListName); + } else { + return PremiumListSqlDao.getLatestRevision(premiumListName); + } + } + + /** + * Returns the premium price for the specified label and registry. + * + *

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). + */ + public static Optional getPremiumPrice(String label, Registry registry) { + if (registry.getPremiumList() == null) { + return Optional.empty(); + } + String premiumListName = registry.getPremiumList().getName(); + Optional primaryResult; + // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check + if (tm().isOfy()) { + 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()) { + 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())); + } + 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. + */ + public static PremiumList save(String name, List inputData) { + PremiumList result; + // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check + if (tm().isOfy()) { + result = PremiumListDatastoreDao.save(name, inputData); + suppressExceptionUnlessInTest( + () -> PremiumListSqlDao.save(name, inputData), "Error when saving premium list to SQL."); + } else { + 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. + */ + public static void delete(PremiumList premiumList) { + // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check + if (tm().isOfy()) { + 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 + // TODO(gbrodman): Use Sarah's DB scheduler instead of this isOfy check + if (tm().isOfy()) { + return PremiumListDatastoreDao.getLatestRevision(premiumListName).isPresent(); + } else { + return PremiumListSqlDao.getLatestRevision(premiumListName).isPresent(); + } + } + + /** + * Returns all {@link PremiumListEntry PremiumListEntries} in the list with the given name. + * + *

This is an expensive operation and should only be used when the entire list is required. + */ + public static Iterable loadAllPremiumListEntries(String premiumListName) { + PremiumList premiumList = + getLatestRevision(premiumListName) + .orElseThrow( + () -> + 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()) { + return PremiumListDatastoreDao.loadPremiumListEntriesUncached(premiumList); + } else { + CurrencyUnit currencyUnit = premiumList.getCurrency(); + return Streams.stream(PremiumListSqlDao.loadPremiumListEntriesUncached(premiumList)) + .map( + premiumEntry -> + new PremiumListEntry.Builder() + .setPrice(Money.of(currencyUnit, premiumEntry.getPrice())) + .setLabel(premiumEntry.getDomainLabel()) + .build()) + .collect(toImmutableList()); + } + } + + private PremiumListDualDao() {} +} diff --git a/core/src/main/java/google/registry/model/registry/label/PremiumListUtils.java b/core/src/main/java/google/registry/model/registry/label/PremiumListUtils.java deleted file mode 100644 index 2803af371..000000000 --- a/core/src/main/java/google/registry/model/registry/label/PremiumListUtils.java +++ /dev/null @@ -1,255 +0,0 @@ -// Copyright 2017 The Nomulus Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package google.registry.model.registry.label; - -import static com.google.common.base.Preconditions.checkState; -import static com.google.common.collect.ImmutableSet.toImmutableSet; -import static com.google.common.collect.Iterables.partition; -import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; -import static google.registry.model.ofy.ObjectifyService.ofy; -import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.BLOOM_FILTER_NEGATIVE; -import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.CACHED_NEGATIVE; -import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.CACHED_POSITIVE; -import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.UNCACHED_NEGATIVE; -import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.UNCACHED_POSITIVE; -import static google.registry.model.registry.label.PremiumList.cachePremiumListEntries; -import static google.registry.model.registry.label.PremiumList.cachePremiumListRevisions; -import static google.registry.model.registry.label.PremiumList.cachePremiumLists; -import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import static org.joda.time.DateTimeZone.UTC; - -import com.google.auto.value.AutoValue; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.cache.CacheLoader.InvalidCacheLoadException; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Streams; -import com.google.common.flogger.FluentLogger; -import com.googlecode.objectify.Key; -import google.registry.model.registry.Registry; -import google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome; -import google.registry.model.registry.label.PremiumList.PremiumListEntry; -import google.registry.model.registry.label.PremiumList.PremiumListRevision; -import google.registry.schema.tld.PremiumListDao; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.concurrent.ExecutionException; -import org.joda.money.Money; -import org.joda.time.DateTime; - -/** Static helper methods for working with {@link PremiumList}s. */ -public final class PremiumListUtils { - - /** The number of premium list entry entities that are created and deleted per batch. */ - private static final int TRANSACTION_BATCH_SIZE = 200; - - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - /** Value type class used by {@link #checkStatus} to return the results of a premiumness check. */ - @AutoValue - abstract static class CheckResults { - static CheckResults create(PremiumListCheckOutcome checkOutcome, Optional premiumPrice) { - return new AutoValue_PremiumListUtils_CheckResults(checkOutcome, premiumPrice); - } - - abstract PremiumListCheckOutcome checkOutcome(); - abstract Optional premiumPrice(); - } - - /** - * Returns the premium price for the specified label and registry, or absent if the label is not - * premium. - */ - public static Optional getPremiumPrice(String label, Registry registry) { - // If the registry has no configured premium list, then no labels are premium. - if (registry.getPremiumList() == null) { - return Optional.empty(); - } - DateTime startTime = DateTime.now(UTC); - String listName = registry.getPremiumList().getName(); - Optional optionalPremiumList = PremiumList.getCached(listName); - checkState(optionalPremiumList.isPresent(), "Could not load premium list '%s'", listName); - PremiumList premiumList = optionalPremiumList.get(); - PremiumListRevision revision; - try { - revision = cachePremiumListRevisions.get(premiumList.getRevisionKey()); - } catch (InvalidCacheLoadException | ExecutionException e) { - throw new RuntimeException( - "Could not load premium list revision " + premiumList.getRevisionKey(), e); - } - checkState( - revision.getProbablePremiumLabels() != null, - "Probable premium labels Bloom filter is null on revision '%s'", - premiumList.getRevisionKey()); - - CheckResults checkResults = checkStatus(revision, label); - DomainLabelMetrics.recordPremiumListCheckOutcome( - registry.getTldStr(), - listName, - checkResults.checkOutcome(), - DateTime.now(UTC).getMillis() - startTime.getMillis()); - - // Also load the value from Cloud SQL, compare the two results, and log if different. - try { - Optional priceFromSql = PremiumListDao.getPremiumPrice(label, registry); - if (!priceFromSql.equals(checkResults.premiumPrice())) { - logger.atWarning().log( - "Unequal prices for domain %s.%s from Datastore (%s) and Cloud SQL (%s).", - label, registry.getTldStr(), checkResults.premiumPrice(), priceFromSql); - } - } catch (Throwable t) { - logger.atSevere().withCause(t).log( - "Error loading price of domain %s.%s from Cloud SQL.", label, registry.getTldStr()); - } - return checkResults.premiumPrice(); - } - - private static CheckResults checkStatus(PremiumListRevision premiumListRevision, String label) { - if (!premiumListRevision.getProbablePremiumLabels().mightContain(label)) { - return CheckResults.create(BLOOM_FILTER_NEGATIVE, Optional.empty()); - } - - Key entryKey = - Key.create(Key.create(premiumListRevision), PremiumListEntry.class, label); - try { - // getIfPresent() returns null if the key is not in the cache - Optional entry = cachePremiumListEntries.getIfPresent(entryKey); - if (entry != null) { - if (entry.isPresent()) { - return CheckResults.create(CACHED_POSITIVE, Optional.of(entry.get().getValue())); - } else { - return CheckResults.create(CACHED_NEGATIVE, Optional.empty()); - } - } - - entry = cachePremiumListEntries.get(entryKey); - if (entry.isPresent()) { - return CheckResults.create(UNCACHED_POSITIVE, Optional.of(entry.get().getValue())); - } else { - return CheckResults.create(UNCACHED_NEGATIVE, Optional.empty()); - } - } catch (InvalidCacheLoadException | ExecutionException e) { - throw new RuntimeException("Could not load premium list entry " + entryKey, e); - } - } - - /** - * Persists a new or updated PremiumList object and its descendant entities to Datastore. - * - *

The flow here is: save the new premium list entries parented on that revision entity, - * save/update the PremiumList, and then delete the old premium list entries associated with the - * old revision. - * - *

This is the only valid way to save these kinds of entities! - */ - public static PremiumList savePremiumListAndEntries( - final PremiumList premiumList, - ImmutableMap premiumListEntries) { - final Optional oldPremiumList = PremiumList.getUncached(premiumList.getName()); - - // Create the new revision (with its Bloom filter) and parent the entries on it. - final PremiumListRevision newRevision = - PremiumListRevision.create(premiumList, premiumListEntries.keySet()); - final Key newRevisionKey = Key.create(newRevision); - ImmutableSet parentedEntries = - parentPremiumListEntriesOnRevision(premiumListEntries.values(), newRevisionKey); - - // Save the new child entities in a series of transactions. - for (final List batch : partition(parentedEntries, TRANSACTION_BATCH_SIZE)) { - tm().transactNew(() -> ofy().save().entities(batch)); - } - - // Save the new PremiumList and revision itself. - PremiumList updated = tm().transactNew(() -> { - DateTime now = tm().getTransactionTime(); - // Assert that the premium list hasn't been changed since we started this process. - PremiumList existing = ofy().load() - .type(PremiumList.class) - .parent(getCrossTldKey()) - .id(premiumList.getName()) - .now(); - checkState( - Objects.equals(existing, oldPremiumList.orElse(null)), - "PremiumList was concurrently edited"); - PremiumList newList = premiumList.asBuilder() - .setLastUpdateTime(now) - .setCreationTime(oldPremiumList.isPresent() ? oldPremiumList.get().creationTime : now) - .setRevision(newRevisionKey) - .build(); - ofy().save().entities(newList, newRevision); - return newList; - }); - - // Invalidate the cache on this premium list so the change will take effect instantly. This only - // clears the cache on the same instance that the update was run on, which will typically be the - // only tools instance. - PremiumList.cachePremiumLists.invalidate(premiumList.getName()); - - // TODO(b/79888775): Enqueue the oldPremiumList for deletion after at least - // RegistryConfig.getDomainLabelListCacheDuration() has elapsed. - return updated; - } - - public static PremiumList savePremiumListAndEntries( - PremiumList premiumList, Iterable premiumListLines) { - return savePremiumListAndEntries(premiumList, premiumList.parse(premiumListLines)); - } - - /** Re-parents the given {@link PremiumListEntry}s on the given {@link PremiumListRevision}. */ - @VisibleForTesting - public static ImmutableSet parentPremiumListEntriesOnRevision( - Iterable entries, final Key revisionKey) { - return Streams.stream(entries) - .map((PremiumListEntry entry) -> entry.asBuilder().setParent(revisionKey).build()) - .collect(toImmutableSet()); - } - - /** Deletes the PremiumList and all of its child entities. */ - public static void deletePremiumList(final PremiumList premiumList) { - tm().transactNew(() -> ofy().delete().entity(premiumList)); - deleteRevisionAndEntriesOfPremiumList(premiumList); - cachePremiumLists.invalidate(premiumList.getName()); - } - - static void deleteRevisionAndEntriesOfPremiumList(final PremiumList premiumList) { - if (premiumList.getRevisionKey() == null) { - return; - } - for (final List> batch : - partition( - ofy().load().type(PremiumListEntry.class).ancestor(premiumList.revisionKey).keys(), - TRANSACTION_BATCH_SIZE)) { - tm().transactNew(() -> ofy().delete().keys(batch)); - } - tm().transactNew(() -> ofy().delete().key(premiumList.getRevisionKey())); - } - - /** - * Returns all {@link PremiumListEntry PremiumListEntries} in the given {@code premiumList}. - * - *

This is an expensive operation and should only be used when the entire list is required. - */ - public static Iterable loadPremiumListEntries(PremiumList premiumList) { - return ofy().load().type(PremiumListEntry.class).ancestor(premiumList.revisionKey).iterable(); - } - - /** Returns whether a PremiumList of the given name exists, bypassing the cache. */ - public static boolean doesPremiumListExist(String name) { - return ofy().load().key(Key.create(getCrossTldKey(), PremiumList.class, name)).now() != null; - } - - private PremiumListUtils() {} -} diff --git a/core/src/main/java/google/registry/schema/tld/PremiumEntry.java b/core/src/main/java/google/registry/schema/tld/PremiumEntry.java index dd2801aff..a060d5c66 100644 --- a/core/src/main/java/google/registry/schema/tld/PremiumEntry.java +++ b/core/src/main/java/google/registry/schema/tld/PremiumEntry.java @@ -50,4 +50,19 @@ public class PremiumEntry extends ImmutableObject implements Serializable, SqlEn public Optional toDatastoreEntity() { return Optional.empty(); // PremiumList is dually-written } + + public BigDecimal getPrice() { + return price; + } + + public String getDomainLabel() { + return domainLabel; + } + + public static PremiumEntry create(BigDecimal price, String domainLabel) { + PremiumEntry result = new PremiumEntry(); + result.price = price; + result.domainLabel = domainLabel; + return result; + } } diff --git a/core/src/main/java/google/registry/schema/tld/PremiumListCache.java b/core/src/main/java/google/registry/schema/tld/PremiumListCache.java deleted file mode 100644 index fd22e4437..000000000 --- a/core/src/main/java/google/registry/schema/tld/PremiumListCache.java +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright 2019 The Nomulus Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package google.registry.schema.tld; - -import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration; -import static google.registry.config.RegistryConfig.getSingletonCachePersistDuration; -import static google.registry.config.RegistryConfig.getStaticPremiumListMaxCachedEntries; -import static google.registry.schema.tld.PremiumListDao.getPriceForLabel; - -import com.google.auto.value.AutoValue; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import google.registry.model.registry.label.PremiumList; -import google.registry.util.NonFinalForTesting; -import java.math.BigDecimal; -import java.util.Optional; -import org.joda.time.Duration; - -/** Caching utils for {@link PremiumList}s. */ -class PremiumListCache { - - /** - * In-memory cache for premium lists. - * - *

This is cached for a shorter duration because we need to periodically reload from the DB to - * check if a new revision has been published, and if so, then use that. - */ - @NonFinalForTesting - static LoadingCache> cachePremiumLists = - createCachePremiumLists(getDomainLabelListCacheDuration()); - - @VisibleForTesting - static LoadingCache> createCachePremiumLists( - Duration cachePersistDuration) { - return CacheBuilder.newBuilder() - .expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis())) - .build( - new CacheLoader>() { - @Override - public Optional load(String premiumListName) { - return PremiumListDao.getLatestRevision(premiumListName); - } - }); - } - - /** - * In-memory price cache for for a given premium list revision and domain label. - * - *

Note that premium list revision ids are globally unique, so this cache is specific to a - * given premium list. Premium list entries might not be present, as indicated by the Optional - * wrapper, and we want to cache that as well. - * - *

This is cached for a long duration (essentially indefinitely) because premium list revisions - * are immutable and cannot ever be changed once created, so the cache need not ever expire. - * - *

A maximum size is set here on the cache because it can potentially grow too big to fit in - * memory if there are a large number of distinct premium list entries being queried (both those - * that exist, as well as those that might exist according to the Bloom filter, must be cached). - * The entries judged least likely to be accessed again will be evicted first. - */ - @NonFinalForTesting - static LoadingCache> cachePremiumEntries = - createCachePremiumEntries(getSingletonCachePersistDuration()); - - @VisibleForTesting - static LoadingCache> createCachePremiumEntries( - Duration cachePersistDuration) { - return CacheBuilder.newBuilder() - .expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis())) - .maximumSize(getStaticPremiumListMaxCachedEntries()) - .build( - new CacheLoader>() { - @Override - public Optional load(RevisionIdAndLabel revisionIdAndLabel) { - return getPriceForLabel(revisionIdAndLabel); - } - }); - } - - @AutoValue - abstract static class RevisionIdAndLabel { - abstract long revisionId(); - - abstract String label(); - - static RevisionIdAndLabel create(long revisionId, String label) { - return new AutoValue_PremiumListCache_RevisionIdAndLabel(revisionId, label); - } - } - - private PremiumListCache() {} -} diff --git a/core/src/main/java/google/registry/schema/tld/PremiumListDao.java b/core/src/main/java/google/registry/schema/tld/PremiumListDao.java deleted file mode 100644 index e2bec623f..000000000 --- a/core/src/main/java/google/registry/schema/tld/PremiumListDao.java +++ /dev/null @@ -1,175 +0,0 @@ -// Copyright 2019 The Nomulus Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package google.registry.schema.tld; - -import static com.google.common.base.Preconditions.checkArgument; -import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; - -import com.google.common.cache.CacheLoader.InvalidCacheLoadException; -import com.google.common.util.concurrent.UncheckedExecutionException; -import google.registry.model.registry.Registry; -import google.registry.model.registry.label.PremiumList; -import google.registry.schema.tld.PremiumListCache.RevisionIdAndLabel; -import java.math.BigDecimal; -import java.util.Optional; -import java.util.concurrent.ExecutionException; -import org.joda.money.Money; - -/** Data access object class for {@link PremiumList}. */ -public class PremiumListDao { - - /** - * Returns the premium price for the specified label and registry, or absent if the label is not - * premium. - */ - public static Optional getPremiumPrice(String label, Registry registry) { - // If the registry has no configured premium list, then no labels are premium. - if (registry.getPremiumList() == null) { - return Optional.empty(); - } - String premiumListName = registry.getPremiumList().getName(); - PremiumList premiumList = - getLatestRevisionCached(premiumListName) - .orElseThrow( - () -> - new IllegalStateException( - String.format("Could not load premium list '%s'", premiumListName))); - return getPremiumPriceFromList(label, premiumList); - } - - /** Persist a new premium list to Cloud SQL. */ - public static void saveNew(PremiumList premiumList) { - jpaTm() - .transact( - () -> { - checkArgument( - !checkExists(premiumList.getName()), - "Premium list '%s' already exists", - premiumList.getName()); - jpaTm().getEntityManager().persist(premiumList); - }); - } - - /** Persist a new revision of an existing premium list to Cloud SQL. */ - public static void update(PremiumList premiumList) { - jpaTm() - .transact( - () -> { - // This check is currently disabled because, during the Cloud SQL migration, we need - // to be able to update premium lists in Datastore while simultaneously creating their - // first revision in Cloud SQL (i.e. if they haven't been migrated over yet). - // TODO(b/147246613): Reinstate this once all premium lists are migrated to Cloud SQL, - // and re-enable the test update_throwsWhenListDoesntExist(). - // checkArgument( - // checkExists(premiumList.getName()), - // "Can't update non-existent premium list '%s'", - // premiumList.getName()); - jpaTm().getEntityManager().persist(premiumList); - }); - } - - /** - * Returns the most recent revision of the PremiumList with the specified name, if it exists. - * - *

Note that this does not load PremiumList.labelsToPrices! If you need to check - * prices, use {@link #getPremiumPrice}. - */ - public static Optional getLatestRevision(String premiumListName) { - return jpaTm() - .transact( - () -> - jpaTm() - .getEntityManager() - .createQuery( - "SELECT pl FROM PremiumList pl WHERE pl.name = :name ORDER BY" - + " pl.revisionId DESC", - PremiumList.class) - .setParameter("name", premiumListName) - .setMaxResults(1) - .getResultStream() - .findFirst()); - } - - static Optional getPriceForLabel(RevisionIdAndLabel revisionIdAndLabel) { - return jpaTm() - .transact( - () -> - jpaTm() - .getEntityManager() - .createQuery( - "SELECT pe.price FROM PremiumEntry pe WHERE pe.revisionId = :revisionId" - + " AND pe.domainLabel = :label", - BigDecimal.class) - .setParameter("revisionId", revisionIdAndLabel.revisionId()) - .setParameter("label", revisionIdAndLabel.label()) - .setMaxResults(1) - .getResultStream() - .findFirst()); - } - - /** Returns the most recent revision of the PremiumList with the specified name, from cache. */ - static Optional getLatestRevisionCached(String premiumListName) { - try { - return PremiumListCache.cachePremiumLists.get(premiumListName); - } catch (ExecutionException e) { - throw new UncheckedExecutionException( - "Could not retrieve premium list named " + premiumListName, e); - } - } - - /** - * Returns whether the premium list of the given name exists. - * - *

This means that at least one premium list revision must exist for the given name. - */ - static boolean checkExists(String premiumListName) { - return jpaTm() - .transact( - () -> - jpaTm() - .getEntityManager() - .createQuery("SELECT 1 FROM PremiumList WHERE name = :name", Integer.class) - .setParameter("name", premiumListName) - .setMaxResults(1) - .getResultList() - .size() - > 0); - } - - private static Optional getPremiumPriceFromList(String label, PremiumList premiumList) { - // Consult the bloom filter and immediately return if the label definitely isn't premium. - if (!premiumList.getBloomFilter().mightContain(label)) { - return Optional.empty(); - } - RevisionIdAndLabel revisionIdAndLabel = - RevisionIdAndLabel.create(premiumList.getRevisionId(), label); - try { - Optional price = PremiumListCache.cachePremiumEntries.get(revisionIdAndLabel); - return price.map( - p -> - Money.of( - premiumList.getCurrency(), - p.setScale(premiumList.getCurrency().getDecimalPlaces()))); - } catch (InvalidCacheLoadException | ExecutionException e) { - throw new RuntimeException( - String.format( - "Could not load premium entry %s for list %s", - revisionIdAndLabel, premiumList.getName()), - e); - } - } - - private PremiumListDao() {} -} diff --git a/core/src/main/java/google/registry/schema/tld/PremiumListSqlDao.java b/core/src/main/java/google/registry/schema/tld/PremiumListSqlDao.java new file mode 100644 index 000000000..4e2a143a0 --- /dev/null +++ b/core/src/main/java/google/registry/schema/tld/PremiumListSqlDao.java @@ -0,0 +1,238 @@ +// Copyright 2019 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.schema.tld; + +import static google.registry.config.RegistryConfig.getDomainLabelListCacheDuration; +import static google.registry.config.RegistryConfig.getSingletonCachePersistDuration; +import static google.registry.config.RegistryConfig.getStaticPremiumListMaxCachedEntries; +import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; + +import com.google.auto.value.AutoValue; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.CacheLoader.InvalidCacheLoadException; +import com.google.common.cache.LoadingCache; +import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumList.PremiumListEntry; +import google.registry.util.NonFinalForTesting; +import java.math.BigDecimal; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.ExecutionException; +import org.joda.money.Money; +import org.joda.time.Duration; + +/** + * Data access object class for accessing {@link PremiumList} objects from Cloud SQL. + * + *

This class handles both the mapping from string to SQL-level PremiumList objects as well as + * the mapping and retrieval of {@link PremiumEntry} objects that correspond to the particular + * {@link PremiumList} object in SQL, and caching these entries so that future lookups can be + * quicker. + */ +public class PremiumListSqlDao { + + /** + * In-memory cache for premium lists. + * + *

This is cached for a shorter duration because we need to periodically reload this entity to + * check if a new revision has been published, and if so, then use that. + * + *

We also cache the absence of premium lists with a given name to avoid unnecessary pointless + * lookups. Note that this cache is only applicable to PremiumList objects stored in SQL. + */ + @NonFinalForTesting + static LoadingCache> premiumListCache = + createPremiumListCache(getDomainLabelListCacheDuration()); + + @VisibleForTesting + public static void setPremiumListCacheForTest(Optional expiry) { + Duration effectiveExpiry = expiry.orElse(getDomainLabelListCacheDuration()); + premiumListCache = createPremiumListCache(effectiveExpiry); + } + + @VisibleForTesting + public static LoadingCache> createPremiumListCache( + Duration cachePersistDuration) { + return CacheBuilder.newBuilder() + .expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis())) + .build( + new CacheLoader>() { + @Override + public Optional load(final String name) { + return jpaTm().doTransactionless(() -> getLatestRevisionUncached(name)); + } + }); + } + + /** + * In-memory price cache for for a given premium list revision and domain label. + * + *

Note that premium list revision ids are globally unique, so this cache is specific to a + * given premium list. Premium list entries might not be present, as indicated by the Optional + * wrapper, and we want to cache that as well. + * + *

This is cached for a long duration (essentially indefinitely) because premium list revisions + * are immutable and cannot ever be changed once created, so the cache need not ever expire. + * + *

A maximum size is set here on the cache because it can potentially grow too big to fit in + * memory if there are a large number of distinct premium list entries being queried (both those + * that exist, as well as those that might exist according to the Bloom filter, must be cached). + * The entries judged least likely to be accessed again will be evicted first. + */ + @NonFinalForTesting + static LoadingCache> premiumEntryCache = + createPremiumEntryCache(getSingletonCachePersistDuration()); + + @VisibleForTesting + static LoadingCache> createPremiumEntryCache( + Duration cachePersistDuration) { + return CacheBuilder.newBuilder() + .expireAfterWrite(java.time.Duration.ofMillis(cachePersistDuration.getMillis())) + .maximumSize(getStaticPremiumListMaxCachedEntries()) + .build( + new CacheLoader>() { + @Override + public Optional load(RevisionIdAndLabel revisionIdAndLabel) { + return getPriceForLabelUncached(revisionIdAndLabel); + } + }); + } + + /** + * Returns the most recent revision of the PremiumList with the specified name, if it exists. + * + *

Note that this does not load PremiumList.labelsToPrices! If you need to check + * prices, use {@link #getPremiumPrice}. + */ + public static Optional getLatestRevision(String premiumListName) { + return premiumListCache.getUnchecked(premiumListName); + } + + /** + * Returns the premium price for the specified label and registry, or absent if the label is not + * premium. + */ + public static Optional getPremiumPrice(String premiumListName, String label) { + Optional maybeLoadedList = getLatestRevision(premiumListName); + if (!maybeLoadedList.isPresent()) { + return Optional.empty(); + } + PremiumList loadedList = maybeLoadedList.get(); + // Consult the bloom filter and immediately return if the label definitely isn't premium. + if (!loadedList.getBloomFilter().mightContain(label)) { + return Optional.empty(); + } + RevisionIdAndLabel revisionIdAndLabel = + RevisionIdAndLabel.create(loadedList.getRevisionId(), label); + try { + Optional price = premiumEntryCache.get(revisionIdAndLabel); + return price.map( + p -> + Money.of( + loadedList.getCurrency(), + p.setScale(loadedList.getCurrency().getDecimalPlaces()))); + } catch (InvalidCacheLoadException | ExecutionException e) { + throw new RuntimeException( + String.format( + "Could not load premium entry %s for list %s", + revisionIdAndLabel, loadedList.getName()), + e); + } + } + + public static PremiumList save(String name, List inputData) { + return save(PremiumListUtils.parseToPremiumList(name, inputData)); + } + + public static PremiumList save(PremiumList premiumList) { + jpaTm().transact(() -> jpaTm().getEntityManager().persist(premiumList)); + premiumListCache.invalidate(premiumList.getName()); + return premiumList; + } + + public static void delete(PremiumList premiumList) { + jpaTm().transact(() -> getLatestRevision(premiumList.getName()).ifPresent(jpaTm()::delete)); + premiumListCache.invalidate(premiumList.getName()); + } + + private static Optional getLatestRevisionUncached(String premiumListName) { + return jpaTm() + .transact( + () -> + jpaTm() + .getEntityManager() + .createQuery( + "FROM PremiumList WHERE name = :name ORDER BY revisionId DESC", + PremiumList.class) + .setParameter("name", premiumListName) + .setMaxResults(1) + .getResultStream() + .findFirst()); + } + + /** + * Returns all {@link PremiumListEntry PremiumListEntries} in the given {@code premiumList}. + * + *

This is an expensive operation and should only be used when the entire list is required. + */ + public static Iterable loadPremiumListEntriesUncached(PremiumList premiumList) { + return jpaTm() + .transact( + () -> + jpaTm() + .getEntityManager() + .createQuery( + "FROM PremiumEntry pe WHERE pe.revisionId = :revisionId", + PremiumEntry.class) + .setParameter("revisionId", premiumList.getRevisionId()) + .getResultList()); + } + + /** + * Loads the price for the given revisionId + label combination. Note that this does a database + * retrieval so it should only be done in a cached context. + */ + static Optional getPriceForLabelUncached(RevisionIdAndLabel revisionIdAndLabel) { + return jpaTm() + .transact( + () -> + jpaTm() + .getEntityManager() + .createQuery( + "SELECT pe.price FROM PremiumEntry pe WHERE pe.revisionId = :revisionId" + + " AND pe.domainLabel = :label", + BigDecimal.class) + .setParameter("revisionId", revisionIdAndLabel.revisionId()) + .setParameter("label", revisionIdAndLabel.label()) + .setMaxResults(1) + .getResultStream() + .findFirst()); + } + + @AutoValue + abstract static class RevisionIdAndLabel { + abstract long revisionId(); + + abstract String label(); + + static RevisionIdAndLabel create(long revisionId, String label) { + return new AutoValue_PremiumListSqlDao_RevisionIdAndLabel(revisionId, label); + } + } + + private PremiumListSqlDao() {} +} diff --git a/core/src/main/java/google/registry/schema/tld/PremiumListUtils.java b/core/src/main/java/google/registry/schema/tld/PremiumListUtils.java index c12d77889..c9b5275c6 100644 --- a/core/src/main/java/google/registry/schema/tld/PremiumListUtils.java +++ b/core/src/main/java/google/registry/schema/tld/PremiumListUtils.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static org.joda.time.DateTimeZone.UTC; -import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; @@ -35,12 +34,9 @@ import org.joda.time.DateTime; /** Static utility methods for {@link PremiumList}. */ public class PremiumListUtils { - public static PremiumList parseToPremiumList(String name, String inputData) { - List inputDataPreProcessed = - Splitter.on('\n').omitEmptyStrings().splitToList(inputData); - + public static PremiumList parseToPremiumList(String name, List inputData) { ImmutableMap prices = - new PremiumList.Builder().setName(name).build().parse(inputDataPreProcessed); + new PremiumList.Builder().setName(name).build().parse(inputData); ImmutableSet currencies = prices.values().stream() .map(e -> e.getValue().getCurrencyUnit()) diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateTldCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateTldCommand.java index d95f183e2..693029185 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateTldCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateTldCommand.java @@ -31,6 +31,7 @@ import google.registry.model.registry.Registry; import google.registry.model.registry.Registry.TldState; import google.registry.model.registry.Registry.TldType; import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.tools.params.OptionalStringParameter; import google.registry.tools.params.TransitionListParameter.BillingCostTransitions; import google.registry.tools.params.TransitionListParameter.TldStateTransitions; @@ -342,7 +343,8 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand { if (premiumListName != null) { if (premiumListName.isPresent()) { - Optional premiumList = PremiumList.getUncached(premiumListName.get()); + Optional premiumList = + PremiumListDualDao.getLatestRevision(premiumListName.get()); checkArgument( premiumList.isPresent(), String.format("The premium list '%s' doesn't exist", premiumListName.get())); diff --git a/core/src/main/java/google/registry/tools/DeletePremiumListCommand.java b/core/src/main/java/google/registry/tools/DeletePremiumListCommand.java index 88b25a837..66c9f86c1 100644 --- a/core/src/main/java/google/registry/tools/DeletePremiumListCommand.java +++ b/core/src/main/java/google/registry/tools/DeletePremiumListCommand.java @@ -15,25 +15,23 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; -import static google.registry.model.registry.label.PremiumListUtils.deletePremiumList; -import static google.registry.model.registry.label.PremiumListUtils.doesPremiumListExist; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; import google.registry.model.registry.label.PremiumList; +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} in Datastore. This command will fail if the premium list + * is currently in use on a tld. */ @Parameters(separators = " =", commandDescription = "Delete a PremiumList from Datastore.") final class DeletePremiumListCommand extends ConfirmingCommand implements CommandWithRemoteApi { - @Nullable - PremiumList premiumList; + @Nullable PremiumList premiumList; @Parameter( names = {"-n", "--name"}, @@ -44,10 +42,10 @@ final class DeletePremiumListCommand extends ConfirmingCommand implements Comman @Override protected void init() { checkArgument( - doesPremiumListExist(name), + PremiumListDualDao.exists(name), "Cannot delete the premium list %s because it doesn't exist.", name); - premiumList = PremiumList.getUncached(name).get(); + premiumList = PremiumListDualDao.getLatestRevision(name).get(); ImmutableSet tldsUsedOn = premiumList.getReferencingTlds(); checkArgument( tldsUsedOn.isEmpty(), @@ -62,7 +60,7 @@ final class DeletePremiumListCommand extends ConfirmingCommand implements Comman @Override protected String execute() { - deletePremiumList(premiumList); + PremiumListDualDao.delete(premiumList); return String.format("Deleted premium list '%s'.\n", premiumList.getName()); } } diff --git a/core/src/main/java/google/registry/tools/server/CreateOrUpdatePremiumListAction.java b/core/src/main/java/google/registry/tools/server/CreateOrUpdatePremiumListAction.java index f335a7ea6..d32492305 100644 --- a/core/src/main/java/google/registry/tools/server/CreateOrUpdatePremiumListAction.java +++ b/core/src/main/java/google/registry/tools/server/CreateOrUpdatePremiumListAction.java @@ -45,25 +45,15 @@ public abstract class CreateOrUpdatePremiumListAction implements Runnable { @Override public void run() { try { - saveToDatastore(); + save(); } catch (IllegalArgumentException e) { logger.atInfo().withCause(e).log( "Usage error in attempting to save premium list from nomulus tool command"); response.setPayload(ImmutableMap.of("error", e.toString(), "status", "error")); - return; } catch (Exception e) { logger.atSevere().withCause(e).log( "Unexpected error saving premium list to Datastore from nomulus tool command"); response.setPayload(ImmutableMap.of("error", e.toString(), "status", "error")); - return; - } - - try { - saveToCloudSql(); - } catch (Throwable e) { - logger.atSevere().withCause(e).log( - "Unexpected error saving premium list to Cloud SQL from nomulus tool command"); - response.setPayload(ImmutableMap.of("error", e.toString(), "status", "error")); } } @@ -78,9 +68,6 @@ public abstract class CreateOrUpdatePremiumListAction implements Runnable { : (inputData.substring(0, MAX_LOGGING_PREMIUM_LIST_LENGTH) + ""))); } - /** Saves the premium list to Datastore. */ - protected abstract void saveToDatastore(); - - /** Saves the premium list to Cloud SQL. */ - protected abstract void saveToCloudSql(); + /** Saves the premium list to both Datastore and Cloud SQL. */ + protected abstract void save(); } diff --git a/core/src/main/java/google/registry/tools/server/CreatePremiumListAction.java b/core/src/main/java/google/registry/tools/server/CreatePremiumListAction.java index 69b5d39b0..401247726 100644 --- a/core/src/main/java/google/registry/tools/server/CreatePremiumListAction.java +++ b/core/src/main/java/google/registry/tools/server/CreatePremiumListAction.java @@ -16,19 +16,15 @@ package google.registry.tools.server; import static com.google.common.base.Preconditions.checkArgument; import static google.registry.model.registry.Registries.assertTldExists; -import static google.registry.model.registry.label.PremiumListUtils.doesPremiumListExist; -import static google.registry.model.registry.label.PremiumListUtils.savePremiumListAndEntries; import static google.registry.request.Action.Method.POST; -import static google.registry.schema.tld.PremiumListUtils.parseToPremiumList; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import com.google.common.flogger.FluentLogger; -import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.request.Action; import google.registry.request.Parameter; import google.registry.request.auth.Auth; -import google.registry.schema.tld.PremiumListDao; import java.util.List; import javax.inject.Inject; @@ -49,46 +45,24 @@ public class CreatePremiumListAction extends CreateOrUpdatePremiumListAction { public static final String PATH = "/_dr/admin/createPremiumList"; @Inject @Parameter(OVERRIDE_PARAM) boolean override; + @Inject CreatePremiumListAction() {} @Override - protected void saveToDatastore() { + protected void save() { checkArgument( - !doesPremiumListExist(name), "A premium list of this name already exists: %s.", name); + !PremiumListDualDao.exists(name), "A premium list of this name already exists: %s.", name); if (!override) { assertTldExists(name); } - logger.atInfo().log("Saving premium list for TLD %s", name); logInputData(); List inputDataPreProcessed = Splitter.on('\n').omitEmptyStrings().splitToList(inputData); - PremiumList premiumList = new PremiumList.Builder().setName(name).build(); - savePremiumListAndEntries(premiumList, inputDataPreProcessed); - + PremiumListDualDao.save(name, inputDataPreProcessed); String message = - String.format( - "Saved premium list %s with %d entries", - premiumList.getName(), inputDataPreProcessed.size()); + String.format("Saved premium list %s with %d entries", name, inputDataPreProcessed.size()); logger.atInfo().log(message); response.setPayload(ImmutableMap.of("status", "success", "message", message)); } - - @Override - protected void saveToCloudSql() { - if (!override) { - assertTldExists(name); - } - logger.atInfo().log("Saving premium list to Cloud SQL for TLD %s", name); - // TODO(mcilwain): Call logInputData() here once Datastore persistence is removed. - - PremiumList premiumList = parseToPremiumList(name, inputData); - PremiumListDao.saveNew(premiumList); - - String message = - String.format( - "Saved premium list %s with %d entries", name, premiumList.getLabelsToPrices().size()); - logger.atInfo().log(message); - // TODO(mcilwain): Call response.setPayload(...) here once Datastore persistence is removed. - } } diff --git a/core/src/main/java/google/registry/tools/server/UpdatePremiumListAction.java b/core/src/main/java/google/registry/tools/server/UpdatePremiumListAction.java index 97d837347..af78b3b19 100644 --- a/core/src/main/java/google/registry/tools/server/UpdatePremiumListAction.java +++ b/core/src/main/java/google/registry/tools/server/UpdatePremiumListAction.java @@ -15,19 +15,16 @@ package google.registry.tools.server; import static com.google.common.base.Preconditions.checkArgument; -import static google.registry.model.registry.label.PremiumListUtils.savePremiumListAndEntries; import static google.registry.request.Action.Method.POST; -import static google.registry.schema.tld.PremiumListUtils.parseToPremiumList; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import com.google.common.flogger.FluentLogger; import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.request.Action; import google.registry.request.auth.Auth; -import google.registry.schema.tld.PremiumListDao; import java.util.List; -import java.util.Optional; import javax.inject.Inject; /** @@ -48,10 +45,9 @@ public class UpdatePremiumListAction extends CreateOrUpdatePremiumListAction { @Inject UpdatePremiumListAction() {} @Override - protected void saveToDatastore() { - Optional existingPremiumList = PremiumList.getUncached(name); + protected void save() { checkArgument( - existingPremiumList.isPresent(), + PremiumListDualDao.exists(name), "Could not update premium list %s because it doesn't exist.", name); @@ -59,8 +55,7 @@ public class UpdatePremiumListAction extends CreateOrUpdatePremiumListAction { logInputData(); List inputDataPreProcessed = Splitter.on('\n').omitEmptyStrings().splitToList(inputData); - PremiumList newPremiumList = - savePremiumListAndEntries(existingPremiumList.get(), inputDataPreProcessed); + PremiumList newPremiumList = PremiumListDualDao.save(name, inputDataPreProcessed); String message = String.format( @@ -69,18 +64,4 @@ public class UpdatePremiumListAction extends CreateOrUpdatePremiumListAction { logger.atInfo().log(message); response.setPayload(ImmutableMap.of("status", "success", "message", message)); } - - @Override - protected void saveToCloudSql() { - logger.atInfo().log("Updating premium list '%s' in Cloud SQL.", name); - // TODO(mcilwain): Add logInputData() call here once DB migration is complete. - PremiumList premiumList = parseToPremiumList(name, inputData); - PremiumListDao.update(premiumList); - String message = - String.format( - "Updated premium list '%s' with %d entries.", - premiumList.getName(), premiumList.getLabelsToPrices().size()); - logger.atInfo().log(message); - // TODO(mcilwain): Call response.setPayload() here once DB migration is complete. - } } diff --git a/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java b/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java index bbafaa9e0..7247ce3a1 100644 --- a/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java +++ b/core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java @@ -53,6 +53,7 @@ import google.registry.model.ofy.CommitLogBucket; import google.registry.model.ofy.CommitLogManifest; import google.registry.model.ofy.CommitLogMutation; import google.registry.model.registrar.RegistrarContact; +import google.registry.model.registry.label.PremiumList; import google.registry.model.registry.label.ReservedList; import google.registry.model.server.Lock; import google.registry.model.tmch.ClaimsListShard; @@ -93,13 +94,14 @@ public class ReplayCommitLogsToSqlActionTest { .withClock(fakeClock) .withOfyTestEntities(TestObject.class) .withJpaUnitTestEntities( - RegistrarContact.class, - TestObject.class, - SqlReplayCheckpoint.class, ContactResource.class, + DelegationSignerData.class, DomainBase.class, GracePeriod.class, - DelegationSignerData.class) + PremiumList.class, + RegistrarContact.class, + SqlReplayCheckpoint.class, + TestObject.class) .build(); /** Local GCS service. */ diff --git a/core/src/test/java/google/registry/beam/initsql/BackupTestStoreTest.java b/core/src/test/java/google/registry/beam/initsql/BackupTestStoreTest.java index 4a9cb3a1c..0f9291b0d 100644 --- a/core/src/test/java/google/registry/beam/initsql/BackupTestStoreTest.java +++ b/core/src/test/java/google/registry/beam/initsql/BackupTestStoreTest.java @@ -34,6 +34,9 @@ import google.registry.model.domain.DomainBase; import google.registry.model.ofy.Ofy; import google.registry.model.registry.Registry; import google.registry.persistence.VKey; +import google.registry.persistence.transaction.JpaTestRules; +import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestExtension; +import google.registry.testing.DatastoreEntityExtension; import google.registry.testing.FakeClock; import google.registry.testing.InjectExtension; import google.registry.tools.LevelDbLogReader; @@ -48,6 +51,7 @@ import org.apache.beam.sdk.values.KV; import org.joda.time.DateTime; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; @@ -58,6 +62,15 @@ public class BackupTestStoreTest { @TempDir File tempDir; + @RegisterExtension + final transient JpaIntegrationTestExtension jpaIntegrationTestExtension = + new JpaTestRules.Builder().buildIntegrationTestRule(); + + @RegisterExtension + @Order(value = 1) + final transient DatastoreEntityExtension datastoreEntityExtension = + new DatastoreEntityExtension(); + @RegisterExtension InjectExtension injectRule = new InjectExtension(); private FakeClock fakeClock; diff --git a/core/src/test/java/google/registry/beam/initsql/CommitLogTransformsTest.java b/core/src/test/java/google/registry/beam/initsql/CommitLogTransformsTest.java index 2ca811e51..15ea045c4 100644 --- a/core/src/test/java/google/registry/beam/initsql/CommitLogTransformsTest.java +++ b/core/src/test/java/google/registry/beam/initsql/CommitLogTransformsTest.java @@ -26,6 +26,9 @@ import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.ofy.Ofy; import google.registry.model.registry.Registry; +import google.registry.persistence.transaction.JpaTestRules; +import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestExtension; +import google.registry.testing.DatastoreEntityExtension; import google.registry.testing.FakeClock; import google.registry.testing.InjectExtension; import java.io.File; @@ -44,6 +47,7 @@ import org.apache.beam.sdk.values.PCollection; import org.joda.time.DateTime; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; @@ -52,6 +56,7 @@ import org.junit.jupiter.api.io.TempDir; class CommitLogTransformsTest implements Serializable { private static final DateTime START_TIME = DateTime.parse("2000-01-01T00:00:00.0Z"); + private final FakeClock fakeClock = new FakeClock(START_TIME); @SuppressWarnings("WeakerAccess") @TempDir @@ -59,11 +64,19 @@ class CommitLogTransformsTest implements Serializable { @RegisterExtension final transient InjectExtension injectRule = new InjectExtension(); + @RegisterExtension + final transient JpaIntegrationTestExtension jpaIntegrationTestExtension = + new JpaTestRules.Builder().withClock(fakeClock).buildIntegrationTestRule(); + + @RegisterExtension + @Order(value = 1) + final transient DatastoreEntityExtension datastoreEntityExtension = + new DatastoreEntityExtension(); + @RegisterExtension final transient TestPipelineExtension testPipeline = TestPipelineExtension.create().enableAbandonedNodeEnforcement(true); - private FakeClock fakeClock; private transient BackupTestStore store; private File commitLogsDir; private File firstCommitLogFile; @@ -75,7 +88,6 @@ class CommitLogTransformsTest implements Serializable { @BeforeEach void beforeEach() throws Exception { - fakeClock = new FakeClock(START_TIME); store = new BackupTestStore(fakeClock); injectRule.setStaticField(Ofy.class, "clock", fakeClock); diff --git a/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java b/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java index 301994c69..55ed0d5b5 100644 --- a/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java +++ b/core/src/test/java/google/registry/beam/initsql/DomainBaseUtilTest.java @@ -68,7 +68,7 @@ public class DomainBaseUtilTest { @RegisterExtension AppEngineExtension appEngineRule = - AppEngineExtension.builder().withDatastore().withClock(fakeClock).build(); + AppEngineExtension.builder().withDatastoreAndCloudSql().withClock(fakeClock).build(); @RegisterExtension InjectExtension injectRule = new InjectExtension(); diff --git a/core/src/test/java/google/registry/beam/initsql/ExportloadingTransformsTest.java b/core/src/test/java/google/registry/beam/initsql/ExportLoadingTransformsTest.java similarity index 92% rename from core/src/test/java/google/registry/beam/initsql/ExportloadingTransformsTest.java rename to core/src/test/java/google/registry/beam/initsql/ExportLoadingTransformsTest.java index 1138cd510..af659a1f9 100644 --- a/core/src/test/java/google/registry/beam/initsql/ExportloadingTransformsTest.java +++ b/core/src/test/java/google/registry/beam/initsql/ExportLoadingTransformsTest.java @@ -27,6 +27,9 @@ import google.registry.model.contact.ContactResource; import google.registry.model.domain.DomainBase; import google.registry.model.ofy.Ofy; import google.registry.model.registry.Registry; +import google.registry.persistence.transaction.JpaTestRules; +import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestExtension; +import google.registry.testing.DatastoreEntityExtension; import google.registry.testing.FakeClock; import google.registry.testing.InjectExtension; import java.io.File; @@ -44,6 +47,7 @@ import org.apache.beam.sdk.values.PCollection; import org.joda.time.DateTime; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; @@ -53,7 +57,7 @@ import org.junit.jupiter.api.io.TempDir; * *

This class implements {@link Serializable} so that test {@link DoFn} classes may be inlined. */ -class ExportloadingTransformsTest implements Serializable { +class ExportLoadingTransformsTest implements Serializable { private static final DateTime START_TIME = DateTime.parse("2000-01-01T00:00:00.0Z"); @@ -68,6 +72,15 @@ class ExportloadingTransformsTest implements Serializable { @RegisterExtension final transient InjectExtension injectRule = new InjectExtension(); + @RegisterExtension + final transient JpaIntegrationTestExtension jpaIntegrationTestExtension = + new JpaTestRules.Builder().buildIntegrationTestRule(); + + @RegisterExtension + @Order(value = 1) + final transient DatastoreEntityExtension datastoreEntityExtension = + new DatastoreEntityExtension(); + @RegisterExtension final transient TestPipelineExtension testPipeline = TestPipelineExtension.create().enableAbandonedNodeEnforcement(true); diff --git a/core/src/test/java/google/registry/beam/initsql/LoadDatastoreSnapshotTest.java b/core/src/test/java/google/registry/beam/initsql/LoadDatastoreSnapshotTest.java index 8af1849dc..9eb72f23a 100644 --- a/core/src/test/java/google/registry/beam/initsql/LoadDatastoreSnapshotTest.java +++ b/core/src/test/java/google/registry/beam/initsql/LoadDatastoreSnapshotTest.java @@ -29,6 +29,9 @@ import google.registry.model.domain.DomainBase; import google.registry.model.eppcommon.AuthInfo.PasswordAuth; import google.registry.model.ofy.Ofy; import google.registry.model.registry.Registry; +import google.registry.persistence.transaction.JpaTestRules; +import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestExtension; +import google.registry.testing.DatastoreEntityExtension; import google.registry.testing.FakeClock; import google.registry.testing.InjectExtension; import java.io.File; @@ -38,6 +41,7 @@ import org.apache.beam.sdk.values.KV; import org.apache.beam.sdk.values.PCollectionTuple; import org.joda.time.DateTime; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; @@ -84,6 +88,15 @@ class LoadDatastoreSnapshotTest { @RegisterExtension final transient InjectExtension injectRule = new InjectExtension(); + @RegisterExtension + final transient JpaIntegrationTestExtension jpaIntegrationTestExtension = + new JpaTestRules.Builder().buildIntegrationTestRule(); + + @RegisterExtension + @Order(value = 1) + final transient DatastoreEntityExtension datastoreEntityExtension = + new DatastoreEntityExtension(); + @RegisterExtension final transient TestPipelineExtension testPipeline = TestPipelineExtension.create().enableAbandonedNodeEnforcement(true); diff --git a/core/src/test/java/google/registry/export/ExportPremiumTermsActionTest.java b/core/src/test/java/google/registry/export/ExportPremiumTermsActionTest.java index 695c6c5bf..caf0639b5 100644 --- a/core/src/test/java/google/registry/export/ExportPremiumTermsActionTest.java +++ b/core/src/test/java/google/registry/export/ExportPremiumTermsActionTest.java @@ -17,8 +17,6 @@ package google.registry.export; import static com.google.common.net.MediaType.PLAIN_TEXT_UTF_8; import static google.registry.export.ExportPremiumTermsAction.EXPORT_MIME_TYPE; import static google.registry.export.ExportPremiumTermsAction.PREMIUM_TERMS_FILENAME; -import static google.registry.model.registry.label.PremiumListUtils.deletePremiumList; -import static google.registry.model.registry.label.PremiumListUtils.savePremiumListAndEntries; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.deleteTld; import static google.registry.testing.DatabaseHelper.persistResource; @@ -39,6 +37,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.net.MediaType; import google.registry.model.registry.Registry; import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.request.Response; import google.registry.storage.drive.DriveConnection; import google.registry.testing.AppEngineExtension; @@ -76,8 +75,7 @@ public class ExportPremiumTermsActionTest { @BeforeEach void beforeEach() throws Exception { createTld("tld"); - PremiumList pl = new PremiumList.Builder().setName("pl-name").build(); - savePremiumListAndEntries(pl, PREMIUM_NAMES); + PremiumList pl = PremiumListDualDao.save("pl-name", PREMIUM_NAMES); persistResource( Registry.get("tld").asBuilder().setPremiumList(pl).setDriveFolderId("folder_id").build()); when(driveConnection.createOrUpdateFile( @@ -106,26 +104,6 @@ public class ExportPremiumTermsActionTest { verifyNoMoreInteractions(response); } - @Test - void test_exportPremiumTerms_success_emptyPremiumList() throws IOException { - PremiumList pl = new PremiumList.Builder().setName("pl-name").build(); - savePremiumListAndEntries(pl, ImmutableList.of()); - runAction("tld"); - - verify(driveConnection) - .createOrUpdateFile( - PREMIUM_TERMS_FILENAME, - EXPORT_MIME_TYPE, - "folder_id", - DISCLAIMER_WITH_NEWLINE.getBytes(UTF_8)); - verifyNoMoreInteractions(driveConnection); - - verify(response).setStatus(SC_OK); - verify(response).setPayload("file_id"); - verify(response).setContentType(PLAIN_TEXT_UTF_8); - verifyNoMoreInteractions(response); - } - @Test void test_exportPremiumTerms_doNothing_listNotConfigured() { persistResource(Registry.get("tld").asBuilder().setPremiumList(null).build()); @@ -165,7 +143,7 @@ public class ExportPremiumTermsActionTest { @Test void test_exportPremiumTerms_failure_noPremiumList() { - deletePremiumList(new PremiumList.Builder().setName("pl-name").build()); + PremiumListDualDao.delete(new PremiumList.Builder().setName("pl-name").build()); assertThrows(RuntimeException.class, () -> runAction("tld")); verifyNoInteractions(driveConnection); diff --git a/core/src/test/java/google/registry/model/registry/RegistryTest.java b/core/src/test/java/google/registry/model/registry/RegistryTest.java index a73398853..ff03d677d 100644 --- a/core/src/test/java/google/registry/model/registry/RegistryTest.java +++ b/core/src/test/java/google/registry/model/registry/RegistryTest.java @@ -41,6 +41,7 @@ import google.registry.model.EntityTestCase; import google.registry.model.registry.Registry.RegistryNotFoundException; import google.registry.model.registry.Registry.TldState; import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.model.registry.label.ReservedList; import google.registry.testing.DualDatabaseTest; import google.registry.testing.TestOfyAndSql; @@ -204,7 +205,7 @@ public class RegistryTest extends EntityTestCase { Registry registry = Registry.get("tld").asBuilder().setPremiumList(pl2).build(); Key plKey = registry.getPremiumList(); assertThat(plKey).isNotNull(); - PremiumList stored = PremiumList.getUncached(plKey.getName()).get(); + PremiumList stored = PremiumListDualDao.getLatestRevision(plKey.getName()).get(); assertThat(stored.getName()).isEqualTo("tld2"); } diff --git a/core/src/test/java/google/registry/model/registry/label/PremiumListUtilsTest.java b/core/src/test/java/google/registry/model/registry/label/PremiumListDatastoreDaoTest.java similarity index 63% rename from core/src/test/java/google/registry/model/registry/label/PremiumListUtilsTest.java rename to core/src/test/java/google/registry/model/registry/label/PremiumListDatastoreDaoTest.java index 90298c622..1c1f3acc0 100644 --- a/core/src/test/java/google/registry/model/registry/label/PremiumListUtilsTest.java +++ b/core/src/test/java/google/registry/model/registry/label/PremiumListDatastoreDaoTest.java @@ -14,6 +14,7 @@ package google.registry.model.registry.label; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static com.google.monitoring.metrics.contrib.DistributionMetricSubject.assertThat; @@ -26,37 +27,33 @@ import static google.registry.model.registry.label.DomainLabelMetrics.PremiumLis import static google.registry.model.registry.label.DomainLabelMetrics.PremiumListCheckOutcome.UNCACHED_POSITIVE; import static google.registry.model.registry.label.DomainLabelMetrics.premiumListChecks; import static google.registry.model.registry.label.DomainLabelMetrics.premiumListProcessingTime; -import static google.registry.model.registry.label.PremiumListUtils.deletePremiumList; -import static google.registry.model.registry.label.PremiumListUtils.doesPremiumListExist; -import static google.registry.model.registry.label.PremiumListUtils.getPremiumPrice; -import static google.registry.model.registry.label.PremiumListUtils.savePremiumListAndEntries; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; +import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import static google.registry.testing.DatabaseHelper.createTld; import static google.registry.testing.DatabaseHelper.loadPremiumListEntries; import static google.registry.testing.DatabaseHelper.persistPremiumList; import static google.registry.testing.DatabaseHelper.persistResource; 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.ImmutableMap; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Streams; import com.googlecode.objectify.Key; -import google.registry.dns.writer.VoidDnsWriter; -import google.registry.model.pricing.StaticPremiumListPricingEngine; 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 google.registry.testing.TestCacheExtension; import java.util.Map; +import java.util.Optional; +import java.util.function.Function; import org.joda.money.Money; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -/** Unit tests for {@link PremiumListUtils}. */ -public class PremiumListUtilsTest { +/** Unit tests for {@link PremiumListDatastoreDao}. */ +public class PremiumListDatastoreDaoTest { @RegisterExtension public final AppEngineExtension appEngine = @@ -70,11 +67,13 @@ public class PremiumListUtilsTest { .withPremiumListEntriesCache(standardDays(1)) .build(); + private PremiumList pl; + @BeforeEach void before() { // createTld() overwrites the premium list, so call it before persisting pl. createTld("tld"); - PremiumList pl = + pl = persistPremiumList( "tld", "lol,USD 999 # yup", @@ -98,66 +97,39 @@ public class PremiumListUtilsTest { .hasNoOtherValues(); } - @Test - void testGetPremiumPrice_returnsNoPriceWhenNoPremiumListConfigured() { - createTld("ghost"); - persistResource( - new Registry.Builder() - .setTldStr("ghost") - .setPremiumPricingEngine(StaticPremiumListPricingEngine.NAME) - .setDnsWriters(ImmutableSet.of(VoidDnsWriter.NAME)) - .build()); - assertThat(Registry.get("ghost").getPremiumList()).isNull(); - assertThat(getPremiumPrice("blah", Registry.get("ghost"))).isEmpty(); - assertThat(premiumListChecks).hasNoOtherValues(); - assertThat(premiumListProcessingTime).hasNoOtherValues(); - } - - @Test - void testGetPremiumPrice_throwsExceptionWhenNonExistentPremiumListConfigured() { - deletePremiumList(PremiumList.getUncached("tld").get()); - IllegalStateException thrown = - assertThrows( - IllegalStateException.class, () -> getPremiumPrice("blah", Registry.get("tld"))); - assertThat(thrown).hasMessageThat().contains("Could not load premium list 'tld'"); - } - @Test void testSave_largeNumberOfEntries_succeeds() { PremiumList premiumList = persistHumongousPremiumList("tld", 2500); assertThat(loadPremiumListEntries(premiumList)).hasSize(2500); - assertThat(getPremiumPrice("7", Registry.get("tld"))).hasValue(Money.parse("USD 100")); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "7", "tld")) + .hasValue(Money.parse("USD 100")); assertMetricOutcomeCount(1, UNCACHED_POSITIVE); } @Test void testSave_updateTime_isUpdatedOnEverySave() { - PremiumList pl = - savePremiumListAndEntries( - new PremiumList.Builder().setName("tld3").build(), ImmutableList.of("slime,USD 10")); - PremiumList newPl = - savePremiumListAndEntries( - new PremiumList.Builder().setName(pl.getName()).build(), - ImmutableList.of("mutants,USD 20")); + PremiumList pl = PremiumListDatastoreDao.save("tld3", ImmutableList.of("slime,USD 10")); + PremiumList newPl = PremiumListDatastoreDao.save("tld3", ImmutableList.of("mutants,USD 20")); assertThat(newPl.getLastUpdateTime()).isGreaterThan(pl.getLastUpdateTime()); } @Test void testSave_creationTime_onlyUpdatedOnFirstCreation() { PremiumList pl = persistPremiumList("tld3", "sludge,JPY 1000"); - PremiumList newPl = savePremiumListAndEntries(pl, ImmutableList.of("sleighbells,CHF 2000")); + PremiumList newPl = + PremiumListDatastoreDao.save("tld3", ImmutableList.of("sleighbells,CHF 2000")); assertThat(newPl.creationTime).isEqualTo(pl.creationTime); } @Test void testExists() { - assertThat(doesPremiumListExist("tld")).isTrue(); - assertThat(doesPremiumListExist("nonExistentPremiumList")).isFalse(); + assertThat(PremiumListDatastoreDao.getLatestRevision("tld")).isPresent(); + assertThat(PremiumListDatastoreDao.getLatestRevision("nonExistentPremiumList")).isEmpty(); } @Test void testGetPremiumPrice_comesFromBloomFilter() throws Exception { - PremiumList pl = PremiumList.getCached("tld").get(); + PremiumList pl = PremiumListDatastoreDao.getLatestRevision("tld").get(); PremiumListEntry entry = persistResource( new PremiumListEntry.Builder() @@ -167,10 +139,10 @@ public class PremiumListUtilsTest { .build()); // "missingno" shouldn't be in the Bloom filter, thus it should return not premium without // attempting to load the entity that is actually present. - assertThat(getPremiumPrice("missingno", Registry.get("tld"))).isEmpty(); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "missingno", "tld")).isEmpty(); // However, if we manually query the cache to force an entity load, it should be found. assertThat( - PremiumList.cachePremiumListEntries.get( + PremiumListDatastoreDao.premiumListEntriesCache.get( Key.create(pl.getRevisionKey(), PremiumListEntry.class, "missingno"))) .hasValue(entry); assertMetricOutcomeCount(1, BLOOM_FILTER_NEGATIVE); @@ -178,8 +150,10 @@ public class PremiumListUtilsTest { @Test void testGetPremiumPrice_cachedSecondTime() { - assertThat(getPremiumPrice("rich", Registry.get("tld"))).hasValue(Money.parse("USD 1999")); - assertThat(getPremiumPrice("rich", Registry.get("tld"))).hasValue(Money.parse("USD 1999")); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "rich", "tld")) + .hasValue(Money.parse("USD 1999")); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "rich", "tld")) + .hasValue(Money.parse("USD 1999")); assertThat(premiumListChecks) .hasValueForLabels(1, "tld", "tld", UNCACHED_POSITIVE.toString()) .and() @@ -197,20 +171,15 @@ public class PremiumListUtilsTest { @Test void testGetPremiumPrice_bloomFilterFalsePositive() { // Remove one of the premium list entries from behind the Bloom filter's back. - tm() - .transactNew( + tm().transactNew( () -> ofy() .delete() - .keys( - Key.create( - PremiumList.getCached("tld").get().getRevisionKey(), - PremiumListEntry.class, - "rich"))); + .keys(Key.create(pl.getRevisionKey(), PremiumListEntry.class, "rich"))); ofy().clearSessionCache(); - assertThat(getPremiumPrice("rich", Registry.get("tld"))).isEmpty(); - assertThat(getPremiumPrice("rich", Registry.get("tld"))).isEmpty(); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "rich", "tld")).isEmpty(); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "rich", "tld")).isEmpty(); assertThat(premiumListChecks) .hasValueForLabels(1, "tld", "tld", UNCACHED_NEGATIVE.toString()) @@ -228,22 +197,26 @@ public class PremiumListUtilsTest { @Test void testSave_removedPremiumListEntries_areNoLongerInDatastore() { - Registry registry = Registry.get("tld"); PremiumList pl = persistPremiumList("tld", "genius,USD 10", "dolt,JPY 1000"); - assertThat(getPremiumPrice("genius", registry)).hasValue(Money.parse("USD 10")); - assertThat(getPremiumPrice("dolt", registry)).hasValue(Money.parse("JPY 1000")); - assertThat(ofy() - .load() - .type(PremiumListEntry.class) - .parent(pl.getRevisionKey()) - .id("dolt") - .now() - .price) + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "genius", "tld")) + .hasValue(Money.parse("USD 10")); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "dolt", "tld")) + .hasValue(Money.parse("JPY 1000")); + assertThat( + ofy() + .load() + .type(PremiumListEntry.class) + .parent(pl.getRevisionKey()) + .id("dolt") + .now() + .price) .isEqualTo(Money.parse("JPY 1000")); - savePremiumListAndEntries(pl, ImmutableList.of("genius,USD 10", "savant,USD 90")); - assertThat(getPremiumPrice("genius", registry)).hasValue(Money.parse("USD 10")); - assertThat(getPremiumPrice("savant", registry)).hasValue(Money.parse("USD 90")); - assertThat(getPremiumPrice("dolt", registry)).isEmpty(); + PremiumListDatastoreDao.save("tld", ImmutableList.of("genius,USD 10", "savant,USD 90")); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "genius", "tld")) + .hasValue(Money.parse("USD 10")); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "savant", "tld")) + .hasValue(Money.parse("USD 90")); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "dolt", "tld")).isEmpty(); // TODO(b/79888775): Assert that the old premium list is enqueued for later deletion. assertThat(premiumListChecks) .hasValueForLabels(4, "tld", "tld", UNCACHED_POSITIVE.toString()) @@ -261,23 +234,22 @@ public class PremiumListUtilsTest { @Test void testGetPremiumPrice_allLabelsAreNonPremium_whenNotInList() { - assertThat(getPremiumPrice("blah", Registry.get("tld"))).isEmpty(); - assertThat(getPremiumPrice("slinge", Registry.get("tld"))).isEmpty(); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "blah", "tld")).isEmpty(); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld", "slinge", "tld")).isEmpty(); assertMetricOutcomeCount(2, BLOOM_FILTER_NEGATIVE); } @Test void testSave_simple() { PremiumList pl = - savePremiumListAndEntries( - new PremiumList.Builder().setName("tld2").build(), - ImmutableList.of("lol , USD 999 # yupper rooni ")); - createTld("tld"); + PremiumListDatastoreDao.save("tld2", ImmutableList.of("lol , USD 999 # yupper rooni ")); persistResource(Registry.get("tld").asBuilder().setPremiumList(pl).build()); - assertThat(getPremiumPrice("lol", Registry.get("tld"))).hasValue(Money.parse("USD 999")); - assertThat(getPremiumPrice("lol ", Registry.get("tld"))).isEmpty(); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld2", "lol", "tld")) + .hasValue(Money.parse("USD 999")); + assertThat(PremiumListDatastoreDao.getPremiumPrice("tld2", "lol ", "tld")).isEmpty(); ImmutableMap entries = - loadPremiumListEntries(PremiumList.getUncached("tld2").get()); + Streams.stream(PremiumListDatastoreDao.loadPremiumListEntriesUncached(pl)) + .collect(toImmutableMap(PremiumListEntry::getLabel, Function.identity())); assertThat(entries.keySet()).containsExactly("lol"); assertThat(entries).doesNotContainKey("lol "); PremiumListEntry entry = entries.get("lol"); @@ -300,47 +272,47 @@ public class PremiumListUtilsTest { @Test void test_saveAndUpdateEntriesTwice() { - PremiumList pl = - savePremiumListAndEntries( - new PremiumList.Builder().setName("pl").build(), ImmutableList.of("test,USD 1")); - ImmutableMap entries = loadPremiumListEntries(pl); + PremiumList firstSave = PremiumListDatastoreDao.save("tld", ImmutableList.of("test,USD 1")); + ImmutableMap entries = + Streams.stream(PremiumListDatastoreDao.loadPremiumListEntriesUncached(firstSave)) + .collect(toImmutableMap(PremiumListEntry::getLabel, Function.identity())); assertThat(entries.keySet()).containsExactly("test"); - assertThat(loadPremiumListEntries(PremiumList.getUncached("pl").get())).isEqualTo(entries); // Save again with no changes, and clear the cache to force a re-load from Datastore. - PremiumList resaved = savePremiumListAndEntries(pl, ImmutableList.of("test,USD 1")); + PremiumList resaved = PremiumListDatastoreDao.save("tld", ImmutableList.of("test,USD 1")); ofy().clearSessionCache(); Map entriesReloaded = - loadPremiumListEntries(PremiumList.getUncached("pl").get()); + Streams.stream(PremiumListDatastoreDao.loadPremiumListEntriesUncached(resaved)) + .collect(toImmutableMap(PremiumListEntry::getLabel, Function.identity())); assertThat(entriesReloaded).hasSize(1); assertThat(entriesReloaded).containsKey("test"); assertThat(entriesReloaded.get("test").parent).isEqualTo(resaved.getRevisionKey()); } - @Test - void test_savePremiumListAndEntries_clearsCache() { - assertThat(PremiumList.cachePremiumLists.getIfPresent("tld")).isNull(); - PremiumList pl = PremiumList.getCached("tld").get(); - assertThat(PremiumList.cachePremiumLists.getIfPresent("tld")).isEqualTo(pl); - savePremiumListAndEntries( - new PremiumList.Builder().setName("tld").build(), ImmutableList.of("test,USD 1")); - assertThat(PremiumList.cachePremiumLists.getIfPresent("tld")).isNull(); - } - @Test void testDelete() { persistPremiumList("gtld1", "trombone,USD 10"); - assertThat(PremiumList.getUncached("gtld1")).isPresent(); - Key parent = PremiumList.getUncached("gtld1").get().getRevisionKey(); - deletePremiumList(PremiumList.getUncached("gtld1").get()); - assertThat(PremiumList.getUncached("gtld1")).isEmpty(); + Optional gtld1 = PremiumListDatastoreDao.getLatestRevision("gtld1"); + assertThat(gtld1).isPresent(); + Key parent = gtld1.get().getRevisionKey(); + PremiumListDatastoreDao.delete(gtld1.get()); + assertThat(PremiumListDatastoreDao.getLatestRevision("gtld1")).isEmpty(); assertThat(ofy().load().type(PremiumListEntry.class).ancestor(parent).list()).isEmpty(); } @Test void testDelete_largeNumberOfEntries_succeeds() { - persistHumongousPremiumList("ginormous", 2500); - deletePremiumList(PremiumList.getUncached("ginormous").get()); - assertThat(PremiumList.getUncached("ginormous")).isEmpty(); + PremiumList large = persistHumongousPremiumList("ginormous", 2500); + PremiumListDatastoreDao.delete(large); + assertThat(PremiumListDatastoreDao.getLatestRevision("ginormous")).isEmpty(); + } + + @Test + void test_savePremiumList_clearsCache() { + assertThat(PremiumListDatastoreDao.premiumListCache.getIfPresent("tld")).isNull(); + PremiumList pl = PremiumListDatastoreDao.getLatestRevision("tld").get(); + assertThat(PremiumListDatastoreDao.premiumListCache.getIfPresent("tld").get()).isEqualTo(pl); + transactIfJpaTm(() -> PremiumListDatastoreDao.save("tld", ImmutableList.of("test,USD 1"))); + assertThat(PremiumListDatastoreDao.premiumListCache.getIfPresent("tld")).isNull(); } /** Persists a premium list with a specified number of nonsense entries. */ 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 new file mode 100644 index 000000000..31a187729 --- /dev/null +++ b/core/src/test/java/google/registry/model/registry/label/PremiumListDualDaoTest.java @@ -0,0 +1,139 @@ +// Copyright 2021 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.model.registry.label; + +import static com.google.common.truth.Truth.assertThat; +import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.testing.DatabaseHelper.newRegistry; +import static google.registry.testing.DatabaseHelper.persistResource; +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.truth.Truth8; +import google.registry.dns.writer.VoidDnsWriter; +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.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(); + + // Set long persist times on caches so they can be tested (cache times default to 0 in tests). + @RegisterExtension + public final TestCacheExtension testCacheExtension = + new TestCacheExtension.Builder() + .withPremiumListsCache(standardDays(1)) + .withPremiumListEntriesCache(standardDays(1)) + .build(); + + @BeforeEach + void before() { + createTld("tld"); + } + + @TestOfyOnly + 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)."); + } + + @TestSqlOnly + void testGetPremiumPrice_secondaryLoadMissingOfy() { + PremiumList premiumList = PremiumListDatastoreDao.getLatestRevision("tld").get(); + PremiumListDatastoreDao.delete(premiumList); + assertThat( + assertThrows( + IllegalStateException.class, + () -> PremiumListDualDao.getPremiumPrice("brass", Registry.get("tld")))) + .hasMessageThat() + .isEqualTo( + "Unequal prices for domain brass.tld from primary SQL DB (Optional[USD 20.00]) " + + "and secondary Datastore db (Optional.empty)."); + } + + @TestOfyOnly + 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])."); + } + + @TestSqlOnly + void testGetPremiumPrice_secondaryDifferentOfy() { + PremiumListDatastoreDao.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 SQL DB " + + "(Optional[USD 20.00]) and secondary Datastore db (Optional[USD 50.00])."); + } + + @TestOfyAndSql + void testGetPremiumPrice_returnsNoPriceWhenNoPremiumListConfigured() { + createTld("ghost"); + persistResource( + new Registry.Builder() + .setTldStr("ghost") + .setPremiumPricingEngine(StaticPremiumListPricingEngine.NAME) + .setDnsWriters(ImmutableSet.of(VoidDnsWriter.NAME)) + .build()); + assertThat(Registry.get("ghost").getPremiumList()).isNull(); + Truth8.assertThat(PremiumListDualDao.getPremiumPrice("blah", Registry.get("ghost"))).isEmpty(); + } + + @TestOfyAndSql + void testGetPremiumPrice_emptyWhenPremiumListDeleted() { + PremiumList toDelete = PremiumListDualDao.getLatestRevision("tld").get(); + PremiumListDualDao.delete(toDelete); + Truth8.assertThat(PremiumListDualDao.getPremiumPrice("blah", Registry.get("tld"))).isEmpty(); + } + + @TestOfyAndSql + 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 c31ac67e2..5bc0a3252 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 @@ -69,7 +69,7 @@ public class PremiumListTest { @Test void testProbablePremiumLabels() { - PremiumList pl = PremiumList.getUncached("tld").get(); + PremiumList pl = PremiumListDualDao.getLatestRevision("tld").get(); PremiumListRevision revision = ofy().load().key(pl.getRevisionKey()).now(); assertThat(revision.getProbablePremiumLabels().mightContain("notpremium")).isFalse(); for (String label : ImmutableList.of("rich", "lol", "johnny-be-goode", "icann")) { @@ -85,7 +85,7 @@ public class PremiumListTest { assertThrows( IllegalStateException.class, () -> - PremiumList.getUncached("tld") + PremiumListDualDao.getLatestRevision("tld") .get() .parse( ImmutableList.of( diff --git a/core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java b/core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java index 216db2753..05e86b56a 100644 --- a/core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java +++ b/core/src/test/java/google/registry/schema/integration/SqlIntegrationTestSuite.java @@ -42,7 +42,7 @@ import google.registry.schema.integration.SqlIntegrationTestSuite.BeforeSuiteTes import google.registry.schema.registrar.RegistrarDaoTest; import google.registry.schema.replay.SqlReplayCheckpointTest; import google.registry.schema.server.LockDaoTest; -import google.registry.schema.tld.PremiumListDaoTest; +import google.registry.schema.tld.PremiumListSqlDaoTest; import google.registry.testing.AppEngineExtension; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -92,7 +92,7 @@ import org.junit.runner.RunWith; KmsSecretRevisionSqlDaoTest.class, LockDaoTest.class, PollMessageTest.class, - PremiumListDaoTest.class, + PremiumListSqlDaoTest.class, RdeRevisionTest.class, RegistrarDaoTest.class, RegistryTest.class, diff --git a/core/src/test/java/google/registry/schema/tld/PremiumListDaoTest.java b/core/src/test/java/google/registry/schema/tld/PremiumListSqlDaoTest.java similarity index 65% rename from core/src/test/java/google/registry/schema/tld/PremiumListDaoTest.java rename to core/src/test/java/google/registry/schema/tld/PremiumListSqlDaoTest.java index bb2bdee4f..f91cbb37e 100644 --- a/core/src/test/java/google/registry/schema/tld/PremiumListDaoTest.java +++ b/core/src/test/java/google/registry/schema/tld/PremiumListSqlDaoTest.java @@ -18,30 +18,30 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; -import static google.registry.testing.DatabaseHelper.createTld; +import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; import static google.registry.testing.DatabaseHelper.newRegistry; import static google.registry.testing.DatabaseHelper.persistResource; import static org.joda.money.CurrencyUnit.JPY; import static org.joda.money.CurrencyUnit.USD; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.joda.time.Duration.standardDays; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.googlecode.objectify.Key; -import google.registry.model.registry.Registry; import google.registry.model.registry.label.PremiumList; import google.registry.testing.AppEngineExtension; import google.registry.testing.FakeClock; +import google.registry.testing.TestCacheExtension; import java.math.BigDecimal; import java.util.Optional; import org.joda.money.CurrencyUnit; import org.joda.money.Money; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -/** Unit tests for {@link PremiumListDao}. */ -public class PremiumListDaoTest { +/** Unit tests for {@link PremiumListSqlDao}. */ +public class PremiumListSqlDaoTest { private final FakeClock fakeClock = new FakeClock(); @@ -53,6 +53,14 @@ public class PremiumListDaoTest { .withClock(fakeClock) .build(); + // Set long persist times on caches so they can be tested (cache times default to 0 in tests). + @RegisterExtension + public final TestCacheExtension testCacheExtension = + new TestCacheExtension.Builder() + .withPremiumListsCache(standardDays(1)) + .withPremiumListEntriesCache(standardDays(1)) + .build(); + private ImmutableMap testPrices; private PremiumList testList; @@ -78,11 +86,12 @@ public class PremiumListDaoTest { @Test void saveNew_worksSuccessfully() { - PremiumListDao.saveNew(testList); + PremiumListSqlDao.save(testList); jpaTm() .transact( () -> { - Optional persistedListOpt = PremiumListDao.getLatestRevision("testname"); + Optional persistedListOpt = + PremiumListSqlDao.getLatestRevision("testname"); assertThat(persistedListOpt).isPresent(); PremiumList persistedList = persistedListOpt.get(); assertThat(persistedList.getLabelsToPrices()).containsExactlyEntriesIn(testPrices); @@ -92,17 +101,17 @@ public class PremiumListDaoTest { @Test void update_worksSuccessfully() { - PremiumListDao.saveNew(testList); - Optional persistedList = PremiumListDao.getLatestRevision("testname"); + PremiumListSqlDao.save(testList); + Optional persistedList = PremiumListSqlDao.getLatestRevision("testname"); assertThat(persistedList).isPresent(); long firstRevisionId = persistedList.get().getRevisionId(); - PremiumListDao.update( + PremiumListSqlDao.save( new PremiumList.Builder() .setName("testname") .setCurrency(USD) .setLabelsToPrices( ImmutableMap.of( - "update", + "save", BigDecimal.valueOf(55343.12), "new", BigDecimal.valueOf(0.01), @@ -113,65 +122,46 @@ public class PremiumListDaoTest { jpaTm() .transact( () -> { - Optional updatedListOpt = PremiumListDao.getLatestRevision("testname"); - assertThat(updatedListOpt).isPresent(); - PremiumList updatedList = updatedListOpt.get(); - assertThat(updatedList.getLabelsToPrices()) + Optional savedListOpt = PremiumListSqlDao.getLatestRevision("testname"); + assertThat(savedListOpt).isPresent(); + PremiumList savedList = savedListOpt.get(); + assertThat(savedList.getLabelsToPrices()) .containsExactlyEntriesIn( ImmutableMap.of( - "update", + "save", BigDecimal.valueOf(55343.12), "new", BigDecimal.valueOf(0.01), "silver", BigDecimal.valueOf(30.03))); - assertThat(updatedList.getCreationTime()).isEqualTo(fakeClock.nowUtc()); - assertThat(updatedList.getRevisionId()).isGreaterThan(firstRevisionId); - assertThat(updatedList.getCreationTime()).isEqualTo(fakeClock.nowUtc()); + assertThat(savedList.getCreationTime()).isEqualTo(fakeClock.nowUtc()); + assertThat(savedList.getRevisionId()).isGreaterThan(firstRevisionId); + assertThat(savedList.getCreationTime()).isEqualTo(fakeClock.nowUtc()); }); } - @Test - void saveNew_throwsWhenPremiumListAlreadyExists() { - PremiumListDao.saveNew(testList); - IllegalArgumentException thrown = - assertThrows(IllegalArgumentException.class, () -> PremiumListDao.saveNew(testList)); - assertThat(thrown).hasMessageThat().isEqualTo("Premium list 'testname' already exists"); - } - - // TODO(b/147246613): Un-ignore this. - @Test - @Disabled - void update_throwsWhenListDoesntExist() { - IllegalArgumentException thrown = - assertThrows(IllegalArgumentException.class, () -> PremiumListDao.update(testList)); - assertThat(thrown) - .hasMessageThat() - .isEqualTo("Can't update non-existent premium list 'testname'"); - } - @Test void checkExists_worksSuccessfully() { - assertThat(PremiumListDao.checkExists("testname")).isFalse(); - PremiumListDao.saveNew(testList); - assertThat(PremiumListDao.checkExists("testname")).isTrue(); + assertThat(PremiumListSqlDao.getLatestRevision("testname")).isEmpty(); + PremiumListSqlDao.save(testList); + assertThat(PremiumListSqlDao.getLatestRevision("testname")).isPresent(); } @Test void getLatestRevision_returnsEmptyForNonexistentList() { - assertThat(PremiumListDao.getLatestRevision("nonexistentlist")).isEmpty(); + assertThat(PremiumListSqlDao.getLatestRevision("nonexistentlist")).isEmpty(); } @Test void getLatestRevision_worksSuccessfully() { - PremiumListDao.saveNew( + PremiumListSqlDao.save( new PremiumList.Builder() .setName("list1") .setCurrency(JPY) .setLabelsToPrices(ImmutableMap.of("wrong", BigDecimal.valueOf(1000.50))) .setCreationTime(fakeClock.nowUtc()) .build()); - PremiumListDao.update( + PremiumListSqlDao.save( new PremiumList.Builder() .setName("list1") .setCurrency(JPY) @@ -181,7 +171,7 @@ public class PremiumListDaoTest { jpaTm() .transact( () -> { - Optional persistedList = PremiumListDao.getLatestRevision("list1"); + Optional persistedList = PremiumListSqlDao.getLatestRevision("list1"); assertThat(persistedList).isPresent(); assertThat(persistedList.get().getName()).isEqualTo("list1"); assertThat(persistedList.get().getCurrency()).isEqualTo(JPY); @@ -190,12 +180,6 @@ public class PremiumListDaoTest { }); } - @Test - void getPremiumPrice_returnsNoneWhenNoPremiumListConfigured() { - persistResource(newRegistry("foobar", "FOOBAR").asBuilder().setPremiumList(null).build()); - assertThat(PremiumListDao.getPremiumPrice("rich", Registry.get("foobar"))).isEmpty(); - } - @Test void getPremiumPrice_worksSuccessfully() { persistResource( @@ -207,28 +191,18 @@ public class PremiumListDaoTest { google.registry.model.registry.label.PremiumList.class, "premlist")) .build()); - PremiumListDao.saveNew( + PremiumListSqlDao.save( new PremiumList.Builder() .setName("premlist") .setCurrency(USD) .setLabelsToPrices(testPrices) .setCreationTime(fakeClock.nowUtc()) .build()); - assertThat(PremiumListDao.getPremiumPrice("silver", Registry.get("foobar"))) + assertThat(PremiumListSqlDao.getPremiumPrice("premlist", "silver")) .hasValue(Money.of(USD, 10.23)); - assertThat(PremiumListDao.getPremiumPrice("gold", Registry.get("foobar"))) + assertThat(PremiumListSqlDao.getPremiumPrice("premlist", "gold")) .hasValue(Money.of(USD, 1305.47)); - assertThat(PremiumListDao.getPremiumPrice("zirconium", Registry.get("foobar"))).isEmpty(); - } - - @Test - void testGetPremiumPrice_throwsWhenPremiumListCantBeLoaded() { - createTld("tld"); - IllegalStateException thrown = - assertThrows( - IllegalStateException.class, - () -> PremiumListDao.getPremiumPrice("foobar", Registry.get("tld"))); - assertThat(thrown).hasMessageThat().isEqualTo("Could not load premium list 'tld'"); + assertThat(PremiumListSqlDao.getPremiumPrice("premlist", "zirconium")).isEmpty(); } @Test @@ -242,7 +216,7 @@ public class PremiumListDaoTest { google.registry.model.registry.label.PremiumList.class, "premlist")) .build()); - PremiumListDao.saveNew( + PremiumListSqlDao.save( new PremiumList.Builder() .setName("premlist") .setCurrency(JPY) @@ -256,14 +230,22 @@ public class PremiumListDaoTest { BigDecimal.valueOf(15000))) .setCreationTime(fakeClock.nowUtc()) .build()); - assertThat(PremiumListDao.getPremiumPrice("silver", Registry.get("foobar"))) - .hasValue(moneyOf(JPY, 10)); - assertThat(PremiumListDao.getPremiumPrice("gold", Registry.get("foobar"))) - .hasValue(moneyOf(JPY, 1000)); - assertThat(PremiumListDao.getPremiumPrice("palladium", Registry.get("foobar"))) + assertThat(PremiumListSqlDao.getPremiumPrice("premlist", "silver")).hasValue(moneyOf(JPY, 10)); + assertThat(PremiumListSqlDao.getPremiumPrice("premlist", "gold")).hasValue(moneyOf(JPY, 1000)); + assertThat(PremiumListSqlDao.getPremiumPrice("premlist", "palladium")) .hasValue(moneyOf(JPY, 15000)); } + @Test + void test_savePremiumList_clearsCache() { + assertThat(PremiumListSqlDao.premiumListCache.getIfPresent("testname")).isNull(); + PremiumListSqlDao.save(testList); + PremiumList pl = PremiumListSqlDao.getLatestRevision("testname").get(); + assertThat(PremiumListSqlDao.premiumListCache.getIfPresent("testname").get()).isEqualTo(pl); + transactIfJpaTm(() -> PremiumListSqlDao.save("testname", ImmutableList.of("test,USD 1"))); + assertThat(PremiumListSqlDao.premiumListCache.getIfPresent("testname")).isNull(); + } + private static Money moneyOf(CurrencyUnit unit, double amount) { return Money.of(unit, BigDecimal.valueOf(amount).setScale(unit.getDecimalPlaces())); } diff --git a/core/src/test/java/google/registry/schema/tld/PremiumListUtilsTest.java b/core/src/test/java/google/registry/schema/tld/PremiumListUtilsTest.java index b0c6dbfe4..0eb282de4 100644 --- a/core/src/test/java/google/registry/schema/tld/PremiumListUtilsTest.java +++ b/core/src/test/java/google/registry/schema/tld/PremiumListUtilsTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static google.registry.schema.tld.PremiumListUtils.parseToPremiumList; import static org.junit.jupiter.api.Assertions.assertThrows; +import com.google.common.collect.ImmutableList; import google.registry.model.registry.label.PremiumList; import google.registry.testing.AppEngineExtension; import java.math.BigDecimal; @@ -34,7 +35,8 @@ class PremiumListUtilsTest { @Test void parseInputToPremiumList_works() { PremiumList premiumList = - parseToPremiumList("testlist", "foo,USD 99.50\n" + "bar,USD 30\n" + "baz,USD 10\n"); + parseToPremiumList( + "testlist", ImmutableList.of("foo,USD 99.50", "bar,USD 30", "baz,USD 10")); assertThat(premiumList.getName()).isEqualTo("testlist"); assertThat(premiumList.getLabelsToPrices()) .containsExactly("foo", twoDigits(99.50), "bar", twoDigits(30), "baz", twoDigits(10)); @@ -47,7 +49,7 @@ class PremiumListUtilsTest { IllegalArgumentException.class, () -> parseToPremiumList( - "testlist", "foo,USD 99.50\n" + "bar,USD 30\n" + "baz,JPY 990\n")); + "testlist", ImmutableList.of("foo,USD 99.50", "bar,USD 30", "baz,JPY 990"))); assertThat(thrown) .hasMessageThat() .isEqualTo("The Cloud SQL schema requires exactly one currency, but got: [JPY, USD]"); diff --git a/core/src/test/java/google/registry/testing/DatabaseHelper.java b/core/src/test/java/google/registry/testing/DatabaseHelper.java index b3d177900..fd78267d6 100644 --- a/core/src/test/java/google/registry/testing/DatabaseHelper.java +++ b/core/src/test/java/google/registry/testing/DatabaseHelper.java @@ -32,7 +32,9 @@ import static google.registry.model.ImmutableObjectSubject.immutableObjectCorres import static google.registry.model.ResourceTransferUtils.createTransferResponse; import static google.registry.model.ofy.ObjectifyService.ofy; import static google.registry.model.registry.Registry.TldState.GENERAL_AVAILABILITY; -import static google.registry.model.registry.label.PremiumListUtils.parentPremiumListEntriesOnRevision; +import static google.registry.model.registry.label.PremiumListDatastoreDao.parentPremiumListEntriesOnRevision; +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.persistence.transaction.TransactionManagerUtil.ofyTmOrDoNothing; import static google.registry.persistence.transaction.TransactionManagerUtil.transactIfJpaTm; @@ -57,6 +59,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Iterables; +import com.google.common.collect.Streams; import com.google.common.net.InetAddresses; import com.googlecode.objectify.Key; import com.googlecode.objectify.cmd.Saver; @@ -100,6 +103,7 @@ import google.registry.model.registry.Registry.TldType; import google.registry.model.registry.label.PremiumList; import google.registry.model.registry.label.PremiumList.PremiumListEntry; import google.registry.model.registry.label.PremiumList.PremiumListRevision; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.model.registry.label.ReservedList; import google.registry.model.reporting.HistoryEntry; import google.registry.model.transfer.ContactTransferData; @@ -108,11 +112,11 @@ import google.registry.model.transfer.TransferData; import google.registry.model.transfer.TransferStatus; import google.registry.persistence.VKey; import google.registry.tmch.LordnTaskUtils; -import java.util.Arrays; import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; import javax.annotation.Nullable; import org.joda.money.CurrencyUnit; import org.joda.money.Money; @@ -387,25 +391,23 @@ public class DatabaseHelper { .build(); PremiumListRevision revision = PremiumListRevision.create(premiumList, entries.keySet()); - if (tm().isOfy()) { - ImmutableList premiumLists = - ImmutableList.of( - premiumList.asBuilder().setRevision(Key.create(revision)).build(), revision); - ImmutableSet entriesOnRevision = - parentPremiumListEntriesOnRevision(entries.values(), Key.create(revision)); - if (alwaysSaveWithBackup) { - tm().transact( - () -> { - tm().putAll(premiumLists); - tm().putAll(entriesOnRevision); - }); - } else { - tm().putAllWithoutBackup(premiumLists); - tm().putAllWithoutBackup(entriesOnRevision); - } + ImmutableList premiumListOfyObjects = + ImmutableList.of( + premiumList.asBuilder().setRevision(Key.create(revision)).build(), revision); + ImmutableSet entriesOnRevision = + parentPremiumListEntriesOnRevision(entries.values(), Key.create(revision)); + if (alwaysSaveWithBackup) { + ofyTm() + .transact( + () -> { + ofyTm().putAll(premiumListOfyObjects); + ofyTm().putAll(entriesOnRevision); + }); } else { - tm().transact(() -> tm().insert(premiumList)); + ofyTm().putAllWithoutBackup(premiumListOfyObjects); + ofyTm().putAllWithoutBackup(entriesOnRevision); } + jpaTm().transact(() -> jpaTm().insert(premiumList)); maybeAdvanceClock(); // The above premiumList is in the session cache and it is different from the corresponding // entity stored in Datastore because it has some @Ignore fields set dedicated for SQL. This @@ -807,7 +809,7 @@ public class DatabaseHelper { /** Assert that the expected billing events are exactly the ones found in the fake Datastore. */ public static void assertBillingEvents(BillingEvent... expected) { - assertBillingEventsEqual(getBillingEvents(), Arrays.asList(expected)); + assertBillingEventsEqual(getBillingEvents(), asList(expected)); } /** Assert that the expected billing events set is exactly the one found in the fake Datastore. */ @@ -820,7 +822,7 @@ public class DatabaseHelper { */ public static void assertBillingEventsForResource( EppResource resource, BillingEvent... expected) { - assertBillingEventsEqual(getBillingEvents(resource), Arrays.asList(expected)); + assertBillingEventsEqual(getBillingEvents(resource), asList(expected)); } /** Assert that there are no billing events. */ @@ -851,15 +853,15 @@ public class DatabaseHelper { } public static void assertPollMessages(String clientId, PollMessage... expected) { - assertPollMessagesEqual(getPollMessages(clientId), Arrays.asList(expected)); + assertPollMessagesEqual(getPollMessages(clientId), asList(expected)); } public static void assertPollMessages(PollMessage... expected) { - assertPollMessagesEqual(getPollMessages(), Arrays.asList(expected)); + assertPollMessagesEqual(getPollMessages(), asList(expected)); } public static void assertPollMessagesForResource(DomainContent domain, PollMessage... expected) { - assertPollMessagesEqual(getPollMessages(domain), Arrays.asList(expected)); + assertPollMessagesEqual(getPollMessages(domain), asList(expected)); } public static ImmutableList getPollMessages() { @@ -1232,19 +1234,8 @@ public class DatabaseHelper { /** Returns the entire map of {@link PremiumListEntry}s for the given {@link PremiumList}. */ public static ImmutableMap loadPremiumListEntries( PremiumList premiumList) { - try { - ImmutableMap.Builder entriesMap = new ImmutableMap.Builder<>(); - if (premiumList.getRevisionKey() != null) { - for (PremiumListEntry entry : - ofy().load().type(PremiumListEntry.class).ancestor(premiumList.getRevisionKey())) { - entriesMap.put(entry.getLabel(), entry); - } - } - return entriesMap.build(); - } catch (Exception e) { - throw new RuntimeException( - "Could not retrieve entries for premium list " + premiumList.getName(), e); - } + return Streams.stream(PremiumListDualDao.loadAllPremiumListEntries(premiumList.getName())) + .collect(toImmutableMap(PremiumListEntry::getLabel, Function.identity())); } /** Loads and returns the registrar with the given client ID, or throws IAE if not present. */ diff --git a/core/src/test/java/google/registry/testing/TestCacheExtension.java b/core/src/test/java/google/registry/testing/TestCacheExtension.java index 0bd776ff6..020c190a9 100644 --- a/core/src/test/java/google/registry/testing/TestCacheExtension.java +++ b/core/src/test/java/google/registry/testing/TestCacheExtension.java @@ -18,7 +18,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import google.registry.model.EppResource; import google.registry.model.index.ForeignKeyIndex; -import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDatastoreDao; +import google.registry.schema.tld.PremiumListSqlDao; import java.util.Map; import java.util.Optional; import org.joda.time.Duration; @@ -70,15 +71,18 @@ public class TestCacheExtension implements BeforeEachCallback, AfterEachCallback public Builder withPremiumListsCache(Duration expiry) { cacheHandlerMap.put( - "PremiumList.cachePremiumLists", - new TestCacheHandler(PremiumList::setPremiumListCacheForTest, expiry)); + "PremiumListSqlDao.premiumListCache", + new TestCacheHandler(PremiumListSqlDao::setPremiumListCacheForTest, expiry)); + cacheHandlerMap.put( + "PremiumListDatastoreDao.premiumListCache", + new TestCacheHandler(PremiumListDatastoreDao::setPremiumListCacheForTest, expiry)); return this; } public Builder withPremiumListEntriesCache(Duration expiry) { cacheHandlerMap.put( "PremiumList.cachePremiumListEntries", - new TestCacheHandler(PremiumList::setPremiumListEntriesCacheForTest, expiry)); + new TestCacheHandler(PremiumListDatastoreDao::setPremiumListEntriesCacheForTest, expiry)); return this; } diff --git a/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java b/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java index b28405804..07da40141 100644 --- a/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java @@ -22,7 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.beust.jcommander.ParameterException; import google.registry.model.registry.Registry; -import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.testing.DeterministicStringGenerator; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -101,7 +101,7 @@ class CreateDomainCommandTest extends EppToolCommandTestCase runCommandForced("--name=" + premiumList.getName())); - assertThat(PremiumList.getUncached(premiumList.getName())).isPresent(); + assertThat(PremiumListDualDao.getLatestRevision(premiumList.getName())).isPresent(); assertThat(thrown) .hasMessageThat() .isEqualTo("Cannot delete premium list because it is used on these tld(s): xn--q9jyb4c"); diff --git a/core/src/test/java/google/registry/tools/server/CreatePremiumListActionTest.java b/core/src/test/java/google/registry/tools/server/CreatePremiumListActionTest.java index a8268e74c..89786c20c 100644 --- a/core/src/test/java/google/registry/tools/server/CreatePremiumListActionTest.java +++ b/core/src/test/java/google/registry/tools/server/CreatePremiumListActionTest.java @@ -16,14 +16,13 @@ package google.registry.tools.server; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; -import static google.registry.model.registry.label.PremiumListUtils.deletePremiumList; -import static google.registry.model.registry.label.PremiumListUtils.getPremiumPrice; import static google.registry.testing.DatabaseHelper.createTlds; import static google.registry.testing.DatabaseHelper.loadPremiumListEntries; import static javax.servlet.http.HttpServletResponse.SC_OK; import google.registry.model.registry.Registry; import google.registry.model.registry.label.PremiumList; +import google.registry.model.registry.label.PremiumListDualDao; import google.registry.testing.AppEngineExtension; import google.registry.testing.FakeJsonResponse; import org.joda.money.Money; @@ -46,7 +45,7 @@ public class CreatePremiumListActionTest { @BeforeEach void beforeEach() { createTlds("foo", "xn--q9jyb4c", "how"); - deletePremiumList(PremiumList.getUncached("foo").get()); + PremiumListDualDao.delete(PremiumListDualDao.getLatestRevision("foo").get()); action = new CreatePremiumListAction(); response = new FakeJsonResponse(); action.response = response; @@ -78,7 +77,8 @@ public class CreatePremiumListActionTest { action.override = true; action.run(); assertThat(response.getStatus()).isEqualTo(SC_OK); - assertThat(loadPremiumListEntries(PremiumList.getUncached("zanzibar").get())).hasSize(1); + assertThat(loadPremiumListEntries(PremiumListDualDao.getLatestRevision("zanzibar").get())) + .hasSize(1); } @Test @@ -87,8 +87,10 @@ public class CreatePremiumListActionTest { action.inputData = "rich,USD 25\nricher,USD 1000\n"; action.run(); assertThat(response.getStatus()).isEqualTo(SC_OK); - assertThat(loadPremiumListEntries(PremiumList.getUncached("foo").get())).hasSize(2); - assertThat(getPremiumPrice("rich", Registry.get("foo"))).hasValue(Money.parse("USD 25")); - assertThat(getPremiumPrice("diamond", Registry.get("foo"))).isEmpty(); + PremiumList premiumList = PremiumListDualDao.getLatestRevision("foo").get(); + assertThat(loadPremiumListEntries(premiumList)).hasSize(2); + assertThat(PremiumListDualDao.getPremiumPrice("rich", Registry.get("foo"))) + .hasValue(Money.parse("USD 25")); + assertThat(PremiumListDualDao.getPremiumPrice("diamond", Registry.get("foo"))).isEmpty(); } } diff --git a/core/src/test/java/google/registry/tools/server/UpdatePremiumListActionTest.java b/core/src/test/java/google/registry/tools/server/UpdatePremiumListActionTest.java index a745d7328..11f3f9130 100644 --- a/core/src/test/java/google/registry/tools/server/UpdatePremiumListActionTest.java +++ b/core/src/test/java/google/registry/tools/server/UpdatePremiumListActionTest.java @@ -15,22 +15,23 @@ package google.registry.tools.server; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth8.assertThat; -import static google.registry.model.registry.label.PremiumListUtils.getPremiumPrice; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; -import static google.registry.schema.tld.PremiumListUtils.parseToPremiumList; import static google.registry.testing.DatabaseHelper.createTlds; import static google.registry.testing.DatabaseHelper.loadPremiumListEntries; import static google.registry.util.ResourceUtils.readResourceUtf8; import static javax.servlet.http.HttpServletResponse.SC_OK; +import com.google.common.base.Splitter; +import com.google.common.truth.Truth8; import google.registry.model.registry.Registry; import google.registry.model.registry.label.PremiumList; -import google.registry.schema.tld.PremiumListDao; +import google.registry.model.registry.label.PremiumListDualDao; +import google.registry.schema.tld.PremiumListSqlDao; import google.registry.testing.AppEngineExtension; import google.registry.testing.DatabaseHelper; import google.registry.testing.FakeJsonResponse; import java.math.BigDecimal; +import java.util.List; import org.joda.money.Money; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -75,24 +76,31 @@ class UpdatePremiumListActionTest { @Test void test_success() { - PremiumListDao.saveNew( - parseToPremiumList( - "foo", readResourceUtf8(DatabaseHelper.class, "default_premium_list_testdata.csv"))); + List inputLines = + Splitter.on('\n') + .omitEmptyStrings() + .splitToList( + readResourceUtf8(DatabaseHelper.class, "default_premium_list_testdata.csv")); + PremiumListDualDao.save("foo", inputLines); action.name = "foo"; action.inputData = "rich,USD 75\nricher,USD 5000\npoor, USD 0.99"; action.run(); assertThat(response.getStatus()).isEqualTo(SC_OK); Registry registry = Registry.get("foo"); - assertThat(loadPremiumListEntries(PremiumList.getUncached("foo").get())).hasSize(3); - assertThat(getPremiumPrice("rich", registry)).hasValue(Money.parse("USD 75")); - assertThat(getPremiumPrice("richer", registry)).hasValue(Money.parse("USD 5000")); - assertThat(getPremiumPrice("poor", registry)).hasValue(Money.parse("USD 0.99")); - assertThat(getPremiumPrice("diamond", registry)).isEmpty(); + assertThat(loadPremiumListEntries(PremiumListDualDao.getLatestRevision("foo").get())) + .hasSize(3); + Truth8.assertThat(PremiumListDualDao.getPremiumPrice("rich", registry)) + .hasValue(Money.parse("USD 75")); + Truth8.assertThat(PremiumListDualDao.getPremiumPrice("richer", registry)) + .hasValue(Money.parse("USD 5000")); + Truth8.assertThat(PremiumListDualDao.getPremiumPrice("poor", registry)) + .hasValue(Money.parse("USD 0.99")); + Truth8.assertThat(PremiumListDualDao.getPremiumPrice("diamond", registry)).isEmpty(); jpaTm() .transact( () -> { - PremiumList persistedList = PremiumListDao.getLatestRevision("foo").get(); + PremiumList persistedList = PremiumListSqlDao.getLatestRevision("foo").get(); assertThat(persistedList.getLabelsToPrices()) .containsEntry("rich", new BigDecimal("75.00")); assertThat(persistedList.getLabelsToPrices())