From 21976bee249851d421164e1189d5027d5d5b2319 Mon Sep 17 00:00:00 2001 From: Ben McIlwain Date: Tue, 14 Jul 2026 13:52:04 -0400 Subject: [PATCH] Support dry-run and build-environment flags in Nomulus creation commands (#3149) When checking in a brand new premium list (production/*.txt) or reserved list (reserved/production/*.txt), presubmit testing commands in gradle-runner.sh fall back from update_premium_list / update_reserved_list (which require existing database entities) to create_premium_list / create_reserved_list. However, because --dry_run (-d) and --build_environment flags were previously only defined in UpdatePremiumListCommand and UpdateReservedListCommand, passing them during non-interactive presubmit creation validation resulted in ParameterException (Was passed main parameter '-d' but no main parameter was defined...) or NullPointerException when ConfirmingCommand attempted to prompt on a null system console. This change moves --dry_run and --build_environment parameters and dontRunCommand() behavior up into CreateOrUpdatePremiumListCommand and CreateOrUpdateReservedListCommand, enabling safe dry-run presubmit validation for newly created lists in CI environments. BUG= http://b/534511633, http://b/529397845 --- .../CreateOrUpdatePremiumListCommand.java | 27 ++++++++++++++++++ .../CreateOrUpdateReservedListCommand.java | 28 +++++++++++++++++++ .../tools/UpdatePremiumListCommand.java | 17 ----------- .../tools/UpdateReservedListCommand.java | 19 ------------- .../tools/CreatePremiumListCommandTest.java | 7 +++++ .../tools/CreateReservedListCommandTest.java | 7 +++++ .../tools/UpdatePremiumListCommandTest.java | 4 +-- .../tools/UpdateReservedListCommandTest.java | 4 +-- 8 files changed, 71 insertions(+), 42 deletions(-) diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdatePremiumListCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdatePremiumListCommand.java index 170b92577..249dd83f3 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdatePremiumListCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdatePremiumListCommand.java @@ -14,6 +14,7 @@ package google.registry.tools; +import static com.google.common.base.Preconditions.checkArgument; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import com.beust.jcommander.Parameter; @@ -39,6 +40,19 @@ abstract class CreateOrUpdatePremiumListCommand extends ConfirmingCommand { protected List inputData; protected CurrencyUnit currency; + @Parameter( + names = {"-d", "--dry_run"}, + description = "Does not execute the entity mutation") + boolean dryRun; + + @Parameter( + names = {"--build_environment"}, + description = + "DO NOT USE THIS FLAG ON THE COMMAND LINE! This flag indicates the command is being run" + + " by the build environment tools. This flag should never be used by a human user" + + " from the command line.") + boolean buildEnv; + @Nullable @Parameter( names = {"-n", "--name"}, @@ -68,4 +82,17 @@ abstract class CreateOrUpdatePremiumListCommand extends ConfirmingCommand { } return message; } + + @Override + protected boolean dontRunCommand() { + return dryRun; + } + + @Override + protected boolean checkExecutionState() { + checkArgument( + !RegistryToolEnvironment.get().equals(RegistryToolEnvironment.PRODUCTION) || buildEnv, + "The --build_environment flag must be used when running in production"); + return super.checkExecutionState(); + } } diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java index a8fe390f9..54837e6e8 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateReservedListCommand.java @@ -14,6 +14,8 @@ package google.registry.tools; +import static com.google.common.base.Preconditions.checkArgument; + import com.beust.jcommander.Parameter; import com.google.common.flogger.FluentLogger; import google.registry.model.tld.label.ReservedList; @@ -35,6 +37,19 @@ public abstract class CreateOrUpdateReservedListCommand extends ConfirmingComman static final FluentLogger logger = FluentLogger.forEnclosingClass(); + @Parameter( + names = {"-d", "--dry_run"}, + description = "Does not execute the entity mutation") + boolean dryRun; + + @Parameter( + names = {"--build_environment"}, + description = + "DO NOT USE THIS FLAG ON THE COMMAND LINE! This flag indicates the command is being run" + + " by the build environment tools. This flag should never be used by a human user" + + " from the command line.") + boolean buildEnv; + @Nullable @Parameter( names = {"-n", "--name"}, @@ -74,4 +89,17 @@ public abstract class CreateOrUpdateReservedListCommand extends ConfirmingComman .collect(Collectors.joining(", ")) + "]"; } + + @Override + protected boolean dontRunCommand() { + return dryRun; + } + + @Override + protected boolean checkExecutionState() { + checkArgument( + !RegistryToolEnvironment.get().equals(RegistryToolEnvironment.PRODUCTION) || buildEnv, + "The --build_environment flag must be used when running in production"); + return super.checkExecutionState(); + } } diff --git a/core/src/main/java/google/registry/tools/UpdatePremiumListCommand.java b/core/src/main/java/google/registry/tools/UpdatePremiumListCommand.java index 43e24195e..e5813ecc1 100644 --- a/core/src/main/java/google/registry/tools/UpdatePremiumListCommand.java +++ b/core/src/main/java/google/registry/tools/UpdatePremiumListCommand.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static google.registry.util.ListNamingUtils.convertFilePathToName; import static java.nio.charset.StandardCharsets.UTF_8; -import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.google.common.base.Strings; import google.registry.model.tld.label.PremiumList; @@ -30,27 +29,11 @@ import java.nio.file.Files; @Parameters(separators = " =", commandDescription = "Update a PremiumList in Database.") class UpdatePremiumListCommand extends CreateOrUpdatePremiumListCommand { - @Parameter( - names = {"-d", "--dry_run"}, - description = "Does not execute the entity mutation") - boolean dryRun; - - @Parameter( - names = {"--build_environment"}, - description = - "DO NOT USE THIS FLAG ON THE COMMAND LINE! This flag indicates the command is being run" - + " by the build environment tools. This flag should never be used by a human user" - + " from the command line.") - boolean buildEnv; - // Indicates if there is a new change made by this command private boolean newChange = false; @Override protected String prompt() throws Exception { - checkArgument( - !RegistryToolEnvironment.get().equals(RegistryToolEnvironment.PRODUCTION) || buildEnv, - "The --build_environment flag must be used when running update_premium_list in production"); name = Strings.isNullOrEmpty(name) ? convertFilePathToName(inputFile) : name; PremiumList existingList = PremiumListDao.getLatestRevision(name) diff --git a/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java b/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java index b3f22c57a..88bcc82a1 100644 --- a/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java +++ b/core/src/main/java/google/registry/tools/UpdateReservedListCommand.java @@ -14,12 +14,10 @@ package google.registry.tools; -import static com.google.common.base.Preconditions.checkArgument; import static google.registry.util.DiffUtils.prettyPrintEntityDeepDiff; import static google.registry.util.ListNamingUtils.convertFilePathToName; import static java.nio.charset.StandardCharsets.UTF_8; -import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.google.common.base.Strings; import google.registry.model.tld.label.ReservedList; @@ -30,28 +28,11 @@ import java.util.List; @Parameters(separators = " =", commandDescription = "Update a ReservedList.") final class UpdateReservedListCommand extends CreateOrUpdateReservedListCommand { - @Parameter( - names = {"-d", "--dry_run"}, - description = "Does not execute the entity mutation") - boolean dryRun; - - @Parameter( - names = {"--build_environment"}, - description = - "DO NOT USE THIS FLAG ON THE COMMAND LINE! This flag indicates the command is being run" - + " by the build environment tools. This flag should never be used by a human user" - + " from the command line.") - boolean buildEnv; - // indicates if there is a new change made by this command private boolean newChange = true; @Override protected String prompt() throws Exception { - checkArgument( - !RegistryToolEnvironment.get().equals(RegistryToolEnvironment.PRODUCTION) || buildEnv, - "The --build_environment flag must be used when running update_reserved_list in" - + " production"); name = Strings.isNullOrEmpty(name) ? convertFilePathToName(input) : name; ReservedList existingReservedList = ReservedList.get(name) diff --git a/core/src/test/java/google/registry/tools/CreatePremiumListCommandTest.java b/core/src/test/java/google/registry/tools/CreatePremiumListCommandTest.java index ace6ce512..8e713ee95 100644 --- a/core/src/test/java/google/registry/tools/CreatePremiumListCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreatePremiumListCommandTest.java @@ -119,4 +119,11 @@ class CreatePremiumListCommandTest + "yet TLD %s does not exist", fileName)); } + + @Test + void testDryRun_doesNotCreateList() throws Exception { + runCommandForced( + "--name=" + TLD_TEST, "--input=" + premiumTermsPath, "--currency=USD", "--dry_run"); + assertThat(PremiumListDao.getLatestRevision(TLD_TEST)).isEmpty(); + } } diff --git a/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java b/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java index e9ffb09b4..70d6e1202 100644 --- a/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateReservedListCommandTest.java @@ -180,4 +180,11 @@ class CreateReservedListCommandTest command.init(); assertThat(command.prompt()).contains("reservedListMap=[]"); } + + @Test + void testDryRun_doesNotCreateList() throws Exception { + runCommandForced( + "--name=xn--q9jyb4c_common-reserved", "--input=" + reservedTermsPath, "--dry_run"); + assertThat(ReservedList.get("xn--q9jyb4c_common-reserved")).isEmpty(); + } } diff --git a/core/src/test/java/google/registry/tools/UpdatePremiumListCommandTest.java b/core/src/test/java/google/registry/tools/UpdatePremiumListCommandTest.java index c494ae2dd..73ca3e2d1 100644 --- a/core/src/test/java/google/registry/tools/UpdatePremiumListCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdatePremiumListCommandTest.java @@ -209,9 +209,7 @@ class UpdatePremiumListCommandTest "--name=" + TLD_TEST, "--input=" + Paths.get(tmpFile.getPath()))); assertThat(thrown.getMessage()) - .isEqualTo( - "The --build_environment flag must be used when running update_premium_list in" - + " production"); + .isEqualTo("The --build_environment flag must be used when running in production"); } @Test diff --git a/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java b/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java index bdc85b255..5593d5694 100644 --- a/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java +++ b/core/src/test/java/google/registry/tools/UpdateReservedListCommandTest.java @@ -139,9 +139,7 @@ class UpdateReservedListCommandTest "--name=xn--q9jyb4c_common-reserved", "--input=" + reservedTermsPath)); assertThat(thrown.getMessage()) - .isEqualTo( - "The --build_environment flag must be used when running update_reserved_list in" - + " production"); + .isEqualTo("The --build_environment flag must be used when running in production"); } @Test