diff --git a/java/google/registry/module/backend/BackendServlet.java b/java/google/registry/module/backend/BackendServlet.java index 4765dc9ad..b5360a3a6 100644 --- a/java/google/registry/module/backend/BackendServlet.java +++ b/java/google/registry/module/backend/BackendServlet.java @@ -14,16 +14,11 @@ package google.registry.module.backend; -import static java.util.Arrays.asList; - -import com.google.common.base.Function; -import com.google.common.collect.FluentIterable; import google.registry.monitoring.metrics.MetricReporter; import google.registry.request.RequestHandler; import google.registry.request.RequestModule; import google.registry.util.FormattingLogger; import java.io.IOException; -import java.lang.reflect.Method; import java.security.Security; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -40,14 +35,7 @@ public final class BackendServlet extends HttpServlet { private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); private static final RequestHandler requestHandler = - RequestHandler.create(BackendRequestComponent.class, FluentIterable - .from(asList(BackendRequestComponent.class.getMethods())) - .transform(new Function() { - @Override - public Method apply(Method method) { - method.setAccessible(true); // Make App Engine's security manager happy. - return method; - }})); + RequestHandler.create(BackendRequestComponent.class); @Override public void init() { diff --git a/java/google/registry/module/frontend/FrontendServlet.java b/java/google/registry/module/frontend/FrontendServlet.java index 969b11994..a1e701b46 100644 --- a/java/google/registry/module/frontend/FrontendServlet.java +++ b/java/google/registry/module/frontend/FrontendServlet.java @@ -14,16 +14,11 @@ package google.registry.module.frontend; -import static java.util.Arrays.asList; - -import com.google.common.base.Function; -import com.google.common.collect.FluentIterable; import google.registry.monitoring.metrics.MetricReporter; import google.registry.request.RequestHandler; import google.registry.request.RequestModule; import google.registry.util.FormattingLogger; import java.io.IOException; -import java.lang.reflect.Method; import java.security.Security; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -40,14 +35,7 @@ public final class FrontendServlet extends HttpServlet { private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); private static final RequestHandler requestHandler = - RequestHandler.create(FrontendRequestComponent.class, FluentIterable - .from(asList(FrontendRequestComponent.class.getMethods())) - .transform(new Function() { - @Override - public Method apply(Method method) { - method.setAccessible(true); // Make App Engine's security manager happy. - return method; - }})); + RequestHandler.create(FrontendRequestComponent.class); @Override public void init() { diff --git a/java/google/registry/module/tools/ToolsServlet.java b/java/google/registry/module/tools/ToolsServlet.java index 3cd2a3d29..7468cae64 100644 --- a/java/google/registry/module/tools/ToolsServlet.java +++ b/java/google/registry/module/tools/ToolsServlet.java @@ -14,14 +14,9 @@ package google.registry.module.tools; -import static java.util.Arrays.asList; - -import com.google.common.base.Function; -import com.google.common.collect.FluentIterable; import google.registry.request.RequestHandler; import google.registry.request.RequestModule; import java.io.IOException; -import java.lang.reflect.Method; import java.security.Security; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -34,14 +29,7 @@ public final class ToolsServlet extends HttpServlet { private static final ToolsComponent component = DaggerToolsComponent.create(); private static final RequestHandler requestHandler = - RequestHandler.create(ToolsRequestComponent.class, FluentIterable - .from(asList(ToolsRequestComponent.class.getMethods())) - .transform(new Function() { - @Override - public Method apply(Method method) { - method.setAccessible(true); // Make App Engine's security manager happy. - return method; - }})); + RequestHandler.create(ToolsRequestComponent.class); @Override public void init() { diff --git a/java/google/registry/request/RequestHandler.java b/java/google/registry/request/RequestHandler.java index ecb718e37..a31a56f7d 100644 --- a/java/google/registry/request/RequestHandler.java +++ b/java/google/registry/request/RequestHandler.java @@ -31,7 +31,6 @@ import com.google.common.base.Optional; import google.registry.util.FormattingLogger; import google.registry.util.NonFinalForTesting; import java.io.IOException; -import java.lang.reflect.Method; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.joda.time.Duration; @@ -76,20 +75,9 @@ public final class RequestHandler { @NonFinalForTesting private static UserService userService = UserServiceFactory.getUserService(); - /** - * Creates a new request processor based off your component methods. - * - *

Warning: When using the App Engine platform, you must call - * {@link Method#setAccessible(boolean) setAccessible(true)} on all your component {@link Method} - * instances, from within the same package as the component. This is due to cross-package - * reflection restrictions. - * - * @param methods is the result of calling {@link Class#getMethods()} on {@code component}, which - * are filtered to only include those with no arguments returning a {@link Runnable} with an - * {@link Action} annotation - */ - public static RequestHandler create(Class component, Iterable methods) { - return new RequestHandler<>(component, Router.create(methods)); + /** Creates a new request processor based off your component methods. */ + public static RequestHandler create(Class component) { + return new RequestHandler<>(component, Router.create(component)); } private final Router router; @@ -102,8 +90,7 @@ public final class RequestHandler { /** * Runs the appropriate action for a servlet request. * - * @param component is an instance of the component type whose methods were passed to - * {@link #create(Class, Iterable)} + * @param component is an instance of the component type passed to {@link #create(Class)} */ public void handleRequest(HttpServletRequest req, HttpServletResponse rsp, C component) throws IOException { diff --git a/java/google/registry/request/Router.java b/java/google/registry/request/Router.java index 0349c3d7c..607249bc2 100644 --- a/java/google/registry/request/Router.java +++ b/java/google/registry/request/Router.java @@ -25,7 +25,7 @@ import java.util.Map; import java.util.TreeMap; /** - * Path prefix request router for Nomulus. + * Path prefix request router. * *

See the documentation of {@link RequestHandler} for more information. * @@ -37,14 +37,15 @@ import java.util.TreeMap; */ final class Router { - static Router create(Iterable componentMethods) { - return new Router(extractRoutesFromComponent(componentMethods)); + /** Create a new Router for the given component class. */ + static Router create(Class componentClass) { + return new Router(componentClass); } private final ImmutableSortedMap routes; - private Router(ImmutableSortedMap routes) { - this.routes = routes; + private Router(Class componentClass) { + this.routes = extractRoutesFromComponent(componentClass); } /** Returns the appropriate action route for a request. */ @@ -61,10 +62,12 @@ final class Router { } private static - ImmutableSortedMap extractRoutesFromComponent(Iterable methods) { + ImmutableSortedMap extractRoutesFromComponent(Class componentClass) { ImmutableSortedMap.Builder routes = new ImmutableSortedMap.Builder<>(Ordering.natural()); - for (Method method : methods) { + for (Method method : componentClass.getMethods()) { + // Make App Engine's security manager happy. + method.setAccessible(true); if (!isDaggerInstantiatorOfType(Runnable.class, method)) { continue; } diff --git a/javatests/google/registry/request/RequestHandlerTest.java b/javatests/google/registry/request/RequestHandlerTest.java index e2cab9df9..cdbc45c9c 100644 --- a/javatests/google/registry/request/RequestHandlerTest.java +++ b/javatests/google/registry/request/RequestHandlerTest.java @@ -24,7 +24,6 @@ import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import com.google.appengine.api.users.UserService; -import com.google.common.collect.ImmutableList; import com.google.common.testing.NullPointerTester; import google.registry.request.HttpException.ServiceUnavailableException; import google.registry.testing.AppEngineRule; @@ -149,8 +148,7 @@ public final class RequestHandlerTest { private final Component component = new Component(); private final StringWriter httpOutput = new StringWriter(); - private final RequestHandler handler = - RequestHandler.create(Component.class, ImmutableList.copyOf(Component.class.getMethods())); + private final RequestHandler handler = RequestHandler.create(Component.class); @Before public void before() throws Exception { diff --git a/javatests/google/registry/request/RouterTest.java b/javatests/google/registry/request/RouterTest.java index 0fa0ac486..2134332db 100644 --- a/javatests/google/registry/request/RouterTest.java +++ b/javatests/google/registry/request/RouterTest.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.base.Function; import com.google.common.base.Optional; -import com.google.common.collect.ImmutableList; import google.registry.testing.ExceptionRule; import java.util.concurrent.Callable; import org.junit.Rule; @@ -33,18 +32,14 @@ public final class RouterTest { @Rule public final ExceptionRule thrown = new ExceptionRule(); - private Router create(Class clazz) { - return Router.create(ImmutableList.copyOf(clazz.getMethods())); - } - //////////////////////////////////////////////////////////////////////////////////////////////// public interface Empty {} @Test public void testRoute_noRoutes_returnsAbsent() throws Exception { - assertThat(create(Empty.class).route("")).isAbsent(); - assertThat(create(Empty.class).route("/")).isAbsent(); + assertThat(Router.create(Empty.class).route("")).isAbsent(); + assertThat(Router.create(Empty.class).route("/")).isAbsent(); } //////////////////////////////////////////////////////////////////////////////////////////////// @@ -61,7 +56,7 @@ public final class RouterTest { @Test public void testRoute_pathMatch_returnsRoute() throws Exception { - Optional route = create(SlothComponent.class).route("/sloth"); + Optional route = Router.create(SlothComponent.class).route("/sloth"); assertThat(route).isPresent(); assertThat(route.get().action().path()).isEqualTo("/sloth"); assertThat(route.get().instantiator()).isInstanceOf(Function.class); @@ -69,12 +64,12 @@ public final class RouterTest { @Test public void testRoute_pathMismatch_returnsAbsent() throws Exception { - assertThat(create(SlothComponent.class).route("/doge")).isAbsent(); + assertThat(Router.create(SlothComponent.class).route("/doge")).isAbsent(); } @Test public void testRoute_pathIsAPrefix_notAllowedByDefault() throws Exception { - assertThat(create(SlothComponent.class).route("/sloth/extra")).isAbsent(); + assertThat(Router.create(SlothComponent.class).route("/sloth/extra")).isAbsent(); } //////////////////////////////////////////////////////////////////////////////////////////////// @@ -91,16 +86,16 @@ public final class RouterTest { @Test public void testRoute_prefixMatches_returnsRoute() throws Exception { - assertThat(create(PrefixComponent.class).route("/prefix")).isPresent(); - assertThat(create(PrefixComponent.class).route("/prefix/extra")).isPresent(); + assertThat(Router.create(PrefixComponent.class).route("/prefix")).isPresent(); + assertThat(Router.create(PrefixComponent.class).route("/prefix/extra")).isPresent(); } @Test public void testRoute_prefixDoesNotMatch_returnsAbsent() throws Exception { - assertThat(create(PrefixComponent.class).route("")).isAbsent(); - assertThat(create(PrefixComponent.class).route("/")).isAbsent(); - assertThat(create(PrefixComponent.class).route("/ulysses")).isAbsent(); - assertThat(create(PrefixComponent.class).route("/man/of/sadness")).isAbsent(); + assertThat(Router.create(PrefixComponent.class).route("")).isAbsent(); + assertThat(Router.create(PrefixComponent.class).route("/")).isAbsent(); + assertThat(Router.create(PrefixComponent.class).route("/ulysses")).isAbsent(); + assertThat(Router.create(PrefixComponent.class).route("/man/of/sadness")).isAbsent(); } //////////////////////////////////////////////////////////////////////////////////////////////// @@ -118,21 +113,21 @@ public final class RouterTest { @Test public void testRoute_prefixAndLongPathMatch_returnsLongerPath() throws Exception { - Optional route = create(LongPathComponent.class).route("/prefix/long"); + Optional route = Router.create(LongPathComponent.class).route("/prefix/long"); assertThat(route).isPresent(); assertThat(route.get().action().path()).isEqualTo("/prefix/long"); } @Test public void testRoute_prefixAndLongerPrefixMatch_returnsLongerPrefix() throws Exception { - Optional route = create(LongPathComponent.class).route("/prefix/longer"); + Optional route = Router.create(LongPathComponent.class).route("/prefix/longer"); assertThat(route).isPresent(); assertThat(route.get().action().path()).isEqualTo("/prefix/long"); } @Test public void testRoute_onlyShortPrefixMatches_returnsShortPrefix() throws Exception { - Optional route = create(LongPathComponent.class).route("/prefix/cat"); + Optional route = Router.create(LongPathComponent.class).route("/prefix/cat"); assertThat(route).isPresent(); assertThat(route.get().action().path()).isEqualTo("/prefix"); } @@ -146,7 +141,7 @@ public final class RouterTest { @Test public void testRoute_methodsInComponentAreIgnored_noRoutes() throws Exception { - assertThat(create(WeirdMethodsComponent.class).route("/sloth")).isAbsent(); + assertThat(Router.create(WeirdMethodsComponent.class).route("/sloth")).isAbsent(); } //////////////////////////////////////////////////////////////////////////////////////////////// @@ -171,6 +166,6 @@ public final class RouterTest { @Test public void testCreate_twoTasksWithSameMethodAndPath_resultsInError() throws Exception { thrown.expect(IllegalArgumentException.class, "Multiple entries with same key"); - create(DuplicateComponent.class); + Router.create(DuplicateComponent.class); } }