From 01591ff88ea5460366732ed9ec26c6a0aabfa7bf Mon Sep 17 00:00:00 2001 From: guyben Date: Tue, 3 Oct 2017 07:41:02 -0700 Subject: [PATCH] Clarify diff display of MutatingCommand Tools inheriting from MutatingCommand print out the change they are going to make and then ask the user to confirm that this is indeed what they wanted to do. The change is outputted as a list of updated values in the form key -> [oldValue, newValue] e.g. dnsPaused -> [true, false] This CL will change the output to be clearer: key: oldValue -> newValue e.g. dnsPaused: true -> false ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=170853745 --- docs/operational-procedures.md | 2 +- .../premium-list-management.md | 2 +- .../reserved-list-management.md | 2 +- .../ImmutableSortedMapTranslatorFactory.java | 10 +++++----- java/google/registry/util/DiffUtils.java | 7 +++---- .../registry/tools/MutatingCommandTest.java | 16 ++++++++-------- .../testdata/update_registrar_email.txt | 18 +++++++++--------- .../google/registry/util/DiffUtilsTest.java | 4 ++-- 8 files changed, 30 insertions(+), 31 deletions(-) diff --git a/docs/operational-procedures.md b/docs/operational-procedures.md index 776f590de..4e365ea71 100644 --- a/docs/operational-procedures.md +++ b/docs/operational-procedures.md @@ -63,7 +63,7 @@ Cursors can be updated as follows: $ nomulus -e {ENVIRONMENT} update_cursors exampletld --type RDE_STAGING \ --timestamp 2016-09-01T00:00:00Z Update Cursor@ahFzfmRvbWFpbi1yZWdpc3RyeXIzCxIPRW50aXR5R3JvdXBSb290Igljcm9zcy10bGQMCxIIUmVnaXN0cnkiB3lvdXR1YmUM_RDE_STAGING -cursorTime -> [2016-09-23T00:00:00.000Z, 2016-09-01T00:00:00.000Z] +cursorTime: 2016-09-23T00:00:00.000Z -> 2016-09-01T00:00:00.000Z Perform this command? (y/N): Y Updated 1 entities. diff --git a/docs/operational-procedures/premium-list-management.md b/docs/operational-procedures/premium-list-management.md index 33b33971d..6a750a034 100644 --- a/docs/operational-procedures/premium-list-management.md +++ b/docs/operational-procedures/premium-list-management.md @@ -76,7 +76,7 @@ parameter: ```shell $ nomulus -e {ENVIRONMENT} update_tld exampletld --premium_list exampletld Update Registry@exampletld -premiumList -> [null, Key(EntityGroupRoot("cross-tld")/PremiumList("exampletld"))] +premiumList: null -> Key(EntityGroupRoot("cross-tld")/PremiumList("exampletld")) Perform this command? (y/N): y Updated 1 entities. diff --git a/docs/operational-procedures/reserved-list-management.md b/docs/operational-procedures/reserved-list-management.md index 4e5df320e..53a2b11d1 100644 --- a/docs/operational-procedures/reserved-list-management.md +++ b/docs/operational-procedures/reserved-list-management.md @@ -124,7 +124,7 @@ parameter: $ nomulus -e {ENVIRONMENT} update_tld exampletld \ --add_reserved_lists common_bad-words Update Registry@exampletld -reservedLists -> [null, [Key(EntityGroupRoot("cross-tld")/ReservedList("common_bad-words"))]] +reservedLists: null -> [Key(EntityGroupRoot("cross-tld")/ReservedList("common_bad-words"))] Perform this command? (y/N): y Updated 1 entities. ``` diff --git a/java/google/registry/model/translators/ImmutableSortedMapTranslatorFactory.java b/java/google/registry/model/translators/ImmutableSortedMapTranslatorFactory.java index 45e9e753e..aa9eae204 100644 --- a/java/google/registry/model/translators/ImmutableSortedMapTranslatorFactory.java +++ b/java/google/registry/model/translators/ImmutableSortedMapTranslatorFactory.java @@ -57,16 +57,16 @@ import javax.annotation.Nullable; *

For example, if you had an {@code ImmutableSortedMap} on a field named * {@code field}, then this would look like:

   {@code
  *
- *   field.key -> [key1, key2]
- *   field.value -> [value1, value2]}
+ * field.key: key1 -> key2 + * field.value: value1 -> value2} * *

If you had an {@code ImmutableSortedMap} on a field named * {@code field}, where {@code EmbeddedClass} defines two {@code foo} and {@code bar} fields, then * the embedded properties might look like:

   {@code
  *
- *   field.key -> [key1, key2]
- *   field.value.foo -> [foo1, foo2]
- *   field.value.bar -> [bar1, bar2]}
+ * field.key: key1 -> key2 + * field.value.foo: foo1 -> foo2 + * field.value.bar: bar1 -> bar2} * * @param key type for sorted map which must be {@link Comparable} * @param value type for sorted map diff --git a/java/google/registry/util/DiffUtils.java b/java/google/registry/util/DiffUtils.java index dc54dd107..f37458959 100644 --- a/java/google/registry/util/DiffUtils.java +++ b/java/google/registry/util/DiffUtils.java @@ -15,7 +15,6 @@ package google.registry.util; import static com.google.common.base.Predicates.notNull; -import static com.google.common.collect.Lists.newArrayList; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; @@ -54,7 +53,7 @@ public final class DiffUtils { @Override public String toString() { // Note that we use newArrayList here instead of ImmutableList because a and b can be null. - return newArrayList(a, b).toString(); + return String.format("%s -> %s", a, b); } } @@ -141,9 +140,9 @@ public final class DiffUtils { && ((DiffPair) value).b instanceof Set) { DiffPair pair = ((DiffPair) value); String prettyLineDiff = prettyPrintSetDiff((Set) pair.a, (Set) pair.b) + "\n"; - output = newPath + ((prettyLineDiff.startsWith("\n")) ? " ->" : " -> ") + prettyLineDiff; + output = newPath + ((prettyLineDiff.startsWith("\n")) ? ":" : ": ") + prettyLineDiff; } else { - output = newPath + " -> " + value + "\n"; + output = newPath + ": " + value + "\n"; } builder.append(output); } diff --git a/javatests/google/registry/tools/MutatingCommandTest.java b/javatests/google/registry/tools/MutatingCommandTest.java index 1acbed98b..a2f8ca994 100644 --- a/javatests/google/registry/tools/MutatingCommandTest.java +++ b/javatests/google/registry/tools/MutatingCommandTest.java @@ -99,16 +99,16 @@ public class MutatingCommandTest { String changes = command.prompt(); assertThat(changes).isEqualTo( "Update HostResource@2-ROID\n" - + "lastEppUpdateTime -> [null, 2014-09-09T09:09:09.000Z]\n" + + "lastEppUpdateTime: null -> 2014-09-09T09:09:09.000Z\n" + "\n" + "Update HostResource@3-ROID\n" - + "currentSponsorClientId -> [TheRegistrar, Registrar2]\n" + + "currentSponsorClientId: TheRegistrar -> Registrar2\n" + "\n" + "Update Registrar@Registrar1\n" - + "billingIdentifier -> [null, 42]\n" + + "billingIdentifier: null -> 42\n" + "\n" + "Update Registrar@Registrar2\n" - + "blockPremiumNames -> [false, true]\n"); + + "blockPremiumNames: false -> true\n"); String results = command.execute(); assertThat(results).isEqualTo("Updated 4 entities.\n"); assertThat(ofy().load().entity(host1).now()).isEqualTo(newHost1); @@ -229,13 +229,13 @@ public class MutatingCommandTest { + host1 + "\n" + "\n" + "Update HostResource@3-ROID\n" - + "currentSponsorClientId -> [TheRegistrar, Registrar2]\n" + + "currentSponsorClientId: TheRegistrar -> Registrar2\n" + "\n" + "Delete Registrar@Registrar1\n" + registrar1 + "\n" + "\n" + "Update Registrar@Registrar2\n" - + "blockPremiumNames -> [false, true]\n"); + + "blockPremiumNames: false -> true\n"); String results = command.execute(); assertThat(results).isEqualTo("Updated 4 entities.\n"); assertThat(ofy().load().entity(host1).now()).isNull(); @@ -269,13 +269,13 @@ public class MutatingCommandTest { + host1 + "\n" + "\n" + "Update HostResource@3-ROID\n" - + "currentSponsorClientId -> [TheRegistrar, Registrar2]\n" + + "currentSponsorClientId: TheRegistrar -> Registrar2\n" + "\n" + "Delete Registrar@Registrar1\n" + registrar1 + "\n" + "\n" + "Update Registrar@Registrar2\n" - + "blockPremiumNames -> [false, true]\n"); + + "blockPremiumNames: false -> true\n"); try { command.execute(); assertWithMessage("Expected transaction to fail with IllegalStateException").fail(); diff --git a/javatests/google/registry/ui/server/registrar/testdata/update_registrar_email.txt b/javatests/google/registry/ui/server/registrar/testdata/update_registrar_email.txt index d6baa9eed..6a6e3c9ed 100644 --- a/javatests/google/registry/ui/server/registrar/testdata/update_registrar_email.txt +++ b/javatests/google/registry/ui/server/registrar/testdata/update_registrar_email.txt @@ -1,13 +1,13 @@ The following changes were made to the registrar: -whoisServer -> [null, foo.bar.baz] -ipAddressWhitelist -> [null, [1.1.1.1/32, 2.2.2.2/32, 4.4.4.4/32]] -localizedAddress.street.0 -> [123 Example Bőulevard, 123 Street Rd] -localizedAddress.street.1 -> [null, Ste 156] -localizedAddress.city -> [Williamsburg, New York] -localizedAddress.zip -> [11211, 10011] -phoneNumber -> [+1.2223334444, +1.2223335555] -emailAddress -> [new.registrar@example.com, thase@the.registrar] -contacts -> +whoisServer: null -> foo.bar.baz +ipAddressWhitelist: null -> [1.1.1.1/32, 2.2.2.2/32, 4.4.4.4/32] +localizedAddress.street.0: 123 Example Bőulevard -> 123 Street Rd +localizedAddress.street.1: null -> Ste 156 +localizedAddress.city: Williamsburg -> New York +localizedAddress.zip: 11211 -> 10011 +phoneNumber: +1.2223334444 -> +1.2223335555 +emailAddress: new.registrar@example.com -> thase@the.registrar +contacts: ADDED: {parent=Key(EntityGroupRoot("cross-tld")/Registrar("TheRegistrar")), name=Extra Terrestrial, emailAddress=etphonehome@example.com, phoneNumber=null, faxNumber=null, types=[ADMIN, BILLING, TECH, WHOIS], gaeUserId=null, visibleInWhoisAsAdmin=true, visibleInWhoisAsTech=false, visibleInDomainWhoisAsAbuse=false} REMOVED: diff --git a/javatests/google/registry/util/DiffUtilsTest.java b/javatests/google/registry/util/DiffUtilsTest.java index bbc50a065..bd541ce56 100644 --- a/javatests/google/registry/util/DiffUtilsTest.java +++ b/javatests/google/registry/util/DiffUtilsTest.java @@ -68,8 +68,8 @@ public class DiffUtilsTest { Map mapB = new HashMap(); mapB.put("a", "tim"); mapB.put("b", ImmutableSet.of()); - // This ensures that it is not outputting a diff of [b -> [null, []]. - assertThat(prettyPrintEntityDeepDiff(mapA, mapB)).isEqualTo("a -> [jim, tim]\n"); + // This ensures that it is not outputting a diff of b: null -> []. + assertThat(prettyPrintEntityDeepDiff(mapA, mapB)).isEqualTo("a: jim -> tim\n"); } @Test