From 5ce1babb5a84ec2532115e7115d84d3cc619a1cb Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 3 Oct 2023 17:37:40 -0400 Subject: [PATCH] Fix response codes returned by JSON formatting them (#2156) * Glue datasource support (#2055) Signed-off-by: Vamsi Manohar * Initial commit of new job APIs (#2050) Signed-off-by: Vamsi Manohar * Create Job API (#2070) * Create Job API Signed-off-by: Vamsi Manohar * Refactor to Async Query API Signed-off-by: Vamsi Manohar --------- Signed-off-by: Vamsi Manohar * Cancel Job API (#2126) Signed-off-by: Vamsi Manohar * Fix response codes returned Signed-off-by: Derek Ho * Remove @opensearch datasource, update with new type Signed-off-by: Derek Ho * Spotless Apply Signed-off-by: Derek Ho * Fix tests Signed-off-by: Derek Ho * Revert change back to json string Signed-off-by: Derek Ho * Update tests to use JSON string literal instead of formatting Signed-off-by: Derek Ho * Update IT Signed-off-by: Derek Ho * Fix show datsources IT Signed-off-by: Derek Ho * Remove files from merge Signed-off-by: Derek Ho --------- Signed-off-by: Vamsi Manohar Signed-off-by: Derek Ho Co-authored-by: Vamsi Manohar --- .../physical/datasource/DataSourceTableScan.java | 2 +- .../datasource/DataSourceTableScanTest.java | 2 +- .../transport/TransportCreateDataSourceAction.java | 14 +++++++++++--- .../transport/TransportUpdateDataSourceAction.java | 14 +++++++++++--- .../TransportCreateDataSourceActionTest.java | 4 ++-- .../TransportUpdateDataSourceActionTest.java | 5 +++-- .../sql/datasource/DataSourceAPIsIT.java | 4 ++-- .../sql/ppl/ShowDataSourcesCommandIT.java | 2 +- 8 files changed, 32 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java b/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java index bc92df7d16..89e21377dc 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScan.java @@ -45,7 +45,7 @@ public String explain() { @Override public void open() { List exprValues = new ArrayList<>(); - Set dataSourceMetadataSet = dataSourceService.getDataSourceMetadata(true); + Set dataSourceMetadataSet = dataSourceService.getDataSourceMetadata(false); for (DataSourceMetadata dataSourceMetadata : dataSourceMetadataSet) { exprValues.add( new ExprTupleValue( diff --git a/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java b/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java index 28851f2454..358a7b43b5 100644 --- a/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java +++ b/core/src/test/java/org/opensearch/sql/planner/physical/datasource/DataSourceTableScanTest.java @@ -66,7 +66,7 @@ void testIterator() { Collections.emptyList(), ImmutableMap.of())) .collect(Collectors.toSet()); - when(dataSourceService.getDataSourceMetadata(true)).thenReturn(dataSourceMetadata); + when(dataSourceService.getDataSourceMetadata(false)).thenReturn(dataSourceMetadata); assertFalse(dataSourceTableScan.hasNext()); dataSourceTableScan.open(); diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportCreateDataSourceAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportCreateDataSourceAction.java index 54ca92b695..b3c1ba4196 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportCreateDataSourceAction.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportCreateDataSourceAction.java @@ -7,6 +7,8 @@ package org.opensearch.sql.datasources.transport; +import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; + import org.opensearch.action.ActionType; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; @@ -17,6 +19,7 @@ import org.opensearch.sql.datasources.model.transport.CreateDataSourceActionRequest; import org.opensearch.sql.datasources.model.transport.CreateDataSourceActionResponse; import org.opensearch.sql.datasources.service.DataSourceServiceImpl; +import org.opensearch.sql.protocol.response.format.JsonResponseFormatter; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; @@ -56,9 +59,14 @@ protected void doExecute( try { DataSourceMetadata dataSourceMetadata = request.getDataSourceMetadata(); dataSourceService.createDataSource(dataSourceMetadata); - actionListener.onResponse( - new CreateDataSourceActionResponse( - "Created DataSource with name " + dataSourceMetadata.getName())); + String responseContent = + new JsonResponseFormatter(PRETTY) { + @Override + protected Object buildJsonObject(String response) { + return response; + } + }.format("Created DataSource with name " + dataSourceMetadata.getName()); + actionListener.onResponse(new CreateDataSourceActionResponse(responseContent)); } catch (Exception e) { actionListener.onFailure(e); } diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportUpdateDataSourceAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportUpdateDataSourceAction.java index 4325282f83..fefd0f3a01 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportUpdateDataSourceAction.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportUpdateDataSourceAction.java @@ -7,6 +7,8 @@ package org.opensearch.sql.datasources.transport; +import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; + import org.opensearch.action.ActionType; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; @@ -16,6 +18,7 @@ import org.opensearch.sql.datasources.model.transport.UpdateDataSourceActionRequest; import org.opensearch.sql.datasources.model.transport.UpdateDataSourceActionResponse; import org.opensearch.sql.datasources.service.DataSourceServiceImpl; +import org.opensearch.sql.protocol.response.format.JsonResponseFormatter; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; @@ -55,9 +58,14 @@ protected void doExecute( ActionListener actionListener) { try { dataSourceService.updateDataSource(request.getDataSourceMetadata()); - actionListener.onResponse( - new UpdateDataSourceActionResponse( - "Updated DataSource with name " + request.getDataSourceMetadata().getName())); + String responseContent = + new JsonResponseFormatter(PRETTY) { + @Override + protected Object buildJsonObject(String response) { + return response; + } + }.format("Updated DataSource with name " + request.getDataSourceMetadata().getName()); + actionListener.onResponse(new UpdateDataSourceActionResponse(responseContent)); } catch (Exception e) { actionListener.onFailure(e); } diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportCreateDataSourceActionTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportCreateDataSourceActionTest.java index f1a3a2875e..e104672fa3 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportCreateDataSourceActionTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportCreateDataSourceActionTest.java @@ -59,8 +59,8 @@ public void testDoExecute() { .onResponse(createDataSourceActionResponseArgumentCaptor.capture()); CreateDataSourceActionResponse createDataSourceActionResponse = createDataSourceActionResponseArgumentCaptor.getValue(); - Assertions.assertEquals( - "Created DataSource with name test_datasource", createDataSourceActionResponse.getResult()); + String responseAsJson = "\"Created DataSource with name test_datasource\""; + Assertions.assertEquals(responseAsJson, createDataSourceActionResponse.getResult()); } @Test diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportUpdateDataSourceActionTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportUpdateDataSourceActionTest.java index 998a1aa7b2..ffcd526f87 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportUpdateDataSourceActionTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportUpdateDataSourceActionTest.java @@ -59,8 +59,9 @@ public void testDoExecute() { .onResponse(updateDataSourceActionResponseArgumentCaptor.capture()); UpdateDataSourceActionResponse updateDataSourceActionResponse = updateDataSourceActionResponseArgumentCaptor.getValue(); - Assertions.assertEquals( - "Updated DataSource with name test_datasource", updateDataSourceActionResponse.getResult()); + String responseAsJson = "\"Updated DataSource with name test_datasource\""; + + Assertions.assertEquals(responseAsJson, updateDataSourceActionResponse.getResult()); } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/datasource/DataSourceAPIsIT.java b/integ-test/src/test/java/org/opensearch/sql/datasource/DataSourceAPIsIT.java index 0b69a459a1..b5f7fb0e54 100644 --- a/integ-test/src/test/java/org/opensearch/sql/datasource/DataSourceAPIsIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/datasource/DataSourceAPIsIT.java @@ -67,7 +67,7 @@ public void createDataSourceAPITest() { Response response = client().performRequest(createRequest); Assert.assertEquals(201, response.getStatusLine().getStatusCode()); String createResponseString = getResponseBody(response); - Assert.assertEquals("Created DataSource with name create_prometheus", createResponseString); + Assert.assertEquals("\"Created DataSource with name create_prometheus\"", createResponseString); // Datasource is not immediately created. so introducing a sleep of 2s. Thread.sleep(2000); @@ -109,7 +109,7 @@ public void updateDataSourceAPITest() { Response updateResponse = client().performRequest(updateRequest); Assert.assertEquals(200, updateResponse.getStatusLine().getStatusCode()); String updateResponseString = getResponseBody(updateResponse); - Assert.assertEquals("Updated DataSource with name update_prometheus", updateResponseString); + Assert.assertEquals("\"Updated DataSource with name update_prometheus\"", updateResponseString); // Datasource is not immediately updated. so introducing a sleep of 2s. Thread.sleep(2000); diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/ShowDataSourcesCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/ShowDataSourcesCommandIT.java index c9c4854212..4f31c5c130 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/ShowDataSourcesCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/ShowDataSourcesCommandIT.java @@ -62,7 +62,7 @@ protected void deleteDataSourceMetadata() throws IOException { @Test public void testShowDataSourcesCommands() throws IOException { JSONObject result = executeQuery("show datasources"); - verifyDataRows(result, rows("my_prometheus", "PROMETHEUS"), rows("@opensearch", "OPENSEARCH")); + verifyDataRows(result, rows("my_prometheus", "PROMETHEUS")); verifyColumn(result, columnName("DATASOURCE_NAME"), columnName("CONNECTOR_TYPE")); }