From fc1857717d8298d0fad6063cbcd258c01a29c4a5 Mon Sep 17 00:00:00 2001 From: sarahcaseybot Date: Tue, 19 Sep 2023 12:11:18 -0400 Subject: [PATCH] Use PrintStream in ConfirmingCommand (#2140) * Use PrintStream in ConfirmingCommand * Add errorPrintStream * remove unneccesary line --- .../registry/tools/ConfirmingCommand.java | 31 ++++++++++++++----- .../registry/tools/CreateDomainCommand.java | 2 +- .../tools/CreateOrUpdateTldCommand.java | 21 +++++++------ .../tools/DeleteAllocationTokensCommand.java | 2 +- .../registry/tools/LoadTestCommand.java | 9 +++--- .../tools/LockOrUnlockDomainCommand.java | 2 +- .../registry/tools/UnrenewDomainCommand.java | 19 +++++++----- .../tools/UpdateAllocationTokensCommand.java | 2 +- ...eCancellationsForBillingEventsCommand.java | 10 +++--- .../registry/tools/CommandTestCase.java | 2 +- .../tools/CreateDomainCommandTest.java | 1 + .../tools/CreateRegistrarCommandTest.java | 1 + .../CreateRegistrarGroupsCommandTest.java | 1 + .../registry/tools/CreateTldCommandTest.java | 1 + .../DeleteAllocationTokensCommandTest.java | 1 + .../registry/tools/LoadTestCommandTest.java | 2 ++ .../registry/tools/LockDomainCommandTest.java | 1 + .../UniformRapidSuspensionCommandTest.java | 1 + .../tools/UnlockDomainCommandTest.java | 1 + .../tools/UnrenewDomainCommandTest.java | 2 ++ .../registry/tools/UpdateTldCommandTest.java | 1 + ...cellationsForBillingEventsCommandTest.java | 4 +++ 22 files changed, 79 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/google/registry/tools/ConfirmingCommand.java b/core/src/main/java/google/registry/tools/ConfirmingCommand.java index 1cb8fe37a..38c562af2 100644 --- a/core/src/main/java/google/registry/tools/ConfirmingCommand.java +++ b/core/src/main/java/google/registry/tools/ConfirmingCommand.java @@ -15,9 +15,12 @@ package google.registry.tools; import static google.registry.tools.CommandUtilities.promptForYes; +import static java.nio.charset.StandardCharsets.UTF_8; import com.beust.jcommander.Parameter; import com.google.common.base.Strings; +import java.io.PrintStream; +import java.io.UnsupportedEncodingException; /** A {@link Command} that implements a confirmation step before executing. */ public abstract class ConfirmingCommand implements Command { @@ -27,23 +30,37 @@ public abstract class ConfirmingCommand implements Command { description = "Do not prompt before executing") boolean force; + public PrintStream printStream; + public PrintStream errorPrintStream; + + protected ConfirmingCommand() { + try { + printStream = new PrintStream(System.out, false, UTF_8.name()); + errorPrintStream = new PrintStream(System.err, false, UTF_8.name()); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException(e); + } + } + @Override public final void run() throws Exception { if (checkExecutionState()) { init(); - printLineIfNotEmpty(prompt()); + printLineIfNotEmpty(prompt(), printStream); 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("Running ... "); - System.out.println(execute()); - printLineIfNotEmpty(postExecute()); + printStream.println("Running ... "); + printStream.println(execute()); + printLineIfNotEmpty(postExecute(), printStream); } else { - System.out.println("Command aborted."); + printStream.println("Command aborted."); } } + printStream.close(); + errorPrintStream.close(); } /** Run any pre-execute command checks and return true if they all pass. */ @@ -76,9 +93,9 @@ public abstract class ConfirmingCommand implements Command { } /** Prints the provided text with a trailing newline, if text is not null or empty. */ - private static void printLineIfNotEmpty(String text) { + private static void printLineIfNotEmpty(String text, PrintStream printStream) { if (!Strings.isNullOrEmpty(text)) { - System.out.println(text); + printStream.println(text); } } } diff --git a/core/src/main/java/google/registry/tools/CreateDomainCommand.java b/core/src/main/java/google/registry/tools/CreateDomainCommand.java index 5d077b3aa..48f8a7032 100644 --- a/core/src/main/java/google/registry/tools/CreateDomainCommand.java +++ b/core/src/main/java/google/registry/tools/CreateDomainCommand.java @@ -78,7 +78,7 @@ final class CreateDomainCommand extends CreateOrUpdateDomainCommand { Money createCost = prices.getCreateCost(); currency = createCost.getCurrencyUnit().getCode(); cost = createCost.multipliedBy(period).getAmount().toString(); - System.out.printf( + printStream.printf( "NOTE: %s is premium at %s per year; sending total cost for %d year(s) of %s %s.\n", domain, createCost, period, currency, cost); } diff --git a/core/src/main/java/google/registry/tools/CreateOrUpdateTldCommand.java b/core/src/main/java/google/registry/tools/CreateOrUpdateTldCommand.java index 9f6caeb89..ae5fc4a00 100644 --- a/core/src/main/java/google/registry/tools/CreateOrUpdateTldCommand.java +++ b/core/src/main/java/google/registry/tools/CreateOrUpdateTldCommand.java @@ -39,6 +39,7 @@ import google.registry.tools.params.OptionalStringParameter; import google.registry.tools.params.StringListParameter; import google.registry.tools.params.TransitionListParameter.BillingCostTransitions; import google.registry.tools.params.TransitionListParameter.TldStateTransitions; +import java.io.UnsupportedEncodingException; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -233,12 +234,11 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand { @Nullable @Parameter( - names = {"--num_dns_publish_locks"}, - description = - "The number of publish locks we allow in parallel for DNS updates under this tld " - + "(1 for TLD-wide locks)", - arity = 1 - ) + names = {"--num_dns_publish_locks"}, + description = + "The number of publish locks we allow in parallel for DNS updates under this tld " + + "(1 for TLD-wide locks)", + arity = 1) Integer numDnsPublishShards; @Nullable @@ -301,7 +301,7 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand { protected abstract void initTldCommand(); @Override - protected final void init() { + protected final void init() throws UnsupportedEncodingException { assertAllowedEnvironment(); initTldCommand(); String duplicates = Joiner.on(", ").join(findDuplicates(mainParameters)); @@ -360,7 +360,7 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand { if (!renewBillingCostTransitions.isEmpty()) { // TODO(b/20764952): need invoicing support for multiple renew billing costs. if (renewBillingCostTransitions.size() > 1) { - System.err.println( + errorPrintStream.println( "----------------------\n" + "WARNING: Do not set multiple renew cost transitions " + "until b/20764952 is fixed.\n" @@ -463,7 +463,8 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand { } } - private void checkReservedListValidityForTld(String tld, Set reservedListNames) { + private void checkReservedListValidityForTld(String tld, Set reservedListNames) + throws UnsupportedEncodingException { ImmutableList.Builder builder = new ImmutableList.Builder<>(); for (String reservedListName : reservedListNames) { if (!reservedListName.startsWith("common_") && !reservedListName.startsWith(tld + "_")) { @@ -476,7 +477,7 @@ abstract class CreateOrUpdateTldCommand extends MutatingCommand { Joiner.on(", ").join(invalidNames), tld); if (overrideReservedListRules) { - System.err.println("Error overridden: " + errMsg); + errorPrintStream.println("Error overridden: " + errMsg); } else { throw new IllegalArgumentException(errMsg); } diff --git a/core/src/main/java/google/registry/tools/DeleteAllocationTokensCommand.java b/core/src/main/java/google/registry/tools/DeleteAllocationTokensCommand.java index ce3fde830..ab5520c79 100644 --- a/core/src/main/java/google/registry/tools/DeleteAllocationTokensCommand.java +++ b/core/src/main/java/google/registry/tools/DeleteAllocationTokensCommand.java @@ -85,7 +85,7 @@ final class DeleteAllocationTokensCommand extends UpdateOrDeleteAllocationTokens if (!dryRun) { tm().delete(tokensToDelete); } - System.out.printf( + printStream.printf( "%s tokens: %s\n", dryRun ? "Would delete" : "Deleted", JOINER.join(tokensToDelete.stream().map(VKey::getKey).sorted().collect(toImmutableList()))); diff --git a/core/src/main/java/google/registry/tools/LoadTestCommand.java b/core/src/main/java/google/registry/tools/LoadTestCommand.java index 192f23879..7a7a1a398 100644 --- a/core/src/main/java/google/registry/tools/LoadTestCommand.java +++ b/core/src/main/java/google/registry/tools/LoadTestCommand.java @@ -14,6 +14,7 @@ package google.registry.tools; + import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.google.common.collect.ImmutableMap; @@ -86,17 +87,17 @@ class LoadTestCommand extends ConfirmingCommand implements CommandWithConnection @Override protected boolean checkExecutionState() { if (RegistryToolEnvironment.get() == RegistryToolEnvironment.PRODUCTION) { - System.err.println("You may not run a load test against production."); + errorPrintStream.println("You may not run a load test against production."); return false; } // Check validity of TLD and Client Id. if (!Tlds.getTlds().contains(tld)) { - System.err.printf("No such TLD: %s\n", tld); + errorPrintStream.printf("No such TLD: %s\n", tld); return false; } if (!Registrar.loadByRegistrarId(clientId).isPresent()) { - System.err.printf("No such client: %s\n", clientId); + errorPrintStream.printf("No such client: %s\n", clientId); return false; } @@ -112,7 +113,7 @@ class LoadTestCommand extends ConfirmingCommand implements CommandWithConnection @Override protected String execute() throws Exception { - System.err.println("Initiating load test..."); + errorPrintStream.println("Initiating load test..."); ImmutableMap params = new ImmutableMap.Builder() .put("tld", tld) diff --git a/core/src/main/java/google/registry/tools/LockOrUnlockDomainCommand.java b/core/src/main/java/google/registry/tools/LockOrUnlockDomainCommand.java index f692cffe5..9e17d59af 100644 --- a/core/src/main/java/google/registry/tools/LockOrUnlockDomainCommand.java +++ b/core/src/main/java/google/registry/tools/LockOrUnlockDomainCommand.java @@ -71,7 +71,7 @@ public abstract class LockOrUnlockDomainCommand extends ConfirmingCommand { } String duplicates = Joiner.on(", ").join(findDuplicates(mainParameters)); checkArgument(duplicates.isEmpty(), "Duplicate domain arguments found: '%s'", duplicates); - System.out.println( + printStream.println( "== ENSURE THAT YOU HAVE AUTHENTICATED THE REGISTRAR BEFORE RUNNING THIS COMMAND =="); } diff --git a/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java b/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java index e0af7a18c..0bf1d6bbc 100644 --- a/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java +++ b/core/src/main/java/google/registry/tools/UnrenewDomainCommand.java @@ -42,6 +42,7 @@ import google.registry.model.poll.PollMessage; import google.registry.model.reporting.HistoryEntry.Type; import google.registry.util.Clock; import google.registry.util.NonFinalForTesting; +import java.io.UnsupportedEncodingException; import java.util.List; import java.util.Optional; import javax.inject.Inject; @@ -76,7 +77,7 @@ class UnrenewDomainCommand extends ConfirmingCommand { StatusValue.SERVER_UPDATE_PROHIBITED); @Override - protected void init() { + protected void init() throws UnsupportedEncodingException { checkArgument(period >= 1 && period <= 9, "Period must be in the range 1-9"); DateTime now = clock.nowUtc(); ImmutableSet.Builder domainsNonexistentBuilder = new ImmutableSet.Builder<>(); @@ -116,20 +117,24 @@ class UnrenewDomainCommand extends ConfirmingCommand { && domainsDeleting.isEmpty() && domainsWithDisallowedStatuses.isEmpty() && domainsExpiringTooSoon.isEmpty()); + if (foundInvalidDomains) { - System.err.print("Found domains that cannot be unrenewed for the following reasons:\n\n"); + errorPrintStream.print( + "Found domains that cannot be unrenewed for the following reasons:\n\n"); } if (!domainsNonexistent.isEmpty()) { - System.err.printf("Domains that don't exist: %s\n\n", domainsNonexistent); + errorPrintStream.printf("Domains that don't exist: %s\n\n", domainsNonexistent); } if (!domainsDeleting.isEmpty()) { - System.err.printf("Domains that are deleted or pending delete: %s\n\n", domainsDeleting); + errorPrintStream.printf( + "Domains that are deleted or pending delete: %s\n\n", domainsDeleting); } if (!domainsWithDisallowedStatuses.isEmpty()) { - System.err.printf("Domains with disallowed statuses: %s\n\n", domainsWithDisallowedStatuses); + errorPrintStream.printf( + "Domains with disallowed statuses: %s\n\n", domainsWithDisallowedStatuses); } if (!domainsExpiringTooSoon.isEmpty()) { - System.err.printf("Domains expiring too soon: %s\n\n", domainsExpiringTooSoon); + errorPrintStream.printf("Domains expiring too soon: %s\n\n", domainsExpiringTooSoon); } checkArgument(!foundInvalidDomains, "Aborting because some domains cannot be unrenewed"); } @@ -154,7 +159,7 @@ class UnrenewDomainCommand extends ConfirmingCommand { protected String execute() { for (String domainName : mainParameters) { tm().transact(() -> unrenewDomain(domainName)); - System.out.printf("Unrenewed %s\n", domainName); + printStream.printf("Unrenewed %s\n", domainName); } return "Successfully unrenewed all domains."; } diff --git a/core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java b/core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java index 9435dd884..0e7228428 100644 --- a/core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java +++ b/core/src/main/java/google/registry/tools/UpdateAllocationTokensCommand.java @@ -208,7 +208,7 @@ final class UpdateAllocationTokensCommand extends UpdateOrDeleteAllocationTokens if (!dryRun) { tm().putAll(batch); } - System.out.printf( + printStream.printf( "%s tokens: %s\n", dryRun ? "Would update" : "Updated", JOINER.join( diff --git a/core/src/main/java/google/registry/tools/javascrap/CreateCancellationsForBillingEventsCommand.java b/core/src/main/java/google/registry/tools/javascrap/CreateCancellationsForBillingEventsCommand.java index 52e540e19..42f4f111d 100644 --- a/core/src/main/java/google/registry/tools/javascrap/CreateCancellationsForBillingEventsCommand.java +++ b/core/src/main/java/google/registry/tools/javascrap/CreateCancellationsForBillingEventsCommand.java @@ -70,14 +70,14 @@ public class CreateCancellationsForBillingEventsCommand extends ConfirmingComman } }); billingEventsToCancel = billingEventsBuilder.build(); - System.out.printf("Found %d BillingEvent(s) to cancel\n", billingEventsToCancel.size()); + printStream.printf("Found %d BillingEvent(s) to cancel\n", billingEventsToCancel.size()); ImmutableSet missingIds = missingIdsBuilder.build(); if (!missingIds.isEmpty()) { - System.out.printf("Missing BillingEvent(s) for IDs %s\n", missingIds); + printStream.printf("Missing BillingEvent(s) for IDs %s\n", missingIds); } ImmutableSet alreadyCancelledIds = alreadyCancelledIdsBuilder.build(); if (!alreadyCancelledIds.isEmpty()) { - System.out.printf( + printStream.printf( "The following BillingEvent IDs were already cancelled: %s\n", alreadyCancelledIds); } } @@ -96,7 +96,7 @@ public class CreateCancellationsForBillingEventsCommand extends ConfirmingComman tm().transact( () -> { if (alreadyCancelled(billingEvent)) { - System.out.printf( + printStream.printf( "BillingEvent %d already cancelled, this is unexpected.\n", billingEvent.getId()); return 0; @@ -111,7 +111,7 @@ public class CreateCancellationsForBillingEventsCommand extends ConfirmingComman .setReason(BillingBase.Reason.ERROR) .setTargetId(billingEvent.getTargetId()) .build()); - System.out.printf( + printStream.printf( "Added BillingCancellation for BillingEvent with ID %d\n", billingEvent.getId()); return 1; diff --git a/core/src/test/java/google/registry/tools/CommandTestCase.java b/core/src/test/java/google/registry/tools/CommandTestCase.java index dfc257ccc..f8f8fa7ff 100644 --- a/core/src/test/java/google/registry/tools/CommandTestCase.java +++ b/core/src/test/java/google/registry/tools/CommandTestCase.java @@ -198,7 +198,7 @@ public abstract class CommandTestCase { } void assertInStderr(String... expected) { - String stderror = new String(stderr.toByteArray(), UTF_8); + String stderror = getStderrAsString(); for (String line : expected) { assertThat(stderror).contains(line); } diff --git a/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java b/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java index 86890f5ad..4816561f9 100644 --- a/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateDomainCommandTest.java @@ -38,6 +38,7 @@ class CreateDomainCommandTest extends EppToolCommandTestCase 2048, ImmutableSet.of("secp256r1", "secp384r1"), fakeClock); + command.printStream = System.out; } @Test diff --git a/core/src/test/java/google/registry/tools/CreateRegistrarGroupsCommandTest.java b/core/src/test/java/google/registry/tools/CreateRegistrarGroupsCommandTest.java index 06176b5bf..860af84b7 100644 --- a/core/src/test/java/google/registry/tools/CreateRegistrarGroupsCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateRegistrarGroupsCommandTest.java @@ -37,6 +37,7 @@ class CreateRegistrarGroupsCommandTest extends CommandTestCase { @Test void testSuccess_setCommonAndReservedListFromOtherTld_withOverride() throws Exception { + command.errorPrintStream = System.err; runReservedListsTestOverride("common_abuse,tld_banned"); String errMsg = "Error overridden: The reserved list(s) tld_banned " diff --git a/core/src/test/java/google/registry/tools/DeleteAllocationTokensCommandTest.java b/core/src/test/java/google/registry/tools/DeleteAllocationTokensCommandTest.java index b9f46ada4..2412262c1 100644 --- a/core/src/test/java/google/registry/tools/DeleteAllocationTokensCommandTest.java +++ b/core/src/test/java/google/registry/tools/DeleteAllocationTokensCommandTest.java @@ -53,6 +53,7 @@ class DeleteAllocationTokensCommandTest extends CommandTestCase { command.setConnection(connection); createTld("example"); persistNewRegistrar("acme", "ACME", Registrar.Type.REAL, 99L); + command.printStream = System.out; + command.errorPrintStream = System.err; } @Test diff --git a/core/src/test/java/google/registry/tools/LockDomainCommandTest.java b/core/src/test/java/google/registry/tools/LockDomainCommandTest.java index 85ef02083..b017812f9 100644 --- a/core/src/test/java/google/registry/tools/LockDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/LockDomainCommandTest.java @@ -50,6 +50,7 @@ class LockDomainCommandTest extends CommandTestCase { new DeterministicStringGenerator(Alphabets.BASE_58), "adminreg", new CloudTasksHelper(fakeClock).getTestCloudTasksUtils()); + command.printStream = System.out; } @Test diff --git a/core/src/test/java/google/registry/tools/UniformRapidSuspensionCommandTest.java b/core/src/test/java/google/registry/tools/UniformRapidSuspensionCommandTest.java index d1f74da9c..869708a21 100644 --- a/core/src/test/java/google/registry/tools/UniformRapidSuspensionCommandTest.java +++ b/core/src/test/java/google/registry/tools/UniformRapidSuspensionCommandTest.java @@ -61,6 +61,7 @@ class UniformRapidSuspensionCommandTest ImmutableSet.of( DomainDsData.create(1, 2, 3, new HexBinaryAdapter().unmarshal("dead")), DomainDsData.create(4, 5, 6, new HexBinaryAdapter().unmarshal("beef"))); + command.printStream = System.out; } private void persistDomainWithHosts( diff --git a/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java b/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java index cce5b08da..c24c36fe3 100644 --- a/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/UnlockDomainCommandTest.java @@ -53,6 +53,7 @@ class UnlockDomainCommandTest extends CommandTestCase { new DeterministicStringGenerator(Alphabets.BASE_58), "adminreg", new CloudTasksHelper(fakeClock).getTestCloudTasksUtils()); + command.printStream = System.out; } private Domain persistLockedDomain(String domainName, String registrarId) { diff --git a/core/src/test/java/google/registry/tools/UnrenewDomainCommandTest.java b/core/src/test/java/google/registry/tools/UnrenewDomainCommandTest.java index b83034819..adaff79a3 100644 --- a/core/src/test/java/google/registry/tools/UnrenewDomainCommandTest.java +++ b/core/src/test/java/google/registry/tools/UnrenewDomainCommandTest.java @@ -55,6 +55,7 @@ public class UnrenewDomainCommandTest extends CommandTestCase { @Test void testSuccess_setCommonAndReservedListFromOtherTld_withOverride() throws Exception { + command.errorPrintStream = System.err; runReservedListsTestOverride("common_abuse,tld_banned"); String errMsg = "Error overridden: The reserved list(s) tld_banned " diff --git a/core/src/test/java/google/registry/tools/javascrap/CreateCancellationsForBillingEventsCommandTest.java b/core/src/test/java/google/registry/tools/javascrap/CreateCancellationsForBillingEventsCommandTest.java index c5a00f504..9972ab0e2 100644 --- a/core/src/test/java/google/registry/tools/javascrap/CreateCancellationsForBillingEventsCommandTest.java +++ b/core/src/test/java/google/registry/tools/javascrap/CreateCancellationsForBillingEventsCommandTest.java @@ -34,6 +34,7 @@ import google.registry.model.reporting.HistoryEntryDao; import google.registry.persistence.VKey; import google.registry.testing.DatabaseHelper; import google.registry.tools.CommandTestCase; +import java.io.PrintStream; import org.joda.money.CurrencyUnit; import org.joda.money.Money; import org.junit.jupiter.api.BeforeEach; @@ -59,6 +60,7 @@ public class CreateCancellationsForBillingEventsCommandTest fakeClock.nowUtc(), fakeClock.nowUtc().plusYears(2)); billingEventToCancel = createBillingEvent(); + command.printStream = System.out; } @Test @@ -97,8 +99,10 @@ public class CreateCancellationsForBillingEventsCommandTest @Test void testAlreadyCancelled() throws Exception { // multiple runs / cancellations should be a no-op + command.printStream = new PrintStream(tmpDir.resolve("test.txt").toFile()); runCommandForced(String.valueOf(billingEventToCancel.getId())); assertBillingEventCancelled(); + command.printStream = System.out; runCommandForced(String.valueOf(billingEventToCancel.getId())); assertBillingEventCancelled(); assertThat(DatabaseHelper.loadAllOf(BillingCancellation.class)).hasSize(1);