From ea3a8dfa9d7d85cf4427d81b1781c99b03918e2a Mon Sep 17 00:00:00 2001
From: nickfelt <nickfelt@google.com>
Date: Thu, 23 Feb 2017 09:36:50 -0800
Subject: [PATCH] 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
---
 java/google/registry/request/Router.java      |  3 +++
 .../registry/request/RequestHandlerTest.java  |  1 +
 .../google/registry/request/RouterTest.java   | 25 +++++++++++--------
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/java/google/registry/request/Router.java b/java/google/registry/request/Router.java
index c3a7e9e90..dcb54bf6f 100644
--- a/java/google/registry/request/Router.java
+++ b/java/google/registry/request/Router.java
@@ -14,6 +14,7 @@
 
 package google.registry.request;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Throwables.throwIfUnchecked;
 
 import com.google.common.base.Function;
@@ -47,6 +48,8 @@ final class Router {
 
   private Router(Class<?> 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. */
diff --git a/javatests/google/registry/request/RequestHandlerTest.java b/javatests/google/registry/request/RequestHandlerTest.java
index 3bb49ac20..8dac65512 100644
--- a/javatests/google/registry/request/RequestHandlerTest.java
+++ b/javatests/google/registry/request/RequestHandlerTest.java
@@ -363,6 +363,7 @@ public final class RequestHandlerTest {
   @Test
   public void testNullness() {
     NullPointerTester tester = new NullPointerTester();
+    tester.setDefault(Class.class, Component.class);
     tester.setDefault(RequestAuthenticator.class, requestAuthenticator);
     tester.setDefault(XsrfTokenManager.class, xsrfTokenManager);
     tester.testAllPublicStaticMethods(RequestHandler.class);
diff --git a/javatests/google/registry/request/RouterTest.java b/javatests/google/registry/request/RouterTest.java
index 34484e2d4..bf284da44 100644
--- a/javatests/google/registry/request/RouterTest.java
+++ b/javatests/google/registry/request/RouterTest.java
@@ -37,15 +37,17 @@ public final class RouterTest {
   public interface Empty {}
 
   @Test
-  public void testRoute_noRoutes_returnsAbsent() throws Exception {
-    assertThat(Router.create(Empty.class).route("")).isAbsent();
-    assertThat(Router.create(Empty.class).route("/")).isAbsent();
+  public void testRoute_noRoutes_throws() throws Exception {
+    thrown.expect(
+        IllegalArgumentException.class,
+        "No routes found for class: google.registry.request.RouterTest.Empty");
+    Router.create(Empty.class);
   }
 
   ////////////////////////////////////////////////////////////////////////////////////////////////
 
   @Action(path = "/sloth")
-  public final class SlothTask implements Runnable {
+  public static final class SlothTask implements Runnable {
     @Override
     public void run() {}
   }
@@ -75,7 +77,7 @@ public final class RouterTest {
   ////////////////////////////////////////////////////////////////////////////////////////////////
 
   @Action(path = "/prefix", isPrefix = true)
-  public final class PrefixTask implements Runnable {
+  public static final class PrefixTask implements Runnable {
     @Override
     public void run() {}
   }
@@ -101,7 +103,7 @@ public final class RouterTest {
   ////////////////////////////////////////////////////////////////////////////////////////////////
 
   @Action(path = "/prefix/long", isPrefix = true)
-  public final class LongTask implements Runnable {
+  public static final class LongTask implements Runnable {
     @Override
     public void run() {}
   }
@@ -140,20 +142,23 @@ public final class RouterTest {
   }
 
   @Test
-  public void testRoute_methodsInComponentAreIgnored_noRoutes() throws Exception {
-    assertThat(Router.create(WeirdMethodsComponent.class).route("/sloth")).isAbsent();
+  public void testRoute_methodsInComponentAreIgnored_throws() throws Exception {
+    thrown.expect(
+        IllegalArgumentException.class,
+        "No routes found for class: google.registry.request.RouterTest.WeirdMethodsComponent");
+    Router.create(WeirdMethodsComponent.class);
   }
 
   ////////////////////////////////////////////////////////////////////////////////////////////////
 
   @Action(path = "/samePathAsOtherTask")
-  public final class DuplicateTask1 implements Runnable {
+  public static final class DuplicateTask1 implements Runnable {
     @Override
     public void run() {}
   }
 
   @Action(path = "/samePathAsOtherTask")
-  public final class DuplicateTask2 implements Runnable {
+  public static final class DuplicateTask2 implements Runnable {
     @Override
     public void run() {}
   }