From 6567fff9e7024883ac548abc920f051bdeddaefb Mon Sep 17 00:00:00 2001 From: vishnu rao Date: Sat, 13 Oct 2018 05:29:14 +0800 Subject: [PATCH] Query Response format to be based on http 'accept' header & Query Payload content type to be based on 'content-type' header (#4033) * o- Query Response format to be based on http 'accept' header & Query Payload contenty type to be based on 'content-type' header * o- Query Response format to be based on http 'accept' header & Query Payload contenty type to be based on 'content-type' header o- if Accept header is absent, it defaults to Content-Type header * Feature: Query Response format to be based on http 'accept' header & Query Payload content type to be based on 'content-type' PR #4033 Minor change to a comment - restoring to previous wording * Feature: Query Response format to be based on http 'accept' header & Query Payload content type to be based on 'content-type' PR #4033 o- minor change to check for empty string --- docs/content/querying/querying.md | 10 +- .../apache/druid/server/QueryResource.java | 23 +++- .../druid/server/QueryResourceTest.java | 121 +++++++++++++++++- 3 files changed, 145 insertions(+), 9 deletions(-) diff --git a/docs/content/querying/querying.md b/docs/content/querying/querying.md index 549493e9481..a73939c716b 100644 --- a/docs/content/querying/querying.md +++ b/docs/content/querying/querying.md @@ -11,12 +11,20 @@ REST query interface. For normal Druid operations, queries should be issued to t to the queryable nodes like this - ```bash - curl -X POST ':/druid/v2/?pretty' -H 'Content-Type:application/json' -d @ + curl -X POST ':/druid/v2/?pretty' -H 'Content-Type:application/json' -H 'Accept:application/json' -d @ ``` Druid's native query language is JSON over HTTP, although many members of the community have contributed different [client libraries](../development/libraries.html) in other languages to query Druid. +The Content-Type/Accept Headers can also take 'application/x-jackson-smile'. + + ```bash + curl -X POST ':/druid/v2/?pretty' -H 'Content-Type:application/json' -H 'Accept:x-jackson-smile' -d @ + ``` + +Note: If Accept header is not provided, it defaults to value of 'Content-Type' header. + Druid's native query is relatively low level, mapping closely to how computations are performed internally. Druid queries are designed to be lightweight and complete very quickly. This means that for more complex analysis, or to build more complex visualizations, multiple Druid queries may be required. 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 70fabd21af9..12416e40ba0 100644 --- a/server/src/main/java/org/apache/druid/server/QueryResource.java +++ b/server/src/main/java/org/apache/druid/server/QueryResource.java @@ -24,6 +24,8 @@ import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.datatype.joda.ser.DateTimeSerializer; import com.fasterxml.jackson.jaxrs.smile.SmileMediaTypes; + +import com.google.common.base.Strings; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; @@ -156,13 +158,19 @@ public class QueryResource implements QueryCountStatsProvider public Response doPost( final InputStream in, @QueryParam("pretty") final String pretty, - @Context final HttpServletRequest req // used to get request content-type, remote address and auth-related headers + @Context final HttpServletRequest req // used to get request content-type,Accept header, remote address and auth-related headers ) throws IOException { final QueryLifecycle queryLifecycle = queryLifecycleFactory.factorize(); Query query = null; - final ResponseContext context = createContext(req.getContentType(), pretty != null); + String acceptHeader = req.getHeader("Accept"); + if (Strings.isNullOrEmpty(acceptHeader)) { + //default to content-type + acceptHeader = req.getContentType(); + } + + final ResponseContext context = createContext(acceptHeader, pretty != null); final String currThreadName = Thread.currentThread().getName(); try { @@ -294,13 +302,13 @@ public class QueryResource implements QueryCountStatsProvider } } - private static Query readQuery( + private Query readQuery( final HttpServletRequest req, final InputStream in, final ResponseContext context ) throws IOException { - Query baseQuery = context.getObjectMapper().readValue(in, Query.class); + Query baseQuery = getMapperForRequest(req.getContentType()).readValue(in, Query.class); String prevEtag = getPreviousEtag(req); if (prevEtag != null) { @@ -317,6 +325,13 @@ public class QueryResource implements QueryCountStatsProvider return req.getHeader(HEADER_IF_NONE_MATCH); } + protected ObjectMapper getMapperForRequest(String requestContentType) + { + boolean isSmile = SmileMediaTypes.APPLICATION_JACKSON_SMILE.equals(requestContentType) || + APPLICATION_SMILE.equals(requestContentType); + return isSmile ? smileMapper : jsonMapper; + } + protected ObjectMapper serializeDataTimeAsLong(ObjectMapper mapper) { return mapper.copy().registerModule(new SimpleModule().addSerializer(DateTime.class, new DateTimeSerializer())); 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 9e48793f247..bcf557b029a 100644 --- a/server/src/test/java/org/apache/druid/server/QueryResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/QueryResourceTest.java @@ -19,6 +19,7 @@ package org.apache.druid.server; +import com.fasterxml.jackson.jaxrs.smile.SmileMediaTypes; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Throwables; @@ -52,6 +53,7 @@ import org.apache.druid.server.security.Authorizer; import org.apache.druid.server.security.AuthorizerMapper; import org.apache.druid.server.security.ForbiddenException; import org.apache.druid.server.security.Resource; +import org.apache.http.HttpStatus; import org.easymock.EasyMock; import org.joda.time.Interval; import org.junit.After; @@ -125,6 +127,7 @@ public class QueryResourceTest public void setup() { 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(); EasyMock.expect(testServletRequest.getRemoteAddr()).andReturn("localhost").anyTimes(); queryManager = new QueryManager(); @@ -187,6 +190,116 @@ public class QueryResourceTest Assert.assertNotNull(response); } + @Test + public void testGoodQueryWithNullAcceptHeader() throws IOException + { + final String acceptHeader = null; + final String contentTypeHeader = MediaType.APPLICATION_JSON; + EasyMock.reset(testServletRequest); + + EasyMock.expect(testServletRequest.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)) + .andReturn(null) + .anyTimes(); + EasyMock.expect(testServletRequest.getAttribute(AuthConfig.DRUID_ALLOW_UNSECURED_PATH)).andReturn(null).anyTimes(); + + EasyMock.expect(testServletRequest.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)) + .andReturn(authenticationResult) + .anyTimes(); + + testServletRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true); + + EasyMock.expect(testServletRequest.getHeader("Accept")).andReturn(acceptHeader).anyTimes(); + EasyMock.expect(testServletRequest.getContentType()).andReturn(contentTypeHeader).anyTimes(); + EasyMock.expect(testServletRequest.getHeader(QueryResource.HEADER_IF_NONE_MATCH)).andReturn(null).anyTimes(); + EasyMock.expect(testServletRequest.getRemoteAddr()).andReturn("localhost").anyTimes(); + + EasyMock.replay(testServletRequest); + Response response = queryResource.doPost( + new ByteArrayInputStream(simpleTimeSeriesQuery.getBytes("UTF-8")), + null /*pretty*/, + testServletRequest + ); + Assert.assertEquals(HttpStatus.SC_OK, response.getStatus()); + //since accept header is null, the response content type should be same as the value of 'Content-Type' header + Assert.assertEquals(contentTypeHeader, (response.getMetadata().get("Content-Type").get(0)).toString()); + Assert.assertNotNull(response); + } + + @Test + public void testGoodQueryWithEmptyAcceptHeader() throws IOException + { + final String acceptHeader = ""; + final String contentTypeHeader = MediaType.APPLICATION_JSON; + EasyMock.reset(testServletRequest); + + EasyMock.expect(testServletRequest.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)) + .andReturn(null) + .anyTimes(); + EasyMock.expect(testServletRequest.getAttribute(AuthConfig.DRUID_ALLOW_UNSECURED_PATH)).andReturn(null).anyTimes(); + + EasyMock.expect(testServletRequest.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)) + .andReturn(authenticationResult) + .anyTimes(); + + testServletRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true); + + EasyMock.expect(testServletRequest.getHeader("Accept")).andReturn(acceptHeader).anyTimes(); + EasyMock.expect(testServletRequest.getContentType()).andReturn(contentTypeHeader).anyTimes(); + EasyMock.expect(testServletRequest.getHeader(QueryResource.HEADER_IF_NONE_MATCH)).andReturn(null).anyTimes(); + EasyMock.expect(testServletRequest.getRemoteAddr()).andReturn("localhost").anyTimes(); + + EasyMock.replay(testServletRequest); + Response response = queryResource.doPost( + new ByteArrayInputStream(simpleTimeSeriesQuery.getBytes("UTF-8")), + null /*pretty*/, + testServletRequest + ); + Assert.assertEquals(HttpStatus.SC_OK, response.getStatus()); + //since accept header is empty, the response content type should be same as the value of 'Content-Type' header + Assert.assertEquals(contentTypeHeader, (response.getMetadata().get("Content-Type").get(0)).toString()); + Assert.assertNotNull(response); + } + + @Test + public void testGoodQueryWithSmileAcceptHeader() 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); + EasyMock.expect(smileRequest.getContentType()).andReturn(MediaType.APPLICATION_JSON).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(authenticationResult) + .anyTimes(); + + smileRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true); + + 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( + new ByteArrayInputStream(simpleTimeSeriesQuery.getBytes("UTF-8")), + null /*pretty*/, + smileRequest + ); + Assert.assertEquals(HttpStatus.SC_OK, response.getStatus()); + Assert.assertEquals(SmileMediaTypes.APPLICATION_JACKSON_SMILE, (response.getMetadata().get("Content-Type").get(0)).toString()); + Assert.assertNotNull(response); + EasyMock.verify(smileRequest); + } + + @Test public void testBadQuery() throws IOException { @@ -287,7 +400,8 @@ public class QueryResourceTest Assert.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); Assert.assertEquals(0, responses.size()); Assert.assertEquals(1, testRequestLogger.getLogs().size()); - Assert.assertEquals(true, testRequestLogger.getLogs().get(0).getQueryStats().getStats().get("success")); + Assert.assertEquals(true, + testRequestLogger.getLogs().get(0).getQueryStats().getStats().get("success")); Assert.assertEquals("druid", testRequestLogger.getLogs().get(0).getQueryStats().getStats().get("identity")); } @@ -401,8 +515,7 @@ public class QueryResourceTest startAwaitLatch.await(); Executors.newSingleThreadExecutor().submit( - new Runnable() - { + new Runnable() { @Override public void run() { @@ -431,7 +544,7 @@ public class QueryResourceTest EasyMock.expect(testServletRequest.getAttribute(AuthConfig.DRUID_ALLOW_UNSECURED_PATH)).andReturn(null).anyTimes(); EasyMock.expect(testServletRequest.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)) - .andReturn(authenticationResult) + .andReturn(authenticationResult) .anyTimes(); testServletRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);