diff --git a/java/google/registry/tools/ConfirmingCommand.java b/java/google/registry/tools/ConfirmingCommand.java index ecb3e11bc..f2d00c5cb 100644 --- a/java/google/registry/tools/ConfirmingCommand.java +++ b/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 ""; diff --git a/java/google/registry/tools/DomainApplicationInfoCommand.java b/java/google/registry/tools/DomainApplicationInfoCommand.java index 777413bc0..b6d71ab17 100644 --- a/java/google/registry/tools/DomainApplicationInfoCommand.java +++ b/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"}, diff --git a/java/google/registry/tools/DomainCheckClaimsCommand.java b/java/google/registry/tools/DomainCheckClaimsCommand.java index a205e9d0d..0536b42e4 100644 --- a/java/google/registry/tools/DomainCheckClaimsCommand.java +++ b/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"}, diff --git a/java/google/registry/tools/DomainCheckCommand.java b/java/google/registry/tools/DomainCheckCommand.java index 7ecb6b5f3..381511df2 100644 --- a/java/google/registry/tools/DomainCheckCommand.java +++ b/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"}, diff --git a/java/google/registry/tools/DomainCheckFeeCommand.java b/java/google/registry/tools/DomainCheckFeeCommand.java index 6106e91d9..8f9182a26 100644 --- a/java/google/registry/tools/DomainCheckFeeCommand.java +++ b/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"}, diff --git a/java/google/registry/tools/EppToolCommand.java b/java/google/registry/tools/EppToolCommand.java index f0ba0e049..957e32dcd 100644 --- a/java/google/registry/tools/EppToolCommand.java +++ b/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 { - checkArgument(!(force && isDryRun()), "--force and --dry_run are incompatible"); - return true; + protected boolean dontRunCommand() { + return isDryRun(); } @Override diff --git a/java/google/registry/tools/MutatingEppToolCommand.java b/java/google/registry/tools/MutatingEppToolCommand.java index 33b802ae1..90410a8e2 100644 --- a/java/google/registry/tools/MutatingEppToolCommand.java +++ b/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; diff --git a/java/google/registry/tools/NonMutatingEppToolCommand.java b/java/google/registry/tools/NonMutatingEppToolCommand.java new file mode 100644 index 000000000..4791fe669 --- /dev/null +++ b/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; + } +} diff --git a/javatests/google/registry/tools/DomainApplicationInfoCommandTest.java b/javatests/google/registry/tools/DomainApplicationInfoCommandTest.java index 49da69c6b..120abfd51 100644 --- a/javatests/google/registry/tools/DomainApplicationInfoCommandTest.java +++ b/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", diff --git a/javatests/google/registry/tools/DomainCheckClaimsCommandTest.java b/javatests/google/registry/tools/DomainCheckClaimsCommandTest.java index eb9a293e8..f4d964662 100644 --- a/javatests/google/registry/tools/DomainCheckClaimsCommandTest.java +++ b/javatests/google/registry/tools/DomainCheckClaimsCommandTest.java @@ -24,55 +24,57 @@ public class DomainCheckClaimsCommandTest extends EppToolCommandTestCase 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")); } } diff --git a/javatests/google/registry/tools/DomainCheckCommandTest.java b/javatests/google/registry/tools/DomainCheckCommandTest.java index aae879b11..4e2918410 100644 --- a/javatests/google/registry/tools/DomainCheckCommandTest.java +++ b/javatests/google/registry/tools/DomainCheckCommandTest.java @@ -24,55 +24,57 @@ public class DomainCheckCommandTest extends EppToolCommandTestCase 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")); } } diff --git a/javatests/google/registry/tools/DomainCheckFeeCommandTest.java b/javatests/google/registry/tools/DomainCheckFeeCommandTest.java index ef829b054..53fb720fe 100644 --- a/javatests/google/registry/tools/DomainCheckFeeCommandTest.java +++ b/javatests/google/registry/tools/DomainCheckFeeCommandTest.java @@ -24,55 +24,57 @@ public class DomainCheckFeeCommandTest extends EppToolCommandTestCase 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")); } } diff --git a/javatests/google/registry/tools/EppToolCommandTest.java b/javatests/google/registry/tools/EppToolCommandTest.java index 1d8162195..f20da8819 100644 --- a/javatests/google/registry/tools/EppToolCommandTest.java +++ b/javatests/google/registry/tools/EppToolCommandTest.java @@ -37,7 +37,7 @@ public class EppToolCommandTest extends EppToolCommandTestCase { List xmlPayloads; @Override - void initEppToolCommand() throws Exception { + void initEppToolCommand() { for (String xmlData : xmlPayloads) { addXmlCommand(clientId, xmlData); } @@ -73,7 +73,7 @@ public class EppToolCommandTest extends EppToolCommandTestCase { } @Test - public void testFailure_nonexistentClientId() throws Exception { + public void testFailure_nonexistentClientId() { IllegalArgumentException thrown = assertThrows( IllegalArgumentException.class, diff --git a/javatests/google/registry/tools/MutatingEppToolCommandTest.java b/javatests/google/registry/tools/MutatingEppToolCommandTest.java new file mode 100644 index 000000000..4c5be0e81 --- /dev/null +++ b/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 { + + /** Dummy implementation of MutatingEppToolCommand. */ + @Parameters(separators = " =", commandDescription = "Dummy MutatingEppToolCommand") + static class TestMutatingEppToolCommand extends MutatingEppToolCommand { + + @Parameter(names = {"--client"}) + String clientId; + + @Parameter + List 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"); + } +} diff --git a/javatests/google/registry/tools/NonMutatingEppToolCommandTest.java b/javatests/google/registry/tools/NonMutatingEppToolCommandTest.java new file mode 100644 index 000000000..d11ad7fd7 --- /dev/null +++ b/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 { + + /** Dummy implementation of NonMutatingEppToolCommand. */ + @Parameters(separators = " =", commandDescription = "Dummy NonMutatingEppToolCommand") + static class TestNonMutatingEppToolCommand extends NonMutatingEppToolCommand { + + @Parameter(names = {"--client"}) + String clientId; + + @Parameter + List 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"); + } +}