drcrallen closed pull request #4033: Query Response format to be based on http
'accept' header & Query Payload content type to be based on 'content-type'
header
URL: https://github.com/apache/incubator-druid/pull/4033
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 '<queryable_host>:<port>/druid/v2/?pretty' -H
'Content-Type:application/json' -d @<query_json_file>
+ curl -X POST '<queryable_host>:<port>/druid/v2/?pretty' -H
'Content-Type:application/json' -H 'Accept:application/json' -d
@<query_json_file>
```
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 '<queryable_host>:<port>/druid/v2/?pretty' -H
'Content-Type:application/json' -H 'Accept:x-jackson-smile' -d
@<query_json_file>
+ ```
+
+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.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 Response cancelQuery(@PathParam("id") String
queryId, @Context final Http
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 void write(OutputStream outputStream) throws
WebApplicationException
}
}
- 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 @@ private static String getPreviousEtag(final
HttpServletRequest req)
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.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 static void staticSetup()
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 void testGoodQuery() throws IOException
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 Access authorize(AuthenticationResult
authenticationResult, Resource reso
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 void run()
startAwaitLatch.await();
Executors.newSingleThreadExecutor().submit(
- new Runnable()
- {
+ new Runnable() {
@Override
public void run()
{
@@ -431,7 +544,7 @@ public void testDenySecuredCancelQuery() throws Exception
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);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
