diff --git a/java/google/registry/model/registry/label/PremiumList.java b/java/google/registry/model/registry/label/PremiumList.java index 2efccfe8f..9fdc7d7c4 100644 --- a/java/google/registry/model/registry/label/PremiumList.java +++ b/java/google/registry/model/registry/label/PremiumList.java @@ -39,7 +39,6 @@ import com.googlecode.objectify.annotation.Cache; import com.googlecode.objectify.annotation.Entity; import com.googlecode.objectify.annotation.Id; import com.googlecode.objectify.annotation.Parent; -import com.googlecode.objectify.cmd.Query; import google.registry.model.Buildable; import google.registry.model.ImmutableObject; import google.registry.model.annotations.ReportedOn; @@ -287,10 +286,6 @@ public final class PremiumList extends BaseDomainLabelList queryEntriesForCurrentRevision() { - return ofy().load().type(PremiumListEntry.class).ancestor(revisionKey); - } - @Override public Builder asBuilder() { return new Builder(clone(this)); diff --git a/java/google/registry/model/registry/label/PremiumListUtils.java b/java/google/registry/model/registry/label/PremiumListUtils.java index 2f3aceed9..0cfd0fc9f 100644 --- a/java/google/registry/model/registry/label/PremiumListUtils.java +++ b/java/google/registry/model/registry/label/PremiumListUtils.java @@ -14,7 +14,6 @@ package google.registry.model.registry.label; -import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.Iterables.partition; import static google.registry.model.common.EntityGroupRoot.getCrossTldKey; @@ -37,7 +36,6 @@ import google.registry.model.registry.Registry; import google.registry.model.registry.label.PremiumList.PremiumListEntry; import google.registry.model.registry.label.PremiumList.PremiumListRevision; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.concurrent.ExecutionException; import org.joda.money.Money; @@ -107,9 +105,7 @@ public final class PremiumListUtils { PremiumListRevision.create(premiumList, premiumListEntries.keySet()); final Key newRevisionKey = Key.create(newRevision); ImmutableSet parentedEntries = - parentPremiumListEntriesOnRevision( - firstNonNull(premiumListEntries.values(), ImmutableSet.of()), - newRevisionKey); + parentPremiumListEntriesOnRevision(premiumListEntries.values(), newRevisionKey); // Save the new child entities in a series of transactions. for (final List batch : @@ -159,6 +155,7 @@ public final class PremiumListUtils { } /** Re-parents the given {@link PremiumListEntry}s on the given {@link PremiumListRevision}. */ + @VisibleForTesting public static ImmutableSet parentPremiumListEntriesOnRevision( Iterable entries, final Key revisionKey) { return FluentIterable.from(entries) @@ -188,7 +185,7 @@ public final class PremiumListUtils { return; } for (final List> batch : partition( - premiumList.queryEntriesForCurrentRevision().keys(), + ofy().load().type(PremiumListEntry.class).ancestor(premiumList.revisionKey).keys(), TRANSACTION_BATCH_SIZE)) { ofy().transactNew(new VoidWork() { @Override @@ -208,28 +205,5 @@ public final class PremiumListUtils { return ofy().load().key(Key.create(getCrossTldKey(), PremiumList.class, name)).now() != null; } - /** - * Loads and returns the entire premium list map. - * - *

This load operation is quite expensive for large premium lists because each premium list - * entry is a separate Datastore entity, and loading them this way bypasses the in-memory caches. - * Do not use this method if all you need to do is check the price of a small number of labels! - */ - @VisibleForTesting - public static Map loadPremiumListEntries(PremiumList premiumList) { - try { - ImmutableMap.Builder entriesMap = new ImmutableMap.Builder<>(); - if (premiumList.getRevisionKey() != null) { - for (PremiumListEntry entry : premiumList.queryEntriesForCurrentRevision()) { - entriesMap.put(entry.getLabel(), entry); - } - } - return entriesMap.build(); - } catch (Exception e) { - throw new RuntimeException( - "Could not retrieve entries for premium list " + premiumList.getName(), e); - } - } - private PremiumListUtils() {} } diff --git a/javatests/google/registry/model/registry/label/PremiumListUtilsTest.java b/javatests/google/registry/model/registry/label/PremiumListUtilsTest.java index 4b3728d12..962254f3c 100644 --- a/javatests/google/registry/model/registry/label/PremiumListUtilsTest.java +++ b/javatests/google/registry/model/registry/label/PremiumListUtilsTest.java @@ -20,13 +20,14 @@ import static google.registry.model.registry.label.PremiumList.cachePremiumListE 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.loadPremiumListEntries; import static google.registry.model.registry.label.PremiumListUtils.savePremiumListAndEntries; import static google.registry.testing.DatastoreHelper.createTld; +import static google.registry.testing.DatastoreHelper.loadPremiumListEntries; import static google.registry.testing.DatastoreHelper.persistPremiumList; import static google.registry.testing.DatastoreHelper.persistResource; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.googlecode.objectify.Key; import google.registry.model.pricing.StaticPremiumListPricingEngine; import google.registry.model.registry.Registry; @@ -187,7 +188,8 @@ public class PremiumListUtilsTest { 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"))).isAbsent(); - Map entries = loadPremiumListEntries(PremiumList.get("tld2").get()); + ImmutableMap entries = + loadPremiumListEntries(PremiumList.get("tld2").get()); assertThat(entries.keySet()).containsExactly("lol"); assertThat(entries).doesNotContainKey("lol "); PremiumListEntry entry = entries.get("lol"); @@ -201,7 +203,7 @@ public class PremiumListUtilsTest { PremiumList pl = savePremiumListAndEntries( new PremiumList.Builder().setName("pl").build(), ImmutableList.of("test,USD 1")); - Map entries = loadPremiumListEntries(pl); + ImmutableMap entries = loadPremiumListEntries(pl); assertThat(entries.keySet()).containsExactly("test"); assertThat(loadPremiumListEntries(PremiumList.get("pl").get())).isEqualTo(entries); // Save again with no changes, and clear the cache to force a re-load from Datastore. diff --git a/javatests/google/registry/testing/DatastoreHelper.java b/javatests/google/registry/testing/DatastoreHelper.java index 6e8a0604f..037ff46cd 100644 --- a/javatests/google/registry/testing/DatastoreHelper.java +++ b/javatests/google/registry/testing/DatastoreHelper.java @@ -987,5 +987,23 @@ public class DatastoreHelper { }}); } + /** 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); + } + } + private DatastoreHelper() {} } diff --git a/javatests/google/registry/tools/DeletePremiumListCommandTest.java b/javatests/google/registry/tools/DeletePremiumListCommandTest.java index f4c191b47..7b85586d8 100644 --- a/javatests/google/registry/tools/DeletePremiumListCommandTest.java +++ b/javatests/google/registry/tools/DeletePremiumListCommandTest.java @@ -17,8 +17,8 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static google.registry.model.ofy.ObjectifyService.ofy; -import static google.registry.model.registry.label.PremiumListUtils.loadPremiumListEntries; import static google.registry.testing.DatastoreHelper.createTld; +import static google.registry.testing.DatastoreHelper.loadPremiumListEntries; import static google.registry.testing.DatastoreHelper.persistPremiumList; import static google.registry.testing.DatastoreHelper.persistResource; diff --git a/javatests/google/registry/tools/server/CreatePremiumListActionTest.java b/javatests/google/registry/tools/server/CreatePremiumListActionTest.java index fc84f34ce..4ac24ee90 100644 --- a/javatests/google/registry/tools/server/CreatePremiumListActionTest.java +++ b/javatests/google/registry/tools/server/CreatePremiumListActionTest.java @@ -17,8 +17,8 @@ package google.registry.tools.server; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.registry.label.PremiumListUtils.deletePremiumList; import static google.registry.model.registry.label.PremiumListUtils.getPremiumPrice; -import static google.registry.model.registry.label.PremiumListUtils.loadPremiumListEntries; import static google.registry.testing.DatastoreHelper.createTlds; +import static google.registry.testing.DatastoreHelper.loadPremiumListEntries; import static javax.servlet.http.HttpServletResponse.SC_OK; import google.registry.model.registry.Registry; diff --git a/javatests/google/registry/tools/server/UpdatePremiumListActionTest.java b/javatests/google/registry/tools/server/UpdatePremiumListActionTest.java index b9007fe9b..a4fcca299 100644 --- a/javatests/google/registry/tools/server/UpdatePremiumListActionTest.java +++ b/javatests/google/registry/tools/server/UpdatePremiumListActionTest.java @@ -16,8 +16,8 @@ package google.registry.tools.server; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.registry.label.PremiumListUtils.getPremiumPrice; -import static google.registry.model.registry.label.PremiumListUtils.loadPremiumListEntries; import static google.registry.testing.DatastoreHelper.createTlds; +import static google.registry.testing.DatastoreHelper.loadPremiumListEntries; import static javax.servlet.http.HttpServletResponse.SC_OK; import google.registry.model.registry.Registry;