From 204901a602bfb151d473adef8586bc13a065d4d0 Mon Sep 17 00:00:00 2001 From: frank chen Date: Tue, 4 May 2021 13:43:47 +0800 Subject: [PATCH] Fix Smile encoding for HTTP response (#10980) * fix Smile encoding bug Signed-off-by: frank chen * Add unit tests * Add IT for smile encoding * Fix cases * Update javadoc Co-authored-by: Jihoon Son * resolve comments Co-authored-by: Jihoon Son --- .../AbstractQueryResourceTestClient.java | 131 +++++++++++--- .../clients/QueryResourceTestClient.java | 37 +++- .../clients/SqlResourceTestClient.java | 6 +- .../utils/AbstractTestQueryHelper.java | 31 +++- .../druid/testing/utils/TestQueryHelper.java | 39 +++++ .../tests/query/ITWikipediaQueryTest.java | 30 +++- .../druid/server/BrokerQueryResource.java | 9 +- .../apache/druid/server/QueryResource.java | 94 ++++++---- .../druid/server/QueryResourceTest.java | 162 ++++++++++++++---- 9 files changed, 435 insertions(+), 104 deletions(-) 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 15851c5dbdd..ce2703cc9f4 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 @@ package org.apache.druid.testing.clients; 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 { - 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 encoderDecoderMap; + + /** + * A encoder/decoder that encodes/decodes requests/responses based on Content-Type. + */ + interface EncoderDecoder + { + byte[] encode(Object content) throws IOException; + + List> 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> decode(byte[] content) throws IOException + { + return om.readValue(content, new TypeReference>>() + { + }); + } + } + + /** + * @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 abstract class AbstractQueryResourceTestClient public List> 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 @@ public abstract class AbstractQueryResourceTestClient "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>>() - { - } - ); + 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 abstract class AbstractQueryResourceTestClient public Future 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 1397fe16be5..33820fe6d70 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 @@ package org.apache.druid.testing.clients; 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 { @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 @@ public class QueryResourceTestClient extends AbstractQueryResourceTestClient { @@ -37,7 +39,9 @@ public class SqlResourceTestClient extends AbstractQueryResourceTestClient { @@ -38,10 +41,46 @@ public class TestQueryHelper extends AbstractTestQueryHelper 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 9fc52094d02..ee53ab5f787 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.apache.druid.tests.TestNGGroup; 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 @@ public class ITWikipediaQueryTest } } - @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 + *

+ * 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 9ef86af8733..b4ebc8c6b57 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 @@ public class BrokerQueryResource extends QueryResource @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 @@ public class BrokerQueryResource extends QueryResource ); } 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 6394395a1f0..b2f7f21e4da 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 @@ public class QueryResource implements QueryCountStatsProvider 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 @@ public class QueryResource implements QueryCountStatsProvider 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 @@ public class QueryResource implements QueryCountStatsProvider } } }, - ioReaderWriter.getContentType() + ioReaderWriter.getResponseWriter().getResponseType() ) .header("X-Druid-Query-Id", queryId); @@ -337,27 +331,27 @@ public class QueryResource implements QueryCountStatsProvider 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 @@ public class QueryResource implements QueryCountStatsProvider .addData("peer", req.getRemoteAddr()) .emit(); - return ioReaderWriter.gotError(e); + return ioReaderWriter.getResponseWriter().gotError(e); } finally { Thread.currentThread().setName(currThreadName); @@ -389,7 +383,7 @@ public class QueryResource implements QueryCountStatsProvider { 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 @@ public class QueryResource implements QueryCountStatsProvider 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 @@ public class QueryResource implements QueryCountStatsProvider 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 @@ public class QueryResource implements QueryCountStatsProvider 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 ce3c032b6b2..026af182208 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.ImmutableMap; 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 @@ import java.util.function.Consumer; 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 @@ public class QueryResourceTest false ); + private ObjectMapper jsonMapper; + private ObjectMapper smileMapper; private QueryResource queryResource; private QueryScheduler queryScheduler; private TestRequestLogger testRequestLogger; @@ -191,6 +196,10 @@ public class QueryResourceTest @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 @@ public class QueryResourceTest AuthTestUtils.TEST_AUTHORIZER_MAPPER, Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of())) ), - JSON_MAPPER, - JSON_MAPPER, + jsonMapper, + smileMapper, queryScheduler, new AuthConfig(), null, @@ -259,8 +268,8 @@ public class QueryResourceTest AuthTestUtils.TEST_AUTHORIZER_MAPPER, Suppliers.ofInstance(overrideConfig) ), - JSON_MAPPER, - JSON_MAPPER, + jsonMapper, + smileMapper, queryScheduler, new AuthConfig(), null, @@ -279,7 +288,7 @@ public class QueryResourceTest final ByteArrayOutputStream baos = new ByteArrayOutputStream(); ((StreamingOutput) response.getEntity()).write(baos); - final List> responses = JSON_MAPPER.readValue( + final List> responses = jsonMapper.readValue( baos.toByteArray(), new TypeReference>>() {} ); @@ -311,8 +320,8 @@ public class QueryResourceTest AuthTestUtils.TEST_AUTHORIZER_MAPPER, Suppliers.ofInstance(overrideConfig) ), - JSON_MAPPER, - JSON_MAPPER, + jsonMapper, + smileMapper, queryScheduler, new AuthConfig(), null, @@ -332,7 +341,7 @@ public class QueryResourceTest final ByteArrayOutputStream baos = new ByteArrayOutputStream(); ((StreamingOutput) response.getEntity()).write(baos); - final List> responses = JSON_MAPPER.readValue( + final List> responses = jsonMapper.readValue( baos.toByteArray(), new TypeReference>>() {} ); @@ -367,7 +376,7 @@ public class QueryResourceTest ).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 @@ public class QueryResourceTest } @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 @@ public class QueryResourceTest //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 @@ public class QueryResourceTest 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 @@ public class QueryResourceTest 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 @@ public class QueryResourceTest ); 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 @@ public class QueryResourceTest ); 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 @@ public class QueryResourceTest ); 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 @@ public class QueryResourceTest authMapper, Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of())) ), - JSON_MAPPER, - JSON_MAPPER, + jsonMapper, + smileMapper, queryScheduler, new AuthConfig(), authMapper, @@ -632,7 +736,7 @@ public class QueryResourceTest final ByteArrayOutputStream baos = new ByteArrayOutputStream(); ((StreamingOutput) response.getEntity()).write(baos); - final List> responses = JSON_MAPPER.readValue( + final List> responses = jsonMapper.readValue( baos.toByteArray(), new TypeReference>>() {} ); @@ -679,8 +783,8 @@ public class QueryResourceTest AuthTestUtils.TEST_AUTHORIZER_MAPPER, Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of())) ), - JSON_MAPPER, - JSON_MAPPER, + jsonMapper, + jsonMapper, queryScheduler, new AuthConfig(), null, @@ -697,7 +801,7 @@ public class QueryResourceTest 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 @@ public class QueryResourceTest authMapper, Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of())) ), - JSON_MAPPER, - JSON_MAPPER, + jsonMapper, + smileMapper, queryScheduler, new AuthConfig(), authMapper, @@ -901,8 +1005,8 @@ public class QueryResourceTest authMapper, Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of())) ), - JSON_MAPPER, - JSON_MAPPER, + jsonMapper, + smileMapper, queryScheduler, new AuthConfig(), authMapper, @@ -995,7 +1099,7 @@ public class QueryResourceTest 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 @@ public class QueryResourceTest 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 @@ public class QueryResourceTest 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 @@ public class QueryResourceTest AuthTestUtils.TEST_AUTHORIZER_MAPPER, Suppliers.ofInstance(new DefaultQueryConfig(ImmutableMap.of())) ), - JSON_MAPPER, - JSON_MAPPER, + jsonMapper, + smileMapper, scheduler, new AuthConfig(), null,