Make Router reject classes with no @Action-returning methods

This provides a safeguard against using TypeInstantiator to resolve the component class, where if resolution is done incorrectly, you end up with java.lang.Object.  Formerly, that would have "succeeded" in generating a Router for Object, which of course has no methods that return @Action classes.  Such a router is pretty useless, so it's better to make Router stricter and have it fail if you give it such a class by accident.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=148353224
This commit is contained in:
nickfelt 2017-02-23 09:36:50 -08:00 committed by Ben McIlwain
parent d8ce444251
commit ea3a8dfa9d
3 changed files with 19 additions and 10 deletions

View file

@ -14,6 +14,7 @@
package google.registry.request; package google.registry.request;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Throwables.throwIfUnchecked; import static com.google.common.base.Throwables.throwIfUnchecked;
import com.google.common.base.Function; import com.google.common.base.Function;
@ -47,6 +48,8 @@ final class Router {
private Router(Class<?> componentClass) { private Router(Class<?> componentClass) {
this.routes = extractRoutesFromComponent(componentClass); this.routes = extractRoutesFromComponent(componentClass);
checkArgument(
!this.routes.isEmpty(), "No routes found for class: %s", componentClass.getCanonicalName());
} }
/** Returns the appropriate action route for a request. */ /** Returns the appropriate action route for a request. */

View file

@ -363,6 +363,7 @@ public final class RequestHandlerTest {
@Test @Test
public void testNullness() { public void testNullness() {
NullPointerTester tester = new NullPointerTester(); NullPointerTester tester = new NullPointerTester();
tester.setDefault(Class.class, Component.class);
tester.setDefault(RequestAuthenticator.class, requestAuthenticator); tester.setDefault(RequestAuthenticator.class, requestAuthenticator);
tester.setDefault(XsrfTokenManager.class, xsrfTokenManager); tester.setDefault(XsrfTokenManager.class, xsrfTokenManager);
tester.testAllPublicStaticMethods(RequestHandler.class); tester.testAllPublicStaticMethods(RequestHandler.class);

View file

@ -37,15 +37,17 @@ public final class RouterTest {
public interface Empty {} public interface Empty {}
@Test @Test
public void testRoute_noRoutes_returnsAbsent() throws Exception { public void testRoute_noRoutes_throws() throws Exception {
assertThat(Router.create(Empty.class).route("")).isAbsent(); thrown.expect(
assertThat(Router.create(Empty.class).route("/")).isAbsent(); IllegalArgumentException.class,
"No routes found for class: google.registry.request.RouterTest.Empty");
Router.create(Empty.class);
} }
//////////////////////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////////////////////
@Action(path = "/sloth") @Action(path = "/sloth")
public final class SlothTask implements Runnable { public static final class SlothTask implements Runnable {
@Override @Override
public void run() {} public void run() {}
} }
@ -75,7 +77,7 @@ public final class RouterTest {
//////////////////////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////////////////////
@Action(path = "/prefix", isPrefix = true) @Action(path = "/prefix", isPrefix = true)
public final class PrefixTask implements Runnable { public static final class PrefixTask implements Runnable {
@Override @Override
public void run() {} public void run() {}
} }
@ -101,7 +103,7 @@ public final class RouterTest {
//////////////////////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////////////////////
@Action(path = "/prefix/long", isPrefix = true) @Action(path = "/prefix/long", isPrefix = true)
public final class LongTask implements Runnable { public static final class LongTask implements Runnable {
@Override @Override
public void run() {} public void run() {}
} }
@ -140,20 +142,23 @@ public final class RouterTest {
} }
@Test @Test
public void testRoute_methodsInComponentAreIgnored_noRoutes() throws Exception { public void testRoute_methodsInComponentAreIgnored_throws() throws Exception {
assertThat(Router.create(WeirdMethodsComponent.class).route("/sloth")).isAbsent(); thrown.expect(
IllegalArgumentException.class,
"No routes found for class: google.registry.request.RouterTest.WeirdMethodsComponent");
Router.create(WeirdMethodsComponent.class);
} }
//////////////////////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////////////////////
@Action(path = "/samePathAsOtherTask") @Action(path = "/samePathAsOtherTask")
public final class DuplicateTask1 implements Runnable { public static final class DuplicateTask1 implements Runnable {
@Override @Override
public void run() {} public void run() {}
} }
@Action(path = "/samePathAsOtherTask") @Action(path = "/samePathAsOtherTask")
public final class DuplicateTask2 implements Runnable { public static final class DuplicateTask2 implements Runnable {
@Override @Override
public void run() {} public void run() {}
} }