Read from bloom filter for premium pricing checks

This also cleans up the PremiumList API so that it only has one
method for checking premium prices, which is by TLD, rather than two.

I will be refactoring a lot of the static methods currently residing in
the PremiumList class into a separate utils class, but I don't want to
include too many changes in this one CL.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=148475345

java/google/registry/config/RegistryConfig.java

@@ -1135,6 +1135,13 @@ public final class RegistryConfig {
     return Duration.standardSeconds(CONFIG_SETTINGS.get().caching.singletonCachePersistSeconds);
   }
 
+  /**
+   * Returns the maximum number of premium list entries across all TLDs to keep in in-memory cache.
+   */
+  public static int getStaticPremiumListMaxCachedEntries() {
+    return CONFIG_SETTINGS.get().caching.staticPremiumListMaxCachedEntries;
+  }
+
   /** Returns the email address that outgoing emails from the app are sent from. */
   public static String getGSuiteOutgoingEmailAddress() {
     return CONFIG_SETTINGS.get().gSuite.outgoingEmailAddress;

java/google/registry/config/RegistryConfigSettings.java

@@ -91,6 +91,7 @@ public class RegistryConfigSettings {
     public int singletonCacheRefreshSeconds;
     public int domainLabelCachingSeconds;
     public int singletonCachePersistSeconds;
+    public int staticPremiumListMaxCachedEntries;
   }
 
   /** Configuration for Registry Data Escrow (RDE). */

java/google/registry/config/files/default-config.yaml

@@ -115,6 +115,14 @@ caching:
   # Length of time that a long-lived singleton in persist mode should be cached.
   singletonCachePersistSeconds: 31557600 # This is one year.
 
+  # Maximum total number of static premium list entry entities to cache in
+  # memory, across all premium lists for all TLDs. Tuning this up will use more
+  # memory (and might require using larger App Engine instances). Note that
+  # premium list entries that are absent are cached in addition to ones that are
+  # present, so the total cache size is not bounded by the total number of
+  # premium price entries that exist.
+  staticPremiumListMaxCachedEntries: 200000
+
 rde:
   # URL prefix of ICANN's server to upload RDE reports to. Nomulus adds /TLD/ID
   # to the end of this to construct the full URL.

java/google/registry/config/files/nomulus-config-unittest.yaml

@@ -17,6 +17,7 @@ caching:
   singletonCacheRefreshSeconds: 0
   domainLabelCachingSeconds: 0
   singletonCachePersistSeconds: 0
+  staticPremiumListMaxCachedEntries: 50
 
 braintree:
   merchantAccountIdsMap:

java/google/registry/model/pricing/StaticPremiumListPricingEngine.java

@@ -15,9 +15,9 @@
 package google.registry.model.pricing;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.emptyToNull;
 import static google.registry.model.registry.Registry.TldState.SUNRISE;
+import static google.registry.model.registry.label.PremiumList.getPremiumPrice;
 import static google.registry.model.registry.label.ReservationType.NAME_COLLISION;
 import static google.registry.model.registry.label.ReservedList.getReservation;
 import static google.registry.util.DomainNameUtils.getTldFromDomainName;
@@ -26,7 +26,6 @@ import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
 import com.google.common.net.InternetDomainName;
 import google.registry.model.registry.Registry;
-import google.registry.model.registry.label.PremiumList;
 import javax.inject.Inject;
 import org.joda.money.Money;
 import org.joda.time.DateTime;
@@ -44,13 +43,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<Money> premiumPrice = Optional.<Money>absent();
+    Optional<Money> premiumPrice = getPremiumPrice(label, registry);
-    if (registry.getPremiumList() != null) {
-      String listName = registry.getPremiumList().getName();
-      Optional<PremiumList> premiumList = PremiumList.get(listName);
-      checkState(premiumList.isPresent(), "Could not load premium list: %s", listName);
-      premiumPrice = premiumList.get().getPremiumPrice(label);
-    }
     boolean isNameCollisionInSunrise =
         registry.getTldState(priceTime).equals(SUNRISE)
             && getReservation(label, tld) == NAME_COLLISION;

java/google/registry/model/registry/Registry.java

@@ -484,6 +484,7 @@ public class Registry extends ImmutableObject implements Buildable {
     return anchorTenantAddGracePeriodLength;
   }
 
+  @Nullable
   public Key<PremiumList> getPremiumList() {
     return premiumList;
   }

java/google/registry/model/registry/label/BaseDomainLabelList.java

@@ -19,7 +19,6 @@ import static com.google.common.base.Strings.isNullOrEmpty;
 import static google.registry.model.common.EntityGroupRoot.getCrossTldKey;
 import static google.registry.model.registry.Registries.getTlds;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.cache.CacheLoader.InvalidCacheLoadException;
 import com.google.common.cache.LoadingCache;
@@ -83,8 +82,7 @@ public abstract class BaseDomainLabelList<T extends Comparable<?>, R extends Dom
    *
    * @param lines the CSV file, line by line
    */
-  @VisibleForTesting
+  public ImmutableMap<String, R> parse(Iterable<String> lines) {
-  protected ImmutableMap<String, R> parse(Iterable<String> lines) {
     Map<String, R> labelsToEntries = new HashMap<>();
     Multiset<String> duplicateLabels = HashMultiset.create();
     for (String line : lines) {

java/google/registry/model/registry/label/PremiumList.java

@@ -13,23 +13,20 @@
 // limitations under the License.
 
 package google.registry.model.registry.label;
-
+import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.appengine.api.datastore.DatastoreServiceFactory.getDatastoreService;
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.Iterables.partition;
 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.model.ofy.Ofy.RECOMMENDED_MEMCACHE_EXPIRATION;
-import static google.registry.util.CollectionUtils.nullToEmpty;
-import static google.registry.util.CollectionUtils.nullToEmptyImmutableCopy;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
-import com.google.appengine.api.datastore.EntityNotFoundException;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
@@ -38,8 +35,9 @@ 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.FluentIterable;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.hash.BloomFilter;
 import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.googlecode.objectify.Key;
@@ -48,8 +46,6 @@ import com.googlecode.objectify.Work;
 import com.googlecode.objectify.annotation.Cache;
 import com.googlecode.objectify.annotation.Entity;
 import com.googlecode.objectify.annotation.Id;
-import com.googlecode.objectify.annotation.Ignore;
-import com.googlecode.objectify.annotation.OnLoad;
 import com.googlecode.objectify.annotation.Parent;
 import com.googlecode.objectify.cmd.Query;
 import google.registry.model.Buildable;
@@ -81,13 +77,6 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
   /** Stores the revision key for the set of currently used premium list entry entities. */
   Key<PremiumListRevision> revisionKey;
 
-  /** The revision to be saved along with this entity. */
-  @Ignore
-  PremiumListRevision revision;
-
-  @Ignore
-  Map<String, PremiumListEntry> premiumListMap;
-
   /** Virtual parent entity for premium list entry entities associated with a single revision. */
   @ReportedOn
   @Entity
@@ -120,7 +109,8 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
     private static final int MAX_BLOOM_FILTER_BYTES = 900000;
 
     /** Returns a new PremiumListRevision for the given key and premium list map. */
-    static PremiumListRevision create(PremiumList parent, Set<String> premiumLabels) {
+    @VisibleForTesting
+    public static PremiumListRevision create(PremiumList parent, Set<String> premiumLabels) {
       PremiumListRevision revision = new PremiumListRevision();
       revision.parent = Key.create(parent);
       revision.revisionId = allocateId();
@@ -143,7 +133,13 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
     }
   }
 
-  private static LoadingCache<String, PremiumList> cache =
+  /**
+   * In-memory cache for premium lists.
+   *
+   * <p>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.
+   */
+  private static final LoadingCache<String, PremiumList> cachePremiumLists =
       CacheBuilder.newBuilder()
           .expireAfterWrite(getDomainLabelListCacheDuration().getMillis(), MILLISECONDS)
           .build(new CacheLoader<String, PremiumList>() {
@@ -161,53 +157,122 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
             }});
 
   /**
-   * Gets the premium price for the specified label on the specified tld, or returns Optional.absent
+   * In-memory cache for {@link PremiumListRevision}s, used for retrieving bloom filters quickly.
-   * if there is no premium price.
+   *
+   * <p>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.
    */
-  public static Optional<Money> getPremiumPrice(String label, String tld) {
+  private static final LoadingCache<Key<PremiumListRevision>, PremiumListRevision>
-    Registry registry = Registry.get(checkNotNull(tld, "tld"));
+      cachePremiumListRevisions =
+          CacheBuilder.newBuilder()
+              .expireAfterWrite(getSingletonCachePersistDuration().getMillis(), MILLISECONDS)
+              .build(
+                  new CacheLoader<Key<PremiumListRevision>, PremiumListRevision>() {
+                    @Override
+                    public PremiumListRevision load(final Key<PremiumListRevision> revisionKey) {
+                      return ofy()
+                          .doTransactionless(
+                              new Work<PremiumListRevision>() {
+                                @Override
+                                public PremiumListRevision run() {
+                                  return ofy().load().key(revisionKey).now();
+                                }});
+                    }});
+
+  /**
+   * In-memory cache for {@link PremiumListEntry}s for a given label and {@link PremiumListRevision}
+   *
+   * <p>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.
+   *
+   * <p>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.
+   *
+   * <p>A maximum size is set here on the cache because it can potentially grow too big to fit in
+   * memory if there are a very large number of premium list entries in the system. The least-
+   * accessed entries will be evicted first.
+   */
+  @VisibleForTesting
+  static final LoadingCache<Key<PremiumListEntry>, Optional<PremiumListEntry>>
+      cachePremiumListEntries =
+          CacheBuilder.newBuilder()
+              .expireAfterWrite(getSingletonCachePersistDuration().getMillis(), MILLISECONDS)
+              .maximumSize(getStaticPremiumListMaxCachedEntries())
+              .build(
+                  new CacheLoader<Key<PremiumListEntry>, Optional<PremiumListEntry>>() {
+                    @Override
+                    public Optional<PremiumListEntry> load(final Key<PremiumListEntry> entryKey) {
+                      return ofy()
+                          .doTransactionless(
+                              new Work<Optional<PremiumListEntry>>() {
+                                @Override
+                                public Optional<PremiumListEntry> run() {
+                                  return Optional.fromNullable(ofy().load().key(entryKey).now());
+                                }});
+                    }});
+
+  /**
+   * Returns the premium price for the specified label and registry, or absent if the label is not
+   * premium.
+   */
+  public static Optional<Money> getPremiumPrice(String label, Registry registry) {
+    // If the registry has no configured premium list, then no labels are premium.
     if (registry.getPremiumList() == null) {
       return Optional.<Money> absent();
     }
     String listName = registry.getPremiumList().getName();
-    Optional<PremiumList> premiumList = get(listName);
+    Optional<PremiumList> optionalPremiumList = get(listName);
-    if (!premiumList.isPresent()) {
+    checkState(optionalPremiumList.isPresent(), "Could not load premium list '%s'", listName);
-      throw new IllegalStateException("Could not load premium list named " + listName);
+    PremiumList premiumList = optionalPremiumList.get();
-    }
+    PremiumListRevision revision;
-    return premiumList.get().getPremiumPrice(label);
-  }
-
-  @OnLoad
-  private void onLoad() {
-    if (revisionKey != null) {
-      revision = ofy().load().key(revisionKey).now();
-    }
-
-    // TODO(b/32383610): Don't load up the premium list entries.
     try {
-      ImmutableMap.Builder<String, PremiumListEntry> entriesMap = new ImmutableMap.Builder<>();
+      revision = cachePremiumListRevisions.get(premiumList.getRevisionKey());
-      if (revisionKey != null) {
+    } catch (InvalidCacheLoadException | ExecutionException e) {
-        for (PremiumListEntry entry : loadEntriesForCurrentRevision()) {
+      throw new RuntimeException(
-          entriesMap.put(entry.getLabel(), entry);
+          "Could not load premium list revision " + premiumList.getRevisionKey(), e);
-        }
+    }
+    checkState(
+        revision.probablePremiumLabels != null,
+        "Probable premium labels bloom filter is null on revision '%s'",
+        premiumList.getRevisionKey());
+
+    if (revision.probablePremiumLabels.mightContain(label)) {
+      Key<PremiumListEntry> entryKey =
+          Key.create(premiumList.getRevisionKey(), PremiumListEntry.class, label);
+      try {
+        Optional<PremiumListEntry> entry = cachePremiumListEntries.get(entryKey);
+        return (entry.isPresent()) ? Optional.of(entry.get().getValue()) : Optional.<Money>absent();
+      } catch (InvalidCacheLoadException | ExecutionException e) {
+        throw new RuntimeException("Could not load premium list entry " + entryKey, e);
       }
-      premiumListMap = entriesMap.build();
+    } else {
-    } catch (Exception e) {
+      return Optional.<Money>absent();
-      throw new RuntimeException("Could not retrieve entries for premium list " + name, e);
     }
   }
 
   /**
-   * Gets the premium price for the specified label in the current PremiumList, or returns
+   * Loads and returns the entire premium list map.
-   * Optional.absent if there is no premium price.
+   *
+   * <p>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!
    */
-  public Optional<Money> getPremiumPrice(String label) {
+  @VisibleForTesting
-    return Optional.fromNullable(
+  public Map<String, PremiumListEntry> loadPremiumListEntries() {
-        premiumListMap.containsKey(label) ? premiumListMap.get(label).getValue() : null);
+    try {
-  }
+      ImmutableMap.Builder<String, PremiumListEntry> entriesMap = new ImmutableMap.Builder<>();
-
+      if (revisionKey != null) {
-  public Map<String, PremiumListEntry> getPremiumListEntries() {
+        for (PremiumListEntry entry : queryEntriesForCurrentRevision()) {
-    return nullToEmptyImmutableCopy(premiumListMap);
+          entriesMap.put(entry.getLabel(), entry);
+        }
+      }
+      return entriesMap.build();
+    } catch (Exception e) {
+      throw new RuntimeException("Could not retrieve entries for premium list " + name, e);
+    }
   }
 
   @VisibleForTesting
@@ -215,15 +280,10 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
     return revisionKey;
   }
 
-  @VisibleForTesting
-  public PremiumListRevision getRevision() {
-    return revision;
-  }
-
   /** Returns the PremiumList with the specified name. */
   public static Optional<PremiumList> get(String name) {
     try {
-      return Optional.of(cache.get(name));
+      return Optional.of(cachePremiumLists.get(name));
     } catch (InvalidCacheLoadException e) {
       return Optional.<PremiumList> absent();
     } catch (ExecutionException e) {
@@ -231,18 +291,9 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
     }
   }
 
-  /**
+  /** Returns whether a PremiumList of the given name exists, bypassing the cache. */
-   * Returns whether a PremiumList of the given name exists, without going through the overhead
-   * of loading up all of the premium list entities. Also does not hit the cache.
-   */
   public static boolean exists(String name) {
-    try {
+    return ofy().load().key(Key.create(getCrossTldKey(), PremiumList.class, name)).now() != null;
-      // Use DatastoreService to bypass the @OnLoad method that loads the premium list entries.
-      getDatastoreService().get(Key.create(getCrossTldKey(), PremiumList.class, name).getRaw());
-      return true;
-    } catch (EntityNotFoundException e) {
-      return false;
-    }
   }
 
   /**
@@ -310,30 +361,53 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
         .build();
   }
 
+  public static PremiumList saveWithEntries(
+      PremiumList premiumList, Iterable<String> premiumListLines) {
+    return saveWithEntries(premiumList, premiumList.parse(premiumListLines));
+  }
+
+  /** Re-parents the given {@link PremiumListEntry}s on the given {@link PremiumListRevision}. */
+  public static ImmutableSet<PremiumListEntry> parentEntriesOnRevision(
+      Iterable<PremiumListEntry> entries, final Key<PremiumListRevision> revisionKey) {
+    return FluentIterable.from(firstNonNull(entries, ImmutableSet.of()))
+        .transform(
+            new Function<PremiumListEntry, PremiumListEntry>() {
+              @Override
+              public PremiumListEntry apply(PremiumListEntry entry) {
+                return entry.asBuilder().setParent(revisionKey).build();
+              }
+            })
+        .toSet();
+  }
+
   /**
-   * Persists a PremiumList object to Datastore.
+   * Persists a new or updated PremiumList object and its descendant entities to Datastore.
    *
-   * <p> The flow here is: save the new premium list entries parented on that revision entity,
+   * <p>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.
+   *
+   * <p>This is the only valid way to save these kinds of entities!
    */
-  public PremiumList saveAndUpdateEntries() {
+  public static PremiumList saveWithEntries(
-    final Optional<PremiumList> oldPremiumList = get(name);
+      final PremiumList premiumList, ImmutableMap<String, PremiumListEntry> premiumListEntries) {
-    // Only update entries if there's actually a new revision of the list to save (which there will
+    final Optional<PremiumList> oldPremiumList = get(premiumList.getName());
-    // be if the list content changes, vs just the description/metadata).
+
-    boolean entriesToUpdate =
+    // Create the new revision (with its bloom filter) and parent the entries on it.
-        !oldPremiumList.isPresent()
+    final PremiumListRevision newRevision =
-            || !Objects.equals(oldPremiumList.get().revisionKey, this.revisionKey);
+        PremiumListRevision.create(premiumList, premiumListEntries.keySet());
-    // If needed, save the new child entities in a series of transactions.
+    final Key<PremiumListRevision> newRevisionKey = Key.create(newRevision);
-    if (entriesToUpdate) {
+    ImmutableSet<PremiumListEntry> parentedEntries =
-      for (final List<PremiumListEntry> batch
+        parentEntriesOnRevision(
-          : partition(premiumListMap.values(), TRANSACTION_BATCH_SIZE)) {
+            firstNonNull(premiumListEntries.values(), ImmutableSet.of()), newRevisionKey);
-        ofy().transactNew(new VoidWork() {
+
-          @Override
+    // Save the new child entities in a series of transactions.
-          public void vrun() {
+    for (final List<PremiumListEntry> batch : partition(parentedEntries, TRANSACTION_BATCH_SIZE)) {
-            ofy().save().entities(batch);
+      ofy().transactNew(new VoidWork() {
-          }});
+        @Override
-      }
+        public void vrun() {
+          ofy().save().entities(batch);
+        }});
     }
 
     // Save the new PremiumList and revision itself.
@@ -342,23 +416,27 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
         public PremiumList run() {
           DateTime now = ofy().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(
+              Objects.equals(existing, oldPremiumList.orNull()),
-                  ofy().load().type(PremiumList.class).parent(getCrossTldKey()).id(name).now(),
-                  oldPremiumList.orNull()),
               "PremiumList was concurrently edited");
-          PremiumList newList = PremiumList.this.asBuilder()
+          PremiumList newList = premiumList.asBuilder()
               .setLastUpdateTime(now)
               .setCreationTime(
                   oldPremiumList.isPresent() ? oldPremiumList.get().creationTime : now)
+              .setRevision(newRevisionKey)
               .build();
-          ofy().save().entities(newList, revision);
+          ofy().save().entities(newList, newRevision);
           return newList;
         }});
     // Update the cache.
-    PremiumList.cache.put(name, updated);
+    cachePremiumLists.put(premiumList.getName(), updated);
-    // If needed and there are any, delete the entities under the old PremiumList.
+    // Delete the entities under the old PremiumList.
-    if (entriesToUpdate && oldPremiumList.isPresent()) {
+    if (oldPremiumList.isPresent()) {
       oldPremiumList.get().deleteRevisionAndEntries();
     }
     return updated;
@@ -377,7 +455,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
         ofy().delete().entity(PremiumList.this);
       }});
     deleteRevisionAndEntries();
-    cache.invalidate(name);
+    cachePremiumLists.invalidate(name);
   }
 
   private void deleteRevisionAndEntries() {
@@ -385,7 +463,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
       return;
     }
     for (final List<Key<PremiumListEntry>> batch : partition(
-        loadEntriesForCurrentRevision().keys(),
+        queryEntriesForCurrentRevision().keys(),
         TRANSACTION_BATCH_SIZE)) {
       ofy().transactNew(new VoidWork() {
         @Override
@@ -400,7 +478,7 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
       }});
   }
 
-  private Query<PremiumListEntry> loadEntriesForCurrentRevision() {
+  private Query<PremiumListEntry> queryEntriesForCurrentRevision() {
     return ofy().load().type(PremiumListEntry.class).ancestor(revisionKey);
   }
 
@@ -418,34 +496,13 @@ public final class PremiumList extends BaseDomainLabelList<Money, PremiumList.Pr
       super(instance);
     }
 
-    private boolean entriesWereUpdated;
+    public Builder setRevision(Key<PremiumListRevision> revision) {
-
+      getInstance().revisionKey = revision;
-    public Builder setPremiumListMap(ImmutableMap<String, PremiumListEntry> premiumListMap) {
-      entriesWereUpdated = true;
-      getInstance().premiumListMap = premiumListMap;
       return this;
     }
 
-    /** Updates the premiumListMap from input lines. */
-    public Builder setPremiumListMapFromLines(Iterable<String> lines) {
-      return setPremiumListMap(getInstance().parse(lines));
-    }
-
     @Override
     public PremiumList build() {
-      final PremiumList instance = getInstance();
-      if (instance.revisionKey == null || entriesWereUpdated) {
-        instance.revision = PremiumListRevision.create(instance, instance.premiumListMap.keySet());
-        instance.revisionKey = Key.create(instance.revision);
-      }
-      // When we build an instance, make sure all entries are parented on its revisionKey.
-      instance.premiumListMap = Maps.transformValues(
-          nullToEmpty(instance.premiumListMap),
-          new Function<PremiumListEntry, PremiumListEntry>() {
-            @Override
-            public PremiumListEntry apply(PremiumListEntry entry) {
-              return entry.asBuilder().setParent(instance.revisionKey).build();
-            }});
       return super.build();
     }
   }

java/google/registry/tools/CreateOrUpdatePremiumListCommand.java

@@ -75,11 +75,8 @@ abstract class CreateOrUpdatePremiumListCommand extends ConfirmingCommand
   protected void init() throws Exception {
     name = isNullOrEmpty(name) ? convertFilePathToName(inputFile) : name;
     List<String> lines = Files.readAllLines(inputFile, UTF_8);
-    // Try constructing the premium list locally to check up front for validation errors.
+    // Try constructing and parsing the premium list locally to check up front for validation errors
-    new PremiumList.Builder()
+    new PremiumList.Builder().setName(name).build().parse(lines);
-        .setName(name)
-        .setPremiumListMapFromLines(lines)
-        .build();
     inputLineCount = lines.size();
   }
 
@@ -108,7 +105,7 @@ abstract class CreateOrUpdatePremiumListCommand extends ConfirmingCommand
         getCommandPath(),
         params.build(),
         MediaType.FORM_DATA,
-        requestBody.getBytes());
+        requestBody.getBytes(UTF_8));
 
     return extractServerResponse(response);
   }

java/google/registry/tools/DeletePremiumListCommand.java

@@ -62,9 +62,6 @@ final class DeletePremiumListCommand extends ConfirmingCommand implements Remote
   @Override
   protected String execute() throws Exception {
     premiumList.delete();
-    return String.format(
+    return String.format("Deleted premium list '%s'.\n", premiumList.getName());
-        "Deleted premium list %s with %d entries.\n",
-        premiumList.getName(),
-        premiumList.getPremiumListEntries().size());
   }
 }

java/google/registry/tools/server/CreatePremiumListAction.java

@@ -16,6 +16,7 @@ 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.PremiumList.saveWithEntries;
 import static google.registry.request.Action.Method.POST;
 
 import com.google.common.base.Splitter;
@@ -52,16 +53,14 @@ public class CreatePremiumListAction extends CreateOrUpdatePremiumListAction {
     logger.infofmt("Got the following input data: %s", inputData);
     List<String> inputDataPreProcessed =
         Splitter.on('\n').omitEmptyStrings().splitToList(inputData);
-    PremiumList premiumList = new PremiumList.Builder()
+    PremiumList premiumList = new PremiumList.Builder().setName(name).build();
-        .setName(name)
+    saveWithEntries(premiumList, inputDataPreProcessed);
-        .setPremiumListMapFromLines(inputDataPreProcessed)
-        .build();
-    premiumList.saveAndUpdateEntries();
 
-    logger.infofmt("Saved premium list %s with entries %s",
+    String message =
-        premiumList.getName(),
+        String.format(
-        premiumList.getPremiumListEntries());
+            "Saved premium list %s with %d entries",
-
+            premiumList.getName(), inputDataPreProcessed.size());
-    response.setPayload(ImmutableMap.of("status", "success"));
+    logger.info(message);
+    response.setPayload(ImmutableMap.of("status", "success", "message", message));
   }
 }

java/google/registry/tools/server/UpdatePremiumListAction.java

@@ -15,6 +15,7 @@
 package google.registry.tools.server;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static google.registry.model.registry.label.PremiumList.saveWithEntries;
 import static google.registry.request.Action.Method.POST;
 
 import com.google.common.base.Optional;
@@ -38,9 +39,9 @@ public class UpdatePremiumListAction extends CreateOrUpdatePremiumListAction {
 
   @Override
   protected void savePremiumList() {
-    Optional<PremiumList> existingName = PremiumList.get(name);
+    Optional<PremiumList> existingPremiumList = PremiumList.get(name);
     checkArgument(
-        existingName.isPresent(),
+        existingPremiumList.isPresent(),
         "Could not update premium list %s because it doesn't exist.",
         name);
 
@@ -48,21 +49,13 @@ public class UpdatePremiumListAction extends CreateOrUpdatePremiumListAction {
     logger.infofmt("Got the following input data: %s", inputData);
     List<String> inputDataPreProcessed =
         Splitter.on('\n').omitEmptyStrings().splitToList(inputData);
-    PremiumList premiumList = existingName.get().asBuilder()
+    PremiumList newPremiumList = saveWithEntries(existingPremiumList.get(), inputDataPreProcessed);
-        .setPremiumListMapFromLines(inputDataPreProcessed)
-        .build();
-    premiumList.saveAndUpdateEntries();
 
-    logger.infofmt("Updated premium list %s with entries %s",
+    String message =
-        premiumList.getName(),
+        String.format(
-        premiumList.getPremiumListEntries());
+            "Updated premium list %s with %d entries.",
-
+            newPremiumList.getName(), inputDataPreProcessed.size());
-    String message = String.format(
+    logger.info(message);
-        "Saved premium list %s with %d entries.\n",
+    response.setPayload(ImmutableMap.of("status", "success", "message", message));
-        premiumList.getName(),
-        premiumList.getPremiumListEntries().size());
-    response.setPayload(ImmutableMap.of(
-        "status", "success",
-        "message", message));
   }
 }

javatests/google/registry/model/registry/label/PremiumListTest.java

@@ -17,7 +17,9 @@ package google.registry.model.registry.label;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.model.registry.label.PremiumList.cachePremiumListEntries;
 import static google.registry.model.registry.label.PremiumList.getPremiumPrice;
+import static google.registry.model.registry.label.PremiumList.saveWithEntries;
 import static google.registry.testing.DatastoreHelper.createTld;
 import static google.registry.testing.DatastoreHelper.persistPremiumList;
 import static google.registry.testing.DatastoreHelper.persistReservedList;
@@ -34,7 +36,6 @@ import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
 import java.util.Map;
 import org.joda.money.Money;
-import org.joda.time.DateTime;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -75,54 +76,49 @@ public class PremiumListTest {
             .setPremiumPricingEngine(StaticPremiumListPricingEngine.NAME)
             .build());
     assertThat(Registry.get("ghost").getPremiumList()).isNull();
-    assertThat(getPremiumPrice("blah", "ghost")).isAbsent();
+    assertThat(getPremiumPrice("blah", Registry.get("ghost"))).isAbsent();
   }
 
   @Test
   public void testGetPremiumPrice_throwsExceptionWhenNonExistentPremiumListConfigured()
       throws Exception {
     PremiumList.get("tld").get().delete();
-    thrown.expect(IllegalStateException.class, "Could not load premium list named tld");
+    thrown.expect(IllegalStateException.class, "Could not load premium list 'tld'");
-    getPremiumPrice("blah", "tld");
+    getPremiumPrice("blah", Registry.get("tld"));
   }
 
   @Test
   public void testSave_largeNumberOfEntries_succeeds() throws Exception {
     PremiumList premiumList = persistHumongousPremiumList("tld", 2500);
-    assertThat(premiumList.getPremiumListEntries()).hasSize(2500);
+    assertThat(premiumList.loadPremiumListEntries()).hasSize(2500);
-    assertThat(premiumList.getPremiumPrice("7")).hasValue(Money.parse("USD 100"));
+    assertThat(getPremiumPrice("7", Registry.get("tld"))).hasValue(Money.parse("USD 100"));
   }
 
   @Test
   public void testSave_updateTime_isUpdatedOnEverySave() throws Exception {
-    PremiumList pl =  new PremiumList.Builder()
+    PremiumList pl =
-        .setName("tld3")
+        saveWithEntries(
-        .setPremiumListMapFromLines(ImmutableList.of("slime,USD 10"))
+            new PremiumList.Builder().setName("tld3").build(), ImmutableList.of("slime,USD 10"));
-        .build()
+    PremiumList newPl =
-        .saveAndUpdateEntries();
+        saveWithEntries(
-    PremiumList newPl = new PremiumList.Builder()
+            new PremiumList.Builder().setName(pl.getName()).build(),
-        .setName(pl.getName())
+            ImmutableList.of("mutants,USD 20"));
-        .setPremiumListMapFromLines(ImmutableList.of("mutants,USD 20"))
-        .build()
-        .saveAndUpdateEntries();
     assertThat(newPl.getLastUpdateTime()).isGreaterThan(pl.getLastUpdateTime());
   }
 
   @Test
   public void testSave_creationTime_onlyUpdatedOnFirstCreation() throws Exception {
     PremiumList pl = persistPremiumList("tld3", "sludge,JPY 1000");
-    DateTime creationTime = pl.creationTime;
+    PremiumList newPl = saveWithEntries(pl, ImmutableList.of("sleighbells,CHF 2000"));
-    pl = pl.asBuilder()
+    assertThat(newPl.creationTime).isEqualTo(pl.creationTime);
-        .setPremiumListMapFromLines(ImmutableList.of("sleighbells,CHF 2000"))
-        .build();
-    assertThat(pl.creationTime).isEqualTo(creationTime);
   }
 
   @Test
   public void testSave_removedPremiumListEntries_areNoLongerInDatastore() throws Exception {
+    Registry registry = Registry.get("tld");
     PremiumList pl = persistPremiumList("tld", "genius,USD 10", "dolt,JPY 1000");
-    assertThat(getPremiumPrice("genius", "tld")).hasValue(Money.parse("USD 10"));
+    assertThat(getPremiumPrice("genius", registry)).hasValue(Money.parse("USD 10"));
-    assertThat(getPremiumPrice("dolt", "tld")).hasValue(Money.parse("JPY 1000"));
+    assertThat(getPremiumPrice("dolt", registry)).hasValue(Money.parse("JPY 1000"));
     assertThat(ofy()
             .load()
             .type(PremiumListEntry.class)
@@ -131,13 +127,10 @@ public class PremiumListTest {
             .now()
             .price)
         .isEqualTo(Money.parse("JPY 1000"));
-    PremiumList pl2 = pl.asBuilder()
+    PremiumList pl2 = saveWithEntries(pl, ImmutableList.of("genius,USD 10", "savant,USD 90"));
-        .setPremiumListMapFromLines(ImmutableList.of("genius,USD 10", "savant,USD 90"))
+    assertThat(getPremiumPrice("genius", registry)).hasValue(Money.parse("USD 10"));
-        .build()
+    assertThat(getPremiumPrice("savant", registry)).hasValue(Money.parse("USD 90"));
-        .saveAndUpdateEntries();
+    assertThat(getPremiumPrice("dolt", registry)).isAbsent();
-    assertThat(getPremiumPrice("genius", "tld")).hasValue(Money.parse("USD 10"));
-    assertThat(getPremiumPrice("savant", "tld")).hasValue(Money.parse("USD 90"));
-    assertThat(getPremiumPrice("dolt", "tld")).isAbsent();
     assertThat(ofy()
             .load()
             .type(PremiumListEntry.class)
@@ -156,59 +149,46 @@ public class PremiumListTest {
 
   @Test
   public void testGetPremiumPrice_allLabelsAreNonPremium_whenNotInList() throws Exception {
-    assertThat(getPremiumPrice("blah", "tld")).isAbsent();
+    assertThat(getPremiumPrice("blah", Registry.get("tld"))).isAbsent();
-    assertThat(getPremiumPrice("slinge", "tld")).isAbsent();
+    assertThat(getPremiumPrice("slinge", Registry.get("tld"))).isAbsent();
   }
 
   @Test
   public void testSave_simple() throws Exception {
-    PremiumList pl = persistPremiumList("tld2", "lol , USD 999 # yupper rooni ");
+    PremiumList pl =
+        saveWithEntries(
+            new PremiumList.Builder().setName("tld2").build(),
+            ImmutableList.of("lol , USD 999 # yupper rooni "));
     createTld("tld");
     persistResource(Registry.get("tld").asBuilder().setPremiumList(pl).build());
-    assertThat(pl.getPremiumPrice("lol")).hasValue(Money.parse("USD 999"));
+    assertThat(getPremiumPrice("lol", Registry.get("tld"))).hasValue(Money.parse("USD 999"));
-    assertThat(getPremiumPrice("lol", "tld")).hasValue(Money.parse("USD 999"));
+    assertThat(getPremiumPrice("lol ", Registry.get("tld"))).isAbsent();
-    assertThat(getPremiumPrice("lol ", "tld")).isAbsent();
+    Map<String, PremiumListEntry> entries =
-    Map<String, PremiumListEntry> entries = PremiumList.get("tld2").get().getPremiumListEntries();
+        PremiumList.get("tld2").get().loadPremiumListEntries();
     assertThat(entries.keySet()).containsExactly("lol");
     assertThat(entries).doesNotContainKey("lol ");
-    PremiumListEntry entry = entries.values().iterator().next();
+    PremiumListEntry entry = entries.get("lol");
     assertThat(entry.comment).isEqualTo("yupper rooni");
     assertThat(entry.price).isEqualTo(Money.parse("USD 999"));
     assertThat(entry.label).isEqualTo("lol");
   }
 
   @Test
-  public void test_saveAndUpdateEntries_twiceOnUnchangedList() throws Exception {
+  public void test_saveAndUpdateEntriesTwice() throws Exception {
     PremiumList pl =
-        new PremiumList.Builder()
+        saveWithEntries(
-            .setName("pl")
+            new PremiumList.Builder().setName("pl").build(), ImmutableList.of("test,USD 1"));
-            .setPremiumListMapFromLines(ImmutableList.of("test,USD 1"))
+    Map<String, PremiumListEntry> entries = pl.loadPremiumListEntries();
-            .build()
-            .saveAndUpdateEntries();
-    Map<String, PremiumListEntry> entries = pl.getPremiumListEntries();
     assertThat(entries.keySet()).containsExactly("test");
-    assertThat(PremiumList.get("pl").get().getPremiumListEntries()).isEqualTo(entries);
+    assertThat(PremiumList.get("pl").get().loadPremiumListEntries()).isEqualTo(entries);
     // Save again with no changes, and clear the cache to force a re-load from Datastore.
-    pl.saveAndUpdateEntries();
+    PremiumList resaved = saveWithEntries(pl, ImmutableList.of("test,USD 1"));
     ofy().clearSessionCache();
-    assertThat(PremiumList.get("pl").get().getPremiumListEntries()).isEqualTo(entries);
+    Map<String, PremiumListEntry> entriesReloaded =
-  }
+        PremiumList.get("pl").get().loadPremiumListEntries();
-
+    assertThat(entriesReloaded).hasSize(1);
-  @Test
+    assertThat(entriesReloaded).containsKey("test");
-  public void test_saveAndUpdateEntries_twiceOnListWithOnlyMetadataChanges() throws Exception {
+    assertThat(entriesReloaded.get("test").parent).isEqualTo(resaved.getRevisionKey());
-    PremiumList pl =
-        new PremiumList.Builder()
-            .setName("pl")
-            .setPremiumListMapFromLines(ImmutableList.of("test,USD 1"))
-            .build()
-            .saveAndUpdateEntries();
-    Map<String, PremiumListEntry> entries = pl.getPremiumListEntries();
-    assertThat(entries.keySet()).containsExactly("test");
-    assertThat(PremiumList.get("pl").get().getPremiumListEntries()).isEqualTo(entries);
-    // Save again with description changed, and clear the cache to force a re-load from Datastore.
-    pl.asBuilder().setDescription("foobar").build().saveAndUpdateEntries();
-    ofy().clearSessionCache();
-    assertThat(PremiumList.get("pl").get().getPremiumListEntries()).isEqualTo(entries);
   }
 
   @Test
@@ -253,22 +233,33 @@ public class PremiumListTest {
   }
 
   @Test
-  public void testAsBuilder_updatingEntitiesreplacesRevisionKey() throws Exception {
+  public void testGetPremiumPrice_comesFromBloomFilter() throws Exception {
     PremiumList pl = PremiumList.get("tld").get();
-    assertThat(pl.asBuilder()
+    PremiumListEntry entry =
-        .setPremiumListMapFromLines(ImmutableList.of("qux,USD 123"))
+        persistResource(
-        .build()
+            new PremiumListEntry.Builder()
-        .getRevisionKey())
+                .setParent(pl.getRevisionKey())
-            .isNotEqualTo(pl.getRevisionKey());
+                .setLabel("missingno")
+                .setPrice(Money.parse("USD 1000"))
+                .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"))).isAbsent();
+    // However, if we manually query the cache to force an entity load, it should be found.
+    assertThat(
+            cachePremiumListEntries.get(
+                Key.create(pl.getRevisionKey(), PremiumListEntry.class, "missingno")))
+        .hasValue(entry);
   }
 
   @Test
   public void testProbablePremiumLabels() throws Exception {
     PremiumList pl = PremiumList.get("tld").get();
-    assertThat(pl.getRevision().probablePremiumLabels.mightContain("notpremium")).isFalse();
+    PremiumListRevision revision = ofy().load().key(pl.getRevisionKey()).now();
+    assertThat(revision.probablePremiumLabels.mightContain("notpremium")).isFalse();
     for (String label : ImmutableList.of("rich", "lol", "johnny-be-goode", "icann")) {
       assertWithMessage(label + " should be a probable premium")
-          .that(pl.getRevision().probablePremiumLabels.mightContain(label))
+          .that(revision.probablePremiumLabels.mightContain(label))
           .isTrue();
     }
   }

javatests/google/registry/testing/DatastoreHelper.java

@@ -27,6 +27,7 @@ import static google.registry.model.EppResourceUtils.createDomainRepoId;
 import static google.registry.model.EppResourceUtils.createRepoId;
 import static google.registry.model.domain.launch.ApplicationStatus.VALIDATED;
 import static google.registry.model.ofy.ObjectifyService.ofy;
+import static google.registry.model.registry.label.PremiumList.parentEntriesOnRevision;
 import static google.registry.pricing.PricingEngineProxy.getDomainRenewCost;
 import static google.registry.util.CollectionUtils.difference;
 import static google.registry.util.CollectionUtils.union;
@@ -35,16 +36,17 @@ import static google.registry.util.DateTimeUtils.START_OF_TIME;
 import static google.registry.util.DomainNameUtils.ACE_PREFIX_REGEX;
 import static google.registry.util.DomainNameUtils.getTldFromDomainName;
 import static google.registry.util.ResourceUtils.readResourceUtf8;
+import static java.util.Arrays.asList;
 import static org.joda.money.CurrencyUnit.USD;
 
 import com.google.common.base.Ascii;
 import com.google.common.base.Function;
-import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
 import com.google.common.base.Splitter;
 import com.google.common.base.Supplier;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.Iterables;
@@ -83,6 +85,8 @@ import google.registry.model.registrar.Registrar;
 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.PremiumList.PremiumListEntry;
+import google.registry.model.registry.label.PremiumList.PremiumListRevision;
 import google.registry.model.registry.label.ReservedList;
 import google.registry.model.reporting.HistoryEntry;
 import google.registry.model.smd.EncodedSignedMark;
@@ -343,23 +347,27 @@ public class DatastoreHelper {
             .build());
   }
 
+  /**
+   * Persists a premium list and its child entities directly without writing commit logs.
+   *
+   * <p>Avoiding commit logs is important because a simple default premium list is persisted for
+   * each TLD that is created in tests, and clocks would need to be mocked using an auto-
+   * incrementing FakeClock for all tests in order to persist the commit logs properly because of
+   * the requirement to have monotonically increasing timestamps.
+   */
   public static PremiumList persistPremiumList(String listName, String... lines) {
-    Optional<PremiumList> existing = PremiumList.get(listName);
+    PremiumList premiumList = new PremiumList.Builder().setName(listName).build();
-    return persistPremiumList(
+    ImmutableMap<String, PremiumListEntry> entries = premiumList.parse(asList(lines));
-        (existing.isPresent() ? existing.get().asBuilder() : new PremiumList.Builder())
+    PremiumListRevision revision = PremiumListRevision.create(premiumList, entries.keySet());
-            .setName(listName)
+    ofy()
-            .setPremiumListMapFromLines(ImmutableList.copyOf(lines))
+        .saveWithoutBackup()
-            .build());
+        .entities(premiumList.asBuilder().setRevision(Key.create(revision)).build(), revision)
-  }
+        .now();
-
+    ofy()
-  private static PremiumList persistPremiumList(PremiumList premiumList) {
+        .saveWithoutBackup()
-    // Persist the list and its child entities directly, rather than using its helper method, so
+        .entities(parentEntriesOnRevision(entries.values(), Key.create(revision)))
-    // that we can avoid writing commit logs. This would cause issues since many tests replace the
+        .now();
-    // clock in Ofy with a non-advancing FakeClock, and commit logs currently require
+    return ofy().load().entity(premiumList).now();
-    // monotonically increasing timestamps.
-    ofy().saveWithoutBackup().entities(premiumList, premiumList.getRevision()).now();
-    ofy().saveWithoutBackup().entities(premiumList.getPremiumListEntries().values()).now();
-    return premiumList;
   }
 
   /** Creates and persists a tld. */

javatests/google/registry/tools/DeletePremiumListCommandTest.java

@@ -32,7 +32,7 @@ public class DeletePremiumListCommandTest extends CommandTestCase<DeletePremiumL
   @Test
   public void testSuccess() throws Exception {
     PremiumList premiumList = persistPremiumList("xn--q9jyb4c", "blah,USD 100");
-    assertThat(premiumList.getPremiumListEntries()).hasSize(1);
+    assertThat(premiumList.loadPremiumListEntries()).hasSize(1);
     runCommand("--force", "--name=xn--q9jyb4c");
     assertThat(PremiumList.get("xn--q9jyb4c")).isAbsent();
 

javatests/google/registry/tools/server/CreatePremiumListActionTest.java

@@ -15,9 +15,11 @@
 package google.registry.tools.server;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.registry.label.PremiumList.getPremiumPrice;
 import static google.registry.testing.DatastoreHelper.createTlds;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
 
+import google.registry.model.registry.Registry;
 import google.registry.model.registry.label.PremiumList;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
@@ -81,10 +83,7 @@ public class CreatePremiumListActionTest {
     action.override = true;
     action.run();
     assertThat(response.getStatus()).isEqualTo(SC_OK);
-    PremiumList premiumList = PremiumList.get("zanzibar").get();
+    assertThat(PremiumList.get("zanzibar").get().loadPremiumListEntries()).hasSize(1);
-    assertThat(premiumList.getPremiumListEntries()).hasSize(1);
-    assertThat(premiumList.getPremiumPrice("zanzibar")).hasValue(Money.parse("USD 100"));
-    assertThat(premiumList.getPremiumPrice("diamond")).isAbsent();
   }
 
   @Test
@@ -93,9 +92,8 @@ public class CreatePremiumListActionTest {
     action.inputData = "rich,USD 25\nricher,USD 1000\n";
     action.run();
     assertThat(response.getStatus()).isEqualTo(SC_OK);
-    PremiumList premiumList = PremiumList.get("foo").get();
+    assertThat(PremiumList.get("foo").get().loadPremiumListEntries()).hasSize(2);
-    assertThat(premiumList.getPremiumListEntries()).hasSize(2);
+    assertThat(getPremiumPrice("rich", Registry.get("foo"))).hasValue(Money.parse("USD 25"));
-    assertThat(premiumList.getPremiumPrice("rich")).hasValue(Money.parse("USD 25"));
+    assertThat(getPremiumPrice("diamond", Registry.get("foo"))).isAbsent();
-    assertThat(premiumList.getPremiumPrice("diamond")).isAbsent();
   }
 }

javatests/google/registry/tools/server/ListPremiumListsActionTest.java

@@ -17,7 +17,6 @@ package google.registry.tools.server;
 import static google.registry.testing.DatastoreHelper.persistPremiumList;
 
 import com.google.common.base.Optional;
-import google.registry.model.registry.label.PremiumList;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -35,10 +34,6 @@ public class ListPremiumListsActionTest extends ListActionTestCase {
   public void init() throws Exception {
     persistPremiumList("xn--q9jyb4c", "rich,USD 100");
     persistPremiumList("how", "richer,JPY 5000");
-    PremiumList.get("how").get().asBuilder()
-        .setDescription("foobar")
-        .build()
-        .saveAndUpdateEntries();
     action = new ListPremiumListsAction();
   }
 

javatests/google/registry/tools/server/UpdatePremiumListActionTest.java

@@ -15,9 +15,11 @@
 package google.registry.tools.server;
 
 import static com.google.common.truth.Truth.assertThat;
+import static google.registry.model.registry.label.PremiumList.getPremiumPrice;
 import static google.registry.testing.DatastoreHelper.createTlds;
 import static javax.servlet.http.HttpServletResponse.SC_OK;
 
+import google.registry.model.registry.Registry;
 import google.registry.model.registry.label.PremiumList;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.ExceptionRule;
@@ -79,11 +81,11 @@ public class UpdatePremiumListActionTest {
     action.inputData = "rich,USD 75\nricher,USD 5000\npoor, USD 0.99";
     action.run();
     assertThat(response.getStatus()).isEqualTo(SC_OK);
-    PremiumList premiumList = PremiumList.get("foo").get();
+    Registry registry = Registry.get("foo");
-    assertThat(premiumList.getPremiumListEntries()).hasSize(3);
+    assertThat(PremiumList.get("foo").get().loadPremiumListEntries()).hasSize(3);
-    assertThat(premiumList.getPremiumPrice("rich")).hasValue(Money.parse("USD 75"));
+    assertThat(getPremiumPrice("rich", registry)).hasValue(Money.parse("USD 75"));
-    assertThat(premiumList.getPremiumPrice("richer")).hasValue(Money.parse("USD 5000"));
+    assertThat(getPremiumPrice("richer", registry)).hasValue(Money.parse("USD 5000"));
-    assertThat(premiumList.getPremiumPrice("poor")).hasValue(Money.parse("USD 0.99"));
+    assertThat(getPremiumPrice("poor", registry)).hasValue(Money.parse("USD 0.99"));
-    assertThat(premiumList.getPremiumPrice("diamond")).isAbsent();
+    assertThat(getPremiumPrice("diamond", registry)).isAbsent();
   }
 }