Simplify logic in create/update tld commands

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

java/google/registry/tools/CreateOrUpdateTldCommand.java

@@ -15,25 +15,18 @@
 package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.collect.Sets.difference;
-import static com.google.common.collect.Sets.intersection;
-import static com.google.common.collect.Sets.union;
 import static google.registry.model.RoidSuffixes.isRoidSuffixUsed;
 import static google.registry.util.CollectionUtils.findDuplicates;
-import static google.registry.util.CollectionUtils.nullToEmpty;
 import static google.registry.util.DomainNameUtils.canonicalizeDomainName;
 
 import com.google.common.base.CharMatcher;
-import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
-import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 
 import com.beust.jcommander.Parameter;
-import com.googlecode.objectify.Key;
 
 import google.registry.model.pricing.StaticPremiumListPricingEngine;
 import google.registry.model.registry.Registries;
@@ -41,7 +34,6 @@ 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.ReservedList;
 import google.registry.tools.params.OptionalStringParameter;
 import google.registry.tools.params.TransitionListParameter.BillingCostTransitions;
 import google.registry.tools.params.TransitionListParameter.TldStateTransitions;
@@ -208,28 +200,16 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand {
       description = "The end of the claims period")
   DateTime claimsPeriodEnd;
 
-  @Nullable
-  Set<String> reservedListNamesToAdd;
-
-  @Nullable
-  Set<String> reservedListNamesToRemove;
-
-  @Nullable
-  Set<String> allowedRegistrantsToAdd;
-
-  @Nullable
-  Set<String> allowedRegistrantsToRemove;
-
-  @Nullable
-  Set<String> allowedNameserversToAdd;
-
-  @Nullable
-  Set<String> allowedNameserversToRemove;
-
   /** Returns the existing registry (for update) or null (for creates). */
   @Nullable
   abstract Registry getOldRegistry(String tld);
 
+  abstract ImmutableSet<String> getAllowedRegistrants(Registry oldRegistry);
+
+  abstract ImmutableSet<String> getAllowedNameservers(Registry oldRegistry);
+
+  abstract ImmutableSet<String> getReservedLists(Registry oldRegistry);
+
   /** Subclasses can override this to set their own properties. */
   void setCommandSpecificProperties(@SuppressWarnings("unused") Registry.Builder builder) {}
 
@@ -354,43 +334,13 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand {
         }
       }
 
-      ImmutableSet<String> newReservedListNames =
+      ImmutableSet<String> newReservedListNames = getReservedLists(oldRegistry);
-          formUpdatedList(
-              "reserved lists",
-              oldRegistry == null ? ImmutableSet.<String>of() : FluentIterable
-                  .from(oldRegistry.getReservedLists())
-                  .transform(
-                      new Function<Key<ReservedList>, String>() {
-                        @Override
-                        public String apply(Key<ReservedList> key) {
-                          return key.getName();
-                        }})
-                  .toSet(),
-              reservedListNames,
-              reservedListNamesToAdd,
-              reservedListNamesToRemove);
       checkReservedListValidityForTld(tld, newReservedListNames);
       builder.setReservedListsByName(newReservedListNames);
 
-      builder.setAllowedRegistrantContactIds(
+      builder.setAllowedRegistrantContactIds(getAllowedRegistrants(oldRegistry));
-          formUpdatedList(
-              "allowed registrants",
-              oldRegistry == null
-                  ? ImmutableSet.<String>of()
-                  : oldRegistry.getAllowedRegistrantContactIds(),
-              allowedRegistrants,
-              allowedRegistrantsToAdd,
-              allowedRegistrantsToRemove));
 
-      builder.setAllowedFullyQualifiedHostNames(
+      builder.setAllowedFullyQualifiedHostNames(getAllowedNameservers(oldRegistry));
-          formUpdatedList(
-              "allowed nameservers",
-              oldRegistry == null
-                  ? ImmutableSet.<String>of()
-                  : oldRegistry.getAllowedFullyQualifiedHostNames(),
-              allowedNameservers,
-              allowedNameserversToAdd,
-              allowedNameserversToRemove));
 
       // Update the Registry object.
       setCommandSpecificProperties(builder);
@@ -398,34 +348,6 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand {
     }
   }
 
-  private static ImmutableSet<String> formUpdatedList(
-      String description,
-      ImmutableSet<String> originals,
-      List<String> toReplace,
-      Set<String> toAdd,
-      Set<String> toRemove) {
-    if (toReplace != null) {
-      return ImmutableSet.copyOf(toReplace);
-    }
-    toAdd = nullToEmpty(toAdd);
-    toRemove = nullToEmpty(toRemove);
-    checkIsEmpty(
-        intersection(toAdd, toRemove),
-        String.format(
-            "Adding and removing the same %s simultaneously doesn't make sense", description));
-    checkIsEmpty(
-        intersection(originals, toAdd),
-        String.format("Cannot add %s that were previously present", description));
-    checkIsEmpty(
-        difference(toRemove, originals),
-        String.format("Cannot remove %s that were not previously present", description));
-    return ImmutableSet.copyOf(difference(union(originals, toAdd), toRemove));
-  }
-
-  private static void checkIsEmpty(Set<String> set, String errorString) {
-    checkArgument(set.isEmpty(), String.format("%s: %s", errorString, set));
-  }
-
   @Override
   public String execute() throws Exception {
     try {

java/google/registry/tools/CreateTldCommand.java

@@ -17,9 +17,11 @@ package google.registry.tools;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static google.registry.model.registry.Registries.getTlds;
+import static google.registry.util.CollectionUtils.nullToEmpty;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
 
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 
 import com.beust.jcommander.Parameter;
@@ -81,4 +83,19 @@ class CreateTldCommand extends CreateOrUpdateTldCommand {
     checkState(!getTlds().contains(tld), "TLD already exists");
     return null;
   }
+
+  @Override
+  ImmutableSet<String> getAllowedRegistrants(Registry oldRegistry) {
+    return ImmutableSet.copyOf(nullToEmpty(allowedRegistrants));
+  }
+
+  @Override
+  ImmutableSet<String> getAllowedNameservers(Registry oldRegistry) {
+    return ImmutableSet.copyOf(nullToEmpty(allowedNameservers));
+  }
+
+  @Override
+  ImmutableSet<String> getReservedLists(Registry oldRegistry) {
+    return ImmutableSet.copyOf(nullToEmpty(reservedListNames));
+  }
 }

java/google/registry/tools/UpdateTldCommand.java

@@ -15,17 +15,25 @@
 package google.registry.tools;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.Sets.difference;
+import static com.google.common.collect.Sets.intersection;
+import static com.google.common.collect.Sets.union;
 import static google.registry.model.registry.Registries.assertTldExists;
 import static google.registry.util.CollectionUtils.nullToEmpty;
 
+import com.google.common.base.Function;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
 
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
+import com.googlecode.objectify.Key;
 
 import google.registry.model.registry.Registry;
+import google.registry.model.registry.label.ReservedList;
 
 import java.util.List;
+import java.util.Set;
 
 import javax.annotation.Nullable;
 
@@ -73,6 +81,44 @@ class UpdateTldCommand extends CreateOrUpdateTldCommand {
     return Registry.get(assertTldExists(tld));
   }
 
+  @Override
+  ImmutableSet<String> getAllowedRegistrants(Registry oldRegistry) {
+    return formUpdatedList(
+        "allowed registrants",
+        oldRegistry.getAllowedRegistrantContactIds(),
+        allowedRegistrants,
+        allowedRegistrantsAdd,
+        allowedRegistrantsRemove);
+  }
+
+  @Override
+  ImmutableSet<String> getAllowedNameservers(Registry oldRegistry) {
+    return formUpdatedList(
+        "allowed nameservers",
+        oldRegistry.getAllowedFullyQualifiedHostNames(),
+        allowedNameservers,
+        allowedNameserversAdd,
+        allowedNameserversRemove);
+  }
+
+  @Override
+  ImmutableSet<String> getReservedLists(Registry oldRegistry) {
+    return formUpdatedList(
+        "reserved lists",
+        FluentIterable
+            .from(oldRegistry.getReservedLists())
+            .transform(
+                new Function<Key<ReservedList>, String>() {
+                  @Override
+                  public String apply(Key<ReservedList> key) {
+                    return key.getName();
+                  }})
+            .toSet(),
+        reservedListNames,
+        reservedListsAdd,
+        reservedListsRemove);
+  }
+
   @Override
   protected void initTldCommand() throws Exception {
     checkConflicts("reserved_lists", reservedListNames, reservedListsAdd, reservedListsRemove);
@@ -80,21 +126,43 @@ class UpdateTldCommand extends CreateOrUpdateTldCommand {
         "allowed_registrants", allowedRegistrants, allowedRegistrantsAdd, allowedRegistrantsRemove);
     checkConflicts(
         "allowed_nameservers", allowedNameservers, allowedNameserversAdd, allowedNameserversRemove);
-    reservedListNamesToAdd = ImmutableSet.copyOf(nullToEmpty(reservedListsAdd));
-    reservedListNamesToRemove = ImmutableSet.copyOf(nullToEmpty(reservedListsRemove));
-    allowedRegistrantsToAdd = ImmutableSet.copyOf(nullToEmpty(allowedRegistrantsAdd));
-    allowedRegistrantsToRemove = ImmutableSet.copyOf(nullToEmpty(allowedRegistrantsRemove));
-    allowedNameserversToAdd = ImmutableSet.copyOf(nullToEmpty(allowedNameserversAdd));
-    allowedNameserversToRemove = ImmutableSet.copyOf(nullToEmpty(allowedNameserversRemove));
   }
 
-  private void checkConflicts(
+  private static ImmutableSet<String> formUpdatedList(
+      String description,
+      ImmutableSet<String> originals,
+      List<String> fullReplacement,
+      List<String> itemsToAdd,
+      List<String> itemsToRemove) {
+    if (fullReplacement != null) {
+      return ImmutableSet.copyOf(fullReplacement);
+    }
+    Set<String> toAdd = ImmutableSet.copyOf(nullToEmpty(itemsToAdd));
+    Set<String> toRemove = ImmutableSet.copyOf(nullToEmpty(itemsToRemove));
+    checkIsEmpty(
+        intersection(toAdd, toRemove),
+        String.format(
+            "Adding and removing the same %s simultaneously doesn't make sense", description));
+    checkIsEmpty(
+        intersection(originals, toAdd),
+        String.format("Cannot add %s that were previously present", description));
+    checkIsEmpty(
+        difference(toRemove, originals),
+        String.format("Cannot remove %s that were not previously present", description));
+    return ImmutableSet.copyOf(difference(union(originals, toAdd), toRemove));
+  }
+
+  private static void checkIsEmpty(Set<String> set, String errorString) {
+    checkArgument(set.isEmpty(), String.format("%s: %s", errorString, set));
+  }
+
+  private static void checkConflicts(
       String baseFlagName, Object overwriteValue, Object addValue, Object removeValue) {
     checkNotBoth(baseFlagName, overwriteValue, "add_" + baseFlagName, addValue);
     checkNotBoth(baseFlagName, overwriteValue, "remove_" + baseFlagName, removeValue);
   }
 
-  private void checkNotBoth(String nameA, Object valueA, String nameB, Object valueB) {
+  private static void checkNotBoth(String nameA, Object valueA, String nameB, Object valueB) {
     checkArgument(valueA == null || valueB == null, "Don't pass both --%s and --%s", nameA, nameB);
   }
 }