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

@@ -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.

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()));

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()
           {