Don't prompt to confirm non-mutating nomulus EPP tool commands

This is accomplished by making all non-mutating commands function with dry run set
to true, which also has the pleasurable side effect of not prompting for dry-run
mutating commands either, which also do nothing different/special on the second
run.

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

java/google/registry/tools/ConfirmingCommand.java

@@ -32,7 +32,11 @@ public abstract class ConfirmingCommand implements Command {
     if (checkExecutionState()) {
       init();
       printLineIfNotEmpty(prompt());
-      if (force || promptForYes("Perform this command?")) {
+      if (dontRunCommand()) {
+        // This typically happens when all of the work is accomplished inside of prompt(), so do
+        // nothing further.
+        return;
+      } else if (force || promptForYes("Perform this command?")) {
         System.out.println(execute());
         printLineIfNotEmpty(postExecute());
       } else {
@@ -49,6 +53,11 @@ public abstract class ConfirmingCommand implements Command {
   /** Initializes the command. */
   protected void init() throws Exception {}
 
+  /** Whether to NOT run the command. Override to true for dry-run commands. */
+  protected boolean dontRunCommand() {
+    return false;
+  }
+
   /** Returns the optional extra confirmation prompt for the command. */
   protected String prompt() throws Exception {
     return "";

java/google/registry/tools/DomainApplicationInfoCommand.java

@@ -25,7 +25,7 @@ import google.registry.tools.soy.DomainApplicationInfoSoyInfo;
 
 /** A command to execute a domain application info EPP command. */
 @Parameters(separators = " =", commandDescription = "Get domain application EPP info")
-final class DomainApplicationInfoCommand extends EppToolCommand {
+final class DomainApplicationInfoCommand extends NonMutatingEppToolCommand {
 
   @Parameter(
       names = {"-c", "--client"},

java/google/registry/tools/DomainCheckClaimsCommand.java

@@ -24,7 +24,7 @@ import java.util.List;
 
 /** A command to execute a domain check claims epp command. */
 @Parameters(separators = " =", commandDescription = "Check claims on domain(s)")
-final class DomainCheckClaimsCommand extends EppToolCommand {
+final class DomainCheckClaimsCommand extends NonMutatingEppToolCommand {
 
   @Parameter(
       names = {"-c", "--client"},

java/google/registry/tools/DomainCheckCommand.java

@@ -24,7 +24,7 @@ import java.util.List;
 
 /** A command to execute a domain check epp command. */
 @Parameters(separators = " =", commandDescription = "Check domain availability")
-final class DomainCheckCommand extends EppToolCommand {
+final class DomainCheckCommand extends NonMutatingEppToolCommand {
 
   @Parameter(
       names = {"-c", "--client"},

java/google/registry/tools/DomainCheckFeeCommand.java

@@ -24,7 +24,7 @@ import java.util.List;
 
 /** A command to execute a domain check fees epp command. */
 @Parameters(separators = " =", commandDescription = "Check domain fees (for a 1-year create)")
-final class DomainCheckFeeCommand extends EppToolCommand {
+final class DomainCheckFeeCommand extends NonMutatingEppToolCommand {
 
   @Parameter(
       names = {"-c", "--client"},

java/google/registry/tools/EppToolCommand.java

@@ -14,7 +14,6 @@
 
 package google.registry.tools;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Strings.nullToEmpty;
 import static com.google.common.collect.Maps.filterValues;
@@ -123,9 +122,8 @@ abstract class EppToolCommand extends ConfirmingCommand implements ServerSideCom
   }
 
   @Override
-  protected boolean checkExecutionState() throws Exception {
+  protected boolean dontRunCommand() {
-    checkArgument(!(force && isDryRun()), "--force and --dry_run are incompatible");
+    return isDryRun();
-    return true;
   }
 
   @Override

java/google/registry/tools/MutatingEppToolCommand.java

@@ -14,6 +14,8 @@
 
 package google.registry.tools;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import com.beust.jcommander.Parameter;
 
 /**
@@ -27,6 +29,12 @@ public abstract class MutatingEppToolCommand extends EppToolCommand {
       description = "Do not actually commit any mutations")
   boolean dryRun;
 
+  @Override
+  protected boolean checkExecutionState() throws Exception {
+    checkArgument(!(force && isDryRun()), "--force and --dry_run are incompatible");
+    return true;
+  }
+
   @Override
   protected boolean isDryRun() {
     return dryRun;

java/google/registry/tools/NonMutatingEppToolCommand.java

@@ -0,0 +1,32 @@
+// Copyright 2018 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.tools;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/** A non-mutating EPP command (that does not prompt for confirmation). */
+abstract class NonMutatingEppToolCommand extends EppToolCommand {
+
+  @Override
+  protected boolean isDryRun() {
+    return true;
+  }
+
+  @Override
+  protected boolean checkExecutionState() {
+    checkArgument(!force, "--force is unnecessary with non-mutating EPP tool commands");
+    return true;
+  }
+}

javatests/google/registry/tools/DomainApplicationInfoCommandTest.java

@@ -25,17 +25,17 @@ public class DomainApplicationInfoCommandTest
 
   @Test
   public void testSuccess() throws Exception {
-    runCommandForced("--client=NewRegistrar", "--domain_name=example.tld",
+    runCommand("--client=NewRegistrar", "--domain_name=example.tld",
         "--phase=landrush", "--id=123");
-    eppVerifier.verifySent("domain_info_landrush.xml");
+    eppVerifier.expectDryRun().verifySent("domain_info_landrush.xml");
   }
 
   @Test
   public void testSuccess_subphase() throws Exception {
     // Sunrush: phase=sunrise, subphase=landrush
-    runCommandForced("--client=NewRegistrar", "--domain_name=example.tld",
+    runCommand("--client=NewRegistrar", "--domain_name=example.tld",
         "--phase=sunrush", "--id=123");
-    eppVerifier.verifySent("domain_info_sunrush.xml");
+    eppVerifier.expectDryRun().verifySent("domain_info_sunrush.xml");
   }
 
   @Test
@@ -43,7 +43,7 @@ public class DomainApplicationInfoCommandTest
     assertThrows(
         IllegalArgumentException.class,
         () ->
-            runCommandForced(
+            runCommand(
                 "--client=NewRegistrar",
                 "--domain_name=example.tld",
                 "--phase=landrise",
@@ -54,14 +54,14 @@ public class DomainApplicationInfoCommandTest
   public void testFailure_missingClientId() throws Exception {
     assertThrows(
         ParameterException.class,
-        () -> runCommandForced("--domain_name=example.tld", "--phase=sunrush", "--id=123"));
+        () -> runCommand("--domain_name=example.tld", "--phase=sunrush", "--id=123"));
   }
 
   @Test
   public void testFailure_missingPhase() throws Exception {
     assertThrows(
         ParameterException.class,
-        () -> runCommandForced("--client=NewRegistrar", "--domain_name=example.tld", "--id=123"));
+        () -> runCommand("--client=NewRegistrar", "--domain_name=example.tld", "--id=123"));
   }
 
   @Test
@@ -69,7 +69,7 @@ public class DomainApplicationInfoCommandTest
     assertThrows(
         ParameterException.class,
         () ->
-            runCommandForced(
+            runCommand(
                 "--client=NewRegistrar", "--domain_name=example.tld", "--phase=landrush"));
   }
 
@@ -78,7 +78,7 @@ public class DomainApplicationInfoCommandTest
     assertThrows(
         ParameterException.class,
         () ->
-            runCommandForced(
+            runCommand(
                 "--client=NewRegistrar",
                 "--domain_name=example.tld",
                 "--phase=landrush",
@@ -91,7 +91,7 @@ public class DomainApplicationInfoCommandTest
     assertThrows(
         ParameterException.class,
         () ->
-            runCommandForced(
+            runCommand(
                 "--client=NewRegistrar",
                 "--domain_name=example.tld",
                 "--phase=landrush",

javatests/google/registry/tools/DomainCheckClaimsCommandTest.java

@@ -24,55 +24,57 @@ public class DomainCheckClaimsCommandTest extends EppToolCommandTestCase<DomainC
 
   @Test
   public void testSuccess() throws Exception {
-    runCommandForced("--client=NewRegistrar", "example.tld");
+    runCommand("--client=NewRegistrar", "example.tld");
-    eppVerifier.verifySent("domain_check_claims.xml");
+    eppVerifier.expectDryRun().verifySent("domain_check_claims.xml");
   }
 
   @Test
   public void testSuccess_multipleTlds() throws Exception {
-    runCommandForced("--client=NewRegistrar", "example.tld", "example.tld2");
+    runCommand("--client=NewRegistrar", "example.tld", "example.tld2");
     eppVerifier
+        .expectDryRun()
         .verifySent("domain_check_claims.xml")
         .verifySent("domain_check_claims_second_tld.xml");
   }
 
   @Test
   public void testSuccess_multipleDomains() throws Exception {
-    runCommandForced(
+    runCommand(
         "--client=NewRegistrar",
         "example.tld",
         "example2.tld",
         "example3.tld");
-    eppVerifier.verifySent("domain_check_claims_multiple.xml");
+    eppVerifier.expectDryRun().verifySent("domain_check_claims_multiple.xml");
   }
 
   @Test
   public void testSuccess_multipleDomainsAndTlds() throws Exception {
-    runCommandForced(
+    runCommand(
         "--client=NewRegistrar",
         "example.tld",
         "example2.tld",
         "example3.tld",
         "example.tld2");
     eppVerifier
+        .expectDryRun()
         .verifySent("domain_check_claims_multiple.xml")
         .verifySent("domain_check_claims_second_tld.xml");
   }
 
   @Test
   public void testFailure_missingClientId() throws Exception {
-    assertThrows(ParameterException.class, () -> runCommandForced("example.tld"));
+    assertThrows(ParameterException.class, () -> runCommand("example.tld"));
   }
 
   @Test
   public void testFailure_NoMainParameter() throws Exception {
-    assertThrows(ParameterException.class, () -> runCommandForced("--client=NewRegistrar"));
+    assertThrows(ParameterException.class, () -> runCommand("--client=NewRegistrar"));
   }
 
   @Test
   public void testFailure_unknownFlag() throws Exception {
     assertThrows(
         ParameterException.class,
-        () -> runCommandForced("--client=NewRegistrar", "--unrecognized=foo", "example.tld"));
+        () -> runCommand("--client=NewRegistrar", "--unrecognized=foo", "example.tld"));
   }
 }

javatests/google/registry/tools/DomainCheckCommandTest.java

@@ -24,55 +24,57 @@ public class DomainCheckCommandTest extends EppToolCommandTestCase<DomainCheckCo
 
   @Test
   public void testSuccess() throws Exception {
-    runCommandForced("--client=NewRegistrar", "example.tld");
+    runCommand("--client=NewRegistrar", "example.tld");
-    eppVerifier.verifySent("domain_check.xml");
+    eppVerifier.expectDryRun().verifySent("domain_check.xml");
   }
 
   @Test
   public void testSuccess_multipleTlds() throws Exception {
-    runCommandForced("--client=NewRegistrar", "example.tld", "example.tld2");
+    runCommand("--client=NewRegistrar", "example.tld", "example.tld2");
     eppVerifier
+        .expectDryRun()
         .verifySent("domain_check.xml")
         .verifySent("domain_check_second_tld.xml");
   }
 
   @Test
   public void testSuccess_multipleDomains() throws Exception {
-    runCommandForced(
+    runCommand(
         "--client=NewRegistrar",
         "example.tld",
         "example2.tld",
         "example3.tld");
-    eppVerifier.verifySent("domain_check_multiple.xml");
+    eppVerifier.expectDryRun().verifySent("domain_check_multiple.xml");
   }
 
   @Test
   public void testSuccess_multipleDomainsAndTlds() throws Exception {
-    runCommandForced(
+    runCommand(
         "--client=NewRegistrar",
         "example.tld",
         "example2.tld",
         "example3.tld",
         "example.tld2");
     eppVerifier
+        .expectDryRun()
         .verifySent("domain_check_multiple.xml")
         .verifySent("domain_check_second_tld.xml");
   }
 
   @Test
   public void testFailure_missingClientId() throws Exception {
-    assertThrows(ParameterException.class, () -> runCommandForced("example.tld"));
+    assertThrows(ParameterException.class, () -> runCommand("example.tld"));
   }
 
   @Test
   public void testFailure_NoMainParameter() throws Exception {
-    assertThrows(ParameterException.class, () -> runCommandForced("--client=NewRegistrar"));
+    assertThrows(ParameterException.class, () -> runCommand("--client=NewRegistrar"));
   }
 
   @Test
   public void testFailure_unknownFlag() throws Exception {
     assertThrows(
         ParameterException.class,
-        () -> runCommandForced("--client=NewRegistrar", "--unrecognized=foo", "example.tld"));
+        () -> runCommand("--client=NewRegistrar", "--unrecognized=foo", "example.tld"));
   }
 }

javatests/google/registry/tools/DomainCheckFeeCommandTest.java

@@ -24,55 +24,57 @@ public class DomainCheckFeeCommandTest extends EppToolCommandTestCase<DomainChec
 
   @Test
   public void testSuccess() throws Exception {
-    runCommandForced("--client=NewRegistrar", "example.tld");
+    runCommand("--client=NewRegistrar", "example.tld");
-    eppVerifier.verifySent("domain_check_fee.xml");
+    eppVerifier.expectDryRun().verifySent("domain_check_fee.xml");
   }
 
   @Test
   public void testSuccess_multipleTlds() throws Exception {
-    runCommandForced("--client=NewRegistrar", "example.tld", "example.tld2");
+    runCommand("--client=NewRegistrar", "example.tld", "example.tld2");
     eppVerifier
+        .expectDryRun()
         .verifySent("domain_check_fee.xml")
         .verifySent("domain_check_fee_second_tld.xml");
   }
 
   @Test
   public void testSuccess_multipleDomains() throws Exception {
-    runCommandForced(
+    runCommand(
         "--client=NewRegistrar",
         "example.tld",
         "example2.tld",
         "example3.tld");
-    eppVerifier.verifySent("domain_check_fee_multiple.xml");
+    eppVerifier.expectDryRun().verifySent("domain_check_fee_multiple.xml");
   }
 
   @Test
   public void testSuccess_multipleDomainsAndTlds() throws Exception {
-    runCommandForced(
+    runCommand(
         "--client=NewRegistrar",
         "example.tld",
         "example2.tld",
         "example3.tld",
         "example.tld2");
     eppVerifier
+        .expectDryRun()
         .verifySent("domain_check_fee_multiple.xml")
         .verifySent("domain_check_fee_second_tld.xml");
   }
 
   @Test
   public void testFailure_missingClientId() throws Exception {
-    assertThrows(ParameterException.class, () -> runCommandForced("example.tld"));
+    assertThrows(ParameterException.class, () -> runCommand("example.tld"));
   }
 
   @Test
   public void testFailure_NoMainParameter() throws Exception {
-    assertThrows(ParameterException.class, () -> runCommandForced("--client=NewRegistrar"));
+    assertThrows(ParameterException.class, () -> runCommand("--client=NewRegistrar"));
   }
 
   @Test
   public void testFailure_unknownFlag() throws Exception {
     assertThrows(
         ParameterException.class,
-        () -> runCommandForced("--client=NewRegistrar", "--unrecognized=foo", "example.tld"));
+        () -> runCommand("--client=NewRegistrar", "--unrecognized=foo", "example.tld"));
   }
 }

javatests/google/registry/tools/EppToolCommandTest.java

@@ -37,7 +37,7 @@ public class EppToolCommandTest extends EppToolCommandTestCase<EppToolCommand> {
     List<String> xmlPayloads;
 
     @Override
-    void initEppToolCommand() throws Exception {
+    void initEppToolCommand() {
       for (String xmlData : xmlPayloads) {
         addXmlCommand(clientId, xmlData);
       }
@@ -73,7 +73,7 @@ public class EppToolCommandTest extends EppToolCommandTestCase<EppToolCommand> {
   }
 
   @Test
-  public void testFailure_nonexistentClientId() throws Exception {
+  public void testFailure_nonexistentClientId() {
     IllegalArgumentException thrown =
         assertThrows(
             IllegalArgumentException.class,

javatests/google/registry/tools/MutatingEppToolCommandTest.java

@@ -0,0 +1,75 @@
+// Copyright 2018 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.tools;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.JUnitBackports.assertThrows;
+
+import com.beust.jcommander.Parameter;
+import com.beust.jcommander.Parameters;
+import google.registry.tools.server.ToolsTestData;
+import java.util.List;
+import org.junit.Test;
+
+/** Unit tests for {@link MutatingEppToolCommand}. */
+public class MutatingEppToolCommandTest extends EppToolCommandTestCase<MutatingEppToolCommand> {
+
+  /** Dummy implementation of MutatingEppToolCommand. */
+  @Parameters(separators = " =", commandDescription = "Dummy MutatingEppToolCommand")
+  static class TestMutatingEppToolCommand extends MutatingEppToolCommand {
+
+    @Parameter(names = {"--client"})
+    String clientId;
+
+    @Parameter
+    List<String> xmlPayloads;
+
+    @Override
+    protected void initMutatingEppToolCommand() {
+      for (String xmlData : xmlPayloads) {
+        addXmlCommand(clientId, xmlData);
+      }
+    }
+  }
+
+  @Override
+  protected MutatingEppToolCommand newCommandInstance() {
+    return new TestMutatingEppToolCommand();
+  }
+
+  @Test
+  public void testSuccess_dryrun() throws Exception {
+    // The choice of xml file is arbitrary.
+    runCommand(
+        "--client=NewRegistrar",
+        "--dry_run",
+        ToolsTestData.loadFile("contact_create.xml"));
+    eppVerifier.expectDryRun().verifySent("contact_create.xml");
+  }
+
+  @Test
+  public void testFailure_cantUseForceWithDryRun() {
+    Exception e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommand(
+                    "--force",
+                    "--dry_run",
+                    "--client=NewRegistrar",
+                    ToolsTestData.loadFile("domain_check.xml")));
+    assertThat(e).hasMessageThat().isEqualTo("--force and --dry_run are incompatible");
+  }
+}

javatests/google/registry/tools/NonMutatingEppToolCommandTest.java

@@ -0,0 +1,72 @@
+// Copyright 2018 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.tools;
+
+import static com.google.common.truth.Truth.assertThat;
+import static google.registry.testing.JUnitBackports.assertThrows;
+
+import com.beust.jcommander.Parameter;
+import com.beust.jcommander.Parameters;
+import google.registry.tools.server.ToolsTestData;
+import java.util.List;
+import org.junit.Test;
+
+/** Unit tests for {@link NonMutatingEppToolCommand}. */
+public class NonMutatingEppToolCommandTest
+    extends EppToolCommandTestCase<NonMutatingEppToolCommand> {
+
+  /** Dummy implementation of NonMutatingEppToolCommand. */
+  @Parameters(separators = " =", commandDescription = "Dummy NonMutatingEppToolCommand")
+  static class TestNonMutatingEppToolCommand extends NonMutatingEppToolCommand {
+
+    @Parameter(names = {"--client"})
+    String clientId;
+
+    @Parameter
+    List<String> xmlPayloads;
+
+    @Override
+    void initEppToolCommand() {
+      for (String xmlData : xmlPayloads) {
+        addXmlCommand(clientId, xmlData);
+      }
+    }
+  }
+
+  @Override
+  protected NonMutatingEppToolCommand newCommandInstance() {
+    return new TestNonMutatingEppToolCommand();
+  }
+
+  @Test
+  public void testSuccess_doesntPrompt_whenNotForced() throws Exception {
+    // The choice of xml file is arbitrary.
+    runCommand("--client=NewRegistrar", ToolsTestData.loadFile("domain_check.xml"));
+    eppVerifier.expectDryRun().verifySent("domain_check.xml");
+  }
+
+  @Test
+  public void testFailure_doesntAllowForce() {
+    Exception e =
+        assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                runCommand(
+                    "--force",
+                    "--client=NewRegistrar",
+                    ToolsTestData.loadFile("domain_check.xml")));
+    assertThat(e).hasMessageThat().contains("--force is unnecessary");
+  }
+}