From 90c53152bf118390f32f30abc90e47eeffe355f7 Mon Sep 17 00:00:00 2001 From: guyben Date: Wed, 1 May 2019 08:32:29 -0700 Subject: [PATCH] Simplify some of the RDAP Action classes Overriding getter methods to change values is a bit overkill when these values are static (don't change based on internal state). Just setting them in the base class' constructor is simpler. Also, we can read the PATH of an Action based on the Annotation instead returning it manually for each Action. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=246135754 --- java/google/registry/rdap/RdapActionBase.java | 24 +++++++++---- .../registry/rdap/RdapAutnumAction.java | 21 ++--------- .../registry/rdap/RdapDomainAction.java | 21 ++--------- .../registry/rdap/RdapDomainSearchAction.java | 21 ++--------- .../registry/rdap/RdapEntityAction.java | 21 ++--------- .../registry/rdap/RdapEntitySearchAction.java | 23 +++--------- java/google/registry/rdap/RdapHelpAction.java | 21 ++--------- java/google/registry/rdap/RdapIpAction.java | 21 ++--------- .../registry/rdap/RdapNameserverAction.java | 21 ++--------- .../rdap/RdapNameserverSearchAction.java | 19 ++-------- .../registry/rdap/RdapSearchActionBase.java | 5 +++ .../registry/rdap/RdapActionBaseTest.java | 36 ++++++++----------- .../registry/rdap/RdapActionBaseTestCase.java | 5 +-- .../registry/rdap/RdapDomainActionTest.java | 2 +- .../rdap/RdapDomainSearchActionTest.java | 6 ++-- .../registry/rdap/RdapEntityActionTest.java | 2 +- .../rdap/RdapEntitySearchActionTest.java | 4 +-- .../registry/rdap/RdapHelpActionTest.java | 2 +- .../rdap/RdapNameserverActionTest.java | 2 +- .../rdap/RdapNameserverSearchActionTest.java | 4 +-- .../rdap/RdapSearchActionTestCase.java | 4 +-- 21 files changed, 81 insertions(+), 204 deletions(-) diff --git a/java/google/registry/rdap/RdapActionBase.java b/java/google/registry/rdap/RdapActionBase.java index 38c3df19b..cccc5a654 100644 --- a/java/google/registry/rdap/RdapActionBase.java +++ b/java/google/registry/rdap/RdapActionBase.java @@ -18,6 +18,7 @@ import static com.google.common.base.Charsets.UTF_8; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.request.Actions.getPathForAction; import static google.registry.util.DateTimeUtils.END_OF_TIME; import static google.registry.util.DomainNameUtils.canonicalizeDomainName; import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; @@ -106,14 +107,25 @@ public abstract class RdapActionBase implements Runnable { final RdapMetrics.RdapMetricInformation.Builder metricInformationBuilder = RdapMetrics.RdapMetricInformation.builder(); - /** Returns a string like "domain name" or "nameserver", used for error strings. */ - abstract String getHumanReadableObjectTypeName(); + private final String humanReadableObjectTypeName; - /** Returns the endpoint type used for recording metrics. */ - abstract EndpointType getEndpointType(); + /** Returns a string like "domain name" or "nameserver", used for error strings. */ + final String getHumanReadableObjectTypeName() { + return humanReadableObjectTypeName; + } + + /** The endpoint type used for recording metrics. */ + private final EndpointType endpointType; /** Returns the servlet action path; used to extract the search string from the incoming path. */ - abstract String getActionPath(); + final String getActionPath() { + return getPathForAction(getClass()); + } + + RdapActionBase(String humanReadableObjectTypeName, EndpointType endpointType) { + this.humanReadableObjectTypeName = humanReadableObjectTypeName; + this.endpointType = endpointType; + } /** * Does the actual search and returns an RDAP JSON object. @@ -135,7 +147,7 @@ public abstract class RdapActionBase implements Runnable { metricInformationBuilder.setRegistrarSpecified(registrarParam.isPresent()); metricInformationBuilder.setRole(getAuthorization().role()); metricInformationBuilder.setRequestMethod(requestMethod); - metricInformationBuilder.setEndpointType(getEndpointType()); + metricInformationBuilder.setEndpointType(endpointType); try { // Extract what we're searching for from the request path. Some RDAP commands use trailing // data in the path itself (e.g. /rdap/domain/mydomain.com), and some use the query string diff --git a/java/google/registry/rdap/RdapAutnumAction.java b/java/google/registry/rdap/RdapAutnumAction.java index 1b749da34..c56c6615f 100644 --- a/java/google/registry/rdap/RdapAutnumAction.java +++ b/java/google/registry/rdap/RdapAutnumAction.java @@ -32,29 +32,14 @@ import javax.inject.Inject; */ @Action( service = Action.Service.PUBAPI, - path = RdapAutnumAction.PATH, + path = "/rdap/autnum/", method = {GET, HEAD}, isPrefix = true, auth = Auth.AUTH_PUBLIC_ANONYMOUS) public class RdapAutnumAction extends RdapActionBase { - public static final String PATH = "/rdap/autnum/"; - - @Inject RdapAutnumAction() {} - - @Override - public String getHumanReadableObjectTypeName() { - return "autnum"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.AUTNUM; - } - - @Override - public String getActionPath() { - return PATH; + @Inject RdapAutnumAction() { + super("authnum", EndpointType.AUTNUM); } @Override diff --git a/java/google/registry/rdap/RdapDomainAction.java b/java/google/registry/rdap/RdapDomainAction.java index a84a25c3e..f5d9f1ff7 100644 --- a/java/google/registry/rdap/RdapDomainAction.java +++ b/java/google/registry/rdap/RdapDomainAction.java @@ -36,29 +36,14 @@ import org.joda.time.DateTime; /** RDAP (new WHOIS) action for domain requests. */ @Action( service = Action.Service.PUBAPI, - path = RdapDomainAction.PATH, + path = "/rdap/domain/", method = {GET, HEAD}, isPrefix = true, auth = Auth.AUTH_PUBLIC) public class RdapDomainAction extends RdapActionBase { - public static final String PATH = "/rdap/domain/"; - - @Inject public RdapDomainAction() {} - - @Override - public String getHumanReadableObjectTypeName() { - return "domain name"; - } - - @Override - public String getActionPath() { - return PATH; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.DOMAIN; + @Inject public RdapDomainAction() { + super("domain name", EndpointType.DOMAIN); } @Override diff --git a/java/google/registry/rdap/RdapDomainSearchAction.java b/java/google/registry/rdap/RdapDomainSearchAction.java index 09a226ea4..1a74a7899 100644 --- a/java/google/registry/rdap/RdapDomainSearchAction.java +++ b/java/google/registry/rdap/RdapDomainSearchAction.java @@ -68,13 +68,11 @@ import org.joda.time.DateTime; */ @Action( service = Action.Service.PUBAPI, - path = RdapDomainSearchAction.PATH, + path = "/rdap/domains", method = {GET, HEAD}, auth = Auth.AUTH_PUBLIC) public class RdapDomainSearchAction extends RdapSearchActionBase { - static final String PATH = "/rdap/domains"; - static final int RESULT_SET_SIZE_SCALING_FACTOR = 30; @NonFinalForTesting @@ -85,21 +83,8 @@ public class RdapDomainSearchAction extends RdapSearchActionBase { @Inject @Parameter("name") Optional nameParam; @Inject @Parameter("nsLdhName") Optional nsLdhNameParam; @Inject @Parameter("nsIp") Optional nsIpParam; - @Inject public RdapDomainSearchAction() {} - - @Override - public String getHumanReadableObjectTypeName() { - return "domain search"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.DOMAINS; - } - - @Override - public String getActionPath() { - return PATH; + @Inject public RdapDomainSearchAction() { + super("domain search", EndpointType.DOMAINS); } /** diff --git a/java/google/registry/rdap/RdapEntityAction.java b/java/google/registry/rdap/RdapEntityAction.java index b6c892720..05aae8474 100644 --- a/java/google/registry/rdap/RdapEntityAction.java +++ b/java/google/registry/rdap/RdapEntityAction.java @@ -47,31 +47,16 @@ import org.joda.time.DateTime; */ @Action( service = Action.Service.PUBAPI, - path = RdapEntityAction.PATH, + path = "/rdap/entity/", method = {GET, HEAD}, isPrefix = true, auth = Auth.AUTH_PUBLIC) public class RdapEntityAction extends RdapActionBase { - public static final String PATH = "/rdap/entity/"; - private static final Pattern ROID_PATTERN = Pattern.compile("[-_.a-zA-Z0-9]+"); - @Inject public RdapEntityAction() {} - - @Override - public String getHumanReadableObjectTypeName() { - return "entity"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.ENTITY; - } - - @Override - public String getActionPath() { - return PATH; + @Inject public RdapEntityAction() { + super("entity", EndpointType.ENTITY); } @Override diff --git a/java/google/registry/rdap/RdapEntitySearchAction.java b/java/google/registry/rdap/RdapEntitySearchAction.java index ecf568f14..3fe0e1daa 100644 --- a/java/google/registry/rdap/RdapEntitySearchAction.java +++ b/java/google/registry/rdap/RdapEntitySearchAction.java @@ -76,17 +76,17 @@ import org.joda.time.DateTime; */ @Action( service = Action.Service.PUBAPI, - path = RdapEntitySearchAction.PATH, + path = "/rdap/entities", method = {GET, HEAD}, auth = Auth.AUTH_PUBLIC) public class RdapEntitySearchAction extends RdapSearchActionBase { - public static final String PATH = "/rdap/entities"; - @Inject @Parameter("fn") Optional fnParam; @Inject @Parameter("handle") Optional handleParam; @Inject @Parameter("subtype") Optional subtypeParam; - @Inject public RdapEntitySearchAction() {} + @Inject public RdapEntitySearchAction() { + super("entity search", EndpointType.ENTITIES); + } private enum QueryType { FULL_NAME, @@ -108,21 +108,6 @@ public class RdapEntitySearchAction extends RdapSearchActionBase { private static final String CONTACT_CURSOR_PREFIX = "c:"; private static final String REGISTRAR_CURSOR_PREFIX = "r:"; - @Override - public String getHumanReadableObjectTypeName() { - return "entity search"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.ENTITIES; - } - - @Override - public String getActionPath() { - return PATH; - } - /** Parses the parameters and calls the appropriate search function. */ @Override public ImmutableMap getJsonObjectForResource( diff --git a/java/google/registry/rdap/RdapHelpAction.java b/java/google/registry/rdap/RdapHelpAction.java index 45000927f..215dca192 100644 --- a/java/google/registry/rdap/RdapHelpAction.java +++ b/java/google/registry/rdap/RdapHelpAction.java @@ -28,29 +28,14 @@ import javax.inject.Inject; /** RDAP (new WHOIS) action for help requests. */ @Action( service = Action.Service.PUBAPI, - path = RdapHelpAction.PATH, + path = "/rdap/help", method = {GET, HEAD}, isPrefix = true, auth = Auth.AUTH_PUBLIC_ANONYMOUS) public class RdapHelpAction extends RdapActionBase { - public static final String PATH = "/rdap/help"; - - @Inject public RdapHelpAction() {} - - @Override - public String getHumanReadableObjectTypeName() { - return "help"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.HELP; - } - - @Override - public String getActionPath() { - return PATH; + @Inject public RdapHelpAction() { + super("help", EndpointType.HELP); } @Override diff --git a/java/google/registry/rdap/RdapIpAction.java b/java/google/registry/rdap/RdapIpAction.java index 54b844cc3..15ce2b3b0 100644 --- a/java/google/registry/rdap/RdapIpAction.java +++ b/java/google/registry/rdap/RdapIpAction.java @@ -32,29 +32,14 @@ import javax.inject.Inject; */ @Action( service = Action.Service.PUBAPI, - path = RdapIpAction.PATH, + path = "/rdap/ip/", method = {GET, HEAD}, isPrefix = true, auth = Auth.AUTH_PUBLIC_ANONYMOUS) public class RdapIpAction extends RdapActionBase { - public static final String PATH = "/rdap/ip/"; - - @Inject RdapIpAction() {} - - @Override - public String getHumanReadableObjectTypeName() { - return "ip"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.IP; - } - - @Override - public String getActionPath() { - return PATH; + @Inject RdapIpAction() { + super("ip", EndpointType.IP); } @Override diff --git a/java/google/registry/rdap/RdapNameserverAction.java b/java/google/registry/rdap/RdapNameserverAction.java index 9c13aa7f0..148945cd6 100644 --- a/java/google/registry/rdap/RdapNameserverAction.java +++ b/java/google/registry/rdap/RdapNameserverAction.java @@ -36,29 +36,14 @@ import org.joda.time.DateTime; /** RDAP (new WHOIS) action for nameserver requests. */ @Action( service = Action.Service.PUBAPI, - path = RdapNameserverAction.PATH, + path = "/rdap/nameserver/", method = {GET, HEAD}, isPrefix = true, auth = Auth.AUTH_PUBLIC_ANONYMOUS) public class RdapNameserverAction extends RdapActionBase { - public static final String PATH = "/rdap/nameserver/"; - - @Inject public RdapNameserverAction() {} - - @Override - public String getHumanReadableObjectTypeName() { - return "nameserver"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.NAMESERVER; - } - - @Override - public String getActionPath() { - return PATH; + @Inject public RdapNameserverAction() { + super("nameserver", EndpointType.NAMESERVER); } @Override diff --git a/java/google/registry/rdap/RdapNameserverSearchAction.java b/java/google/registry/rdap/RdapNameserverSearchAction.java index 91c8af0b8..5f4f47602 100644 --- a/java/google/registry/rdap/RdapNameserverSearchAction.java +++ b/java/google/registry/rdap/RdapNameserverSearchAction.java @@ -58,7 +58,7 @@ import org.joda.time.DateTime; */ @Action( service = Action.Service.PUBAPI, - path = RdapNameserverSearchAction.PATH, + path = "/rdap/nameservers", method = {GET, HEAD}, auth = Auth.AUTH_PUBLIC_ANONYMOUS) public class RdapNameserverSearchAction extends RdapSearchActionBase { @@ -67,21 +67,8 @@ public class RdapNameserverSearchAction extends RdapSearchActionBase { @Inject @Parameter("name") Optional nameParam; @Inject @Parameter("ip") Optional ipParam; - @Inject public RdapNameserverSearchAction() {} - - @Override - public String getHumanReadableObjectTypeName() { - return "nameserver search"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.NAMESERVERS; - } - - @Override - public String getActionPath() { - return PATH; + @Inject public RdapNameserverSearchAction() { + super("nameserver search", EndpointType.NAMESERVERS); } private enum CursorType { diff --git a/java/google/registry/rdap/RdapSearchActionBase.java b/java/google/registry/rdap/RdapSearchActionBase.java index e99b8c717..4b8d46547 100644 --- a/java/google/registry/rdap/RdapSearchActionBase.java +++ b/java/google/registry/rdap/RdapSearchActionBase.java @@ -19,6 +19,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMap; +import google.registry.rdap.RdapMetrics.EndpointType; import google.registry.request.Parameter; import google.registry.request.ParameterMap; import google.registry.request.RequestUrl; @@ -44,6 +45,10 @@ public abstract class RdapSearchActionBase extends RdapActionBase { protected Optional cursorString; + RdapSearchActionBase(String humanReadableObjectTypeName, EndpointType endpointType) { + super(humanReadableObjectTypeName, endpointType); + } + /** * Decodes the cursor token passed in the HTTP request. * diff --git a/javatests/google/registry/rdap/RdapActionBaseTest.java b/javatests/google/registry/rdap/RdapActionBaseTest.java index 04c4e7bdd..13efb4951 100644 --- a/javatests/google/registry/rdap/RdapActionBaseTest.java +++ b/javatests/google/registry/rdap/RdapActionBaseTest.java @@ -17,6 +17,7 @@ package google.registry.rdap; import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN; import static com.google.common.truth.Truth.assertThat; import static google.registry.request.Action.Method.GET; +import static google.registry.request.Action.Method.HEAD; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.TestDataHelper.loadFile; import static org.mockito.Mockito.verify; @@ -29,6 +30,7 @@ import google.registry.rdap.RdapMetrics.SearchType; import google.registry.rdap.RdapMetrics.WildcardType; import google.registry.rdap.RdapSearchResults.IncompletenessWarningType; import google.registry.request.Action; +import google.registry.request.auth.Auth; import java.util.Optional; import org.json.simple.JSONValue; import org.junit.Before; @@ -41,29 +43,19 @@ import org.junit.runners.JUnit4; public class RdapActionBaseTest extends RdapActionBaseTestCase { public RdapActionBaseTest() { - super(RdapTestAction.class, RdapTestAction.PATH); + super(RdapTestAction.class); } - /** - * Dummy RdapActionBase subclass used for testing. - */ + /** Dummy RdapActionBase subclass used for testing. */ + @Action( + service = Action.Service.PUBAPI, + path = "/rdap/test/", + method = {GET, HEAD}, + auth = Auth.AUTH_PUBLIC_ANONYMOUS) public static class RdapTestAction extends RdapActionBase { - public static final String PATH = "/rdap/test/"; - - @Override - public String getHumanReadableObjectTypeName() { - return "human-readable string"; - } - - @Override - public EndpointType getEndpointType() { - return EndpointType.HELP; - } - - @Override - public String getActionPath() { - return PATH; + public RdapTestAction() { + super("human-readable string", EndpointType.HELP); } @Override @@ -90,7 +82,7 @@ public class RdapActionBaseTest extends RdapActionBaseTestCase { protected final String actionPath; protected final Class rdapActionClass; - protected RdapActionBaseTestCase(Class rdapActionClass, String actionPath) { + protected RdapActionBaseTestCase(Class rdapActionClass) { this.rdapActionClass = rdapActionClass; - this.actionPath = actionPath; + this.actionPath = Actions.getPathForAction(rdapActionClass); } @Before diff --git a/javatests/google/registry/rdap/RdapDomainActionTest.java b/javatests/google/registry/rdap/RdapDomainActionTest.java index 9930aefee..90ee442e7 100644 --- a/javatests/google/registry/rdap/RdapDomainActionTest.java +++ b/javatests/google/registry/rdap/RdapDomainActionTest.java @@ -59,7 +59,7 @@ import org.junit.runners.JUnit4; public class RdapDomainActionTest extends RdapActionBaseTestCase { public RdapDomainActionTest() { - super(RdapDomainAction.class, RdapDomainAction.PATH); + super(RdapDomainAction.class); } @Before diff --git a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java index 7e86a18df..875523db8 100644 --- a/javatests/google/registry/rdap/RdapDomainSearchActionTest.java +++ b/javatests/google/registry/rdap/RdapDomainSearchActionTest.java @@ -71,7 +71,7 @@ import org.junit.runners.JUnit4; public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { public RdapDomainSearchActionTest() { - super(RdapDomainSearchAction.class, RdapDomainSearchAction.PATH); + super(RdapDomainSearchAction.class); } private Registrar registrar; @@ -96,7 +96,7 @@ public class RdapDomainSearchActionTest extends RdapSearchActionTestCase { public RdapEntityActionTest() { - super(RdapEntityAction.class, RdapEntityAction.PATH); + super(RdapEntityAction.class); } private Registrar registrarLol; diff --git a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java index 82e49afe7..2696a12b8 100644 --- a/javatests/google/registry/rdap/RdapEntitySearchActionTest.java +++ b/javatests/google/registry/rdap/RdapEntitySearchActionTest.java @@ -58,7 +58,7 @@ import org.junit.runners.JUnit4; public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { public RdapEntitySearchActionTest() { - super(RdapEntitySearchAction.class, RdapEntitySearchAction.PATH); + super(RdapEntitySearchAction.class); } private enum QueryType { @@ -418,7 +418,7 @@ public class RdapEntitySearchActionTest extends RdapSearchActionTestCase { public RdapHelpActionTest() { - super(RdapHelpAction.class, RdapHelpAction.PATH); + super(RdapHelpAction.class); } private Object generateExpectedJson(String name, String expectedOutputFile) { diff --git a/javatests/google/registry/rdap/RdapNameserverActionTest.java b/javatests/google/registry/rdap/RdapNameserverActionTest.java index 6bd4555e7..9eb5d9cff 100644 --- a/javatests/google/registry/rdap/RdapNameserverActionTest.java +++ b/javatests/google/registry/rdap/RdapNameserverActionTest.java @@ -45,7 +45,7 @@ import org.junit.runners.JUnit4; public class RdapNameserverActionTest extends RdapActionBaseTestCase { public RdapNameserverActionTest() { - super(RdapNameserverAction.class, RdapNameserverAction.PATH); + super(RdapNameserverAction.class); } @Before diff --git a/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java b/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java index 82adab075..57b7273a4 100644 --- a/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java +++ b/javatests/google/registry/rdap/RdapNameserverSearchActionTest.java @@ -58,7 +58,7 @@ public class RdapNameserverSearchActionTest extends RdapSearchActionTestCase { public RdapNameserverSearchActionTest() { - super(RdapNameserverSearchAction.class, RdapNameserverSearchAction.PATH); + super(RdapNameserverSearchAction.class); } private DomainBase domainCatLol; @@ -263,7 +263,7 @@ public class RdapNameserverSearchActionTest @Test public void testInvalidPath_rejected() { - action.requestPath = RdapDomainSearchAction.PATH + "/path"; + action.requestPath = actionPath + "/path"; action.run(); assertThat(response.getStatus()).isEqualTo(400); verifyErrorMetrics(Optional.empty(), 400); diff --git a/javatests/google/registry/rdap/RdapSearchActionTestCase.java b/javatests/google/registry/rdap/RdapSearchActionTestCase.java index eaa19eca8..b006bcfd3 100644 --- a/javatests/google/registry/rdap/RdapSearchActionTestCase.java +++ b/javatests/google/registry/rdap/RdapSearchActionTestCase.java @@ -30,8 +30,8 @@ import org.junit.Before; public class RdapSearchActionTestCase extends RdapActionBaseTestCase { - protected RdapSearchActionTestCase(Class rdapActionClass, String path) { - super(rdapActionClass, path); + protected RdapSearchActionTestCase(Class rdapActionClass) { + super(rdapActionClass); } SearchType metricSearchType = SearchType.NONE;