From 1e7fc4d64d9a3f4209cd3745b648741c988b53a8 Mon Sep 17 00:00:00 2001 From: nickfelt Date: Wed, 22 Feb 2017 14:31:15 -0800 Subject: [PATCH] Remove Builder type param on RequestComponentBuilder/RequestHandler It turns out this type parameter was never necessary. A builder only needs the reflexive second type parameter when you want to have a builder inheritance hierarchy where the descendant builders have methods that the ancestor builder doesn't. In that case, the type param enables the ancestor builder's setter methods to automatically return the correct derived type, so that if you start with a derived builder, you can call a setter method inherited from an ancestor and then continue the chain with setters from the derived builder (e.g. new ContactResource.Builder().setCreationTime(now).setContactId(), which otherwise would have returned an EppResource.Builder from setCreationTime(), at which point the call to setContactId() would not compile). Even then, it's not strictly necessary to use the type parameter, since you could instead just have each derived type override every inherited method to specify itself as the return type. But that would be a lot of extra boilerplate and brittleness. Anyway, in this case, there is a builder hierarchy, but RequestComponentBuilder specifies all the methods that we're ever going to want on our builders, so there's never any need to be able to call specific derived builder methods. We only even need the individual builder classes so that Dagger can generate them separately for each component. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=148269178 --- .../module/backend/BackendRequestComponent.java | 2 +- .../module/backend/BackendRequestHandler.java | 3 +-- .../module/frontend/FrontendRequestComponent.java | 2 +- .../module/frontend/FrontendRequestHandler.java | 3 +-- .../module/tools/ToolsRequestComponent.java | 2 +- .../registry/module/tools/ToolsRequestHandler.java | 3 +-- .../registry/request/RequestComponentBuilder.java | 4 ++-- java/google/registry/request/RequestHandler.java | 13 ++++++------- .../google/registry/request/RequestHandlerTest.java | 6 +++--- 9 files changed, 17 insertions(+), 21 deletions(-) diff --git a/java/google/registry/module/backend/BackendRequestComponent.java b/java/google/registry/module/backend/BackendRequestComponent.java index 6e733cf51..7677596eb 100644 --- a/java/google/registry/module/backend/BackendRequestComponent.java +++ b/java/google/registry/module/backend/BackendRequestComponent.java @@ -136,7 +136,7 @@ interface BackendRequestComponent { VerifyEntityIntegrityAction verifyEntityIntegrityAction(); @Subcomponent.Builder - abstract class Builder implements RequestComponentBuilder { + abstract class Builder implements RequestComponentBuilder { @Override public abstract Builder requestModule(RequestModule requestModule); @Override public abstract BackendRequestComponent build(); } diff --git a/java/google/registry/module/backend/BackendRequestHandler.java b/java/google/registry/module/backend/BackendRequestHandler.java index b4faf38b4..c2881c1ad 100644 --- a/java/google/registry/module/backend/BackendRequestHandler.java +++ b/java/google/registry/module/backend/BackendRequestHandler.java @@ -22,8 +22,7 @@ import javax.inject.Inject; import javax.inject.Provider; /** Request handler for the backend module. */ -public class BackendRequestHandler - extends RequestHandler { +public class BackendRequestHandler extends RequestHandler { @Inject BackendRequestHandler( Provider componentBuilderProvider, diff --git a/java/google/registry/module/frontend/FrontendRequestComponent.java b/java/google/registry/module/frontend/FrontendRequestComponent.java index d88cd4ca7..db02d5862 100644 --- a/java/google/registry/module/frontend/FrontendRequestComponent.java +++ b/java/google/registry/module/frontend/FrontendRequestComponent.java @@ -81,7 +81,7 @@ interface FrontendRequestComponent { WhoisServer whoisServer(); @Subcomponent.Builder - abstract class Builder implements RequestComponentBuilder { + abstract class Builder implements RequestComponentBuilder { @Override public abstract Builder requestModule(RequestModule requestModule); @Override public abstract FrontendRequestComponent build(); } diff --git a/java/google/registry/module/frontend/FrontendRequestHandler.java b/java/google/registry/module/frontend/FrontendRequestHandler.java index edcb07ccd..3063d2f98 100644 --- a/java/google/registry/module/frontend/FrontendRequestHandler.java +++ b/java/google/registry/module/frontend/FrontendRequestHandler.java @@ -22,8 +22,7 @@ import javax.inject.Inject; import javax.inject.Provider; /** Request handler for the frontend module. */ -public class FrontendRequestHandler - extends RequestHandler { +public class FrontendRequestHandler extends RequestHandler { @Inject FrontendRequestHandler( Provider componentBuilderProvider, diff --git a/java/google/registry/module/tools/ToolsRequestComponent.java b/java/google/registry/module/tools/ToolsRequestComponent.java index ac438428a..cf0eb2db9 100644 --- a/java/google/registry/module/tools/ToolsRequestComponent.java +++ b/java/google/registry/module/tools/ToolsRequestComponent.java @@ -81,7 +81,7 @@ interface ToolsRequestComponent { VerifyOteAction verifyOteAction(); @Subcomponent.Builder - abstract class Builder implements RequestComponentBuilder { + abstract class Builder implements RequestComponentBuilder { @Override public abstract Builder requestModule(RequestModule requestModule); @Override public abstract ToolsRequestComponent build(); } diff --git a/java/google/registry/module/tools/ToolsRequestHandler.java b/java/google/registry/module/tools/ToolsRequestHandler.java index d21a4c402..eee4e8538 100644 --- a/java/google/registry/module/tools/ToolsRequestHandler.java +++ b/java/google/registry/module/tools/ToolsRequestHandler.java @@ -22,8 +22,7 @@ import javax.inject.Inject; import javax.inject.Provider; /** Request handler for the tools module. */ -public class ToolsRequestHandler - extends RequestHandler { +public class ToolsRequestHandler extends RequestHandler { @Inject ToolsRequestHandler( Provider componentBuilderProvider, diff --git a/java/google/registry/request/RequestComponentBuilder.java b/java/google/registry/request/RequestComponentBuilder.java index 83ae2a21c..bcfacfa9b 100644 --- a/java/google/registry/request/RequestComponentBuilder.java +++ b/java/google/registry/request/RequestComponentBuilder.java @@ -19,7 +19,7 @@ package google.registry.request; * * @see RequestHandler */ -public interface RequestComponentBuilder> { - B requestModule(RequestModule requestModule); +public interface RequestComponentBuilder { + RequestComponentBuilder requestModule(RequestModule requestModule); C build(); } diff --git a/java/google/registry/request/RequestHandler.java b/java/google/registry/request/RequestHandler.java index 336c7c991..df3576cdb 100644 --- a/java/google/registry/request/RequestHandler.java +++ b/java/google/registry/request/RequestHandler.java @@ -67,14 +67,13 @@ import javax.servlet.http.HttpServletResponse; *

This class also enforces the {@link Action#requireLogin() requireLogin} setting. * * @param request component type - * @param builder for the request component */ -public class RequestHandler> { +public class RequestHandler { private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); private final Router router; - private final Provider requestComponentBuilderProvider; + private final Provider> requestComponentBuilderProvider; private final UserService userService; private final RequestAuthenticator requestAuthenticator; private final XsrfTokenManager xsrfTokenManager; @@ -94,7 +93,7 @@ public class RequestHandler> { * @param xsrfTokenManager an instance of the {@link XsrfTokenManager} class */ protected RequestHandler( - Provider requestComponentBuilderProvider, + Provider> requestComponentBuilderProvider, UserService userService, RequestAuthenticator requestAuthenticator, XsrfTokenManager xsrfTokenManager) { @@ -103,9 +102,9 @@ public class RequestHandler> { } /** Creates a new RequestHandler with an explicit component class for test purposes. */ - public static > RequestHandler createForTest( + public static RequestHandler createForTest( Class component, - Provider requestComponentBuilderProvider, + Provider> requestComponentBuilderProvider, UserService userService, RequestAuthenticator requestAuthenticator, XsrfTokenManager xsrfTokenManager) { @@ -119,7 +118,7 @@ public class RequestHandler> { private RequestHandler( @Nullable Class component, - Provider requestComponentBuilderProvider, + Provider> requestComponentBuilderProvider, UserService userService, RequestAuthenticator requestAuthenticator, XsrfTokenManager xsrfTokenManager) { diff --git a/javatests/google/registry/request/RequestHandlerTest.java b/javatests/google/registry/request/RequestHandlerTest.java index da2125588..3bb49ac20 100644 --- a/javatests/google/registry/request/RequestHandlerTest.java +++ b/javatests/google/registry/request/RequestHandlerTest.java @@ -194,7 +194,7 @@ public final class RequestHandlerTest { } /** Fake Builder for the fake component above to satisfy RequestHandler expectations. */ - public abstract class Builder implements RequestComponentBuilder { + public abstract class Builder implements RequestComponentBuilder { @Override public Builder requestModule(RequestModule requestModule) { component.setRequestModule(requestModule); @@ -222,7 +222,7 @@ public final class RequestHandlerTest { private final Component component = new Component(); private final StringWriter httpOutput = new StringWriter(); - private RequestHandler handler; + private RequestHandler handler; private AuthResult providedAuthResult = null; private final User testUser = new User("test@example.com", "test@example.com"); private RequestAuthenticator requestAuthenticator; @@ -244,7 +244,7 @@ public final class RequestHandlerTest { new LegacyAuthenticationMechanism(userService)); // Initialize here, not inline, so that we pick up the mocked UserService. - handler = RequestHandler.createForTest( + handler = RequestHandler.createForTest( Component.class, Providers.of(new Builder() { @Override