Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Fix response codes returned by JSON formatting them #2200

Merged
merged 1 commit into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public String explain() {
@Override
public void open() {
List<ExprValue> exprValues = new ArrayList<>();
Set<DataSourceMetadata> dataSourceMetadataSet = dataSourceService.getDataSourceMetadata(true);
Set<DataSourceMetadata> dataSourceMetadataSet = dataSourceService.getDataSourceMetadata(false);
for (DataSourceMetadata dataSourceMetadata : dataSourceMetadataSet) {
exprValues.add(
new ExprTupleValue(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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<String>(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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -55,9 +58,14 @@ protected void doExecute(
ActionListener<UpdateDataSourceActionResponse> actionListener) {
try {
dataSourceService.updateDataSource(request.getDataSourceMetadata());
actionListener.onResponse(
new UpdateDataSourceActionResponse(
"Updated DataSource with name " + request.getDataSourceMetadata().getName()));
String responseContent =
new JsonResponseFormatter<String>(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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

Expand Down