From 04402057a92b970b32911baa813e8d654af39a81 Mon Sep 17 00:00:00 2001 From: Daniel Tang Date: Fri, 7 Apr 2017 11:37:45 -0700 Subject: [PATCH 1/3] make ServletInitializationParameters an AutoValue This change cleans up ServletInitializationParameters to use AutoValue, since this is a pretty standard value class with a builder. API compatibility is retained except for setIllegalArgumentIsBackendError being renamed to setIllegalArgumentBackendError. The "restricted" parameter is now also marked as deprecated, as it no longer serves any purpose. --- .../spi/ServletInitializationParameters.java | 292 ++++++++---------- .../ServletInitializationParametersTest.java | 4 +- 2 files changed, 134 insertions(+), 162 deletions(-) diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java b/endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java index 98a2a4a6..10bef9d7 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java @@ -15,6 +15,7 @@ */ package com.google.api.server.spi; +import com.google.auto.value.AutoValue; import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Splitter; @@ -28,170 +29,68 @@ /** * Initialization parameters supported by the {@link EndpointsServlet}. */ -public class ServletInitializationParameters { - private static final String INIT_PARAM_NAME_SERVICES = "services"; - private static final String INIT_PARAM_NAME_RESTRICTED = "restricted"; - private static final String INIT_PARAM_NAME_CLIENT_ID_WHITELIST_ENABLED = - "clientIdWhitelistEnabled"; - private static final String INIT_PARAM_NAME_ILLEGAL_ARGUMENT_BACKEND_ERROR = - "illegalArgumentIsBackendError"; - private static final String INIT_PARAM_NAME_ENABLE_EXCEPTION_COMPATIBILITY = - "enableExceptionCompatibility"; +@AutoValue +public abstract class ServletInitializationParameters { + // Initialization parameter names used to extract values from a ServletConfig. + private static final String SERVICES = "services"; + private static final String RESTRICTED = "restricted"; + private static final String CLIENT_ID_WHITELIST_ENABLED = "clientIdWhitelistEnabled"; + private static final String ILLEGAL_ARGUMENT_BACKEND_ERROR = "illegalArgumentIsBackendError"; + private static final String EXCEPTION_COMPATIBILITY = "enableExceptionCompatibility"; private static final Splitter CSV_SPLITTER = Splitter.on(',').omitEmptyStrings().trimResults(); private static final Joiner CSV_JOINER = Joiner.on(',').skipNulls(); private static final Function, String> CLASS_TO_NAME = new Function, String>() { - @Override public String apply(Class clazz) { + @Override + public String apply(Class clazz) { return clazz.getName(); } }; - private final ImmutableSet> serviceClasses; - private final boolean isServletRestricted; - private final boolean isClientIdWhitelistEnabled; - private final boolean isIllegalArgumentBackendError; - private final boolean isExceptionCompatibilityEnabled; - - /** - * Returns a new {@link Builder} for this class. - */ - public static Builder builder() { - return new Builder(); - } - - /** - * Constructs a new instance from the provided {@link ServletConfig} and {@link ClassLoader}. - */ - public static ServletInitializationParameters fromServletConfig( - ServletConfig config, ClassLoader classLoader) throws ServletException { - Builder builder = builder(); - if (config != null) { - String serviceClassNames = config.getInitParameter(INIT_PARAM_NAME_SERVICES); - if (serviceClassNames != null) { - for (String serviceClassName : CSV_SPLITTER.split(serviceClassNames)) { - builder.addServiceClass(getClassForName(serviceClassName, classLoader)); - } - } - String isServletRestricted = config.getInitParameter(INIT_PARAM_NAME_RESTRICTED); - if (isServletRestricted != null) { - builder.setRestricted(parseBoolean(isServletRestricted, "is servlet restricted")); - } - String isClientIdWhitelistEnabled = - config.getInitParameter(INIT_PARAM_NAME_CLIENT_ID_WHITELIST_ENABLED); - if (isClientIdWhitelistEnabled != null) { - builder.setClientIdWhitelistEnabled( - parseBoolean(isClientIdWhitelistEnabled, "is the client id whitelist enabled")); - } - String isIllegalArgumentBackendError = - config.getInitParameter(INIT_PARAM_NAME_ILLEGAL_ARGUMENT_BACKEND_ERROR); - if (isIllegalArgumentBackendError != null) { - builder.setIllegalArgumentIsBackendError(parseBoolean( - isIllegalArgumentBackendError, "is IllegalArgumentException a backend error")); - } - String isExceptionCompatibilityEnabled = - config.getInitParameter(INIT_PARAM_NAME_ENABLE_EXCEPTION_COMPATIBILITY); - if (isExceptionCompatibilityEnabled != null) { - builder.setExceptionCompatibilityEnabled( - parseBoolean(isExceptionCompatibilityEnabled, "is exception compatibility enabled")); - } - } - return builder.build(); - } - - private static boolean parseBoolean(String booleanString, String descriptionForErrors) { - if ("true".equalsIgnoreCase(booleanString)) { - return true; - } else if ("false".equalsIgnoreCase(booleanString)) { - return false; - } - throw new IllegalArgumentException(String.format( - "Expected 'true' or 'false' for %s servlet initialization parameter but got '%s'", - descriptionForErrors, booleanString)); - } - - private static Class getClassForName(String className, ClassLoader classLoader) - throws ServletException { - try { - return Class.forName(className, true, classLoader); - } catch (ClassNotFoundException e) { - throw new ServletException(String.format("Cannot find service class: %s", className), e); - } - } - /** * Returns the endpoint service classes to serve. */ - public ImmutableSet> getServiceClasses() { - return serviceClasses; - } + public abstract ImmutableSet> getServiceClasses(); /** - * Returns {@code true} if the SPI servlet is restricted. + * Returns if the SPI servlet is restricted. + * + * @deprecated No longer serves any purpose and will be removed in a future release */ - public boolean isServletRestricted() { - return isServletRestricted; - } + @Deprecated + public abstract boolean isServletRestricted(); /** - * Returns {@code true} if client ID whitelisting is enabled. + * Returns if client ID whitelisting is enabled. */ - public boolean isClientIdWhitelistEnabled() { - return isClientIdWhitelistEnabled; - } + public abstract boolean isClientIdWhitelistEnabled(); /** - * Returns {@code true} if an {@link IllegalArgumentException} should be returned as a backend - * error (500) instead of a user error (400). + * Returns if an {@link IllegalArgumentException} should be returned as a backend error (500 + * level) instead of a user error (400 level). */ - public boolean isIllegalArgumentBackendError() { - return isIllegalArgumentBackendError; - } + public abstract boolean isIllegalArgumentBackendError(); /** - * Returns {@code true} if v1.0 style exceptions should be returned to users. In v1.0, certain - * codes are not permissible, and other codes are translated to other status codes. + * Returns if v1.0 style exceptions should be returned to users. In v1.0, certain codes are not + * permissible, and other codes are translated to other status codes. */ - public boolean isExceptionCompatibilityEnabled() { - return isExceptionCompatibilityEnabled; - } + public abstract boolean isExceptionCompatibilityEnabled(); - /** - * Returns the parameters as a {@link java.util.Map} of parameter name to {@link String} value. - */ - public ImmutableMap asMap() { - ImmutableMap.Builder parameterNameToValue = ImmutableMap.builder(); - parameterNameToValue.put(INIT_PARAM_NAME_SERVICES, - CSV_JOINER.join(Iterables.transform(serviceClasses, CLASS_TO_NAME))); - parameterNameToValue.put(INIT_PARAM_NAME_RESTRICTED, Boolean.toString(isServletRestricted)); - parameterNameToValue.put( - INIT_PARAM_NAME_CLIENT_ID_WHITELIST_ENABLED, Boolean.toString(isClientIdWhitelistEnabled)); - parameterNameToValue.put(INIT_PARAM_NAME_ILLEGAL_ARGUMENT_BACKEND_ERROR, - Boolean.toString(isIllegalArgumentBackendError)); - parameterNameToValue.put(INIT_PARAM_NAME_ENABLE_EXCEPTION_COMPATIBILITY, - Boolean.toString(isExceptionCompatibilityEnabled)); - return parameterNameToValue.build(); - } - - private ServletInitializationParameters( - ImmutableSet> serviceClasses, boolean isServletRestricted, - boolean isClientIdWhitelistEnabled, boolean isIllegalArgumentBackendError, - boolean isExceptionCompatibilityEnabled) { - this.serviceClasses = serviceClasses; - this.isServletRestricted = isServletRestricted; - this.isClientIdWhitelistEnabled = isClientIdWhitelistEnabled; - this.isIllegalArgumentBackendError = isIllegalArgumentBackendError; - this.isExceptionCompatibilityEnabled = isExceptionCompatibilityEnabled; + public static Builder builder() { + return new AutoValue_ServletInitializationParameters.Builder() + .setServletRestricted(true) + .setClientIdWhitelistEnabled(true) + .setIllegalArgumentBackendError(false) + .setExceptionCompatibilityEnabled(true); } /** * A builder for {@link ServletInitializationParameters}. */ - public static class Builder { + @AutoValue.Builder + public abstract static class Builder { private final ImmutableSet.Builder> serviceClasses = ImmutableSet.builder(); - private boolean isServletRestricted = true; - private boolean isClientIdWhitelistEnabled = true; - private boolean isIllegalArgumentBackendError = false; - private boolean isExceptionCompatibilityEnabled = true; /** * Adds an endpoint service class to serve. @@ -210,45 +109,118 @@ public Builder addServiceClasses(Iterable> serviceClasses) { } /** - * Sets if the SPI servlet is restricted ({@code true}) or not ({@code false}). If this - * method is not called, it defaults to {@code true}. + * Sets the complete list of endpoint service classes to serve. */ - public Builder setRestricted(boolean isServletRestricted) { - this.isServletRestricted = isServletRestricted; - return this; - } + public abstract Builder setServiceClasses(ImmutableSet> clazzes); /** - * Sets if the client ID whitelist is enabled ({@code true}) or not ({@code false}). If this - * method is not called, it defaults to {@code true}. + * Sets if the servlet is restricted. Defaults to {@code true}. + * + * @deprecated No longer serves any purpose and will be removed in a future release */ - public Builder setClientIdWhitelistEnabled(boolean isClientIdWhitelistEnabled) { - this.isClientIdWhitelistEnabled = isClientIdWhitelistEnabled; - return this; - } + @Deprecated + public abstract Builder setServletRestricted(boolean servletRestricted); /** - * Sets if an {@link IllegalArgumentException} should be treated as a backend error (500) - * instead of a user error (400). + * Sets if the servlet is restricted. Retained for API compatibility. + * + * @deprecated Retained for API compatibility */ - public Builder setIllegalArgumentIsBackendError(boolean isIllegalArgumentBackendError) { - this.isIllegalArgumentBackendError = isIllegalArgumentBackendError; - return this; + @Deprecated + public Builder setRestricted(boolean servletRestricted) { + return setServletRestricted(servletRestricted); } - public Builder setExceptionCompatibilityEnabled(boolean isExceptionCompatibilityEnabled) { - this.isExceptionCompatibilityEnabled = isExceptionCompatibilityEnabled; - return this; - } + /** + * Sets if the client ID whitelist is enabled, defaulting to {@code true}. + */ + public abstract Builder setClientIdWhitelistEnabled(boolean clientIdWhitelist); /** - * Builds a new {@link ServletInitializationParameters} instance with the values from this - * builder. + * Sets if an {@link IllegalArgumentException} should be treated as a backend error (500) + * instead of a user error (400). Defaults to {@code false}. + */ + public abstract Builder setIllegalArgumentBackendError(boolean illegalArgumentBackendError); + + /** + * Sets if v1.0 style exceptions should be returned to users. In v1.0, certain codes are not + * permissible, and other codes are translated to other status codes. Defaults to {@code true}. */ + public abstract Builder setExceptionCompatibilityEnabled(boolean exceptionCompatibility); + + abstract ServletInitializationParameters autoBuild(); + public ServletInitializationParameters build() { - return new ServletInitializationParameters( - serviceClasses.build(), isServletRestricted, isClientIdWhitelistEnabled, - isIllegalArgumentBackendError, isExceptionCompatibilityEnabled); + return setServiceClasses(serviceClasses.build()).autoBuild(); } } + + /** + * Constructs a new instance from the provided {@link ServletConfig} and {@link ClassLoader}. + */ + public static ServletInitializationParameters fromServletConfig( + ServletConfig config, ClassLoader classLoader) throws ServletException { + Builder builder = builder(); + if (config != null) { + String serviceClassNames = config.getInitParameter(SERVICES); + if (serviceClassNames != null) { + for (String serviceClassName : CSV_SPLITTER.split(serviceClassNames)) { + builder.addServiceClass(getClassForName(serviceClassName, classLoader)); + } + } + String servletRestricted = config.getInitParameter(RESTRICTED); + if (servletRestricted != null) { + builder.setServletRestricted(parseBoolean(servletRestricted, RESTRICTED)); + } + String clientIdWhitelist = config.getInitParameter(CLIENT_ID_WHITELIST_ENABLED); + if (clientIdWhitelist != null) { + builder.setClientIdWhitelistEnabled( + parseBoolean(clientIdWhitelist, CLIENT_ID_WHITELIST_ENABLED)); + } + String illegalArgumentBackendError = config.getInitParameter(ILLEGAL_ARGUMENT_BACKEND_ERROR); + if (illegalArgumentBackendError != null) { + builder.setIllegalArgumentBackendError( + parseBoolean(illegalArgumentBackendError, ILLEGAL_ARGUMENT_BACKEND_ERROR)); + } + String exceptionCompatibility = config.getInitParameter(EXCEPTION_COMPATIBILITY); + if (exceptionCompatibility != null) { + builder.setExceptionCompatibilityEnabled( + parseBoolean(exceptionCompatibility, EXCEPTION_COMPATIBILITY)); + } + } + return builder.build(); + } + + private static boolean parseBoolean(String booleanString, String descriptionForErrors) { + if ("true".equalsIgnoreCase(booleanString)) { + return true; + } else if ("false".equalsIgnoreCase(booleanString)) { + return false; + } + throw new IllegalArgumentException(String.format( + "Expected 'true' or 'false' for '%s' servlet initialization parameter but got '%s'", + descriptionForErrors, booleanString)); + } + + private static Class getClassForName(String className, ClassLoader classLoader) + throws ServletException { + try { + return Class.forName(className, true, classLoader); + } catch (ClassNotFoundException e) { + throw new ServletException(String.format("Cannot find service class: %s", className), e); + } + } + + /** + * Returns the parameters as a {@link java.util.Map} of parameter name to {@link String} value. + */ + public ImmutableMap asMap() { + return ImmutableMap.builder() + .put(SERVICES, CSV_JOINER.join(Iterables.transform(getServiceClasses(), CLASS_TO_NAME))) + .put(RESTRICTED, Boolean.toString(isServletRestricted())) + .put(CLIENT_ID_WHITELIST_ENABLED, Boolean.toString(isClientIdWhitelistEnabled())) + .put(ILLEGAL_ARGUMENT_BACKEND_ERROR, Boolean.toString(isIllegalArgumentBackendError())) + .put(EXCEPTION_COMPATIBILITY, Boolean.toString(isExceptionCompatibilityEnabled())) + .build(); + } } diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java index 1bb3b592..49980db8 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java @@ -59,7 +59,7 @@ public void testBuilder_emptySetsAndTrue() { .setClientIdWhitelistEnabled(true) .setRestricted(true) .addServiceClasses(ImmutableSet.>of()) - .setIllegalArgumentIsBackendError(true) + .setIllegalArgumentBackendError(true) .setExceptionCompatibilityEnabled(true) .build(); assertThat(initParameters.getServiceClasses()).isEmpty(); @@ -76,7 +76,7 @@ public void testBuilder_oneEntrySetsAndFalse() { .setRestricted(false) .addServiceClass(String.class) .setClientIdWhitelistEnabled(false) - .setIllegalArgumentIsBackendError(false) + .setIllegalArgumentBackendError(false) .setExceptionCompatibilityEnabled(false) .build(); assertThat(initParameters.getServiceClasses()).containsExactly(String.class); From abb4b83edcb66dbe391bf45f6150eb6cd7001ee3 Mon Sep 17 00:00:00 2001 From: Daniel Tang Date: Fri, 7 Apr 2017 12:38:19 -0700 Subject: [PATCH 2/3] add prettyPrint servlet initialization parameter This new parameter is a boolean value that controls whether or not prettyPrint is enabled by default. Requests can still enable pretty printing by adding a query parameter if necessary. This value is set to true by default. This value is propagated for each request via EndpointsContext, which requires some amount of refactoring to plumb the value to StandardParameters.shouldPrettyPrint. --- .../api/server/spi/EndpointsContext.java | 8 +++- .../api/server/spi/EndpointsServlet.java | 3 +- .../spi/ServletInitializationParameters.java | 19 +++++++- .../spi/config/model/StandardParameters.java | 9 +++- .../spi/dispatcher/DispatcherContext.java | 2 +- .../spi/handlers/EndpointsMethodHandler.java | 6 +-- .../RestServletRequestParamReader.java | 11 +++-- .../request/ServletRequestParamReader.java | 19 ++++---- .../ServletInitializationParametersTest.java | 48 +++++++++++++------ .../config/model/StandardParametersTest.java | 41 +++++++++++++--- .../spi/handlers/ApiProxyHandlerTest.java | 3 +- .../handlers/EndpointsMethodHandlerTest.java | 2 +- .../spi/handlers/ExplorerHandlerTest.java | 2 +- .../RestServletRequestParamReaderTest.java | 9 +++- .../ServletRequestParamReaderTest.java | 18 +++++-- 15 files changed, 148 insertions(+), 52 deletions(-) diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/EndpointsContext.java b/endpoints-framework/src/main/java/com/google/api/server/spi/EndpointsContext.java index 6e1ac233..205b8361 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/EndpointsContext.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/EndpointsContext.java @@ -27,12 +27,14 @@ public class EndpointsContext extends DispatcherContext { private final HttpServletRequest request; private final HttpServletResponse response; + private final boolean prettyPrint; public EndpointsContext(String httpMethod, String path, HttpServletRequest request, - HttpServletResponse response) { + HttpServletResponse response, boolean prettyPrint) { super(httpMethod, path); this.request = Preconditions.checkNotNull(request, "request"); this.response = Preconditions.checkNotNull(response, "response"); + this.prettyPrint = prettyPrint; } public HttpServletRequest getRequest() { @@ -42,4 +44,8 @@ public HttpServletRequest getRequest() { public HttpServletResponse getResponse() { return response; } + + public boolean isPrettyPrintEnabled() { + return prettyPrint; + } } diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/EndpointsServlet.java b/endpoints-framework/src/main/java/com/google/api/server/spi/EndpointsServlet.java index fa7d91ba..52a2fc52 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/EndpointsServlet.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/EndpointsServlet.java @@ -67,7 +67,8 @@ public void service(HttpServletRequest request, HttpServletResponse response) th } else { String path = Strings.stripSlash( request.getRequestURI().substring(request.getServletPath().length())); - EndpointsContext context = new EndpointsContext(method, path, request, response); + EndpointsContext context = new EndpointsContext(method, path, request, response, + initParameters.isPrettyPrintEnabled()); if (!dispatcher.dispatch(method, path, context)) { response.setStatus(HttpServletResponse.SC_NOT_FOUND); response.getWriter().append("Not Found"); diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java b/endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java index 10bef9d7..28044ab3 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/ServletInitializationParameters.java @@ -37,6 +37,7 @@ public abstract class ServletInitializationParameters { private static final String CLIENT_ID_WHITELIST_ENABLED = "clientIdWhitelistEnabled"; private static final String ILLEGAL_ARGUMENT_BACKEND_ERROR = "illegalArgumentIsBackendError"; private static final String EXCEPTION_COMPATIBILITY = "enableExceptionCompatibility"; + private static final String PRETTY_PRINT = "prettyPrint"; private static final Splitter CSV_SPLITTER = Splitter.on(',').omitEmptyStrings().trimResults(); private static final Joiner CSV_JOINER = Joiner.on(',').skipNulls(); @@ -77,12 +78,18 @@ public String apply(Class clazz) { */ public abstract boolean isExceptionCompatibilityEnabled(); + /** + * Returns if pretty printing should be enabled for responses by default. Defaults to true. + */ + public abstract boolean isPrettyPrintEnabled(); + public static Builder builder() { return new AutoValue_ServletInitializationParameters.Builder() .setServletRestricted(true) .setClientIdWhitelistEnabled(true) .setIllegalArgumentBackendError(false) - .setExceptionCompatibilityEnabled(true); + .setExceptionCompatibilityEnabled(true) + .setPrettyPrintEnabled(true); } /** @@ -148,6 +155,11 @@ public Builder setRestricted(boolean servletRestricted) { */ public abstract Builder setExceptionCompatibilityEnabled(boolean exceptionCompatibility); + /** + * Sets if pretty printing should be enabled for responses by default. Defaults to {@code true}. + */ + public abstract Builder setPrettyPrintEnabled(boolean prettyPrint); + abstract ServletInitializationParameters autoBuild(); public ServletInitializationParameters build() { @@ -187,6 +199,10 @@ public static ServletInitializationParameters fromServletConfig( builder.setExceptionCompatibilityEnabled( parseBoolean(exceptionCompatibility, EXCEPTION_COMPATIBILITY)); } + String prettyPrint = config.getInitParameter(PRETTY_PRINT); + if (prettyPrint != null) { + builder.setPrettyPrintEnabled(parseBoolean(prettyPrint, PRETTY_PRINT)); + } } return builder.build(); } @@ -221,6 +237,7 @@ public ImmutableMap asMap() { .put(CLIENT_ID_WHITELIST_ENABLED, Boolean.toString(isClientIdWhitelistEnabled())) .put(ILLEGAL_ARGUMENT_BACKEND_ERROR, Boolean.toString(isIllegalArgumentBackendError())) .put(EXCEPTION_COMPATIBILITY, Boolean.toString(isExceptionCompatibilityEnabled())) + .put(PRETTY_PRINT, Boolean.toString(isPrettyPrintEnabled())) .build(); } } diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/StandardParameters.java b/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/StandardParameters.java index 119b4ef2..b73bd5ee 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/StandardParameters.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/config/model/StandardParameters.java @@ -15,6 +15,7 @@ */ package com.google.api.server.spi.config.model; +import com.google.api.server.spi.EndpointsContext; import com.google.common.collect.ImmutableSet; import javax.servlet.http.HttpServletRequest; @@ -45,8 +46,12 @@ public static boolean isStandardParamName(String paramName) { return STANDARD_PARAM_NAMES.contains(paramName); } - public static boolean shouldPrettyPrint(HttpServletRequest request) { + public static boolean shouldPrettyPrint(EndpointsContext context) { + HttpServletRequest request = context.getRequest(); String prettyPrintStr = request.getParameter("prettyPrint"); - return prettyPrintStr == null || "true".equals(prettyPrintStr.toLowerCase()); + if (prettyPrintStr == null) { + return context.isPrettyPrintEnabled(); + } + return "true".equals(prettyPrintStr.toLowerCase()); } } diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/dispatcher/DispatcherContext.java b/endpoints-framework/src/main/java/com/google/api/server/spi/dispatcher/DispatcherContext.java index e1b3b0a4..d6d392a2 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/dispatcher/DispatcherContext.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/dispatcher/DispatcherContext.java @@ -26,7 +26,7 @@ public class DispatcherContext { private final String httpMethod; private final String path; - private ImmutableMap rawPathParameters; + private ImmutableMap rawPathParameters = ImmutableMap.of(); public DispatcherContext(String httpMethod, String path) { this.httpMethod = Preconditions.checkNotNull(httpMethod, "httpMethod").toUpperCase(); diff --git a/endpoints-framework/src/main/java/com/google/api/server/spi/handlers/EndpointsMethodHandler.java b/endpoints-framework/src/main/java/com/google/api/server/spi/handlers/EndpointsMethodHandler.java index bf6cd501..5ef37421 100644 --- a/endpoints-framework/src/main/java/com/google/api/server/spi/handlers/EndpointsMethodHandler.java +++ b/endpoints-framework/src/main/java/com/google/api/server/spi/handlers/EndpointsMethodHandler.java @@ -82,15 +82,15 @@ public DispatcherHandler getRestHandler() { @VisibleForTesting protected ParamReader createRestParamReader(EndpointsContext context, ApiSerializationConfig serializationConfig) { - return new RestServletRequestParamReader(endpointMethod, context.getRequest(), - servletContext, serializationConfig, methodConfig, context.getRawPathParameters()); + return new RestServletRequestParamReader(endpointMethod, context, + servletContext, serializationConfig, methodConfig); } @VisibleForTesting protected ResultWriter createResultWriter(EndpointsContext context, ApiSerializationConfig serializationConfig) { return new RestResponseResultWriter(context.getResponse(), serializationConfig, - StandardParameters.shouldPrettyPrint(context.getRequest()), + StandardParameters.shouldPrettyPrint(context), initParameters.isExceptionCompatibilityEnabled()); } 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 index 43560d09..65aedd21 100644 --- 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 @@ -16,6 +16,7 @@ package com.google.api.server.spi.request; import com.google.api.server.spi.EndpointMethod; +import com.google.api.server.spi.EndpointsContext; import com.google.api.server.spi.IoUtil; import com.google.api.server.spi.ServiceException; import com.google.api.server.spi.Strings; @@ -59,11 +60,10 @@ public class RestServletRequestParamReader extends ServletRequestParamReader { private final Map parameterConfigMap; public RestServletRequestParamReader(EndpointMethod method, - HttpServletRequest servletRequest, ServletContext servletContext, - ApiSerializationConfig serializationConfig, ApiMethodConfig methodConfig, - Map rawPathParameters) { - super(method, servletRequest, servletContext, serializationConfig, methodConfig); - this.rawPathParameters = rawPathParameters; + EndpointsContext endpointsContext, ServletContext servletContext, + ApiSerializationConfig serializationConfig, ApiMethodConfig methodConfig) { + super(method, endpointsContext, servletContext, serializationConfig, methodConfig); + this.rawPathParameters = endpointsContext.getRawPathParameters(); ImmutableMap.Builder builder = ImmutableMap.builder(); for (ApiParameterConfig config : methodConfig.getParameterConfigs()) { if (config.getName() != null) { @@ -82,6 +82,7 @@ public Object[] read() throws ServiceException { if (method.getParameterClasses().length == 0) { return new Object[0]; } + HttpServletRequest servletRequest = endpointsContext.getRequest(); String requestBody = IoUtil.readRequestBody(servletRequest); logger.log(Level.FINE, "requestBody=" + requestBody); // Unlike the Lily protocol, which essentially always requires a JSON body to exist (due to 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 index 80a688a3..a74fda37 100644 --- 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 @@ -17,6 +17,7 @@ import com.google.api.server.spi.ConfiguredObjectMapper; import com.google.api.server.spi.EndpointMethod; +import com.google.api.server.spi.EndpointsContext; import com.google.api.server.spi.IoUtil; import com.google.api.server.spi.ServiceException; import com.google.api.server.spi.auth.common.User; @@ -162,7 +163,7 @@ protected Object[] deserializeParams(JsonNode node) throws IOException, IllegalA logger.log(Level.FINE, "deserialize: App Engine User injected into param[{0}]", i); } else if (clazz == HttpServletRequest.class) { // HttpServletRequest type parameter requires no Named annotation (ignored if present) - params[i] = servletRequest; + params[i] = endpointsContext.getRequest(); logger.log(Level.FINE, "deserialize: HttpServletRequest injected into param[{0}]", i); } else if (clazz == ServletContext.class) { // ServletContext type parameter requires no Named annotation (ignored if present) @@ -201,21 +202,21 @@ protected Object[] deserializeParams(JsonNode node) throws IOException, IllegalA @VisibleForTesting User getUser() throws ServiceException { - return Auth.from(servletRequest).authenticate(); + return Auth.from(endpointsContext.getRequest()).authenticate(); } @VisibleForTesting com.google.appengine.api.users.User getAppEngineUser() throws ServiceException { - return Auth.from(servletRequest).authenticateAppEngineUser(); + return Auth.from(endpointsContext.getRequest()).authenticateAppEngineUser(); } private Object getStandardParamValue(JsonNode body, String paramName) { if (!StandardParameters.isStandardParamName(paramName)) { throw new IllegalArgumentException("paramName"); } else if (StandardParameters.USER_IP.equals(paramName)) { - return servletRequest.getRemoteAddr(); + return endpointsContext.getRequest().getRemoteAddr(); } else if (StandardParameters.PRETTY_PRINT.equals(paramName)) { - return StandardParameters.shouldPrettyPrint(servletRequest); + return StandardParameters.shouldPrettyPrint(endpointsContext); } JsonNode value = body.get(paramName); if (value == null && StandardParameters.ALT.equals(paramName)) { @@ -288,21 +289,21 @@ public Blob deserialize(JsonParser jsonParser, DeserializationContext context) } } - protected final HttpServletRequest servletRequest; + protected final EndpointsContext endpointsContext; private final ServletContext servletContext; protected final ObjectReader objectReader; protected final ApiMethodConfig methodConfig; public ServletRequestParamReader( EndpointMethod method, - HttpServletRequest servletRequest, + EndpointsContext endpointsContext, ServletContext servletContext, ApiSerializationConfig serializationConfig, ApiMethodConfig methodConfig) { super(method); this.methodConfig = methodConfig; - this.servletRequest = servletRequest; + this.endpointsContext = endpointsContext; this.servletContext = servletContext; LinkedHashSet modules = new LinkedHashSet<>(); @@ -320,7 +321,7 @@ public Object[] read() throws ServiceException { // Assumes input stream to be encoded in UTF-8 // TODO: Take charset from content-type as encoding try { - String requestBody = IoUtil.readStream(servletRequest.getInputStream()); + String requestBody = IoUtil.readStream(endpointsContext.getRequest().getInputStream()); logger.log(Level.FINE, "requestBody=" + requestBody); if (requestBody == null || requestBody.trim().isEmpty()) { return new Object[0]; diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java index 49980db8..6edfe20f 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java @@ -50,7 +50,8 @@ public void testBuilder_defaults() { assertTrue(initParameters.isClientIdWhitelistEnabled()); assertFalse(initParameters.isIllegalArgumentBackendError()); assertTrue(initParameters.isExceptionCompatibilityEnabled()); - verifyAsMap(initParameters, "", "true", "true", "false", "true"); + assertTrue(initParameters.isPrettyPrintEnabled()); + verifyAsMap(initParameters, "", "true", "true", "false", "true", "true"); } @Test @@ -61,13 +62,14 @@ public void testBuilder_emptySetsAndTrue() { .addServiceClasses(ImmutableSet.>of()) .setIllegalArgumentBackendError(true) .setExceptionCompatibilityEnabled(true) + .setPrettyPrintEnabled(true) .build(); assertThat(initParameters.getServiceClasses()).isEmpty(); assertTrue(initParameters.isServletRestricted()); assertTrue(initParameters.isClientIdWhitelistEnabled()); assertTrue(initParameters.isIllegalArgumentBackendError()); assertTrue(initParameters.isExceptionCompatibilityEnabled()); - verifyAsMap(initParameters, "", "true", "true", "true", "true"); + verifyAsMap(initParameters, "", "true", "true", "true", "true", "true"); } @Test @@ -78,11 +80,13 @@ public void testBuilder_oneEntrySetsAndFalse() { .setClientIdWhitelistEnabled(false) .setIllegalArgumentBackendError(false) .setExceptionCompatibilityEnabled(false) + .setPrettyPrintEnabled(false) .build(); assertThat(initParameters.getServiceClasses()).containsExactly(String.class); assertFalse(initParameters.isServletRestricted()); assertFalse(initParameters.isClientIdWhitelistEnabled()); - verifyAsMap(initParameters, String.class.getName(), "false", "false", "false", "false"); + verifyAsMap( + initParameters, String.class.getName(), "false", "false", "false", "false", "false"); } @Test @@ -92,7 +96,7 @@ public void testBuilder_twoEntrySets() { .build(); assertThat(initParameters.getServiceClasses()).containsExactly(String.class, Integer.class); verifyAsMap(initParameters, String.class.getName() + ',' + Integer.class.getName(), "true", - "true", "false", "true"); + "true", "false", "true", "true"); } @Test @@ -107,51 +111,58 @@ public void testFromServletConfig_nullConfig() throws ServletException { @Test public void testFromServletConfig_nullValues() throws ServletException { ServletInitializationParameters initParameters = - fromServletConfig(null, null, null, null); + fromServletConfig(null, null, null, null, null, null); assertThat(initParameters.getServiceClasses()).isEmpty(); assertTrue(initParameters.isServletRestricted()); assertTrue(initParameters.isClientIdWhitelistEnabled()); assertFalse(initParameters.isIllegalArgumentBackendError()); + assertTrue(initParameters.isExceptionCompatibilityEnabled()); + assertTrue(initParameters.isPrettyPrintEnabled()); } @Test public void testFromServletConfig_emptySetsAndFalse() throws ServletException { ServletInitializationParameters initParameters = - fromServletConfig("", "false", "false", "false"); + fromServletConfig("", "false", "false", "false", "false", "false"); assertThat(initParameters.getServiceClasses()).isEmpty(); assertFalse(initParameters.isServletRestricted()); assertFalse(initParameters.isClientIdWhitelistEnabled()); assertFalse(initParameters.isIllegalArgumentBackendError()); + assertFalse(initParameters.isExceptionCompatibilityEnabled()); + assertFalse(initParameters.isPrettyPrintEnabled()); } @Test public void testFromServletConfig_oneEntrySetsAndTrue() throws ServletException { ServletInitializationParameters initParameters = - fromServletConfig(String.class.getName(), "true", "true", "true"); + fromServletConfig(String.class.getName(), "true", "true", "true", "true", "true"); assertThat(initParameters.getServiceClasses()).containsExactly(String.class); assertTrue(initParameters.isServletRestricted()); assertTrue(initParameters.isClientIdWhitelistEnabled()); assertTrue(initParameters.isIllegalArgumentBackendError()); + assertTrue(initParameters.isExceptionCompatibilityEnabled()); + assertTrue(initParameters.isPrettyPrintEnabled()); } @Test public void testFromServletConfig_twoEntrySets() throws ServletException { ServletInitializationParameters initParameters = fromServletConfig( - String.class.getName() + ',' + Integer.class.getName(), null, null, null); + String.class.getName() + ',' + Integer.class.getName(), null, null, null, null, null); assertThat(initParameters.getServiceClasses()).containsExactly(String.class, Integer.class); } @Test public void testFromServletConfig_skipsEmptyElements() throws ServletException { ServletInitializationParameters initParameters = fromServletConfig( - ",," + String.class.getName() + ",,," + Integer.class.getName() + ",", null, null, null); + ",," + String.class.getName() + ",,," + Integer.class.getName() + ",", null, null, null, + null, null); assertThat(initParameters.getServiceClasses()).containsExactly(String.class, Integer.class); } @Test public void testFromServletConfig_invalidRestrictedThrows() throws ServletException { try { - fromServletConfig(null, "yes", null, null); + fromServletConfig(null, "yes", null, null, null, null); fail("Expected IllegalArgumentException"); } catch (IllegalArgumentException expected) { // expected @@ -161,22 +172,26 @@ public void testFromServletConfig_invalidRestrictedThrows() throws ServletExcept private void verifyAsMap( ServletInitializationParameters initParameters, String serviceClasses, String isServletRestricted, String isClientIdWhitelistEnabled, - String isIllegalArgumentBackendError, String isExceptionCompatibilityEnabled) { + String isIllegalArgumentBackendError, String isExceptionCompatibilityEnabled, + String isPrettyPrintEnabled) { Map map = initParameters.asMap(); - assertEquals(5, map.size()); + assertEquals(6, map.size()); assertEquals(serviceClasses, map.get("services")); assertEquals(isServletRestricted, map.get("restricted")); assertEquals(isClientIdWhitelistEnabled, map.get("clientIdWhitelistEnabled")); assertEquals(isIllegalArgumentBackendError, map.get("illegalArgumentIsBackendError")); assertEquals(isExceptionCompatibilityEnabled, map.get("enableExceptionCompatibility")); + assertEquals(isPrettyPrintEnabled, map.get("prettyPrint")); } private ServletInitializationParameters fromServletConfig( String serviceClasses, String isServletRestricted, - String isClientIdWhitelistEnabled, String isIllegalArgumentBackendError) + String isClientIdWhitelistEnabled, String isIllegalArgumentBackendError, + String isExceptionCompatibilityEnabled, String isPrettyPrintEnabled) throws ServletException { ServletConfig servletConfig = new StubServletConfig(serviceClasses, - isServletRestricted, isClientIdWhitelistEnabled, isIllegalArgumentBackendError); + isServletRestricted, isClientIdWhitelistEnabled, isIllegalArgumentBackendError, + isExceptionCompatibilityEnabled, isPrettyPrintEnabled); return ServletInitializationParameters.fromServletConfig( servletConfig, getClass().getClassLoader()); } @@ -186,12 +201,15 @@ private static class StubServletConfig implements ServletConfig { public StubServletConfig( String serviceClasses, String isServletRestricted, String isClientIdWhitelistEnabled, - String isIllegalArgumentBackendError) { + String isIllegalArgumentBackendError, String isExceptionCompatibilityEnabled, + String isPrettyPrintEnabled) { initParameters = Maps.newHashMap(); initParameters.put("services", serviceClasses); initParameters.put("restricted", isServletRestricted); initParameters.put("clientIdWhitelistEnabled", isClientIdWhitelistEnabled); initParameters.put("illegalArgumentIsBackendError", isIllegalArgumentBackendError); + initParameters.put("enableExceptionCompatibility", isExceptionCompatibilityEnabled); + initParameters.put("prettyPrint", isPrettyPrintEnabled); } @Override diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/StandardParametersTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/StandardParametersTest.java index f10b1385..72536d0e 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/StandardParametersTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/config/model/StandardParametersTest.java @@ -17,10 +17,13 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.api.server.spi.EndpointsContext; + import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; /** * Tests for {@link StandardParameters}. @@ -28,22 +31,48 @@ @RunWith(JUnit4.class) public class StandardParametersTest { @Test - public void shouldPrettyPrint_defaultValueIsTrue() { - assertThat(StandardParameters.shouldPrettyPrint(new MockHttpServletRequest())).isTrue(); + public void shouldPrettyPrint_defaultValueIsTrue_globalDefaultTrue() { + assertThat(StandardParameters.shouldPrettyPrint( + getEndpointsContext(new MockHttpServletRequest(), true))).isTrue(); + } + + @Test + public void shouldPrettyPrint_false_globalDefaultTrue() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setParameter("prettyPrint", "false"); + assertThat(StandardParameters.shouldPrettyPrint(getEndpointsContext(request, true))).isFalse(); + } + + @Test + public void shouldPrettyPrint_true_globalDefaultTrue() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setParameter("prettyPrint", "true"); + assertThat(StandardParameters.shouldPrettyPrint(getEndpointsContext(request, true))).isTrue(); + } + + @Test + public void shouldPrettyPrint_defaultValueIsFalse_globalDefaultFalse() { + assertThat(StandardParameters.shouldPrettyPrint( + getEndpointsContext(new MockHttpServletRequest(), false))).isFalse(); } @Test - public void shouldPrettyPrint_false() { + public void shouldPrettyPrint_false_globalDefaultFalse() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("prettyPrint", "false"); - assertThat(StandardParameters.shouldPrettyPrint(request)).isFalse(); + assertThat(StandardParameters.shouldPrettyPrint(getEndpointsContext(request, false))).isFalse(); } @Test - public void shouldPrettyPrint_true() { + public void shouldPrettyPrint_true_globalDefaultFalse() { MockHttpServletRequest request = new MockHttpServletRequest(); request.setParameter("prettyPrint", "true"); - assertThat(StandardParameters.shouldPrettyPrint(request)).isTrue(); + assertThat(StandardParameters.shouldPrettyPrint(getEndpointsContext(request, false))).isTrue(); + } + + private EndpointsContext getEndpointsContext( + MockHttpServletRequest request, boolean prettyPrint) { + return new EndpointsContext("GET", "/", request, new MockHttpServletResponse(), prettyPrint); } @Test diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/ApiProxyHandlerTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/ApiProxyHandlerTest.java index 85c26b3b..b48c4a18 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/ApiProxyHandlerTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/ApiProxyHandlerTest.java @@ -47,7 +47,8 @@ private void testWithServletPath(String servletPath) throws Exception { request.setServletPath(servletPath); MockHttpServletResponse response = new MockHttpServletResponse(); ApiProxyHandler handler = new ApiProxyHandler(); - EndpointsContext context = new EndpointsContext("GET", "static/proxy.html", request, response); + EndpointsContext context = + new EndpointsContext("GET", "static/proxy.html", request, response, true); handler.handle(context); diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/EndpointsMethodHandlerTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/EndpointsMethodHandlerTest.java index d75ec4e0..ff1fc3bf 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/EndpointsMethodHandlerTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/EndpointsMethodHandlerTest.java @@ -66,7 +66,7 @@ public void setUp() throws Exception { classLoader = EndpointsMethodHandlerTest.class.getClassLoader(); request = new MockHttpServletRequest(); response = new MockHttpServletResponse(); - context = new EndpointsContext("", "", request, response); + context = new EndpointsContext("", "", request, response, true); systemService = SystemService.builder() .withDefaults(classLoader) .addService(TestEndpoint.class, new TestEndpoint()) diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/ExplorerHandlerTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/ExplorerHandlerTest.java index 08d2b3af..47d59d36 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/ExplorerHandlerTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/handlers/ExplorerHandlerTest.java @@ -55,7 +55,7 @@ private void testHandle(String scheme, int port, String expectedLocation) throws request.setRequestURI("/_ah/api/explorer/"); MockHttpServletResponse response = new MockHttpServletResponse(); ExplorerHandler handler = new ExplorerHandler(); - EndpointsContext context = new EndpointsContext("GET", "explorer", request, response); + EndpointsContext context = new EndpointsContext("GET", "explorer", request, response, true); handler.handle(context); assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_FOUND); diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/request/RestServletRequestParamReaderTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/request/RestServletRequestParamReaderTest.java index 165418da..ace37865 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/request/RestServletRequestParamReaderTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/request/RestServletRequestParamReaderTest.java @@ -19,6 +19,7 @@ import static org.junit.Assert.fail; import com.google.api.server.spi.EndpointMethod; +import com.google.api.server.spi.EndpointsContext; import com.google.api.server.spi.ServiceContext; import com.google.api.server.spi.TypeLoader; import com.google.api.server.spi.config.Api; @@ -41,6 +42,7 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -202,8 +204,11 @@ public void arrayPathParam() throws Exception { } private RestServletRequestParamReader createReader(Map rawPathParameters) { - return new RestServletRequestParamReader(endpointMethod, request, null, - serializationConfig, methodConfig, rawPathParameters); + EndpointsContext endpointsContext = + new EndpointsContext("GET", "/", request, new MockHttpServletResponse(), true); + endpointsContext.setRawPathParameters(rawPathParameters); + return new RestServletRequestParamReader(endpointMethod, endpointsContext, null, + serializationConfig, methodConfig); } public static class TestResource { diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/request/ServletRequestParamReaderTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/request/ServletRequestParamReaderTest.java index 2708a4c2..e166c98a 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/request/ServletRequestParamReaderTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/request/ServletRequestParamReaderTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.when; import com.google.api.server.spi.EndpointMethod; +import com.google.api.server.spi.EndpointsContext; import com.google.api.server.spi.auth.common.User; import com.google.api.server.spi.config.AuthLevel; import com.google.api.server.spi.config.Named; @@ -39,6 +40,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.reflect.TypeToken; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -87,6 +89,15 @@ public class ServletRequestParamReaderTest { @Mock private ServletContext context; + @Mock + private EndpointsContext endpointsContext; + + @Before + public void setUp() { + when(endpointsContext.getRequest()).thenReturn(request); + when(endpointsContext.isPrettyPrintEnabled()).thenReturn(true); + } + @Test public void testRead() throws Exception { Object[] params = readExecuteMethod(ImmutableMap.builder() @@ -644,7 +655,8 @@ public void user(TestUser user) {} final TestUser user = new TestUser("test"); Method method = TestUserEndpoint.class.getDeclaredMethod("user", TestUser.class); ParamReader reader = new ServletRequestParamReader( - EndpointMethod.create(method.getDeclaringClass(), method), request, context, null, null) { + EndpointMethod.create(method.getDeclaringClass(), method), endpointsContext, context, null, + null) { @Override User getUser() { return user; @@ -832,8 +844,8 @@ private Object[] readParameters(final String input, EndpointMethod method, ApiMethodConfig methodConfig, final User user, final com.google.appengine.api.users.User appEngineUser) throws Exception { - ParamReader reader = new ServletRequestParamReader( - method, request, context, null, methodConfig) { + ParamReader reader = new ServletRequestParamReader(method, endpointsContext, context, null, + methodConfig) { @Override User getUser() { return user; From d4d8337e6edecc2d4124cad6cbb33ed6fa0470a6 Mon Sep 17 00:00:00 2001 From: Daniel Tang Date: Fri, 7 Apr 2017 12:42:44 -0700 Subject: [PATCH 3/3] use Truth in ServletInitializationParametersTest --- .../ServletInitializationParametersTest.java | 73 +++++++++---------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java b/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java index 6edfe20f..11b39395 100644 --- a/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java +++ b/endpoints-framework/src/test/java/com/google/api/server/spi/ServletInitializationParametersTest.java @@ -16,9 +16,6 @@ package com.google.api.server.spi; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.common.collect.ImmutableSet; @@ -46,11 +43,11 @@ public void testBuilder_defaults() { ServletInitializationParameters initParameters = ServletInitializationParameters.builder() .build(); assertThat(initParameters.getServiceClasses()).isEmpty(); - assertTrue(initParameters.isServletRestricted()); - assertTrue(initParameters.isClientIdWhitelistEnabled()); - assertFalse(initParameters.isIllegalArgumentBackendError()); - assertTrue(initParameters.isExceptionCompatibilityEnabled()); - assertTrue(initParameters.isPrettyPrintEnabled()); + assertThat(initParameters.isServletRestricted()).isTrue(); + assertThat(initParameters.isClientIdWhitelistEnabled()).isTrue(); + assertThat(initParameters.isIllegalArgumentBackendError()).isFalse(); + assertThat(initParameters.isExceptionCompatibilityEnabled()).isTrue(); + assertThat(initParameters.isPrettyPrintEnabled()).isTrue(); verifyAsMap(initParameters, "", "true", "true", "false", "true", "true"); } @@ -65,10 +62,10 @@ public void testBuilder_emptySetsAndTrue() { .setPrettyPrintEnabled(true) .build(); assertThat(initParameters.getServiceClasses()).isEmpty(); - assertTrue(initParameters.isServletRestricted()); - assertTrue(initParameters.isClientIdWhitelistEnabled()); - assertTrue(initParameters.isIllegalArgumentBackendError()); - assertTrue(initParameters.isExceptionCompatibilityEnabled()); + assertThat(initParameters.isServletRestricted()).isTrue(); + assertThat(initParameters.isClientIdWhitelistEnabled()).isTrue(); + assertThat(initParameters.isIllegalArgumentBackendError()).isTrue(); + assertThat(initParameters.isExceptionCompatibilityEnabled()).isTrue(); verifyAsMap(initParameters, "", "true", "true", "true", "true", "true"); } @@ -83,8 +80,8 @@ public void testBuilder_oneEntrySetsAndFalse() { .setPrettyPrintEnabled(false) .build(); assertThat(initParameters.getServiceClasses()).containsExactly(String.class); - assertFalse(initParameters.isServletRestricted()); - assertFalse(initParameters.isClientIdWhitelistEnabled()); + assertThat(initParameters.isServletRestricted()).isFalse(); + assertThat(initParameters.isClientIdWhitelistEnabled()).isFalse(); verifyAsMap( initParameters, String.class.getName(), "false", "false", "false", "false", "false"); } @@ -104,8 +101,8 @@ public void testFromServletConfig_nullConfig() throws ServletException { ServletInitializationParameters initParameters = ServletInitializationParameters.fromServletConfig(null, getClass().getClassLoader()); assertThat(initParameters.getServiceClasses()).isEmpty(); - assertTrue(initParameters.isServletRestricted()); - assertTrue(initParameters.isClientIdWhitelistEnabled()); + assertThat(initParameters.isServletRestricted()).isTrue(); + assertThat(initParameters.isClientIdWhitelistEnabled()).isTrue(); } @Test @@ -113,11 +110,11 @@ public void testFromServletConfig_nullValues() throws ServletException { ServletInitializationParameters initParameters = fromServletConfig(null, null, null, null, null, null); assertThat(initParameters.getServiceClasses()).isEmpty(); - assertTrue(initParameters.isServletRestricted()); - assertTrue(initParameters.isClientIdWhitelistEnabled()); - assertFalse(initParameters.isIllegalArgumentBackendError()); - assertTrue(initParameters.isExceptionCompatibilityEnabled()); - assertTrue(initParameters.isPrettyPrintEnabled()); + assertThat(initParameters.isServletRestricted()).isTrue(); + assertThat(initParameters.isClientIdWhitelistEnabled()).isTrue(); + assertThat(initParameters.isIllegalArgumentBackendError()).isFalse(); + assertThat(initParameters.isExceptionCompatibilityEnabled()).isTrue(); + assertThat(initParameters.isPrettyPrintEnabled()).isTrue(); } @Test @@ -125,11 +122,11 @@ public void testFromServletConfig_emptySetsAndFalse() throws ServletException { ServletInitializationParameters initParameters = fromServletConfig("", "false", "false", "false", "false", "false"); assertThat(initParameters.getServiceClasses()).isEmpty(); - assertFalse(initParameters.isServletRestricted()); - assertFalse(initParameters.isClientIdWhitelistEnabled()); - assertFalse(initParameters.isIllegalArgumentBackendError()); - assertFalse(initParameters.isExceptionCompatibilityEnabled()); - assertFalse(initParameters.isPrettyPrintEnabled()); + assertThat(initParameters.isServletRestricted()).isFalse(); + assertThat(initParameters.isClientIdWhitelistEnabled()).isFalse(); + assertThat(initParameters.isIllegalArgumentBackendError()).isFalse(); + assertThat(initParameters.isExceptionCompatibilityEnabled()).isFalse(); + assertThat(initParameters.isPrettyPrintEnabled()).isFalse(); } @Test @@ -137,11 +134,11 @@ public void testFromServletConfig_oneEntrySetsAndTrue() throws ServletException ServletInitializationParameters initParameters = fromServletConfig(String.class.getName(), "true", "true", "true", "true", "true"); assertThat(initParameters.getServiceClasses()).containsExactly(String.class); - assertTrue(initParameters.isServletRestricted()); - assertTrue(initParameters.isClientIdWhitelistEnabled()); - assertTrue(initParameters.isIllegalArgumentBackendError()); - assertTrue(initParameters.isExceptionCompatibilityEnabled()); - assertTrue(initParameters.isPrettyPrintEnabled()); + assertThat(initParameters.isServletRestricted()).isTrue(); + assertThat(initParameters.isClientIdWhitelistEnabled()).isTrue(); + assertThat(initParameters.isIllegalArgumentBackendError()).isTrue(); + assertThat(initParameters.isExceptionCompatibilityEnabled()).isTrue(); + assertThat(initParameters.isPrettyPrintEnabled()).isTrue(); } @Test @@ -175,13 +172,13 @@ private void verifyAsMap( String isIllegalArgumentBackendError, String isExceptionCompatibilityEnabled, String isPrettyPrintEnabled) { Map map = initParameters.asMap(); - assertEquals(6, map.size()); - assertEquals(serviceClasses, map.get("services")); - assertEquals(isServletRestricted, map.get("restricted")); - assertEquals(isClientIdWhitelistEnabled, map.get("clientIdWhitelistEnabled")); - assertEquals(isIllegalArgumentBackendError, map.get("illegalArgumentIsBackendError")); - assertEquals(isExceptionCompatibilityEnabled, map.get("enableExceptionCompatibility")); - assertEquals(isPrettyPrintEnabled, map.get("prettyPrint")); + assertThat(map).hasSize(6); + assertThat(map.get("services")).isEqualTo(serviceClasses); + assertThat(map.get("restricted")).isEqualTo(isServletRestricted); + assertThat(map.get("clientIdWhitelistEnabled")).isEqualTo(isClientIdWhitelistEnabled); + assertThat(map.get("illegalArgumentIsBackendError")).isEqualTo(isIllegalArgumentBackendError); + assertThat(map.get("enableExceptionCompatibility")).isEqualTo(isExceptionCompatibilityEnabled); + assertThat(map.get("prettyPrint")).isEqualTo(isPrettyPrintEnabled); } private ServletInitializationParameters fromServletConfig(