From 7768937bc5fb8a64ad9135ba8f5ba0a9deb5f8d3 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 3 Jun 2025 10:20:31 +0800 Subject: [PATCH 1/7] Improve IT --- .../druid/tests/query/ITSqlQueryTest.java | 84 ++++++++----------- 1 file changed, 35 insertions(+), 49 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java b/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java index 46d6a938321e..98fd0a9cea15 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java @@ -46,7 +46,7 @@ /** * Test the SQL endpoint with different Content-Type */ -@Test(groups = {TestNGGroup.QUERY, TestNGGroup.CENTRALIZED_DATASOURCE_SCHEMA}) +@Test(groups = {TestNGGroup.QUERY}) @Guice(moduleFactory = DruidTestModuleFactory.class) public class ITSqlQueryTest { @@ -133,10 +133,10 @@ private void assertEquals(String expected, String actual) } } - private void assertEquals(int expected, int actual) + private void assertEquals(int expected, int actual, String message) { if (expected != actual) { - throw new ISE("Expected [%d] but got [%d]", expected, actual); + throw new ISE("Expected [%d] but got [%d]: %s", expected, actual, message); } } @@ -159,13 +159,11 @@ public void testNullContentType() { executeQuery( null, - (request) -> { - request.setEntity(new StringEntity("select 1")); - }, + (request) -> request.setEntity(new StringEntity("select 1")), (statusCode, responseEntity) -> { - assertEquals(HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE, statusCode, responseBody); assertStringCompare("Unsupported Content-Type:", responseBody, responseBody::contains); } ); @@ -176,13 +174,11 @@ public void testUnsupportedContentType() { executeQuery( "application/xml", - (request) -> { - request.setEntity(new StringEntity("select 1")); - }, + (request) -> request.setEntity(new StringEntity("select 1")), (statusCode, responseEntity) -> { - assertEquals(HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE, statusCode, responseBody); assertStringCompare("Unsupported Content-Type:", responseBody, responseBody::contains); } ); @@ -193,13 +189,11 @@ public void testTextPlain() { executeQuery( MediaType.TEXT_PLAIN, - (request) -> { - request.setEntity(new StringEntity("select \n1")); - }, + (request) -> request.setEntity(new StringEntity("select \n1")), (statusCode, responseEntity) -> { - assertEquals(200, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":1}]", responseBody); } ); @@ -210,13 +204,11 @@ public void testFormURLEncoded() { executeQuery( MediaType.APPLICATION_FORM_URLENCODED, - (request) -> { - request.setEntity(new StringEntity(URLEncoder.encode("select 'x % y'", StandardCharsets.UTF_8))); - }, + (request) -> request.setEntity(new StringEntity(URLEncoder.encode("select 'x % y'", StandardCharsets.UTF_8))), (statusCode, responseEntity) -> { - assertEquals(200, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":\"x % y\"}]", responseBody); } ); @@ -227,13 +219,11 @@ public void testFormURLEncoded_InvalidEncoding() { executeQuery( MediaType.APPLICATION_FORM_URLENCODED, - (request) -> { - request.setEntity(new StringEntity("select 'x % y'")); - }, + (request) -> request.setEntity(new StringEntity("select 'x % y'")), (statusCode, responseEntity) -> { - assertEquals(400, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(400, statusCode, responseBody); assertStringCompare("Unable to decoded", responseBody, responseBody::contains); } ); @@ -244,13 +234,11 @@ public void testJSON() { executeQuery( MediaType.APPLICATION_JSON, - (request) -> { - request.setEntity(new StringEntity(StringUtils.format("{\"query\":\"select 567\"}"))); - }, + (request) -> request.setEntity(new StringEntity(StringUtils.format("{\"query\":\"select 567\"}"))), (statusCode, responseEntity) -> { - assertEquals(200, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":567}]", responseBody); } ); @@ -261,13 +249,11 @@ public void testInvalidJSONFormat() { executeQuery( MediaType.APPLICATION_JSON, - (request) -> { - request.setEntity(new StringEntity(StringUtils.format("{\"query\":select 567}"))); - }, + (request) -> request.setEntity(new StringEntity(StringUtils.format("{\"query\":select 567}"))), (statusCode, responseEntity) -> { - assertEquals(400, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(400, statusCode, responseBody); assertStringCompare("Malformed SQL query", responseBody, responseBody::contains); } ); @@ -282,9 +268,9 @@ public void testEmptyQuery_TextPlain() // Empty query, DO NOTHING }, (statusCode, responseEntity) -> { - assertEquals(400, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } ); @@ -299,9 +285,9 @@ public void testEmptyQuery_UrlEncoded() // Empty query, DO NOTHING }, (statusCode, responseEntity) -> { - assertEquals(400, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } ); @@ -313,13 +299,13 @@ public void testBlankQuery_TextPlain() executeQuery( MediaType.TEXT_PLAIN, (request) -> { - // an query with blank characters + // a query with blank characters request.setEntity(new StringEntity(" ")); }, (statusCode, responseEntity) -> { - assertEquals(400, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } ); @@ -334,9 +320,9 @@ public void testEmptyQuery_JSON() // Empty query, DO NOTHING }, (statusCode, responseEntity) -> { - assertEquals(400, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } ); @@ -356,9 +342,9 @@ public void testMultipleContentType() request.setEntity(new StringEntity(StringUtils.format("SELECT 1"))); }, (statusCode, responseEntity) -> { - assertEquals(200, statusCode); - String responseBody = EntityUtils.toString(responseEntity).trim(); + + assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":1}]", responseBody); } ); From ce443c5e7846431e90c1692240bf26287804700d Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 3 Jun 2025 18:51:54 +0800 Subject: [PATCH 2/7] Use TestClient --- .../druid/tests/query/ITSqlQueryTest.java | 162 ++++++++---------- .../org/apache/druid/sql/http/SqlQuery.java | 2 +- 2 files changed, 76 insertions(+), 88 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java b/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java index 98fd0a9cea15..62dd7ebabc9b 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java @@ -23,22 +23,22 @@ import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.logger.Logger; +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.StatusResponseHandler; +import org.apache.druid.java.util.http.client.response.StatusResponseHolder; import org.apache.druid.testing.IntegrationTestingConfig; import org.apache.druid.testing.guice.DruidTestModuleFactory; +import org.apache.druid.testing.guice.TestClient; import org.apache.druid.tests.TestNGGroup; -import org.apache.http.HttpEntity; -import org.apache.http.HttpStatus; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.entity.StringEntity; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.util.EntityUtils; +import org.jboss.netty.handler.codec.http.HttpMethod; +import org.jboss.netty.handler.codec.http.HttpResponseStatus; import org.testng.annotations.Guice; import org.testng.annotations.Test; import javax.ws.rs.core.MediaType; import java.io.IOException; +import java.net.URL; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.function.Function; @@ -55,19 +55,23 @@ public class ITSqlQueryTest @Inject IntegrationTestingConfig config; + @Inject + @TestClient + HttpClient httpClient; + interface IExecutable { - void execute(String endpoint) throws IOException; + void execute(String endpoint) throws Exception; } interface OnRequest { - void on(HttpPost request) throws IOException; + void on(Request request) throws IOException; } interface OnResponse { - void on(int statusCode, HttpEntity response) throws IOException; + void on(int statusCode, String response) throws IOException; } private void executeWithRetry(String endpoint, String contentType, IExecutable executable) @@ -79,7 +83,7 @@ private void executeWithRetry(String endpoint, String contentType, IExecutable e executable.execute(endpoint); return; } - catch (IOException e) { + catch (Exception e) { // Only catch IOException lastException = e; } @@ -95,28 +99,34 @@ private void executeWithRetry(String endpoint, String contentType, IExecutable e private void executeQuery( String contentType, + String query, OnRequest onRequest, OnResponse onResponse ) { IExecutable executable = (endpoint) -> { - try (CloseableHttpClient client = HttpClientBuilder.create().build()) { - HttpPost request = new HttpPost(endpoint); - if (contentType != null) { - request.addHeader("Content-Type", contentType); - } + Request request = new Request(HttpMethod.POST, new URL(endpoint)); + if (contentType != null) { + request.addHeader("Content-Type", contentType); + } + + if (query != null) { + request.setContent(query.getBytes(StandardCharsets.UTF_8)); + } + + if (onRequest != null) { onRequest.on(request); - - try (CloseableHttpResponse response = client.execute(request)) { - HttpEntity responseEntity = response.getEntity(); - assertNotNull(responseEntity); - - onResponse.on( - response.getStatusLine().getStatusCode(), - responseEntity - ); - } } + + StatusResponseHolder response = httpClient.go(request, StatusResponseHandler.getInstance()) + .get(); + + assertNotNull(response); + + onResponse.on( + response.getStatus().getCode(), + response.getContent().trim() + ); }; // Send query to broker to exeucte @@ -159,11 +169,10 @@ public void testNullContentType() { executeQuery( null, - (request) -> request.setEntity(new StringEntity("select 1")), - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - - assertEquals(HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE, statusCode, responseBody); + "select 1", + (request) -> {}, + (statusCode, responseBody) -> { + assertEquals(HttpResponseStatus.UNSUPPORTED_MEDIA_TYPE.getCode(), statusCode, responseBody); assertStringCompare("Unsupported Content-Type:", responseBody, responseBody::contains); } ); @@ -174,11 +183,10 @@ public void testUnsupportedContentType() { executeQuery( "application/xml", - (request) -> request.setEntity(new StringEntity("select 1")), - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - - assertEquals(HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE, statusCode, responseBody); + "select 1", + (request) -> {}, + (statusCode, responseBody) -> { + assertEquals(HttpResponseStatus.UNSUPPORTED_MEDIA_TYPE.getCode(), statusCode, responseBody); assertStringCompare("Unsupported Content-Type:", responseBody, responseBody::contains); } ); @@ -189,10 +197,9 @@ public void testTextPlain() { executeQuery( MediaType.TEXT_PLAIN, - (request) -> request.setEntity(new StringEntity("select \n1")), - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + "select \n1", + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":1}]", responseBody); } @@ -204,10 +211,9 @@ public void testFormURLEncoded() { executeQuery( MediaType.APPLICATION_FORM_URLENCODED, - (request) -> request.setEntity(new StringEntity(URLEncoder.encode("select 'x % y'", StandardCharsets.UTF_8))), - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + URLEncoder.encode("select 'x % y'", StandardCharsets.UTF_8), + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":\"x % y\"}]", responseBody); } @@ -219,12 +225,11 @@ public void testFormURLEncoded_InvalidEncoding() { executeQuery( MediaType.APPLICATION_FORM_URLENCODED, - (request) -> request.setEntity(new StringEntity("select 'x % y'")), - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + "select 'x % y'", + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); - assertStringCompare("Unable to decoded", responseBody, responseBody::contains); + assertStringCompare("Unable to decode", responseBody, responseBody::contains); } ); } @@ -234,10 +239,9 @@ public void testJSON() { executeQuery( MediaType.APPLICATION_JSON, - (request) -> request.setEntity(new StringEntity(StringUtils.format("{\"query\":\"select 567\"}"))), - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + "{\"query\":\"select 567\"}", + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":567}]", responseBody); } @@ -249,10 +253,9 @@ public void testInvalidJSONFormat() { executeQuery( MediaType.APPLICATION_JSON, - (request) -> request.setEntity(new StringEntity(StringUtils.format("{\"query\":select 567}"))), - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + "{\"query\":select 567}", + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Malformed SQL query", responseBody, responseBody::contains); } @@ -264,12 +267,9 @@ public void testEmptyQuery_TextPlain() { executeQuery( MediaType.TEXT_PLAIN, - (request) -> { - // Empty query, DO NOTHING - }, - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + null, + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } @@ -281,12 +281,9 @@ public void testEmptyQuery_UrlEncoded() { executeQuery( MediaType.APPLICATION_FORM_URLENCODED, - (request) -> { - // Empty query, DO NOTHING - }, - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + null, + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } @@ -298,13 +295,9 @@ public void testBlankQuery_TextPlain() { executeQuery( MediaType.TEXT_PLAIN, - (request) -> { - // a query with blank characters - request.setEntity(new StringEntity(" ")); - }, - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + " ", + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } @@ -316,12 +309,9 @@ public void testEmptyQuery_JSON() { executeQuery( MediaType.APPLICATION_JSON, - (request) -> { - // Empty query, DO NOTHING - }, - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + null, + (request) -> {}, + (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } @@ -336,14 +326,12 @@ public void testMultipleContentType() { executeQuery( MediaType.TEXT_PLAIN, + "SELECT 1", (request) -> { // Add one more Content-Type header request.addHeader("Content-Type", MediaType.APPLICATION_JSON); - request.setEntity(new StringEntity(StringUtils.format("SELECT 1"))); }, - (statusCode, responseEntity) -> { - String responseBody = EntityUtils.toString(responseEntity).trim(); - + (statusCode, responseBody) -> { assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":1}]", responseBody); } diff --git a/sql/src/main/java/org/apache/druid/sql/http/SqlQuery.java b/sql/src/main/java/org/apache/druid/sql/http/SqlQuery.java index 71f9c14563ce..022a3c4ed54f 100644 --- a/sql/src/main/java/org/apache/druid/sql/http/SqlQuery.java +++ b/sql/src/main/java/org/apache/druid/sql/http/SqlQuery.java @@ -308,7 +308,7 @@ private static SqlQuery from( catch (IllegalArgumentException e) { throw new HttpException( Response.Status.BAD_REQUEST, - "Unable to decoded URL-Encoded SQL query: " + e.getMessage() + "Unable to decode URL-Encoded SQL query: " + e.getMessage() ); } From 73875a683283eb5ccd7e32ec61b32b369a50a551 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 3 Jun 2025 18:53:43 +0800 Subject: [PATCH 3/7] Remove dependency --- integration-tests/pom.xml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml index 60cbe4edcb4d..f54e88c93044 100644 --- a/integration-tests/pom.xml +++ b/integration-tests/pom.xml @@ -418,14 +418,6 @@ com.google.protobuf protobuf-java - - org.apache.httpcomponents - httpcore - - - org.apache.httpcomponents - httpclient - From 6919094e436691d196c320283ebd6041d4ad3745 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 3 Jun 2025 21:28:14 +0800 Subject: [PATCH 4/7] address comments --- .../druid/tests/query/ITSqlQueryTest.java | 78 +++++++++---------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java b/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java index 62dd7ebabc9b..62b55ae81f42 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java @@ -22,7 +22,6 @@ import com.google.inject.Inject; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.java.util.common.logger.Logger; 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.StatusResponseHandler; @@ -30,6 +29,7 @@ import org.apache.druid.testing.IntegrationTestingConfig; import org.apache.druid.testing.guice.DruidTestModuleFactory; import org.apache.druid.testing.guice.TestClient; +import org.apache.druid.testing.utils.ITRetryUtil; import org.apache.druid.tests.TestNGGroup; import org.jboss.netty.handler.codec.http.HttpMethod; import org.jboss.netty.handler.codec.http.HttpResponseStatus; @@ -50,8 +50,6 @@ @Guice(moduleFactory = DruidTestModuleFactory.class) public class ITSqlQueryTest { - private static final Logger LOG = new Logger(ITSqlQueryTest.class); - @Inject IntegrationTestingConfig config; @@ -76,25 +74,16 @@ interface OnResponse private void executeWithRetry(String endpoint, String contentType, IExecutable executable) { - Throwable lastException = null; - for (int i = 1; i <= 5; i++) { - LOG.info("Query to %s with Content-Type = %s, tries = %s", endpoint, contentType, i); - try { - executable.execute(endpoint); - return; - } - catch (Exception e) { - // Only catch IOException - lastException = e; - } - try { - Thread.sleep(200); - } - catch (InterruptedException ignored) { - break; - } - } - throw new ISE(contentType + " failed after 5 tries, last exception: " + lastException); + // Retry 5 times with 200 ms delay + ITRetryUtil.retryUntilEquals( + () -> { + executable.execute(endpoint); + return true; + }, true, + 200, + 5, + StringUtils.format("Query to %s with Content-Type = %s", endpoint, contentType) + ); } private void executeQuery( @@ -109,20 +98,20 @@ private void executeQuery( if (contentType != null) { request.addHeader("Content-Type", contentType); } - + if (query != null) { request.setContent(query.getBytes(StandardCharsets.UTF_8)); } - + if (onRequest != null) { onRequest.on(request); } StatusResponseHolder response = httpClient.go(request, StatusResponseHandler.getInstance()) .get(); - + assertNotNull(response); - + onResponse.on( response.getStatus().getCode(), response.getContent().trim() @@ -132,7 +121,7 @@ private void executeQuery( // Send query to broker to exeucte executeWithRetry(StringUtils.format("%s/druid/v2/sql/", config.getBrokerUrl()), contentType, executable); - // Send query to router to execute + // Send a query to router to execute executeWithRetry(StringUtils.format("%s/druid/v2/sql/", config.getRouterUrl()), contentType, executable); } @@ -170,7 +159,8 @@ public void testNullContentType() executeQuery( null, "select 1", - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(HttpResponseStatus.UNSUPPORTED_MEDIA_TYPE.getCode(), statusCode, responseBody); assertStringCompare("Unsupported Content-Type:", responseBody, responseBody::contains); @@ -184,7 +174,8 @@ public void testUnsupportedContentType() executeQuery( "application/xml", "select 1", - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(HttpResponseStatus.UNSUPPORTED_MEDIA_TYPE.getCode(), statusCode, responseBody); assertStringCompare("Unsupported Content-Type:", responseBody, responseBody::contains); @@ -198,7 +189,8 @@ public void testTextPlain() executeQuery( MediaType.TEXT_PLAIN, "select \n1", - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":1}]", responseBody); @@ -212,7 +204,8 @@ public void testFormURLEncoded() executeQuery( MediaType.APPLICATION_FORM_URLENCODED, URLEncoder.encode("select 'x % y'", StandardCharsets.UTF_8), - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":\"x % y\"}]", responseBody); @@ -226,7 +219,8 @@ public void testFormURLEncoded_InvalidEncoding() executeQuery( MediaType.APPLICATION_FORM_URLENCODED, "select 'x % y'", - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Unable to decode", responseBody, responseBody::contains); @@ -240,7 +234,8 @@ public void testJSON() executeQuery( MediaType.APPLICATION_JSON, "{\"query\":\"select 567\"}", - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(200, statusCode, responseBody); assertEquals("[{\"EXPR$0\":567}]", responseBody); @@ -254,7 +249,8 @@ public void testInvalidJSONFormat() executeQuery( MediaType.APPLICATION_JSON, "{\"query\":select 567}", - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Malformed SQL query", responseBody, responseBody::contains); @@ -268,7 +264,8 @@ public void testEmptyQuery_TextPlain() executeQuery( MediaType.TEXT_PLAIN, null, - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); @@ -282,7 +279,8 @@ public void testEmptyQuery_UrlEncoded() executeQuery( MediaType.APPLICATION_FORM_URLENCODED, null, - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); @@ -296,7 +294,8 @@ public void testBlankQuery_TextPlain() executeQuery( MediaType.TEXT_PLAIN, " ", - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); @@ -310,7 +309,8 @@ public void testEmptyQuery_JSON() executeQuery( MediaType.APPLICATION_JSON, null, - (request) -> {}, + (request) -> { + }, (statusCode, responseBody) -> { assertEquals(400, statusCode, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); @@ -319,7 +319,7 @@ public void testEmptyQuery_JSON() } /** - * When multiple Content-Type headers are set, the first one(in this case it's the text format) should be used. + * When multiple Content-Type headers are set, the first one (in this case, it's the text format) should be used. */ @Test public void testMultipleContentType() From c1e4742a4db09ad842ab4647bb1c34fb9817023f Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 3 Jun 2025 21:51:13 +0800 Subject: [PATCH 5/7] Clean assert --- .../druid/tests/query/ITSqlQueryTest.java | 59 ++++++------------- 1 file changed, 19 insertions(+), 40 deletions(-) diff --git a/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java b/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java index 62b55ae81f42..05fca8cf3027 100644 --- a/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java +++ b/integration-tests/src/test/java/org/apache/druid/tests/query/ITSqlQueryTest.java @@ -33,6 +33,7 @@ import org.apache.druid.tests.TestNGGroup; import org.jboss.netty.handler.codec.http.HttpMethod; import org.jboss.netty.handler.codec.http.HttpResponseStatus; +import org.testng.Assert; import org.testng.annotations.Guice; import org.testng.annotations.Test; @@ -109,8 +110,7 @@ private void executeQuery( StatusResponseHolder response = httpClient.go(request, StatusResponseHandler.getInstance()) .get(); - - assertNotNull(response); + Assert.assertNotNull(response); onResponse.on( response.getStatus().getCode(), @@ -121,31 +121,10 @@ private void executeQuery( // Send query to broker to exeucte executeWithRetry(StringUtils.format("%s/druid/v2/sql/", config.getBrokerUrl()), contentType, executable); - // Send a query to router to execute + // Send query to router to execute executeWithRetry(StringUtils.format("%s/druid/v2/sql/", config.getRouterUrl()), contentType, executable); } - private void assertEquals(String expected, String actual) - { - if (!expected.equals(actual)) { - throw new ISE("Expected [%s] but got [%s]", expected, actual); - } - } - - private void assertEquals(int expected, int actual, String message) - { - if (expected != actual) { - throw new ISE("Expected [%d] but got [%d]: %s", expected, actual, message); - } - } - - private void assertNotNull(Object object) - { - if (object == null) { - throw new ISE("Expected not null"); - } - } - private void assertStringCompare(String expected, String actual, Function predicate) { if (!predicate.apply(expected)) { @@ -162,7 +141,7 @@ public void testNullContentType() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(HttpResponseStatus.UNSUPPORTED_MEDIA_TYPE.getCode(), statusCode, responseBody); + Assert.assertEquals(statusCode, HttpResponseStatus.UNSUPPORTED_MEDIA_TYPE.getCode(), responseBody); assertStringCompare("Unsupported Content-Type:", responseBody, responseBody::contains); } ); @@ -177,7 +156,7 @@ public void testUnsupportedContentType() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(HttpResponseStatus.UNSUPPORTED_MEDIA_TYPE.getCode(), statusCode, responseBody); + Assert.assertEquals(statusCode, HttpResponseStatus.UNSUPPORTED_MEDIA_TYPE.getCode(), responseBody); assertStringCompare("Unsupported Content-Type:", responseBody, responseBody::contains); } ); @@ -192,8 +171,8 @@ public void testTextPlain() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(200, statusCode, responseBody); - assertEquals("[{\"EXPR$0\":1}]", responseBody); + Assert.assertEquals(statusCode, 200, responseBody); + Assert.assertEquals(responseBody, "[{\"EXPR$0\":1}]"); } ); } @@ -207,8 +186,8 @@ public void testFormURLEncoded() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(200, statusCode, responseBody); - assertEquals("[{\"EXPR$0\":\"x % y\"}]", responseBody); + Assert.assertEquals(statusCode, 200, responseBody); + Assert.assertEquals(responseBody, "[{\"EXPR$0\":\"x % y\"}]"); } ); } @@ -222,7 +201,7 @@ public void testFormURLEncoded_InvalidEncoding() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(400, statusCode, responseBody); + Assert.assertEquals(statusCode, 400, responseBody); assertStringCompare("Unable to decode", responseBody, responseBody::contains); } ); @@ -237,8 +216,8 @@ public void testJSON() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(200, statusCode, responseBody); - assertEquals("[{\"EXPR$0\":567}]", responseBody); + Assert.assertEquals(statusCode, 200, responseBody); + Assert.assertEquals(responseBody, "[{\"EXPR$0\":567}]"); } ); } @@ -252,7 +231,7 @@ public void testInvalidJSONFormat() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(400, statusCode, responseBody); + Assert.assertEquals(statusCode, 400, responseBody); assertStringCompare("Malformed SQL query", responseBody, responseBody::contains); } ); @@ -267,7 +246,7 @@ public void testEmptyQuery_TextPlain() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(400, statusCode, responseBody); + Assert.assertEquals(statusCode, 400, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } ); @@ -282,7 +261,7 @@ public void testEmptyQuery_UrlEncoded() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(400, statusCode, responseBody); + Assert.assertEquals(statusCode, 400, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } ); @@ -297,7 +276,7 @@ public void testBlankQuery_TextPlain() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(400, statusCode, responseBody); + Assert.assertEquals(statusCode, 400, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } ); @@ -312,7 +291,7 @@ public void testEmptyQuery_JSON() (request) -> { }, (statusCode, responseBody) -> { - assertEquals(400, statusCode, responseBody); + Assert.assertEquals(statusCode, 400, responseBody); assertStringCompare("Empty query", responseBody, responseBody::contains); } ); @@ -332,8 +311,8 @@ public void testMultipleContentType() request.addHeader("Content-Type", MediaType.APPLICATION_JSON); }, (statusCode, responseBody) -> { - assertEquals(200, statusCode, responseBody); - assertEquals("[{\"EXPR$0\":1}]", responseBody); + Assert.assertEquals(statusCode, 200, responseBody); + Assert.assertEquals(responseBody, "[{\"EXPR$0\":1}]"); } ); } From eaf2e0866e408c6284270280a699d38b71b90499 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Wed, 4 Jun 2025 00:28:58 +0800 Subject: [PATCH 6/7] Fix IT --- .../org/apache/druid/server/AsyncQueryForwardingServlet.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/src/main/java/org/apache/druid/server/AsyncQueryForwardingServlet.java b/services/src/main/java/org/apache/druid/server/AsyncQueryForwardingServlet.java index 5b8e9c17f393..dba842ec6896 100644 --- a/services/src/main/java/org/apache/druid/server/AsyncQueryForwardingServlet.java +++ b/services/src/main/java/org/apache/druid/server/AsyncQueryForwardingServlet.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.smile.SmileFactory; import com.fasterxml.jackson.jaxrs.smile.SmileMediaTypes; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; @@ -484,7 +485,7 @@ private void setProxyRequestContent(Request proxyRequest, HttpServletRequest cli byte[] bytes = objectMapper.writeValueAsBytes(content); proxyRequest.content(new BytesContentProvider(bytes)); proxyRequest.getHeaders().put(HttpHeader.CONTENT_LENGTH, String.valueOf(bytes.length)); - proxyRequest.getHeaders().put(HttpHeader.CONTENT_TYPE, MediaType.APPLICATION_JSON); + proxyRequest.getHeaders().put(HttpHeader.CONTENT_TYPE, objectMapper.getFactory() instanceof SmileFactory ? SmileMediaTypes.APPLICATION_JACKSON_SMILE : MediaType.APPLICATION_JSON); } catch (JsonProcessingException e) { throw new RuntimeException(e); From b8a6718d353a02daea4e3c2525c7d8bf67ccc262 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Wed, 4 Jun 2025 09:53:39 +0800 Subject: [PATCH 7/7] Fix dependency check --- services/pom.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/pom.xml b/services/pom.xml index 570e48da1669..fad8933fb2c3 100644 --- a/services/pom.xml +++ b/services/pom.xml @@ -125,6 +125,10 @@ com.fasterxml.jackson.jaxrs jackson-jaxrs-smile-provider + + com.fasterxml.jackson.dataformat + jackson-dataformat-smile + com.opencsv opencsv