Don't allow duplicates in premium/reserved lists

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

docs/operational-procedures/premium-list-management.md

@@ -7,8 +7,9 @@ prices of domain labels (i.e. the part of the domain name without the TLD) by
 checking for their presence on a list of prices in Datastore. `nomulus` is used
 to load and update these lists from flat text files. The format of this list is
 simple: It is a newline-delimited CSV text file with each line containing the
-label and its price (including currency specifier in ISO-4217 format). As an
+label and its price (including currency specifier in ISO-4217 format). Any
-example:
+individual label may not appear more than once in the file. Here's an example of
+the formatting:
 
 ```
 premium,USD 100

docs/operational-procedures/reserved-list-management.md

@@ -6,7 +6,7 @@ for various reasons, usually because of potential abuse.
 ## Reserved list file format
 
 Reserved lists are handled in a similar way to [premium
-lists](./premium-list-management.md), except that rather than each label having
+lists](./premium-list-management.md), except that instead of each label having
 a price, it has a reservation type. The valid values for reservation types are:
 
 *   **`UNRESERVED`** - The default value for any label that isn't reserved.
@@ -29,11 +29,11 @@ a price, it has a reservation type. The valid values for reservation types are:
     specified.
 
 The reservation types are listed in order of increasing precedence, so if a
-label is included on the same list multiple times, or on different lists that
+label is included on different lists that are applied to a single TLD, whichever
-are applied to a single TLD, whichever reservation type is later in the list
+reservation type is later in the list takes precedence. E.g. a label being fully
-takes precedence. E.g. a label being fully blocked in one list always supersedes
+blocked in one list always supersedes it being allowed in sunrise from another
-it being allowed in sunrise from another list. In general `FULLY_BLOCKED` is by
+list. In general `FULLY_BLOCKED` is by far the most widely used reservation type
-far the most widely used reservation type for typical TLD use cases.
+for typical TLD use cases.
 
 Here's an example of a small reserved list. Note that
 `RESERVED_FOR_ANCHOR_TENANT` is the only reservation type that has a third entry

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

@@ -23,10 +23,11 @@ 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;
+import com.google.common.collect.HashMultiset;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Ordering;
+import com.google.common.collect.Multiset;
 import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.annotation.Id;
@@ -85,16 +86,26 @@ public abstract class BaseDomainLabelList<T extends Comparable<?>, R extends Dom
   @VisibleForTesting
   protected ImmutableMap<String, R> parse(Iterable<String> lines) {
     Map<String, R> labelsToEntries = new HashMap<>();
+    Multiset<String> duplicateLabels = HashMultiset.create();
     for (String line : lines) {
       R entry = createFromLine(line);
       if (entry == null) {
         continue;
       }
       String label = entry.getLabel();
-      // Adds the label to the list of all labels if it (a) doesn't already exist, or (b) already
+      // Check if the label was already processed for this list (which is an error), and if so,
-      // exists, but the new value has higher priority (as determined by sort order).
+      // accumulate it so that a list of all duplicates can be thrown.
-      labelsToEntries.put(
+      if (labelsToEntries.containsKey(label)) {
-          label, Ordering.natural().nullsFirst().max(labelsToEntries.get(label), entry));
+        duplicateLabels.add(label, duplicateLabels.contains(label) ? 1 : 2);
+      } else {
+        labelsToEntries.put(label, entry);
+      }
+    }
+    if (!duplicateLabels.isEmpty()) {
+      throw new IllegalStateException(
+          String.format(
+              "List '%s' cannot contain duplicate labels. Dupes (with counts) were: %s",
+              name, duplicateLabels));
     }
     return ImmutableMap.copyOf(labelsToEntries);
   }

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

@@ -273,6 +273,18 @@ public class PremiumListTest {
     }
   }
 
+  @Test
+  public void testParse_cannotIncludeDuplicateLabels() {
+    thrown.expect(
+        IllegalStateException.class,
+        "List 'tld' cannot contain duplicate labels. Dupes (with counts) were: [lol x 2]");
+    PremiumList.get("tld")
+        .get()
+        .parse(
+            ImmutableList.of(
+                "lol,USD 100", "rofl,USD 90", "paper,USD 80", "wood,USD 70", "lol,USD 200"));
+  }
+
   /** Persists a premium list with a specified number of nonsense entries. */
   private PremiumList persistHumongousPremiumList(String name, int size) {
     String[] entries = new String[size];

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

@@ -291,6 +291,21 @@ public class ReservedListTest {
             .build());
   }
 
+  @Test
+  public void testParse_cannotIncludeDuplicateLabels() {
+    ReservedList rl = new ReservedList.Builder().setName("blah").build();
+    thrown.expect(
+        IllegalStateException.class,
+        "List 'blah' cannot contain duplicate labels. Dupes (with counts) were: [lol x 2]");
+    rl.parse(
+        ImmutableList.of(
+            "lol,FULLY_BLOCKED",
+            "rofl,FULLY_BLOCKED",
+            "paper,FULLY_BLOCKED",
+            "wood,FULLY_BLOCKED",
+            "lol,FULLY_BLOCKED"));
+  }
+
   /** Gets the name of a reserved list. */
   public static final Function<Key<ReservedList>, String> GET_NAME_FUNCTION =
       new Function<Key<ReservedList>, String>() {

javatests/google/registry/testing/default_premium_list_testdata.csv

@@ -10,4 +10,3 @@ palladium,USD 877
 aluminum,USD 11
 copper,USD 15
 brass,USD 20
-platinum,USD 80000 #this should be ignored as a cheaper duplicate