Skip to content

Refactor param reader to handle properly name conflicts between param… #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
clementdenis merged 1 commit into from
Oct 22, 2019
Merged

Refactor param reader to handle properly name conflicts between param… #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 27 additions & 73 deletions ...mework/src/main/java/com/google/api/server/spi/request/RestServletRequestParamReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
package com.google.api.server.spi.request;

import com.fasterxml.jackson.databind.exc.InvalidFormatException;
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
import com.google.api.server.spi.EndpointMethod;
import com.google.api.server.spi.EndpointsContext;
import com.google.api.server.spi.IoUtil;
Expand All @@ -25,8 +25,6 @@
import com.google.api.server.spi.config.model.ApiParameterConfig;
import com.google.api.server.spi.config.model.ApiSerializationConfig;
import com.google.api.server.spi.response.BadRequestException;
import com.google.api.server.spi.types.DateAndTime;
import com.google.api.server.spi.types.SimpleDate;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;

Expand All @@ -42,9 +40,6 @@

import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.text.MessageFormat;
import java.util.Arrays;
import java.util.Date;
import java.util.Enumeration;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -91,7 +86,8 @@ public Object[] read() throws ServiceException {
return new Object[0];
}
HttpServletRequest servletRequest = endpointsContext.getRequest();
JsonNode node;
ObjectNode body = (ObjectNode) objectReader.createObjectNode();
ObjectNode params = (ObjectNode) objectReader.createObjectNode();
// multipart/form-data requests can be used for requests which have no resource body. In
// this case, each part represents a named parameter instead.
if (ServletFileUpload.isMultipartContent(servletRequest)) {
Expand All @@ -107,7 +103,7 @@ public Object[] read() throws ServiceException {
throw new BadRequestException("unable to parse multipart form field");
}
}
node = obj;
params = obj;
} catch (FileUploadException e) {
throw new BadRequestException("unable to parse multipart request", e);
}
Expand All @@ -117,102 +113,60 @@ public Object[] read() throws ServiceException {
// Unlike the Lily protocol, which essentially always requires a JSON body to exist (due to
// path and query parameters being injected into the body), bodies are optional here, so we
// create an empty body and inject named parameters to make deserialize work.
node = Strings.isEmptyOrWhitespace(requestBody) ? objectReader.createObjectNode()
: objectReader.readTree(requestBody);
}
if (!node.isObject()) {
throw new BadRequestException("expected a JSON object body");
if (!Strings.isEmptyOrWhitespace(requestBody)) {
JsonNode node = objectReader.readTree(requestBody);
if (!node.isObject()) {
throw new BadRequestException("expected a JSON object body");
}
body = (ObjectNode) node;
}
}
ObjectNode body = (ObjectNode) node;
Map<String, Class<?>> parameterMap = getParameterMap(method);
// First add query parameters, then add path parameters. If the parameters already exist in
// the resource, then the they aren't added to the body object. For compatibility reasons,
// the order of precedence is resource field > query parameter > path parameter.
for (Enumeration<?> e = servletRequest.getParameterNames(); e.hasMoreElements(); ) {
String parameterName = (String) e.nextElement();
if (!body.has(parameterName)) {
Class<?> parameterClass = parameterMap.get(parameterName);
ApiParameterConfig parameterConfig = parameterConfigMap.get(parameterName);
if (parameterClass != null && parameterConfig.isRepeated()) {
ArrayNode values = body.putArray(parameterName);
for (String value : servletRequest.getParameterValues(parameterName)) {
values.add(value);
}
} else {
body.put(parameterName, servletRequest.getParameterValues(parameterName)[0]);
Class<?> parameterClass = parameterMap.get(parameterName);
ApiParameterConfig parameterConfig = parameterConfigMap.get(parameterName);
if (parameterClass != null && parameterConfig.isRepeated()) {
ArrayNode values = params.putArray(parameterName);
for (String value : servletRequest.getParameterValues(parameterName)) {
values.add(value);
}
} else {
params.put(parameterName, servletRequest.getParameterValues(parameterName)[0]);
}
}
for (Entry<String, String> entry : rawPathParameters.entrySet()) {
String parameterName = entry.getKey();
Class<?> parameterClass = parameterMap.get(parameterName);
if (parameterClass != null && !body.has(parameterName)) {
if (parameterClass != null && !params.has(parameterName)) {
if (parameterConfigMap.get(parameterName).isRepeated()) {
ArrayNode values = body.putArray(parameterName);
ArrayNode values = params.putArray(parameterName);
for (String value : COMPOSITE_PATH_SPLITTER.split(entry.getValue())) {
values.add(value);
}
} else {
body.put(parameterName, entry.getValue());
params.put(parameterName, entry.getValue());
}
}
}
for (Entry<String, ApiParameterConfig> entry : parameterConfigMap.entrySet()) {
if (!body.has(entry.getKey()) && entry.getValue().getDefaultValue() != null) {
body.put(entry.getKey(), entry.getValue().getDefaultValue());
if (!params.has(entry.getKey()) && entry.getValue().getDefaultValue() != null) {
params.put(entry.getKey(), entry.getValue().getDefaultValue());
}
}
return deserializeParams(body);
} catch (InvalidFormatException e) {
return deserializeParams(body, params);
} catch (MismatchedInputException e) {
logger.atInfo().withCause(e).log("Unable to read request parameter(s)");
throw translate(e);
throw translateJsonException(e);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException
| IOException e) {
logger.atInfo().withCause(e).log("Unable to read request parameter(s)");
throw new BadRequestException("Parse error", "parseError", e);
}
}

private BadRequestException translate(InvalidFormatException e) {
String messagePattern = "Invalid {0} value \"{1}\".{2}";
String message;
String reason = "parseError";
if (e.getTargetType().isEnum()) {
message = MessageFormat.format(messagePattern, "enum",
e.getValue(),
" Valid values are " + Arrays.toString(e.getTargetType().getEnumConstants())
);
} else if (isNumber(e.getTargetType())) {
message = MessageFormat.format(messagePattern, "number", e.getValue(), "");
} else if (isBoolean(e.getTargetType())) {
message = MessageFormat.format(messagePattern,"boolean", e.getValue(), " Valid values are [true, false]");
} else if (isDate(e.getTargetType())) {
message = MessageFormat.format(messagePattern, "date", e.getValue(), "");
} else {
message = "Parse error";
}

return new BadRequestException(message, reason, e);
}

private boolean isBoolean(Class<?> clazz) {
return Boolean.class.equals(clazz) || boolean.class.equals(clazz);
}

private boolean isDate(Class<?> clazz) {
return Date.class.isAssignableFrom(clazz)
|| DateAndTime.class.equals(clazz)
|| SimpleDate.class.equals(clazz);
}

private boolean isNumber(Class<?> clazz) {
return Number.class.isAssignableFrom(clazz)
|| byte.class.equals(clazz)
|| int.class.equals(clazz)
|| long.class.equals(clazz)
|| float.class.equals(clazz)
|| double.class.equals(clazz);
}

private static ImmutableMap<String, Class<?>> getParameterMap(EndpointMethod method)
throws IllegalAccessException, NoSuchMethodException, InvocationTargetException {
Expand Down
70 changes: 65 additions & 5 deletions ...-framework/src/main/java/com/google/api/server/spi/request/ServletRequestParamReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
package com.google.api.server.spi.request;

import com.fasterxml.jackson.core.Base64Variants;
import com.fasterxml.jackson.databind.JsonMappingException.Reference;
import com.fasterxml.jackson.databind.exc.InvalidFormatException;
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.api.server.spi.ConfiguredObjectMapper;
import com.google.api.server.spi.EndpointMethod;
import com.google.api.server.spi.EndpointsContext;
Expand Down Expand Up @@ -54,6 +57,7 @@
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.ParameterizedType;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -65,6 +69,7 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import java.util.stream.Collectors;
import javax.servlet.ServletContext;
import javax.servlet.http.HttpServletRequest;

Expand Down Expand Up @@ -128,7 +133,7 @@ protected static List<String> getParameterNames(EndpointMethod endpointMethod)
return parameterNames;
}

protected Object[] deserializeParams(JsonNode node) throws IOException, IllegalAccessException,
protected Object[] deserializeParams(JsonNode body, JsonNode parameters) throws IOException, IllegalAccessException,
InvocationTargetException, NoSuchMethodException, ServiceException {
EndpointMethod method = getMethod();
Class<?>[] paramClasses = method.getParameterClasses();
Expand Down Expand Up @@ -174,13 +179,13 @@ protected Object[] deserializeParams(JsonNode node) throws IOException, IllegalA
} else {
String name = parameterNames.get(i);
if (Strings.isNullOrEmpty(name)) {
params[i] = (node == null) ? null : objectReader.forType(clazz).readValue(node);
params[i] = (body == null) ? null : objectReader.forType(clazz).readValue(body);
logger.atFine().log("deserialize: %s %s injected into unnamed param[%d]",
clazz, params[i], i);
} else if (StandardParameters.isStandardParamName(name)) {
params[i] = getStandardParamValue(node, name);
params[i] = getStandardParamValue(parameters, name);
} else {
JsonNode nodeValue = node.get(name);
JsonNode nodeValue = parameters.get(name);
if (nodeValue == null) {
params[i] = null;
} else {
Expand Down Expand Up @@ -356,10 +361,65 @@ public Object[] read() throws ServiceException {
return new Object[0];
}
JsonNode node = objectReader.readTree(requestBody);
return deserializeParams(node);
if (!node.isObject()) {
throw new BadRequestException("expected a JSON object body");
}
//this convention comes from gapi.client to separate params and body
JsonNode resource = node.get("resource");
((ObjectNode) node).remove("resource");
return deserializeParams(resource, node);
} catch (MismatchedInputException e) {
logger.atInfo().withCause(e).log("Unable to read request parameter(s)");
throw translateJsonException(e);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException
| IOException e) {
throw new BadRequestException(e);
}
}

BadRequestException translateJsonException(MismatchedInputException e) {
String reason = "parseError";
Class<?> targetType = e.getTargetType();
String fieldPath = e.getPath().stream().map(Reference::getFieldName)
.collect(Collectors.joining("."));
String message = "Parse error at '" + fieldPath
+ "' ('" + targetType.getSimpleName() + "' type)";
String messagePattern = ": invalid {0} value \"{1}\".{2}";
if (e instanceof InvalidFormatException) {
Object value = ((InvalidFormatException) e).getValue();
if (targetType.isEnum()) {
message += MessageFormat.format(messagePattern, "enum",
value,
" Valid values are " + Arrays.toString(targetType.getEnumConstants())
);
} else if (isNumber(targetType)) {
message += MessageFormat.format(messagePattern, "number", value, "");
} else if (isBoolean(targetType)) {
message += MessageFormat.format(messagePattern,"boolean", value, " Valid values are [true, false]");
} else if (isDate(targetType)) {
message += MessageFormat.format(messagePattern, "date", value, "");
}
}

return new BadRequestException(message, reason, e);
}

private boolean isBoolean(Class<?> clazz) {
return Boolean.class.equals(clazz) || boolean.class.equals(clazz);
}

private boolean isDate(Class<?> clazz) {
return Date.class.isAssignableFrom(clazz)
|| DateAndTime.class.equals(clazz)
|| SimpleDate.class.equals(clazz);
}

private boolean isNumber(Class<?> clazz) {
return Number.class.isAssignableFrom(clazz)
|| byte.class.equals(clazz)
|| int.class.equals(clazz)
|| long.class.equals(clazz)
|| float.class.equals(clazz)
|| double.class.equals(clazz);
}
}
9 changes: 5 additions & 4 deletions endpoints-framework/src/test/java/com/google/api/server/spi/EndpointsServletTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;

import java.nio.charset.StandardCharsets;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -102,7 +103,7 @@ public void empty() throws IOException {
public void echo() throws IOException {
req.setRequestURI("/_ah/api/test/v2/echo");
req.setMethod("POST");
req.setParameter("x", "1");
req.setContent("{\"x\":1}".getBytes(StandardCharsets.UTF_8));

servlet.service(req, resp);

Expand All @@ -117,7 +118,7 @@ public void echo() throws IOException {
public void contentLengthHeaderNull() throws IOException {
req.setRequestURI("/_ah/api/test/v2/echo");
req.setMethod("POST");
req.setParameter("x", "1");
req.setContent("{\"x\":1}".getBytes(StandardCharsets.UTF_8));

servlet.service(req, resp);

Expand All @@ -133,7 +134,7 @@ public void contentLengthHeaderPresent() throws IOException, ServletException {

req.setRequestURI("/_ah/api/test/v2/echo");
req.setMethod("POST");
req.setParameter("x", "1");
req.setContent("{\"x\":1}".getBytes(StandardCharsets.UTF_8));

servlet.service(req, resp);

Expand All @@ -145,7 +146,7 @@ public void methodOverride() throws IOException {
req.setRequestURI("/_ah/api/test/v2/increment");
req.setMethod("POST");
req.addHeader("X-HTTP-Method-Override", "PATCH");
req.setParameter("x", "1");
req.setContent("{\"x\":1}".getBytes(StandardCharsets.UTF_8));

servlet.service(req, resp);

Expand Down
Loading