Fix Smile encoding for HTTP response (#10980)
* fix Smile encoding bug
Signed-off-by: frank chen <frank.chen021@outlook.com>
* Add unit tests
* Add IT for smile encoding
* Fix cases
* Update javadoc
Co-authored-by: Jihoon Son <jihoonson@apache.org>
* resolve comments
Co-authored-by: Jihoon Son <jihoonson@apache.org>
diff --git a/integration-tests/src/main/java/org/apache/druid/testing/clients/AbstractQueryResourceTestClient.java b/integration-tests/src/main/java/org/apache/druid/testing/clients/AbstractQueryResourceTestClient.java
index 15851c5..ce2703c 100644
--- a/integration-tests/src/main/java/org/apache/druid/testing/clients/AbstractQueryResourceTestClient.java
+++ b/integration-tests/src/main/java/org/apache/druid/testing/clients/AbstractQueryResourceTestClient.java
@@ -21,38 +21,115 @@
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.jaxrs.smile.SmileMediaTypes;
import com.google.inject.Inject;
+import org.apache.druid.guice.annotations.Smile;
+import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.http.client.HttpClient;
import org.apache.druid.java.util.http.client.Request;
+import org.apache.druid.java.util.http.client.response.BytesFullResponseHandler;
+import org.apache.druid.java.util.http.client.response.BytesFullResponseHolder;
import org.apache.druid.java.util.http.client.response.StatusResponseHandler;
import org.apache.druid.java.util.http.client.response.StatusResponseHolder;
-import org.apache.druid.testing.IntegrationTestingConfig;
import org.apache.druid.testing.guice.TestClient;
+import org.jboss.netty.handler.codec.http.HttpHeaders;
import org.jboss.netty.handler.codec.http.HttpMethod;
import org.jboss.netty.handler.codec.http.HttpResponseStatus;
+import javax.annotation.Nullable;
+import javax.ws.rs.core.MediaType;
+import java.io.IOException;
import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Future;
public abstract class AbstractQueryResourceTestClient<QueryType>
{
- private final ObjectMapper jsonMapper;
- private final HttpClient httpClient;
- final String routerUrl;
+ final String contentTypeHeader;
+ /**
+ * a 'null' means the Content-Type in response defaults to Content-Type of request
+ */
+ final String acceptHeader;
+
+ final ObjectMapper jsonMapper;
+ final ObjectMapper smileMapper;
+ final HttpClient httpClient;
+ final String routerUrl;
+ final Map<String, EncoderDecoder> encoderDecoderMap;
+
+ /**
+ * A encoder/decoder that encodes/decodes requests/responses based on Content-Type.
+ */
+ interface EncoderDecoder
+ {
+ byte[] encode(Object content) throws IOException;
+
+ List<Map<String, Object>> decode(byte[] content) throws IOException;
+ }
+
+ static class ObjectMapperEncoderDecoder implements EncoderDecoder
+ {
+ private final ObjectMapper om;
+
+ ObjectMapperEncoderDecoder(ObjectMapper om)
+ {
+ this.om = om;
+ }
+
+ @Override
+ public byte[] encode(Object content) throws IOException
+ {
+ return om.writeValueAsBytes(content);
+ }
+
+ @Override
+ public List<Map<String, Object>> decode(byte[] content) throws IOException
+ {
+ return om.readValue(content, new TypeReference<List<Map<String, Object>>>()
+ {
+ });
+ }
+ }
+
+ /**
+ * @param contentTypeHeader Content-Type header of HTTP request
+ * @param acceptHeader Accept header of HTTP request. If it's null, Content-Type in response defaults to Content-Type in request
+ */
@Inject
AbstractQueryResourceTestClient(
ObjectMapper jsonMapper,
+ @Smile ObjectMapper smileMapper,
@TestClient HttpClient httpClient,
- IntegrationTestingConfig config
+ String routerUrl,
+ String contentTypeHeader,
+ @Nullable String acceptHeader
)
{
this.jsonMapper = jsonMapper;
+ this.smileMapper = smileMapper;
this.httpClient = httpClient;
- this.routerUrl = config.getRouterUrl();
+ this.routerUrl = routerUrl;
+
+ this.encoderDecoderMap = new HashMap<>();
+ this.encoderDecoderMap.put(MediaType.APPLICATION_JSON, new ObjectMapperEncoderDecoder(jsonMapper));
+ this.encoderDecoderMap.put(SmileMediaTypes.APPLICATION_JACKSON_SMILE, new ObjectMapperEncoderDecoder(smileMapper));
+
+ if (!this.encoderDecoderMap.containsKey(contentTypeHeader)) {
+ throw new IAE("Invalid Content-Type[%s]", contentTypeHeader);
+ }
+ this.contentTypeHeader = contentTypeHeader;
+
+ if (acceptHeader != null) {
+ if (!this.encoderDecoderMap.containsKey(acceptHeader)) {
+ throw new IAE("Invalid Accept[%s]", acceptHeader);
+ }
+ }
+ this.acceptHeader = acceptHeader;
}
public abstract String getBrokerURL();
@@ -60,12 +137,18 @@
public List<Map<String, Object>> query(String url, QueryType query)
{
try {
- StatusResponseHolder response = httpClient.go(
- new Request(HttpMethod.POST, new URL(url)).setContent(
- "application/json",
- jsonMapper.writeValueAsBytes(query)
- ),
- StatusResponseHandler.getInstance()
+ String expectedResponseType = this.contentTypeHeader;
+
+ Request request = new Request(HttpMethod.POST, new URL(url));
+ request.setContent(this.contentTypeHeader, encoderDecoderMap.get(this.contentTypeHeader).encode(query));
+ if (this.acceptHeader != null) {
+ expectedResponseType = this.acceptHeader;
+ request.addHeader(HttpHeaders.Names.ACCEPT, this.acceptHeader);
+ }
+
+ BytesFullResponseHolder response = httpClient.go(
+ request,
+ new BytesFullResponseHandler()
).get();
if (!response.getStatus().equals(HttpResponseStatus.OK)) {
@@ -73,15 +156,20 @@
"Error while querying[%s] status[%s] content[%s]",
getBrokerURL(),
response.getStatus(),
- response.getContent()
+ new String(response.getContent(), StandardCharsets.UTF_8)
);
}
- return jsonMapper.readValue(
- response.getContent(), new TypeReference<List<Map<String, Object>>>()
- {
- }
- );
+ String responseType = response.getResponse().headers().get(HttpHeaders.Names.CONTENT_TYPE);
+ if (!expectedResponseType.equals(responseType)) {
+ throw new ISE(
+ "Content-Type[%s] in HTTP response does not match the expected[%s]",
+ responseType,
+ expectedResponseType
+ );
+ }
+
+ return this.encoderDecoderMap.get(responseType).decode(response.getContent());
}
catch (Exception e) {
throw new RuntimeException(e);
@@ -91,11 +179,10 @@
public Future<StatusResponseHolder> queryAsync(String url, QueryType query)
{
try {
+ Request request = new Request(HttpMethod.POST, new URL(url));
+ request.setContent(MediaType.APPLICATION_JSON, encoderDecoderMap.get(MediaType.APPLICATION_JSON).encode(query));
return httpClient.go(
- new Request(HttpMethod.POST, new URL(url)).setContent(
- "application/json",
- jsonMapper.writeValueAsBytes(query)
- ),
+ request,
StatusResponseHandler.getInstance()
);
}
diff --git a/integration-tests/src/main/java/org/apache/druid/testing/clients/QueryResourceTestClient.java b/integration-tests/src/main/java/org/apache/druid/testing/clients/QueryResourceTestClient.java
index 1397fe1..33820fe 100644
--- a/integration-tests/src/main/java/org/apache/druid/testing/clients/QueryResourceTestClient.java
+++ b/integration-tests/src/main/java/org/apache/druid/testing/clients/QueryResourceTestClient.java
@@ -22,23 +22,40 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.inject.Inject;
+import org.apache.druid.guice.annotations.Smile;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.http.client.HttpClient;
import org.apache.druid.query.Query;
import org.apache.druid.testing.IntegrationTestingConfig;
import org.apache.druid.testing.guice.TestClient;
+import javax.annotation.Nullable;
+import javax.ws.rs.core.MediaType;
+
public class QueryResourceTestClient extends AbstractQueryResourceTestClient<Query>
{
@Inject
QueryResourceTestClient(
ObjectMapper jsonMapper,
+ @Smile ObjectMapper smileMapper,
@TestClient HttpClient httpClient,
IntegrationTestingConfig config
)
{
- super(jsonMapper, httpClient, config);
+ this(jsonMapper, smileMapper, httpClient, config.getRouterUrl(), MediaType.APPLICATION_JSON, null);
+ }
+
+ private QueryResourceTestClient(
+ ObjectMapper jsonMapper,
+ @Smile ObjectMapper smileMapper,
+ @TestClient HttpClient httpClient,
+ String routerUrl,
+ String contentType,
+ String accept
+ )
+ {
+ super(jsonMapper, smileMapper, httpClient, routerUrl, contentType, accept);
}
@Override
@@ -50,4 +67,22 @@
);
}
+ /**
+ * clone a new instance of current object with given encoding.
+ * Note: For {@link AbstractQueryResourceTestClient#queryAsync(String, Object)} operation, contentType could only be application/json
+ *
+ * @param contentType Content-Type header of request. Cannot be NULL. Both application/json and application/x-jackson-smile are allowed
+ * @param accept Accept header of request. Both application/json and application/x-jackson-smile are allowed
+ */
+ public QueryResourceTestClient withEncoding(String contentType, @Nullable String accept)
+ {
+ return new QueryResourceTestClient(
+ this.jsonMapper,
+ this.smileMapper,
+ this.httpClient,
+ this.routerUrl,
+ contentType,
+ accept
+ );
+ }
}
diff --git a/integration-tests/src/main/java/org/apache/druid/testing/clients/SqlResourceTestClient.java b/integration-tests/src/main/java/org/apache/druid/testing/clients/SqlResourceTestClient.java
index d8f85c6..b429318 100644
--- a/integration-tests/src/main/java/org/apache/druid/testing/clients/SqlResourceTestClient.java
+++ b/integration-tests/src/main/java/org/apache/druid/testing/clients/SqlResourceTestClient.java
@@ -27,6 +27,8 @@
import org.apache.druid.testing.IntegrationTestingConfig;
import org.apache.druid.testing.guice.TestClient;
+import javax.ws.rs.core.MediaType;
+
public class SqlResourceTestClient extends AbstractQueryResourceTestClient<SqlQuery>
{
@@ -37,7 +39,9 @@
IntegrationTestingConfig config
)
{
- super(jsonMapper, httpClient, config);
+ // currently smile encoding is not supported on SQL endpoint
+ // so no need to pass smile ObjectMapper
+ super(jsonMapper, null, httpClient, config.getRouterUrl(), MediaType.APPLICATION_JSON, null);
}
@Override
diff --git a/integration-tests/src/main/java/org/apache/druid/testing/utils/AbstractTestQueryHelper.java b/integration-tests/src/main/java/org/apache/druid/testing/utils/AbstractTestQueryHelper.java
index ebaed72..bd6866a 100644
--- a/integration-tests/src/main/java/org/apache/druid/testing/utils/AbstractTestQueryHelper.java
+++ b/integration-tests/src/main/java/org/apache/druid/testing/utils/AbstractTestQueryHelper.java
@@ -43,8 +43,8 @@
public static final Logger LOG = new Logger(TestQueryHelper.class);
- private final AbstractQueryResourceTestClient queryClient;
- private final ObjectMapper jsonMapper;
+ protected final AbstractQueryResourceTestClient queryClient;
+ protected final ObjectMapper jsonMapper;
protected final String broker;
protected final String brokerTLS;
protected final String router;
@@ -57,12 +57,31 @@
IntegrationTestingConfig config
)
{
+ this(
+ jsonMapper,
+ queryClient,
+ config.getBrokerUrl(),
+ config.getBrokerTLSUrl(),
+ config.getRouterUrl(),
+ config.getRouterTLSUrl()
+ );
+ }
+
+ AbstractTestQueryHelper(
+ ObjectMapper jsonMapper,
+ AbstractQueryResourceTestClient queryClient,
+ String broker,
+ String brokerTLS,
+ String router,
+ String routerTLS
+ )
+ {
this.jsonMapper = jsonMapper;
this.queryClient = queryClient;
- this.broker = config.getBrokerUrl();
- this.brokerTLS = config.getBrokerTLSUrl();
- this.router = config.getRouterUrl();
- this.routerTLS = config.getRouterTLSUrl();
+ this.broker = broker;
+ this.brokerTLS = brokerTLS;
+ this.router = router;
+ this.routerTLS = routerTLS;
}
public abstract String getQueryURL(String schemeAndHost);
diff --git a/integration-tests/src/main/java/org/apache/druid/testing/utils/TestQueryHelper.java b/integration-tests/src/main/java/org/apache/druid/testing/utils/TestQueryHelper.java
index a79332a..fb48b73 100644
--- a/integration-tests/src/main/java/org/apache/druid/testing/utils/TestQueryHelper.java
+++ b/integration-tests/src/main/java/org/apache/druid/testing/utils/TestQueryHelper.java
@@ -25,6 +25,9 @@
import org.apache.druid.testing.IntegrationTestingConfig;
import org.apache.druid.testing.clients.QueryResourceTestClient;
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+
public class TestQueryHelper extends AbstractTestQueryHelper<QueryWithResults>
{
@@ -38,10 +41,46 @@
super(jsonMapper, queryClient, config);
}
+ private TestQueryHelper(
+ ObjectMapper jsonMapper,
+ QueryResourceTestClient queryResourceTestClient,
+ String broker,
+ String brokerTLS,
+ String router,
+ String routerTLS
+ )
+ {
+ super(
+ jsonMapper,
+ queryResourceTestClient,
+ broker,
+ brokerTLS,
+ router,
+ routerTLS
+ );
+ }
+
@Override
public String getQueryURL(String schemeAndHost)
{
return StringUtils.format("%s/druid/v2?pretty", schemeAndHost);
}
+ /**
+ * clone a new instance of current object with given encoding
+ *
+ * @param contentType Content-Type header of request. Cannot be NULL. Both application/json and application/x-jackson-smile are allowed
+ * @param accept Accept header of request. Both application/json and application/x-jackson-smile are allowed
+ */
+ public TestQueryHelper withEncoding(@NotNull String contentType, @Nullable String accept)
+ {
+ return new TestQueryHelper(
+ this.jsonMapper,
+ ((QueryResourceTestClient) this.queryClient).withEncoding(contentType, accept),
+ this.broker,
+ this.brokerTLS,
+ this.router,
+ this.routerTLS
+ );
+ }
}
diff --git a/integration-tests/src/test/java/org/apache/druid/tests/query/ITWikipediaQueryTest.java b/integration-tests/src/test/java/org/apache/druid/tests/query/ITWikipediaQueryTest.java
index 9fc5209..ee53ab5 100644
--- a/integration-tests/src/test/java/org/apache/druid/tests/query/ITWikipediaQueryTest.java
+++ b/integration-tests/src/test/java/org/apache/druid/tests/query/ITWikipediaQueryTest.java
@@ -19,6 +19,7 @@
package org.apache.druid.tests.query;
+import com.fasterxml.jackson.jaxrs.smile.SmileMediaTypes;
import com.google.common.collect.ImmutableMap;
import com.google.inject.Inject;
import org.apache.druid.java.util.common.logger.Logger;
@@ -36,9 +37,11 @@
import org.jboss.netty.handler.codec.http.HttpResponseStatus;
import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Guice;
import org.testng.annotations.Test;
+import javax.ws.rs.core.MediaType;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
@@ -79,9 +82,32 @@
}
}
- @Test
- public void testWikipediaQueriesFromFile() throws Exception
+ /**
+ * A combination of request Content-Type and Accept HTTP header
+ * The first is Content-Type which can not be null while the 2nd is Accept which could be null
+ * <p>
+ * When Accept is null, its value defaults to value of Content-Type
+ */
+ @DataProvider
+ public static Object[][] encodingCombination()
{
+ return new Object[][]{
+ {MediaType.APPLICATION_JSON, null},
+ {MediaType.APPLICATION_JSON, MediaType.APPLICATION_JSON},
+ {MediaType.APPLICATION_JSON, SmileMediaTypes.APPLICATION_JACKSON_SMILE},
+ {SmileMediaTypes.APPLICATION_JACKSON_SMILE, null},
+ {SmileMediaTypes.APPLICATION_JACKSON_SMILE, MediaType.APPLICATION_JSON},
+ {SmileMediaTypes.APPLICATION_JACKSON_SMILE, SmileMediaTypes.APPLICATION_JACKSON_SMILE},
+ };
+ }
+
+ @Test(dataProvider = "encodingCombination")
+ public void testWikipediaQueriesFromFile(String contentType, String accept)
+ throws Exception
+ {
+ // run tests on a new query helper
+ TestQueryHelper queryHelper = this.queryHelper.withEncoding(contentType, accept);
+
queryHelper.testQueriesFromFile(WIKIPEDIA_QUERIES_RESOURCE);
}
diff --git a/server/src/main/java/org/apache/druid/server/BrokerQueryResource.java b/server/src/main/java/org/apache/druid/server/BrokerQueryResource.java
index 9ef86af..b4ebc8c 100644
--- a/server/src/main/java/org/apache/druid/server/BrokerQueryResource.java
+++ b/server/src/main/java/org/apache/druid/server/BrokerQueryResource.java
@@ -91,11 +91,10 @@
@Context final HttpServletRequest req
) throws IOException
{
- final ResourceIOReaderWriter ioReaderWriter =
- createResourceIOReaderWriter(req.getContentType(), pretty != null);
+ final ResourceIOReaderWriter ioReaderWriter = createResourceIOReaderWriter(req, pretty != null);
try {
- Query<?> query = ioReaderWriter.getInputMapper().readValue(in, Query.class);
- return ioReaderWriter.ok(
+ Query<?> query = ioReaderWriter.getRequestMapper().readValue(in, Query.class);
+ return ioReaderWriter.getResponseWriter().ok(
ServerViewUtil.getTargetLocations(
brokerServerView,
query.getDataSource(),
@@ -105,7 +104,7 @@
);
}
catch (Exception e) {
- return ioReaderWriter.gotError(e);
+ return ioReaderWriter.getResponseWriter().gotError(e);
}
}
}
diff --git a/server/src/main/java/org/apache/druid/server/QueryResource.java b/server/src/main/java/org/apache/druid/server/QueryResource.java
index 6394395..b2f7f21 100644
--- a/server/src/main/java/org/apache/druid/server/QueryResource.java
+++ b/server/src/main/java/org/apache/druid/server/QueryResource.java
@@ -183,13 +183,7 @@
final QueryLifecycle queryLifecycle = queryLifecycleFactory.factorize();
Query<?> query = null;
- String acceptHeader = req.getHeader("Accept");
- if (Strings.isNullOrEmpty(acceptHeader)) {
- //default to content-type
- acceptHeader = req.getContentType();
- }
-
- final ResourceIOReaderWriter ioReaderWriter = createResourceIOReaderWriter(acceptHeader, pretty != null);
+ final ResourceIOReaderWriter ioReaderWriter = createResourceIOReaderWriter(req, pretty != null);
final String currThreadName = Thread.currentThread().getName();
try {
@@ -235,7 +229,7 @@
QueryContexts.isSerializeDateTimeAsLong(query, false)
|| (!shouldFinalize && QueryContexts.isSerializeDateTimeAsLongInner(query, false));
- final ObjectWriter jsonWriter = ioReaderWriter.newOutputWriter(
+ final ObjectWriter jsonWriter = ioReaderWriter.getResponseWriter().newOutputWriter(
queryLifecycle.getToolChest(),
queryLifecycle.getQuery(),
serializeDateTimeAsLong
@@ -276,7 +270,7 @@
}
}
},
- ioReaderWriter.getContentType()
+ ioReaderWriter.getResponseWriter().getResponseType()
)
.header("X-Druid-Query-Id", queryId);
@@ -337,27 +331,27 @@
catch (QueryInterruptedException e) {
interruptedQueryCount.incrementAndGet();
queryLifecycle.emitLogsAndMetrics(e, req.getRemoteAddr(), -1);
- return ioReaderWriter.gotError(e);
+ return ioReaderWriter.getResponseWriter().gotError(e);
}
catch (QueryTimeoutException timeout) {
timedOutQueryCount.incrementAndGet();
queryLifecycle.emitLogsAndMetrics(timeout, req.getRemoteAddr(), -1);
- return ioReaderWriter.gotTimeout(timeout);
+ return ioReaderWriter.getResponseWriter().gotTimeout(timeout);
}
catch (QueryCapacityExceededException cap) {
failedQueryCount.incrementAndGet();
queryLifecycle.emitLogsAndMetrics(cap, req.getRemoteAddr(), -1);
- return ioReaderWriter.gotLimited(cap);
+ return ioReaderWriter.getResponseWriter().gotLimited(cap);
}
catch (QueryUnsupportedException unsupported) {
failedQueryCount.incrementAndGet();
queryLifecycle.emitLogsAndMetrics(unsupported, req.getRemoteAddr(), -1);
- return ioReaderWriter.gotUnsupported(unsupported);
+ return ioReaderWriter.getResponseWriter().gotUnsupported(unsupported);
}
catch (BadJsonQueryException | ResourceLimitExceededException e) {
interruptedQueryCount.incrementAndGet();
queryLifecycle.emitLogsAndMetrics(e, req.getRemoteAddr(), -1);
- return ioReaderWriter.gotBadQuery(e);
+ return ioReaderWriter.getResponseWriter().gotBadQuery(e);
}
catch (ForbiddenException e) {
// don't do anything for an authorization failure, ForbiddenExceptionMapper will catch this later and
@@ -374,7 +368,7 @@
.addData("peer", req.getRemoteAddr())
.emit();
- return ioReaderWriter.gotError(e);
+ return ioReaderWriter.getResponseWriter().gotError(e);
}
finally {
Thread.currentThread().setName(currThreadName);
@@ -389,7 +383,7 @@
{
Query baseQuery;
try {
- baseQuery = ioReaderWriter.getInputMapper().readValue(in, Query.class);
+ baseQuery = ioReaderWriter.getRequestMapper().readValue(in, Query.class);
}
catch (JsonParseException e) {
throw new BadJsonQueryException(e);
@@ -415,47 +409,71 @@
return mapper.copy().registerModule(new SimpleModule().addSerializer(DateTime.class, new DateTimeSerializer()));
}
- protected ResourceIOReaderWriter createResourceIOReaderWriter(String requestType, boolean pretty)
+ protected ResourceIOReaderWriter createResourceIOReaderWriter(HttpServletRequest req, boolean pretty)
{
- boolean isSmile = SmileMediaTypes.APPLICATION_JACKSON_SMILE.equals(requestType) ||
- APPLICATION_SMILE.equals(requestType);
- String contentType = isSmile ? SmileMediaTypes.APPLICATION_JACKSON_SMILE : MediaType.APPLICATION_JSON;
+ String requestType = req.getContentType();
+ String acceptHeader = req.getHeader("Accept");
+
+ // response type defaults to Content-Type if 'Accept' header not provided
+ String responseType = Strings.isNullOrEmpty(acceptHeader) ? requestType : acceptHeader;
+
+ boolean isRequestSmile = SmileMediaTypes.APPLICATION_JACKSON_SMILE.equals(requestType) || APPLICATION_SMILE.equals(requestType);
+ boolean isResponseSmile = SmileMediaTypes.APPLICATION_JACKSON_SMILE.equals(responseType) || APPLICATION_SMILE.equals(responseType);
+
return new ResourceIOReaderWriter(
- contentType,
- isSmile ? smileMapper : jsonMapper,
- isSmile ? serializeDateTimeAsLongSmileMapper : serializeDateTimeAsLongJsonMapper,
- pretty
- );
+ isRequestSmile ? smileMapper : jsonMapper,
+ new ResourceIOWriter(isResponseSmile ? SmileMediaTypes.APPLICATION_JACKSON_SMILE : MediaType.APPLICATION_JSON,
+ isResponseSmile ? smileMapper : jsonMapper,
+ isResponseSmile ? serializeDateTimeAsLongSmileMapper : serializeDateTimeAsLongJsonMapper,
+ pretty
+ ));
}
protected static class ResourceIOReaderWriter
{
- private final String contentType;
+ private final ObjectMapper requestMapper;
+ private final ResourceIOWriter writer;
+
+ public ResourceIOReaderWriter(ObjectMapper requestMapper, ResourceIOWriter writer)
+ {
+ this.requestMapper = requestMapper;
+ this.writer = writer;
+ }
+
+ public ObjectMapper getRequestMapper()
+ {
+ return requestMapper;
+ }
+
+ public ResourceIOWriter getResponseWriter()
+ {
+ return writer;
+ }
+ }
+
+ protected static class ResourceIOWriter
+ {
+ private final String responseType;
private final ObjectMapper inputMapper;
private final ObjectMapper serializeDateTimeAsLongInputMapper;
private final boolean isPretty;
- ResourceIOReaderWriter(
- String contentType,
+ ResourceIOWriter(
+ String responseType,
ObjectMapper inputMapper,
ObjectMapper serializeDateTimeAsLongInputMapper,
boolean isPretty
)
{
- this.contentType = contentType;
+ this.responseType = responseType;
this.inputMapper = inputMapper;
this.serializeDateTimeAsLongInputMapper = serializeDateTimeAsLongInputMapper;
this.isPretty = isPretty;
}
- String getContentType()
+ String getResponseType()
{
- return contentType;
- }
-
- ObjectMapper getInputMapper()
- {
- return inputMapper;
+ return responseType;
}
ObjectWriter newOutputWriter(
@@ -476,7 +494,7 @@
Response ok(Object object) throws IOException
{
- return Response.ok(newOutputWriter(null, null, false).writeValueAsString(object), contentType).build();
+ return Response.ok(newOutputWriter(null, null, false).writeValueAsString(object), responseType).build();
}
Response gotError(Exception e) throws IOException
@@ -510,7 +528,7 @@
Response buildNonOkResponse(int status, Exception e) throws JsonProcessingException
{
return Response.status(status)
- .type(contentType)
+ .type(responseType)
.entity(newOutputWriter(null, null, false).writeValueAsBytes(e))
.build();
}
diff --git a/server/src/test/java/org/apache/druid/server/QueryResourceTest.java b/server/src/test/java/org/apache/druid/server/QueryResourceTest.java
index ce3c032..026af18 100644
--- a/server/src/test/java/org/apache/druid/server/QueryResourceTest.java
+++ b/server/src/test/java/org/apache/druid/server/QueryResourceTest.java
@@ -28,6 +28,10 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import org.apache.druid.guice.GuiceInjectors;
+import org.apache.druid.guice.annotations.Smile;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.concurrent.Execs;
import org.apache.druid.java.util.common.guava.LazySequence;
@@ -96,7 +100,6 @@
public class QueryResourceTest
{
private static final QueryToolChestWarehouse WAREHOUSE = new MapQueryToolChestWarehouse(ImmutableMap.of());
- private static final ObjectMapper JSON_MAPPER = new DefaultObjectMapper();
private static final AuthenticationResult AUTHENTICATION_RESULT =
new AuthenticationResult("druid", "druid", null, null);
@@ -178,6 +181,8 @@
false
);
+ private ObjectMapper jsonMapper;
+ private ObjectMapper smileMapper;
private QueryResource queryResource;
private QueryScheduler queryScheduler;
private TestRequestLogger testRequestLogger;
@@ -191,6 +196,10 @@
@Before
public void setup()
{
+ Injector injector = GuiceInjectors.makeStartupInjector();
+ jsonMapper = injector.getInstance(ObjectMapper.class);
+ smileMapper = injector.getInstance(Key.get(ObjectMapper.class, Smile.class));
+
EasyMock.expect(testServletRequest.getContentType()).andReturn(MediaType.APPLICATION_JSON).anyTimes();
EasyMock.expect(testServletRequest.getHeader("Accept")).andReturn(MediaType.APPLICATION_JSON).anyTimes();
EasyMock.expect(testServletRequest.getHeader(QueryResource.HEADER_IF_NONE_MATCH)).andReturn(null).anyTimes();
@@ -213,8 +222,8 @@
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of()))
),
- JSON_MAPPER,
- JSON_MAPPER,
+ jsonMapper,
+ smileMapper,
queryScheduler,
new AuthConfig(),
null,
@@ -259,8 +268,8 @@
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
Suppliers.ofInstance(overrideConfig)
),
- JSON_MAPPER,
- JSON_MAPPER,
+ jsonMapper,
+ smileMapper,
queryScheduler,
new AuthConfig(),
null,
@@ -279,7 +288,7 @@
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
((StreamingOutput) response.getEntity()).write(baos);
- final List<Result<TimeBoundaryResultValue>> responses = JSON_MAPPER.readValue(
+ final List<Result<TimeBoundaryResultValue>> responses = jsonMapper.readValue(
baos.toByteArray(),
new TypeReference<List<Result<TimeBoundaryResultValue>>>() {}
);
@@ -311,8 +320,8 @@
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
Suppliers.ofInstance(overrideConfig)
),
- JSON_MAPPER,
- JSON_MAPPER,
+ jsonMapper,
+ smileMapper,
queryScheduler,
new AuthConfig(),
null,
@@ -332,7 +341,7 @@
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
((StreamingOutput) response.getEntity()).write(baos);
- final List<Result<TimeBoundaryResultValue>> responses = JSON_MAPPER.readValue(
+ final List<Result<TimeBoundaryResultValue>> responses = jsonMapper.readValue(
baos.toByteArray(),
new TypeReference<List<Result<TimeBoundaryResultValue>>>() {}
);
@@ -367,7 +376,7 @@
).toString();
Assert.assertEquals(
expectedException,
- JSON_MAPPER.readValue((byte[]) response.getEntity(), QueryInterruptedException.class).toString()
+ jsonMapper.readValue((byte[]) response.getEntity(), QueryInterruptedException.class).toString()
);
}
@@ -457,7 +466,7 @@
}
@Test
- public void testGoodQueryWithSmileAcceptHeader() throws IOException
+ public void testGoodQueryWithJsonRequestAndSmileAcceptHeader() throws IOException
{
//Doing a replay of testServletRequest for teardown to succeed.
//We dont use testServletRequest in this testcase
@@ -466,6 +475,8 @@
//Creating our own Smile Servlet request, as to not disturb the remaining tests.
// else refactoring required for this class. i know this kinda makes the class somewhat Dirty.
final HttpServletRequest smileRequest = EasyMock.createMock(HttpServletRequest.class);
+
+ // Set Content-Type to JSON
EasyMock.expect(smileRequest.getContentType()).andReturn(MediaType.APPLICATION_JSON).anyTimes();
EasyMock.expect(smileRequest.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED))
@@ -479,6 +490,7 @@
smileRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+ // Set Accept to Smile
EasyMock.expect(smileRequest.getHeader("Accept")).andReturn(SmileMediaTypes.APPLICATION_JACKSON_SMILE).anyTimes();
EasyMock.expect(smileRequest.getHeader(QueryResource.HEADER_IF_NONE_MATCH)).andReturn(null).anyTimes();
EasyMock.expect(smileRequest.getRemoteAddr()).andReturn("localhost").anyTimes();
@@ -490,6 +502,98 @@
smileRequest
);
Assert.assertEquals(HttpStatus.SC_OK, response.getStatus());
+
+ // Content-Type in response should be Smile
+ Assert.assertEquals(SmileMediaTypes.APPLICATION_JACKSON_SMILE, (response.getMetadata().get("Content-Type").get(0)).toString());
+ Assert.assertNotNull(response);
+ EasyMock.verify(smileRequest);
+ }
+
+ @Test
+ public void testGoodQueryWithSmileRequestAndSmileAcceptHeader() throws IOException
+ {
+ //Doing a replay of testServletRequest for teardown to succeed.
+ //We dont use testServletRequest in this testcase
+ EasyMock.replay(testServletRequest);
+
+ //Creating our own Smile Servlet request, as to not disturb the remaining tests.
+ // else refactoring required for this class. i know this kinda makes the class somewhat Dirty.
+ final HttpServletRequest smileRequest = EasyMock.createMock(HttpServletRequest.class);
+
+ // Set Content-Type to Smile
+ EasyMock.expect(smileRequest.getContentType()).andReturn(SmileMediaTypes.APPLICATION_JACKSON_SMILE).anyTimes();
+
+ EasyMock.expect(smileRequest.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED))
+ .andReturn(null)
+ .anyTimes();
+ EasyMock.expect(smileRequest.getAttribute(AuthConfig.DRUID_ALLOW_UNSECURED_PATH)).andReturn(null).anyTimes();
+
+ EasyMock.expect(smileRequest.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT))
+ .andReturn(AUTHENTICATION_RESULT)
+ .anyTimes();
+
+ smileRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+
+ // Set Accept to Smile
+ EasyMock.expect(smileRequest.getHeader("Accept")).andReturn(SmileMediaTypes.APPLICATION_JACKSON_SMILE).anyTimes();
+ EasyMock.expect(smileRequest.getHeader(QueryResource.HEADER_IF_NONE_MATCH)).andReturn(null).anyTimes();
+ EasyMock.expect(smileRequest.getRemoteAddr()).andReturn("localhost").anyTimes();
+
+ EasyMock.replay(smileRequest);
+ Response response = queryResource.doPost(
+ // Write input in Smile encoding
+ new ByteArrayInputStream(smileMapper.writeValueAsBytes(jsonMapper.readTree(SIMPLE_TIMESERIES_QUERY))),
+ null /*pretty*/,
+ smileRequest
+ );
+ Assert.assertEquals(HttpStatus.SC_OK, response.getStatus());
+
+ // Content-Type in response should be Smile
+ Assert.assertEquals(SmileMediaTypes.APPLICATION_JACKSON_SMILE, (response.getMetadata().get("Content-Type").get(0)).toString());
+ Assert.assertNotNull(response);
+ EasyMock.verify(smileRequest);
+ }
+
+ @Test
+ public void testGoodQueryWithSmileRequestNoSmileAcceptHeader() throws IOException
+ {
+ //Doing a replay of testServletRequest for teardown to succeed.
+ //We dont use testServletRequest in this testcase
+ EasyMock.replay(testServletRequest);
+
+ //Creating our own Smile Servlet request, as to not disturb the remaining tests.
+ // else refactoring required for this class. i know this kinda makes the class somewhat Dirty.
+ final HttpServletRequest smileRequest = EasyMock.createMock(HttpServletRequest.class);
+
+ // Set Content-Type to Smile
+ EasyMock.expect(smileRequest.getContentType()).andReturn(SmileMediaTypes.APPLICATION_JACKSON_SMILE).anyTimes();
+
+ EasyMock.expect(smileRequest.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED))
+ .andReturn(null)
+ .anyTimes();
+ EasyMock.expect(smileRequest.getAttribute(AuthConfig.DRUID_ALLOW_UNSECURED_PATH)).andReturn(null).anyTimes();
+
+ EasyMock.expect(smileRequest.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT))
+ .andReturn(AUTHENTICATION_RESULT)
+ .anyTimes();
+
+ smileRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+
+ // DO NOT set Accept to Smile, Content-Type in response will be default to Content-Type in request
+ EasyMock.expect(smileRequest.getHeader("Accept")).andReturn(null).anyTimes();
+ EasyMock.expect(smileRequest.getHeader(QueryResource.HEADER_IF_NONE_MATCH)).andReturn(null).anyTimes();
+ EasyMock.expect(smileRequest.getRemoteAddr()).andReturn("localhost").anyTimes();
+
+ EasyMock.replay(smileRequest);
+ Response response = queryResource.doPost(
+ // Write input in Smile encoding
+ new ByteArrayInputStream(smileMapper.writeValueAsBytes(jsonMapper.readTree(SIMPLE_TIMESERIES_QUERY))),
+ null /*pretty*/,
+ smileRequest
+ );
+ Assert.assertEquals(HttpStatus.SC_OK, response.getStatus());
+
+ // Content-Type in response will be default to Content-Type in request
Assert.assertEquals(SmileMediaTypes.APPLICATION_JACKSON_SMILE, (response.getMetadata().get("Content-Type").get(0)).toString());
Assert.assertNotNull(response);
EasyMock.verify(smileRequest);
@@ -506,7 +610,7 @@
);
Assert.assertNotNull(response);
Assert.assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus());
- QueryException e = JSON_MAPPER.readValue((byte[]) response.getEntity(), QueryException.class);
+ QueryException e = jsonMapper.readValue((byte[]) response.getEntity(), QueryException.class);
Assert.assertEquals(BadJsonQueryException.ERROR_CODE, e.getErrorCode());
Assert.assertEquals(BadJsonQueryException.ERROR_CLASS, e.getErrorClass());
}
@@ -525,7 +629,7 @@
);
Assert.assertNotNull(response);
Assert.assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus());
- QueryException e = JSON_MAPPER.readValue((byte[]) response.getEntity(), QueryException.class);
+ QueryException e = jsonMapper.readValue((byte[]) response.getEntity(), QueryException.class);
Assert.assertEquals(ResourceLimitExceededException.ERROR_CODE, e.getErrorCode());
Assert.assertEquals(ResourceLimitExceededException.class.getName(), e.getErrorClass());
}
@@ -546,7 +650,7 @@
);
Assert.assertNotNull(response);
Assert.assertEquals(QueryUnsupportedException.STATUS_CODE, response.getStatus());
- QueryException ex = JSON_MAPPER.readValue((byte[]) response.getEntity(), QueryException.class);
+ QueryException ex = jsonMapper.readValue((byte[]) response.getEntity(), QueryException.class);
Assert.assertEquals(errorMessage, ex.getMessage());
Assert.assertEquals(QueryUnsupportedException.ERROR_CODE, ex.getErrorCode());
}
@@ -603,8 +707,8 @@
authMapper,
Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of()))
),
- JSON_MAPPER,
- JSON_MAPPER,
+ jsonMapper,
+ smileMapper,
queryScheduler,
new AuthConfig(),
authMapper,
@@ -632,7 +736,7 @@
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
((StreamingOutput) response.getEntity()).write(baos);
- final List<Result<TimeBoundaryResultValue>> responses = JSON_MAPPER.readValue(
+ final List<Result<TimeBoundaryResultValue>> responses = jsonMapper.readValue(
baos.toByteArray(),
new TypeReference<List<Result<TimeBoundaryResultValue>>>() {}
);
@@ -679,8 +783,8 @@
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of()))
),
- JSON_MAPPER,
- JSON_MAPPER,
+ jsonMapper,
+ jsonMapper,
queryScheduler,
new AuthConfig(),
null,
@@ -697,7 +801,7 @@
Assert.assertEquals(QueryTimeoutException.STATUS_CODE, response.getStatus());
QueryTimeoutException ex;
try {
- ex = JSON_MAPPER.readValue((byte[]) response.getEntity(), QueryTimeoutException.class);
+ ex = jsonMapper.readValue((byte[]) response.getEntity(), QueryTimeoutException.class);
}
catch (IOException e) {
throw new RuntimeException(e);
@@ -777,8 +881,8 @@
authMapper,
Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of()))
),
- JSON_MAPPER,
- JSON_MAPPER,
+ jsonMapper,
+ smileMapper,
queryScheduler,
new AuthConfig(),
authMapper,
@@ -901,8 +1005,8 @@
authMapper,
Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of()))
),
- JSON_MAPPER,
- JSON_MAPPER,
+ jsonMapper,
+ smileMapper,
queryScheduler,
new AuthConfig(),
authMapper,
@@ -995,7 +1099,7 @@
Assert.assertEquals(QueryCapacityExceededException.STATUS_CODE, response.getStatus());
QueryCapacityExceededException ex;
try {
- ex = JSON_MAPPER.readValue((byte[]) response.getEntity(), QueryCapacityExceededException.class);
+ ex = jsonMapper.readValue((byte[]) response.getEntity(), QueryCapacityExceededException.class);
}
catch (IOException e) {
throw new RuntimeException(e);
@@ -1036,7 +1140,7 @@
Assert.assertEquals(QueryCapacityExceededException.STATUS_CODE, response.getStatus());
QueryCapacityExceededException ex;
try {
- ex = JSON_MAPPER.readValue((byte[]) response.getEntity(), QueryCapacityExceededException.class);
+ ex = jsonMapper.readValue((byte[]) response.getEntity(), QueryCapacityExceededException.class);
}
catch (IOException e) {
throw new RuntimeException(e);
@@ -1088,7 +1192,7 @@
Assert.assertEquals(QueryCapacityExceededException.STATUS_CODE, response.getStatus());
QueryCapacityExceededException ex;
try {
- ex = JSON_MAPPER.readValue((byte[]) response.getEntity(), QueryCapacityExceededException.class);
+ ex = jsonMapper.readValue((byte[]) response.getEntity(), QueryCapacityExceededException.class);
}
catch (IOException e) {
throw new RuntimeException(e);
@@ -1160,8 +1264,8 @@
AuthTestUtils.TEST_AUTHORIZER_MAPPER,
Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of()))
),
- JSON_MAPPER,
- JSON_MAPPER,
+ jsonMapper,
+ smileMapper,
scheduler,
new AuthConfig(),
null,
