allow Authenticators to throw ServiceException

tangiel · tangiel · commit 0caa0d4e4801 · 2017-02-28T15:43:35.000-08:00
This feature request lets Authenticators throw ServiceExcpetions, which
are then written in the standard JSON format. This change also refactors
exception handling when calling an API method to clean up the logic a
bit, and change the logging level so that 5xx-level ServiceExceptions
log at the WARNING level, and others at INFO.
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/SystemService.java b/endpoints-framework/src/main/java/com/google/api/server/spi/SystemService.java
@@ -345,30 +345,17 @@ public Method findServiceMethod(Object service, String methodName) throws Servic
   public void invokeServiceMethod(Object service, Method method, ParamReader paramReader,
       ResultWriter resultWriter) throws IOException {
 
-    Object[] params;
     try {
-      params = paramReader.read();
+      Object[] params = paramReader.read();
       logger.log(Level.FINE, "params={0} (String)", Arrays.toString(params));
-    } catch (BadRequestException e) {
-      resultWriter.writeError(e);
-      return;
-    }
-
-    Object response;
-    try {
-      response = method.invoke(service, params);
-    } catch (IllegalArgumentException e) {
-      logger.log(Level.SEVERE, "exception occurred while calling backend method", e);
-      resultWriter.writeError(new BadRequestException(e));
-      return;
-    } catch (IllegalAccessException e) {
+      Object response = method.invoke(service, params);
+      resultWriter.write(response);
+    } catch (IllegalArgumentException | IllegalAccessException e) {
       logger.log(Level.SEVERE, "exception occurred while calling backend method", e);
       resultWriter.writeError(new BadRequestException(e));
-      return;
     } catch (InvocationTargetException e) {
       Throwable cause = e.getCause();
       Level level = Level.INFO;
-
       if (cause instanceof ServiceException) {
         resultWriter.writeError((ServiceException) cause);
       } else if (cause instanceof IllegalArgumentException) {
@@ -378,18 +365,19 @@ public void invokeServiceMethod(Object service, Method method, ParamReader param
       } else if (isOAuthRequestException(cause.getClass())) {
         resultWriter.writeError(new UnauthorizedException(cause));
       } else if (cause.getCause() != null && cause.getCause() instanceof ServiceException) {
-        cause = cause.getCause();
-        resultWriter.writeError((ServiceException) cause);
+        ServiceException serviceException = (ServiceException) cause.getCause();
+        level = getLoggingLevel(serviceException);
+        resultWriter.writeError(serviceException);
       } else {
         level = Level.SEVERE;
         resultWriter.writeError(new InternalServerErrorException(cause));
       }
-      logger.log(level, "exception occurred while calling backend method",
-          cause);
-      return;
+      logger.log(level, "exception occurred while calling backend method", cause);
+    } catch (ServiceException e) {
+      Level level = getLoggingLevel(e);
+      logger.log(level, "exception occurred while calling backend method", e);
+      resultWriter.writeError(e);
     }
-
-    resultWriter.write(response);
   }
 
   /**
@@ -413,6 +401,10 @@ private void validateRegisteredServices(ApiConfigValidator validator) throws Api
     }
   }
 
+  private static Level getLoggingLevel(ServiceException e) {
+    return e.getStatusCode() >= 500 ? Level.SEVERE : Level.INFO;
+  }
+
   private static boolean isOAuthRequestException(Class<?> clazz) {
     while (Object.class != clazz) {
       if (OAUTH_EXCEPTION_CLASS.equals(clazz.getName())) {
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/Authenticator.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/Authenticator.java
@@ -15,6 +15,7 @@
  */
 package com.google.api.server.spi.config;
 
+import com.google.api.server.spi.ServiceException;
 import com.google.api.server.spi.auth.common.User;
 
 import javax.servlet.http.HttpServletRequest;
@@ -35,5 +36,5 @@ public interface Authenticator {
    *
    * @return The authenticated user or null if there is no auth or auth has failed.
    */
-  User authenticate(HttpServletRequest request);
+  User authenticate(HttpServletRequest request) throws ServiceException;
 }
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/request/Auth.java b/endpoints-framework/src/main/java/com/google/api/server/spi/request/Auth.java
@@ -16,6 +16,7 @@
 package com.google.api.server.spi.request;
 
 import com.google.api.server.spi.EnvUtil;
+import com.google.api.server.spi.ServiceException;
 import com.google.api.server.spi.auth.EndpointsAuthenticator;
 import com.google.api.server.spi.auth.common.User;
 import com.google.api.server.spi.config.Authenticator;
@@ -92,7 +93,7 @@ Iterable<Authenticator> getAuthenticatorInstances() {
   /**
    * Authenticate the request and retrieve a {@code User}. Should only run once per request.
    */
-  User authenticate() {
+  User authenticate() throws ServiceException {
     Iterable<Authenticator> authenticators = getAuthenticatorInstances();
     User user = null;
     if (authenticators != null) {
@@ -110,7 +111,7 @@ User authenticate() {
    * Authenticate the request and retrieve an {@code com.google.appengine.api.users.User}. Should
    * only run once per request.
    */
-  com.google.appengine.api.users.User authenticateAppEngineUser() {
+  com.google.appengine.api.users.User authenticateAppEngineUser() throws ServiceException {
     if (!EnvUtil.isRunningOnAppEngine()) {
       return null;
     }
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/request/ParamReader.java b/endpoints-framework/src/main/java/com/google/api/server/spi/request/ParamReader.java
@@ -15,7 +15,7 @@
  */
 package com.google.api.server.spi.request;
 
-import com.google.api.server.spi.response.BadRequestException;
+import com.google.api.server.spi.ServiceException;
 
 /**
  * Reads a request and returns an array of parameter values.
@@ -24,8 +24,8 @@ public interface ParamReader {
 
   /**
    * Reads parameters in JSON into an Object array to be used to invoke an Endpoint method.
-   * @throws BadRequestException when reading of input stream failed, input JSON is invalid,
-   * or cannot be mapped into parameter objects.
+   * @throws ServiceException when reading of input stream failed, input JSON is invalid,
+   * or cannot be mapped into parameter objects, or user authentication fails.
    */
-  Object[] read() throws BadRequestException;
+  Object[] read() throws ServiceException;
 }
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java b/endpoints-framework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java
@@ -17,6 +17,7 @@
 
 import com.google.api.server.spi.EndpointMethod;
 import com.google.api.server.spi.IoUtil;
+import com.google.api.server.spi.ServiceException;
 import com.google.api.server.spi.Strings;
 import com.google.api.server.spi.config.model.ApiMethodConfig;
 import com.google.api.server.spi.config.model.ApiParameterConfig;
@@ -69,7 +70,7 @@ public RestServletRequestParamReader(EndpointMethod method,
   }
 
   @Override
-  public Object[] read() throws BadRequestException {
+  public Object[] read() throws ServiceException {
     // Assumes input stream to be encoded in UTF-8
     // TODO: Take charset from content-type as encoding
     try {
diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/request/ServletRequestParamReader.java b/endpoints-framework/src/main/java/com/google/api/server/spi/request/ServletRequestParamReader.java
@@ -18,6 +18,7 @@
 import com.google.api.server.spi.ConfiguredObjectMapper;
 import com.google.api.server.spi.EndpointMethod;
 import com.google.api.server.spi.IoUtil;
+import com.google.api.server.spi.ServiceException;
 import com.google.api.server.spi.auth.common.User;
 import com.google.api.server.spi.config.Named;
 import com.google.api.server.spi.config.annotationreader.AnnotationUtil;
@@ -122,7 +123,7 @@ protected static List<String> getParameterNames(EndpointMethod endpointMethod)
   }
 
   protected Object[] deserializeParams(JsonNode node) throws IOException, IllegalAccessException,
-      InvocationTargetException, NoSuchMethodException {
+      InvocationTargetException, NoSuchMethodException, ServiceException {
     EndpointMethod method = getMethod();
     Class<?>[] paramClasses = method.getParameterClasses();
     TypeToken<?>[] paramTypes = method.getParameterTypes();
@@ -187,12 +188,12 @@ protected Object[] deserializeParams(JsonNode node) throws IOException, IllegalA
   }
 
   @VisibleForTesting
-  User getUser() {
+  User getUser() throws ServiceException {
     return Auth.from(servletRequest).authenticate();
   }
 
   @VisibleForTesting
-  com.google.appengine.api.users.User getAppEngineUser() {
+  com.google.appengine.api.users.User getAppEngineUser() throws ServiceException {
     return Auth.from(servletRequest).authenticateAppEngineUser();
   }
 
@@ -297,7 +298,7 @@ public ServletRequestParamReader(EndpointMethod method, HttpServletRequest servl
   }
 
   @Override
-  public Object[] read() throws BadRequestException {
+  public Object[] read() throws ServiceException {
     // Assumes input stream to be encoded in UTF-8
     // TODO: Take charset from content-type as encoding
     try {

Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@`
`15`	`15`	`*/`
`16`	`16`	`package com.google.api.server.spi.config;`
`17`	`17`
	`18`	`+import com.google.api.server.spi.ServiceException;`
`18`	`19`	`import com.google.api.server.spi.auth.common.User;`
`19`	`20`
`20`	`21`	`import javax.servlet.http.HttpServletRequest;`
`@@ -35,5 +36,5 @@ public interface Authenticator {`
`35`	`36`	`*`
`36`	`37`	`* @return The authenticated user or null if there is no auth or auth has failed.`
`37`	`38`	`*/`
`38`		`- User authenticate(HttpServletRequest request);`
	`39`	`+ User authenticate(HttpServletRequest request) throws ServiceException;`
`39`	`40`	`}`