From c3723bfa2fd24317f68f5d9ef64ad48d7b26441e Mon Sep 17 00:00:00 2001 From: nickfelt Date: Fri, 23 Sep 2016 14:05:22 -0700 Subject: [PATCH] Refactor GetEppResourceCommand hierarchy This refactors the GetEppResourceCommand hierarchy a bit so that instead of using the type param on the class to do implicit loading (which doesn't work that well any more for domain applications anyway), we just do the loading in each child class and rely on the parent class only for printing and setting common flags. I did this to make it possible for loadByForeignKey() to have strong typing (in a future CL), but I think this changes stands on its own merits for making the logic here more straightforward and actually somewhat shorter. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=134115072 --- .../registry/tools/GetApplicationCommand.java | 13 ++++--- .../tools/GetApplicationIdsCommand.java | 12 +++--- .../registry/tools/GetContactCommand.java | 14 ++++--- .../registry/tools/GetDomainCommand.java | 12 +++--- .../registry/tools/GetEppResourceCommand.java | 38 ++++++------------- .../google/registry/tools/GetHostCommand.java | 12 +++--- .../tools/GetApplicationCommandTest.java | 8 ++-- .../tools/GetApplicationIdsCommandTest.java | 8 ---- 8 files changed, 50 insertions(+), 67 deletions(-) diff --git a/java/google/registry/tools/GetApplicationCommand.java b/java/google/registry/tools/GetApplicationCommand.java index 77868e92b..a95cce9ae 100644 --- a/java/google/registry/tools/GetApplicationCommand.java +++ b/java/google/registry/tools/GetApplicationCommand.java @@ -14,16 +14,16 @@ package google.registry.tools; +import static google.registry.model.EppResourceUtils.loadDomainApplication; + import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; -import google.registry.model.domain.DomainApplication; import google.registry.tools.Command.GtechCommand; import java.util.List; /** Command to show a domain application. */ -@Parameters(separators = " =", commandDescription = "Show domain application record(s)") -final class GetApplicationCommand extends GetEppResourceCommand - implements GtechCommand { +@Parameters(separators = " =", commandDescription = "Show domain application resource(s)") +final class GetApplicationCommand extends GetEppResourceCommand implements GtechCommand { @Parameter( description = "Domain application id(s)", @@ -31,9 +31,10 @@ final class GetApplicationCommand extends GetEppResourceCommand mainParameters; @Override - public void processParameters() { + public void runAndPrint() { for (String applicationId : mainParameters) { - printResource(applicationId); + printResource( + "Application", applicationId, loadDomainApplication(applicationId, readTimestamp)); } } } diff --git a/java/google/registry/tools/GetApplicationIdsCommand.java b/java/google/registry/tools/GetApplicationIdsCommand.java index 4979e91d5..207026fa1 100644 --- a/java/google/registry/tools/GetApplicationIdsCommand.java +++ b/java/google/registry/tools/GetApplicationIdsCommand.java @@ -17,6 +17,7 @@ package google.registry.tools; import static google.registry.model.index.DomainApplicationIndex.loadActiveApplicationsByDomainName; import static google.registry.model.registry.Registries.assertTldExists; import static google.registry.model.registry.Registries.findTldForNameOrThrow; +import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; @@ -26,12 +27,12 @@ import google.registry.model.domain.DomainApplication; import google.registry.tools.Command.GtechCommand; import google.registry.tools.Command.RemoteApiCommand; import java.util.List; +import org.joda.time.DateTime; /** Command to generate a list of all applications for a given domain name(s). */ @Parameters(separators = " =", commandDescription = "Generate list of application IDs and sponsors for given domain name(s)") -final class GetApplicationIdsCommand extends GetEppResourceCommand - implements RemoteApiCommand, GtechCommand { +final class GetApplicationIdsCommand implements RemoteApiCommand, GtechCommand { @Parameter( description = "Fully qualified domain name(s)", @@ -39,10 +40,7 @@ final class GetApplicationIdsCommand extends GetEppResourceCommand mainParameters; @Override - void processParameters() { - // Note that this function explicitly performs its own resource load and print, ignoring - // GetEppResourceCommand.printResource(), because we do NOT want to display the entire - // application to gTech users -- just the ID# and sponsor. + public void run() { for (String domainName : mainParameters) { InternetDomainName tld = findTldForNameOrThrow(InternetDomainName.from(domainName)); assertTldExists(tld.toString()); @@ -55,7 +53,7 @@ final class GetApplicationIdsCommand extends GetEppResourceCommand applications = ImmutableList.copyOf( - loadActiveApplicationsByDomainName(domainName, readTimestamp)); + loadActiveApplicationsByDomainName(domainName, DateTime.now(UTC))); if (applications.isEmpty()) { System.out.printf(" No applications exist for \'%s\'.%n", domainName); } else { diff --git a/java/google/registry/tools/GetContactCommand.java b/java/google/registry/tools/GetContactCommand.java index e5327f152..9dc71b192 100644 --- a/java/google/registry/tools/GetContactCommand.java +++ b/java/google/registry/tools/GetContactCommand.java @@ -14,6 +14,8 @@ package google.registry.tools; +import static google.registry.model.EppResourceUtils.loadByForeignKey; + import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import google.registry.model.contact.ContactResource; @@ -21,9 +23,8 @@ import google.registry.tools.Command.GtechCommand; import java.util.List; /** Command to show one or more contacts. */ -@Parameters(separators = " =", commandDescription = "Show contact record(s)") -final class GetContactCommand extends GetEppResourceCommand - implements GtechCommand { +@Parameters(separators = " =", commandDescription = "Show contact resource(s)") +final class GetContactCommand extends GetEppResourceCommand implements GtechCommand { @Parameter( description = "Contact id(s)", @@ -31,9 +32,10 @@ final class GetContactCommand extends GetEppResourceCommand private List mainParameters; @Override - public void processParameters() { - for (String contactNameString : mainParameters) { - printResource(contactNameString); + public void runAndPrint() { + for (String contactId : mainParameters) { + printResource( + "Contact", contactId, loadByForeignKey(ContactResource.class, contactId, readTimestamp)); } } } diff --git a/java/google/registry/tools/GetDomainCommand.java b/java/google/registry/tools/GetDomainCommand.java index 26e9a9cb1..93d45e721 100644 --- a/java/google/registry/tools/GetDomainCommand.java +++ b/java/google/registry/tools/GetDomainCommand.java @@ -14,6 +14,8 @@ package google.registry.tools; +import static google.registry.model.EppResourceUtils.loadByForeignKey; + import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import google.registry.model.domain.DomainResource; @@ -21,9 +23,8 @@ import google.registry.tools.Command.GtechCommand; import java.util.List; /** Command to show a domain resource. */ -@Parameters(separators = " =", commandDescription = "Show domain record(s)") -final class GetDomainCommand extends GetEppResourceCommand - implements GtechCommand { +@Parameters(separators = " =", commandDescription = "Show domain resource(s)") +final class GetDomainCommand extends GetEppResourceCommand implements GtechCommand { @Parameter( description = "Fully qualified domain name(s)", @@ -31,9 +32,10 @@ final class GetDomainCommand extends GetEppResourceCommand private List mainParameters; @Override - public void processParameters() { + public void runAndPrint() { for (String domainName : mainParameters) { - printResource(domainName); + printResource( + "Domain", domainName, loadByForeignKey(DomainResource.class, domainName, readTimestamp)); } } } diff --git a/java/google/registry/tools/GetEppResourceCommand.java b/java/google/registry/tools/GetEppResourceCommand.java index ed4f240ee..1b8bfd2e8 100644 --- a/java/google/registry/tools/GetEppResourceCommand.java +++ b/java/google/registry/tools/GetEppResourceCommand.java @@ -15,31 +15,22 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; -import static google.registry.model.EppResourceUtils.loadByForeignKey; -import static google.registry.model.EppResourceUtils.loadDomainApplication; import static org.joda.time.DateTimeZone.UTC; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.googlecode.objectify.Key; import google.registry.model.EppResource; -import google.registry.model.domain.DomainApplication; import google.registry.tools.Command.RemoteApiCommand; -import google.registry.util.TypeUtils.TypeInstantiator; +import javax.annotation.Nullable; import org.joda.time.DateTime; -/** - * Abstract command to show a resource. - * - * @param {@link EppResource} subclass. - */ +/** Abstract command to print one or more resources to stdout. */ @Parameters(separators = " =") -abstract class GetEppResourceCommand implements RemoteApiCommand { +abstract class GetEppResourceCommand implements RemoteApiCommand { private final DateTime now = DateTime.now(UTC); - private Class clazz = new TypeInstantiator(getClass()){}.getExactType(); - @Parameter( names = "--read_timestamp", description = "Timestamp to use when reading. May not be in the past.") @@ -50,31 +41,26 @@ abstract class GetEppResourceCommand implements RemoteApi description = "Fully expand the requested resource. NOTE: Output may be lengthy.") boolean expand; - /** Resolve any parameters into ids for loadResource. */ - abstract void processParameters(); + /** Runs the command's own logic that calls {@link #printResource}. */ + abstract void runAndPrint(); /** - * Load a resource by ID and output. Append the websafe key to the output for use in e.g. - * manual mapreduce calls. + * Prints a possibly-null resource to stdout, using resourceType and uniqueId to construct a + * nice error message if the resource was null (i.e. doesn't exist). + * + *

The websafe key is appended to the output for use in e.g. manual mapreduce calls. */ - void printResource(String uniqueId) { - EppResource resource = - (clazz == DomainApplication.class) - ? loadDomainApplication(uniqueId, readTimestamp) - : loadByForeignKey(clazz, uniqueId, readTimestamp); + void printResource(String resourceType, String uniqueId, @Nullable EppResource resource) { System.out.println(resource != null ? String.format("%s\n\nWebsafe key: %s", expand ? resource.toHydratedString() : resource, Key.create(resource).getString()) - : String.format( - "%s '%s' does not exist or is deleted\n", - clazz.getSimpleName().replace("Resource", ""), - uniqueId)); + : String.format("%s '%s' does not exist or is deleted\n", resourceType, uniqueId)); } @Override public void run() { checkArgument(!readTimestamp.isBefore(now), "--read_timestamp may not be in the past"); - processParameters(); + runAndPrint(); } } diff --git a/java/google/registry/tools/GetHostCommand.java b/java/google/registry/tools/GetHostCommand.java index 9e7a28bf9..b3333054e 100644 --- a/java/google/registry/tools/GetHostCommand.java +++ b/java/google/registry/tools/GetHostCommand.java @@ -14,6 +14,8 @@ package google.registry.tools; +import static google.registry.model.EppResourceUtils.loadByForeignKey; + import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import google.registry.model.host.HostResource; @@ -21,9 +23,8 @@ import google.registry.tools.Command.GtechCommand; import java.util.List; /** Command to show one or more host resources. */ -@Parameters(separators = " =", commandDescription = "Show host record(s)") -final class GetHostCommand extends GetEppResourceCommand - implements GtechCommand { +@Parameters(separators = " =", commandDescription = "Show host resource(s)") +final class GetHostCommand extends GetEppResourceCommand implements GtechCommand { @Parameter( description = "Fully qualified host name(s)", @@ -31,9 +32,10 @@ final class GetHostCommand extends GetEppResourceCommand private List mainParameters; @Override - public void processParameters() { + public void runAndPrint() { for (String hostName : mainParameters) { - printResource(hostName); + printResource( + "Host", hostName, loadByForeignKey(HostResource.class, hostName, readTimestamp)); } } } diff --git a/javatests/google/registry/tools/GetApplicationCommandTest.java b/javatests/google/registry/tools/GetApplicationCommandTest.java index 28a671648..ff26d19f0 100644 --- a/javatests/google/registry/tools/GetApplicationCommandTest.java +++ b/javatests/google/registry/tools/GetApplicationCommandTest.java @@ -70,20 +70,20 @@ public class GetApplicationCommandTest extends CommandTestCase