From 32f95774b92bbb8f2956ca8e4fd7f17d7961e32f Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Tue, 10 Oct 2023 09:34:17 -0400 Subject: [PATCH 01/21] Implement patch API for datasources Signed-off-by: Derek Ho <dxho@amazon.com> --- .../sql/datasource/DataSourceService.java | 9 ++- .../PatchDataSourceActionRequest.java | 44 +++++++++++ .../PatchDataSourceActionResponse.java | 32 ++++++++ .../rest/RestDataSourceQueryAction.java | 64 +++++++++++----- .../service/DataSourceServiceImpl.java | 12 +++ .../TransportPatchDataSourceAction.java | 73 +++++++++++++++++++ 6 files changed, 216 insertions(+), 18 deletions(-) create mode 100644 datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java create mode 100644 datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionResponse.java create mode 100644 datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java diff --git a/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java b/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java index 6dace50f99..6d95e8f34b 100644 --- a/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java +++ b/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java @@ -56,12 +56,19 @@ public interface DataSourceService { void createDataSource(DataSourceMetadata metadata); /** - * Updates {@link DataSource} corresponding to dataSourceMetadata. + * Updates {@link DataSource} corresponding to dataSourceMetadata (all fields needed). * * @param dataSourceMetadata {@link DataSourceMetadata}. */ void updateDataSource(DataSourceMetadata dataSourceMetadata); + /** + * Patches {@link DataSource} corresponding to the given name (only fields to be changed needed). + * + * @param dataSourceMetadata {@link DataSourceMetadata}. + */ + void patchDataSource(DataSourceMetadata dataSourceMetadata); + /** * Deletes {@link DataSource} corresponding to the DataSource name. * diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java new file mode 100644 index 0000000000..4895d36d6f --- /dev/null +++ b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java @@ -0,0 +1,44 @@ +/* + * + * * Copyright OpenSearch Contributors + * * SPDX-License-Identifier: Apache-2.0 + * + */ + +package org.opensearch.sql.datasources.model.transport; + +import lombok.Getter; +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.sql.datasource.model.DataSourceMetadata; + +import java.io.IOException; + +import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; + +public class PatchDataSourceActionRequest extends ActionRequest { + + @Getter private DataSourceMetadata dataSourceMetadata; + + /** Constructor of UpdateDataSourceActionRequest from StreamInput. */ + public PatchDataSourceActionRequest(StreamInput in) throws IOException { + super(in); + } + + public PatchDataSourceActionRequest(DataSourceMetadata dataSourceMetadata) { + this.dataSourceMetadata = dataSourceMetadata; + } + + @Override + public ActionRequestValidationException validate() { + if (this.dataSourceMetadata.getName().equals(DEFAULT_DATASOURCE_NAME)) { + ActionRequestValidationException exception = new ActionRequestValidationException(); + exception.addValidationError( + "Not allowed to update datasource with name : " + DEFAULT_DATASOURCE_NAME); + return exception; + } else { + return null; + } + } +} diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionResponse.java b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionResponse.java new file mode 100644 index 0000000000..7eeb0e5551 --- /dev/null +++ b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionResponse.java @@ -0,0 +1,32 @@ +/* + * + * * Copyright OpenSearch Contributors + * * SPDX-License-Identifier: Apache-2.0 + * + */ + +package org.opensearch.sql.datasources.model.transport; + +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import java.io.IOException; + +@RequiredArgsConstructor +public class PatchDataSourceActionResponse extends ActionResponse { + + @Getter private final String result; + + public PatchDataSourceActionResponse(StreamInput in) throws IOException { + super(in); + result = in.readString(); + } + + @Override + public void writeTo(StreamOutput streamOutput) throws IOException { + streamOutput.writeString(result); + } +} diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java index 2947afc5b9..b539afd021 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java @@ -10,10 +10,7 @@ import static org.opensearch.core.rest.RestStatus.BAD_REQUEST; import static org.opensearch.core.rest.RestStatus.NOT_FOUND; import static org.opensearch.core.rest.RestStatus.SERVICE_UNAVAILABLE; -import static org.opensearch.rest.RestRequest.Method.DELETE; -import static org.opensearch.rest.RestRequest.Method.GET; -import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.rest.RestRequest.Method.PUT; +import static org.opensearch.rest.RestRequest.Method.*; import com.google.common.collect.ImmutableList; import java.io.IOException; @@ -32,18 +29,8 @@ import org.opensearch.sql.datasource.model.DataSourceMetadata; import org.opensearch.sql.datasources.exceptions.DataSourceNotFoundException; import org.opensearch.sql.datasources.exceptions.ErrorMessage; -import org.opensearch.sql.datasources.model.transport.CreateDataSourceActionRequest; -import org.opensearch.sql.datasources.model.transport.CreateDataSourceActionResponse; -import org.opensearch.sql.datasources.model.transport.DeleteDataSourceActionRequest; -import org.opensearch.sql.datasources.model.transport.DeleteDataSourceActionResponse; -import org.opensearch.sql.datasources.model.transport.GetDataSourceActionRequest; -import org.opensearch.sql.datasources.model.transport.GetDataSourceActionResponse; -import org.opensearch.sql.datasources.model.transport.UpdateDataSourceActionRequest; -import org.opensearch.sql.datasources.model.transport.UpdateDataSourceActionResponse; -import org.opensearch.sql.datasources.transport.TransportCreateDataSourceAction; -import org.opensearch.sql.datasources.transport.TransportDeleteDataSourceAction; -import org.opensearch.sql.datasources.transport.TransportGetDataSourceAction; -import org.opensearch.sql.datasources.transport.TransportUpdateDataSourceAction; +import org.opensearch.sql.datasources.model.transport.*; +import org.opensearch.sql.datasources.transport.*; import org.opensearch.sql.datasources.utils.Scheduler; import org.opensearch.sql.datasources.utils.XContentParserUtils; @@ -98,6 +85,18 @@ public List<Route> routes() { */ new Route(PUT, BASE_DATASOURCE_ACTION_URL), + /* + * PATCH datasources + * Request body: + * Ref + * [org.opensearch.sql.plugin.transport.datasource.model.PatchDataSourceActionRequest] + * Response body: + * Ref + * [org.opensearch.sql.plugin.transport.datasource.model.PatchDataSourceActionResponse] + */ + + new Route(PATCH, BASE_DATASOURCE_ACTION_URL), + /* * DELETE datasources * Request body: Ref @@ -122,7 +121,9 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient return executeUpdateRequest(restRequest, nodeClient); case DELETE: return executeDeleteRequest(restRequest, nodeClient); - default: + case PATCH: + return executePatchRequest(restRequest, nodeClient); + default: return restChannel -> restChannel.sendResponse( new BytesRestResponse( @@ -216,6 +217,35 @@ public void onFailure(Exception e) { })); } + private RestChannelConsumer executePatchRequest(RestRequest restRequest, NodeClient nodeClient) + throws IOException { + DataSourceMetadata dataSourceMetadata = + XContentParserUtils.toDataSourceMetadata(restRequest.contentParser()); + return restChannel -> + Scheduler.schedule( + nodeClient, + () -> + nodeClient.execute( + TransportPatchDataSourceAction.ACTION_TYPE, + new PatchDataSourceActionRequest(dataSourceMetadata), + new ActionListener<>() { + @Override + public void onResponse( + PatchDataSourceActionResponse patchDataSourceActionResponse) { + restChannel.sendResponse( + new BytesRestResponse( + RestStatus.OK, + "application/json; charset=UTF-8", + patchDataSourceActionResponse.getResult())); + } + + @Override + public void onFailure(Exception e) { + handleException(e, restChannel); + } + })); + } + private RestChannelConsumer executeDeleteRequest(RestRequest restRequest, NodeClient nodeClient) { String dataSourceName = restRequest.param("dataSourceName"); diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java index d6c1907f84..931112aa77 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java @@ -98,6 +98,18 @@ public void updateDataSource(DataSourceMetadata dataSourceMetadata) { } } + @Override + public void patchDataSource(DataSourceMetadata dataSourceMetadata) { + validateDataSourceMetaData(dataSourceMetadata); + if (!dataSourceMetadata.getName().equals(DEFAULT_DATASOURCE_NAME)) { + this.dataSourceLoaderCache.getOrLoadDataSource(dataSourceMetadata); + this.dataSourceMetadataStorage.updateDataSourceMetadata(dataSourceMetadata); + } else { + throw new UnsupportedOperationException( + "Not allowed to update default datasource :" + DEFAULT_DATASOURCE_NAME); + } + } + @Override public void deleteDataSource(String dataSourceName) { if (dataSourceName.equals(DEFAULT_DATASOURCE_NAME)) { diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java new file mode 100644 index 0000000000..ea7155db24 --- /dev/null +++ b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java @@ -0,0 +1,73 @@ +/* + * + * * Copyright OpenSearch Contributors + * * SPDX-License-Identifier: Apache-2.0 + * + */ + +package org.opensearch.sql.datasources.transport; + +import org.opensearch.action.ActionType; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.HandledTransportAction; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.action.ActionListener; +import org.opensearch.sql.datasource.DataSourceService; +import org.opensearch.sql.datasources.model.transport.PatchDataSourceActionRequest; +import org.opensearch.sql.datasources.model.transport.PatchDataSourceActionResponse; +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; + +import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; + +public class TransportPatchDataSourceAction + extends HandledTransportAction<PatchDataSourceActionRequest, PatchDataSourceActionResponse> { + + public static final String NAME = "cluster:admin/opensearch/ql/datasources/update"; + public static final ActionType<PatchDataSourceActionResponse> ACTION_TYPE = + new ActionType<>(NAME, PatchDataSourceActionResponse::new); + + private DataSourceService dataSourceService; + + /** + * TransportUpdateDataSourceAction action for updating datasource. + * + * @param transportService transportService. + * @param actionFilters actionFilters. + * @param dataSourceService dataSourceService. + */ + @Inject + public TransportPatchDataSourceAction( + TransportService transportService, + ActionFilters actionFilters, + DataSourceServiceImpl dataSourceService) { + super( + TransportPatchDataSourceAction.NAME, + transportService, + actionFilters, + PatchDataSourceActionRequest::new); + this.dataSourceService = dataSourceService; + } + + @Override + protected void doExecute( + Task task, + PatchDataSourceActionRequest request, + ActionListener<PatchDataSourceActionResponse> actionListener) { + try { + dataSourceService.updateDataSource(request.getDataSourceMetadata()); + 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 PatchDataSourceActionResponse(responseContent)); + } catch (Exception e) { + actionListener.onFailure(e); + } + } +} From 9ed692f72f8a13d42e5467c185a3702d47eaa63a Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Tue, 10 Oct 2023 10:55:56 -0400 Subject: [PATCH 02/21] Change patch implementation to Map Signed-off-by: Derek Ho <dxho@amazon.com> --- .../PatchDataSourceActionRequest.java | 18 ++--- .../PatchDataSourceActionResponse.java | 3 +- .../rest/RestDataSourceQueryAction.java | 81 +++++++++---------- .../service/DataSourceServiceImpl.java | 2 +- .../TransportPatchDataSourceAction.java | 4 +- .../utils/XContentParserUtils.java | 53 ++++++++++++ 6 files changed, 106 insertions(+), 55 deletions(-) diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java index 4895d36d6f..7800c06e70 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java @@ -7,32 +7,32 @@ package org.opensearch.sql.datasources.model.transport; +import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; + +import java.io.IOException; +import java.util.Map; import lombok.Getter; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.sql.datasource.model.DataSourceMetadata; - -import java.io.IOException; - -import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; public class PatchDataSourceActionRequest extends ActionRequest { - @Getter private DataSourceMetadata dataSourceMetadata; + @Getter private Map<String, Object> dataSourceData; /** Constructor of UpdateDataSourceActionRequest from StreamInput. */ public PatchDataSourceActionRequest(StreamInput in) throws IOException { super(in); } - public PatchDataSourceActionRequest(DataSourceMetadata dataSourceMetadata) { - this.dataSourceMetadata = dataSourceMetadata; + public PatchDataSourceActionRequest(Map<String, Object> dataSourceData) { + this.dataSourceData = dataSourceData; } @Override public ActionRequestValidationException validate() { - if (this.dataSourceMetadata.getName().equals(DEFAULT_DATASOURCE_NAME)) { + if (this.dataSourceData.get(NAME_FIELD).equals(DEFAULT_DATASOURCE_NAME)) { ActionRequestValidationException exception = new ActionRequestValidationException(); exception.addValidationError( "Not allowed to update datasource with name : " + DEFAULT_DATASOURCE_NAME); diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionResponse.java b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionResponse.java index 7eeb0e5551..18873a6731 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionResponse.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionResponse.java @@ -7,14 +7,13 @@ package org.opensearch.sql.datasources.model.transport; +import java.io.IOException; import lombok.Getter; import lombok.RequiredArgsConstructor; import org.opensearch.core.action.ActionResponse; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import java.io.IOException; - @RequiredArgsConstructor public class PatchDataSourceActionResponse extends ActionResponse { diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java index b539afd021..c207f55738 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/rest/RestDataSourceQueryAction.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.util.List; import java.util.Locale; +import java.util.Map; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.OpenSearchException; @@ -85,17 +86,16 @@ public List<Route> routes() { */ new Route(PUT, BASE_DATASOURCE_ACTION_URL), - /* - * PATCH datasources - * Request body: - * Ref - * [org.opensearch.sql.plugin.transport.datasource.model.PatchDataSourceActionRequest] - * Response body: - * Ref - * [org.opensearch.sql.plugin.transport.datasource.model.PatchDataSourceActionResponse] - */ - - new Route(PATCH, BASE_DATASOURCE_ACTION_URL), + /* + * PATCH datasources + * Request body: + * Ref + * [org.opensearch.sql.plugin.transport.datasource.model.PatchDataSourceActionRequest] + * Response body: + * Ref + * [org.opensearch.sql.plugin.transport.datasource.model.PatchDataSourceActionResponse] + */ + new Route(PATCH, BASE_DATASOURCE_ACTION_URL), /* * DELETE datasources @@ -121,9 +121,9 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient return executeUpdateRequest(restRequest, nodeClient); case DELETE: return executeDeleteRequest(restRequest, nodeClient); - case PATCH: - return executePatchRequest(restRequest, nodeClient); - default: + case PATCH: + return executePatchRequest(restRequest, nodeClient); + default: return restChannel -> restChannel.sendResponse( new BytesRestResponse( @@ -217,34 +217,33 @@ public void onFailure(Exception e) { })); } - private RestChannelConsumer executePatchRequest(RestRequest restRequest, NodeClient nodeClient) - throws IOException { - DataSourceMetadata dataSourceMetadata = - XContentParserUtils.toDataSourceMetadata(restRequest.contentParser()); - return restChannel -> - Scheduler.schedule( - nodeClient, - () -> - nodeClient.execute( - TransportPatchDataSourceAction.ACTION_TYPE, - new PatchDataSourceActionRequest(dataSourceMetadata), - new ActionListener<>() { - @Override - public void onResponse( - PatchDataSourceActionResponse patchDataSourceActionResponse) { - restChannel.sendResponse( - new BytesRestResponse( - RestStatus.OK, - "application/json; charset=UTF-8", - patchDataSourceActionResponse.getResult())); - } + private RestChannelConsumer executePatchRequest(RestRequest restRequest, NodeClient nodeClient) + throws IOException { + Map<String, Object> dataSourceData = XContentParserUtils.toMap(restRequest.contentParser()); + return restChannel -> + Scheduler.schedule( + nodeClient, + () -> + nodeClient.execute( + TransportPatchDataSourceAction.ACTION_TYPE, + new PatchDataSourceActionRequest(dataSourceData), + new ActionListener<>() { + @Override + public void onResponse( + PatchDataSourceActionResponse patchDataSourceActionResponse) { + restChannel.sendResponse( + new BytesRestResponse( + RestStatus.OK, + "application/json; charset=UTF-8", + patchDataSourceActionResponse.getResult())); + } - @Override - public void onFailure(Exception e) { - handleException(e, restChannel); - } - })); - } + @Override + public void onFailure(Exception e) { + handleException(e, restChannel); + } + })); + } private RestChannelConsumer executeDeleteRequest(RestRequest restRequest, NodeClient nodeClient) { diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java index 931112aa77..4c184fd4bc 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java @@ -106,7 +106,7 @@ public void patchDataSource(DataSourceMetadata dataSourceMetadata) { this.dataSourceMetadataStorage.updateDataSourceMetadata(dataSourceMetadata); } else { throw new UnsupportedOperationException( - "Not allowed to update default datasource :" + DEFAULT_DATASOURCE_NAME); + "Not allowed to update default datasource :" + DEFAULT_DATASOURCE_NAME); } } diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java index ea7155db24..8284714b4f 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.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; @@ -20,8 +22,6 @@ import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; -import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; - public class TransportPatchDataSourceAction extends HandledTransportAction<PatchDataSourceActionRequest, PatchDataSourceActionResponse> { diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java index 261f13870a..1df8a069f7 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java @@ -90,6 +90,59 @@ public static DataSourceMetadata toDataSourceMetadata(XContentParser parser) thr name, description, connector, allowedRoles, properties, resultIndex); } + public static Map<String, Object> toMap(XContentParser parser) throws IOException { + Map<String, Object> resultMap = new HashMap<>(); + String name; + String description; + List<String> allowedRoles = new ArrayList<>(); + Map<String, String> properties = new HashMap<>(); + String resultIndex; + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String fieldName = parser.currentName(); + parser.nextToken(); + switch (fieldName) { + case NAME_FIELD: + name = parser.textOrNull(); + resultMap.put(NAME_FIELD, name); + break; + case DESCRIPTION_FIELD: + description = parser.textOrNull(); + resultMap.put(DESCRIPTION_FIELD, description); + break; + case CONNECTOR_FIELD: + // no-op - datasource connector should not be modified + break; + case ALLOWED_ROLES_FIELD: + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + allowedRoles.add(parser.text()); + } + resultMap.put(ALLOWED_ROLES_FIELD, allowedRoles); + break; + case PROPERTIES_FIELD: + ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_OBJECT) { + String key = parser.currentName(); + parser.nextToken(); + String value = parser.textOrNull(); + properties.put(key, value); + } + resultMap.put(PROPERTIES_FIELD, properties); + break; + case RESULT_INDEX_FIELD: + resultIndex = parser.textOrNull(); + resultMap.put(RESULT_INDEX_FIELD, resultIndex); + break; + default: + throw new IllegalArgumentException("Unknown field: " + fieldName); + } + } + if (resultMap.get(NAME_FIELD) == null || resultMap.get(NAME_FIELD) == "") { + throw new IllegalArgumentException("Name is a required field."); + } + return resultMap; + } + /** * Converts json string to DataSourceMetadata. * From 9f4506a06f7608d8c06f44226ba82620adae89df Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Tue, 10 Oct 2023 14:52:37 -0400 Subject: [PATCH 03/21] Fix up, everything complete except unit test Signed-off-by: Derek Ho <dxho@amazon.com> --- .../sql/datasource/DataSourceService.java | 5 ++- .../service/DataSourceMetadataStorage.java | 8 ++++ .../service/DataSourceServiceImpl.java | 16 +++---- .../OpenSearchDataSourceMetadataStorage.java | 42 +++++++++++++++++++ .../TransportPatchDataSourceAction.java | 5 ++- .../utils/XContentParserUtils.java | 34 +++++++++++++++ 6 files changed, 95 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java b/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java index 6d95e8f34b..162fe9e8f8 100644 --- a/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java +++ b/core/src/main/java/org/opensearch/sql/datasource/DataSourceService.java @@ -5,6 +5,7 @@ package org.opensearch.sql.datasource; +import java.util.Map; import java.util.Set; import org.opensearch.sql.datasource.model.DataSource; import org.opensearch.sql.datasource.model.DataSourceMetadata; @@ -65,9 +66,9 @@ public interface DataSourceService { /** * Patches {@link DataSource} corresponding to the given name (only fields to be changed needed). * - * @param dataSourceMetadata {@link DataSourceMetadata}. + * @param dataSourceData */ - void patchDataSource(DataSourceMetadata dataSourceMetadata); + void patchDataSource(Map<String, Object> dataSourceData); /** * Deletes {@link DataSource} corresponding to the DataSource name. diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceMetadataStorage.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceMetadataStorage.java index 4d59c68fa0..6d65c2882d 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceMetadataStorage.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceMetadataStorage.java @@ -8,6 +8,7 @@ package org.opensearch.sql.datasources.service; import java.util.List; +import java.util.Map; import java.util.Optional; import org.opensearch.sql.datasource.model.DataSource; import org.opensearch.sql.datasource.model.DataSourceMetadata; @@ -47,6 +48,13 @@ public interface DataSourceMetadataStorage { */ void updateDataSourceMetadata(DataSourceMetadata dataSourceMetadata); + /** + * Patches {@link DataSourceMetadata} in underlying storage. + * + * @param dataSourceData + */ + void patchDataSourceMetadata(Map<String, Object> dataSourceData); + /** * Deletes {@link DataSourceMetadata} corresponding to the datasourceName from underlying storage. * diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java index 4c184fd4bc..6fd29c8dcc 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java @@ -6,15 +6,11 @@ package org.opensearch.sql.datasources.service; import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; import com.google.common.base.Preconditions; import com.google.common.base.Strings; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; +import java.util.*; import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.datasource.DataSourceService; import org.opensearch.sql.datasource.model.DataSource; @@ -99,11 +95,9 @@ public void updateDataSource(DataSourceMetadata dataSourceMetadata) { } @Override - public void patchDataSource(DataSourceMetadata dataSourceMetadata) { - validateDataSourceMetaData(dataSourceMetadata); - if (!dataSourceMetadata.getName().equals(DEFAULT_DATASOURCE_NAME)) { - this.dataSourceLoaderCache.getOrLoadDataSource(dataSourceMetadata); - this.dataSourceMetadataStorage.updateDataSourceMetadata(dataSourceMetadata); + public void patchDataSource(Map<String, Object> dataSourceData) { + if (!dataSourceData.get(NAME_FIELD).equals(DEFAULT_DATASOURCE_NAME)) { + this.dataSourceMetadataStorage.patchDataSourceMetadata(dataSourceData); } else { throw new UnsupportedOperationException( "Not allowed to update default datasource :" + DEFAULT_DATASOURCE_NAME); diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java b/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java index 6659e54342..d5f0212c6c 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java @@ -7,6 +7,9 @@ package org.opensearch.sql.datasources.storage; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.PROPERTIES_FIELD; + import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -161,6 +164,35 @@ public void updateDataSourceMetadata(DataSourceMetadata dataSourceMetadata) { } } + @Override + public void patchDataSourceMetadata(Map<String, Object> dataSourceData) { + encryptDecryptAuthenticationData(dataSourceData, true); + UpdateRequest updateRequest = + new UpdateRequest(DATASOURCE_INDEX_NAME, (String) dataSourceData.get(NAME_FIELD)); + UpdateResponse updateResponse; + try (ThreadContext.StoredContext storedContext = + client.threadPool().getThreadContext().stashContext()) { + updateRequest.doc(XContentParserUtils.convertMapToXContent(dataSourceData)); + updateRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); + ActionFuture<UpdateResponse> updateResponseActionFuture = client.update(updateRequest); + updateResponse = updateResponseActionFuture.actionGet(); + } catch (DocumentMissingException exception) { + throw new DataSourceNotFoundException( + "Datasource with name: " + dataSourceData.get(NAME_FIELD) + " doesn't exist"); + } catch (Exception e) { + throw new RuntimeException(e); + } + + if (updateResponse.getResult().equals(DocWriteResponse.Result.UPDATED) + || updateResponse.getResult().equals(DocWriteResponse.Result.NOOP)) { + LOG.debug("DatasourceMetadata : {} successfully updated", dataSourceData.get(NAME_FIELD)); + } else { + throw new RuntimeException( + "Saving dataSource metadata information failed with result : " + + updateResponse.getResult().getLowercase()); + } + } + @Override public void deleteDataSourceMetadata(String datasourceName) { DeleteRequest deleteRequest = new DeleteRequest(DATASOURCE_INDEX_NAME); @@ -263,6 +295,16 @@ private DataSourceMetadata encryptDecryptAuthenticationData( return dataSourceMetadata; } + // Encrypt and Decrypt irrespective of auth type.If properties name ends in username, password, + // secret_key and access_key. + private Map<String, Object> encryptDecryptAuthenticationData( + Map<String, Object> dataSourceData, Boolean isEncryption) { + Map<String, String> propertiesMap = (Map<String, String>) dataSourceData.get(PROPERTIES_FIELD); + handleBasicAuthPropertiesEncryptionDecryption(propertiesMap, isEncryption); + handleSigV4PropertiesEncryptionDecryption(propertiesMap, isEncryption); + return dataSourceData; + } + private void handleBasicAuthPropertiesEncryptionDecryption( Map<String, String> propertiesMap, Boolean isEncryption) { ArrayList<String> list = new ArrayList<>(); diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java index 8284714b4f..c2e6ba6b78 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java @@ -7,6 +7,7 @@ package org.opensearch.sql.datasources.transport; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; import static org.opensearch.sql.protocol.response.format.JsonResponseFormatter.Style.PRETTY; import org.opensearch.action.ActionType; @@ -57,14 +58,14 @@ protected void doExecute( PatchDataSourceActionRequest request, ActionListener<PatchDataSourceActionResponse> actionListener) { try { - dataSourceService.updateDataSource(request.getDataSourceMetadata()); + dataSourceService.patchDataSource(request.getDataSourceData()); String responseContent = new JsonResponseFormatter<String>(PRETTY) { @Override protected Object buildJsonObject(String response) { return response; } - }.format("Updated DataSource with name " + request.getDataSourceMetadata().getName()); + }.format("Updated DataSource with name " + request.getDataSourceData().get(NAME_FIELD)); actionListener.onResponse(new PatchDataSourceActionResponse(responseContent)); } catch (Exception e) { actionListener.onFailure(e); diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java index 1df8a069f7..aeaeaf8b97 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java @@ -186,4 +186,38 @@ public static XContentBuilder convertToXContent(DataSourceMetadata metadata) thr builder.endObject(); return builder; } + + /** + * Converts Map<String, Object> (partial DataSourceMetadata) to XContentBuilder. + * + * @param dataSourceData + * @return XContentBuilder {@link XContentBuilder} + * @throws Exception Exception. + */ + public static XContentBuilder convertMapToXContent(Map<String, Object> dataSourceData) + throws Exception { + + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + if (dataSourceData.containsKey(DESCRIPTION_FIELD)) { + builder.field(DESCRIPTION_FIELD, dataSourceData.get(DESCRIPTION_FIELD)); + } + if (dataSourceData.containsKey(ALLOWED_ROLES_FIELD)) { + builder.field( + ALLOWED_ROLES_FIELD, ((List<String>) dataSourceData.get(ALLOWED_ROLES_FIELD)).toArray()); + } + if (dataSourceData.containsKey(PROPERTIES_FIELD)) { + builder.startObject(PROPERTIES_FIELD); + for (Map.Entry<String, String> entry : + ((Map<String, String>) dataSourceData.get(PROPERTIES_FIELD)).entrySet()) { + builder.field(entry.getKey(), entry.getValue()); + } + builder.endObject(); + } + if (dataSourceData.containsKey(RESULT_INDEX_FIELD)) { + builder.field(RESULT_INDEX_FIELD, dataSourceData.get(RESULT_INDEX_FIELD)); + } + builder.endObject(); + return builder; + } } From 84522f101902267a87500c867b61788de7aa7631 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Wed, 11 Oct 2023 11:24:58 -0400 Subject: [PATCH 04/21] Revise PR to use existing functions Signed-off-by: Derek Ho <dxho@amazon.com> --- .../service/DataSourceMetadataStorage.java | 8 ----- .../service/DataSourceServiceImpl.java | 36 +++++++++++++++++-- .../OpenSearchDataSourceMetadataStorage.java | 30 ---------------- 3 files changed, 34 insertions(+), 40 deletions(-) diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceMetadataStorage.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceMetadataStorage.java index 6d65c2882d..4d59c68fa0 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceMetadataStorage.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceMetadataStorage.java @@ -8,7 +8,6 @@ package org.opensearch.sql.datasources.service; import java.util.List; -import java.util.Map; import java.util.Optional; import org.opensearch.sql.datasource.model.DataSource; import org.opensearch.sql.datasource.model.DataSourceMetadata; @@ -48,13 +47,6 @@ public interface DataSourceMetadataStorage { */ void updateDataSourceMetadata(DataSourceMetadata dataSourceMetadata); - /** - * Patches {@link DataSourceMetadata} in underlying storage. - * - * @param dataSourceData - */ - void patchDataSourceMetadata(Map<String, Object> dataSourceData); - /** * Deletes {@link DataSourceMetadata} corresponding to the datasourceName from underlying storage. * diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java index 6fd29c8dcc..9173c1b6d6 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java @@ -6,7 +6,7 @@ package org.opensearch.sql.datasources.service; import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; -import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.*; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -86,7 +86,6 @@ public void createDataSource(DataSourceMetadata metadata) { public void updateDataSource(DataSourceMetadata dataSourceMetadata) { validateDataSourceMetaData(dataSourceMetadata); if (!dataSourceMetadata.getName().equals(DEFAULT_DATASOURCE_NAME)) { - this.dataSourceLoaderCache.getOrLoadDataSource(dataSourceMetadata); this.dataSourceMetadataStorage.updateDataSourceMetadata(dataSourceMetadata); } else { throw new UnsupportedOperationException( @@ -97,6 +96,10 @@ public void updateDataSource(DataSourceMetadata dataSourceMetadata) { @Override public void patchDataSource(Map<String, Object> dataSourceData) { if (!dataSourceData.get(NAME_FIELD).equals(DEFAULT_DATASOURCE_NAME)) { + DataSourceMetadata dataSourceMetadata = + getRawDataSourceMetadata((String) dataSourceData.get(NAME_FIELD)); + replaceOldDatasourceMetadata(dataSourceData, dataSourceMetadata); + updateDataSource(dataSourceMetadata); this.dataSourceMetadataStorage.patchDataSourceMetadata(dataSourceData); } else { throw new UnsupportedOperationException( @@ -140,6 +143,35 @@ private void validateDataSourceMetaData(DataSourceMetadata metadata) { + " Properties are required parameters."); } + /** + * Replaces the fields in the map of the given metadata. + * + * @param dataSourceData + * @param metadata {@link DataSourceMetadata}. + */ + private void replaceOldDatasourceMetadata( + Map<String, Object> dataSourceData, DataSourceMetadata metadata) { + + for (String key : dataSourceData.keySet()) { + switch (key) { + // Name and connector should not be modified + case DESCRIPTION_FIELD: + metadata.setDescription((String) dataSourceData.get(DESCRIPTION_FIELD)); + break; + case ALLOWED_ROLES_FIELD: + metadata.setAllowedRoles((List<String>) dataSourceData.get(ALLOWED_ROLES_FIELD)); + break; + case PROPERTIES_FIELD: + Map<String, String> properties = new HashMap<>(metadata.getProperties()); + properties.putAll(((Map<String, String>) dataSourceData.get(PROPERTIES_FIELD))); + break; + case NAME_FIELD: + case CONNECTOR_FIELD: + break; + } + } + } + @Override public DataSourceMetadata getRawDataSourceMetadata(String dataSourceName) { if (dataSourceName.equals(DEFAULT_DATASOURCE_NAME)) { diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java b/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java index d5f0212c6c..8707fc5ddd 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java @@ -7,7 +7,6 @@ package org.opensearch.sql.datasources.storage; -import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; import static org.opensearch.sql.datasources.utils.XContentParserUtils.PROPERTIES_FIELD; import java.io.IOException; @@ -164,35 +163,6 @@ public void updateDataSourceMetadata(DataSourceMetadata dataSourceMetadata) { } } - @Override - public void patchDataSourceMetadata(Map<String, Object> dataSourceData) { - encryptDecryptAuthenticationData(dataSourceData, true); - UpdateRequest updateRequest = - new UpdateRequest(DATASOURCE_INDEX_NAME, (String) dataSourceData.get(NAME_FIELD)); - UpdateResponse updateResponse; - try (ThreadContext.StoredContext storedContext = - client.threadPool().getThreadContext().stashContext()) { - updateRequest.doc(XContentParserUtils.convertMapToXContent(dataSourceData)); - updateRequest.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); - ActionFuture<UpdateResponse> updateResponseActionFuture = client.update(updateRequest); - updateResponse = updateResponseActionFuture.actionGet(); - } catch (DocumentMissingException exception) { - throw new DataSourceNotFoundException( - "Datasource with name: " + dataSourceData.get(NAME_FIELD) + " doesn't exist"); - } catch (Exception e) { - throw new RuntimeException(e); - } - - if (updateResponse.getResult().equals(DocWriteResponse.Result.UPDATED) - || updateResponse.getResult().equals(DocWriteResponse.Result.NOOP)) { - LOG.debug("DatasourceMetadata : {} successfully updated", dataSourceData.get(NAME_FIELD)); - } else { - throw new RuntimeException( - "Saving dataSource metadata information failed with result : " - + updateResponse.getResult().getLowercase()); - } - } - @Override public void deleteDataSourceMetadata(String datasourceName) { DeleteRequest deleteRequest = new DeleteRequest(DATASOURCE_INDEX_NAME); From 9382ff31c02a4673d06d69ea3838cb14ad006a5f Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Wed, 11 Oct 2023 11:27:14 -0400 Subject: [PATCH 05/21] Remove unused utility function Signed-off-by: Derek Ho <dxho@amazon.com> --- .../utils/XContentParserUtils.java | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java index aeaeaf8b97..1df8a069f7 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java @@ -186,38 +186,4 @@ public static XContentBuilder convertToXContent(DataSourceMetadata metadata) thr builder.endObject(); return builder; } - - /** - * Converts Map<String, Object> (partial DataSourceMetadata) to XContentBuilder. - * - * @param dataSourceData - * @return XContentBuilder {@link XContentBuilder} - * @throws Exception Exception. - */ - public static XContentBuilder convertMapToXContent(Map<String, Object> dataSourceData) - throws Exception { - - XContentBuilder builder = XContentFactory.jsonBuilder(); - builder.startObject(); - if (dataSourceData.containsKey(DESCRIPTION_FIELD)) { - builder.field(DESCRIPTION_FIELD, dataSourceData.get(DESCRIPTION_FIELD)); - } - if (dataSourceData.containsKey(ALLOWED_ROLES_FIELD)) { - builder.field( - ALLOWED_ROLES_FIELD, ((List<String>) dataSourceData.get(ALLOWED_ROLES_FIELD)).toArray()); - } - if (dataSourceData.containsKey(PROPERTIES_FIELD)) { - builder.startObject(PROPERTIES_FIELD); - for (Map.Entry<String, String> entry : - ((Map<String, String>) dataSourceData.get(PROPERTIES_FIELD)).entrySet()) { - builder.field(entry.getKey(), entry.getValue()); - } - builder.endObject(); - } - if (dataSourceData.containsKey(RESULT_INDEX_FIELD)) { - builder.field(RESULT_INDEX_FIELD, dataSourceData.get(RESULT_INDEX_FIELD)); - } - builder.endObject(); - return builder; - } } From 91899fe46c2b1c25ee3ab9649c54738279bff258 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Thu, 12 Oct 2023 14:54:50 -0400 Subject: [PATCH 06/21] Add tests Signed-off-by: Derek Ho <dxho@amazon.com> --- .../PatchDataSourceActionRequest.java | 5 ++ .../service/DataSourceServiceImpl.java | 1 - .../TransportPatchDataSourceActionTest.java | 79 +++++++++++++++++++ .../sql/datasource/DataSourceAPIsIT.java | 29 +++++++ .../sql/legacy/SQLIntegTestCase.java | 10 +++ 5 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceActionTest.java diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java index 7800c06e70..9443ea561e 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/model/transport/PatchDataSourceActionRequest.java @@ -8,6 +8,7 @@ package org.opensearch.sql.datasources.model.transport; import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.CONNECTOR_FIELD; import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; import java.io.IOException; @@ -37,6 +38,10 @@ public ActionRequestValidationException validate() { exception.addValidationError( "Not allowed to update datasource with name : " + DEFAULT_DATASOURCE_NAME); return exception; + } else if (this.dataSourceData.get(CONNECTOR_FIELD) != null) { + ActionRequestValidationException exception = new ActionRequestValidationException(); + exception.addValidationError("Not allowed to update connector for datasource"); + return exception; } else { return null; } diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java index 9173c1b6d6..664a675708 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java @@ -100,7 +100,6 @@ public void patchDataSource(Map<String, Object> dataSourceData) { getRawDataSourceMetadata((String) dataSourceData.get(NAME_FIELD)); replaceOldDatasourceMetadata(dataSourceData, dataSourceMetadata); updateDataSource(dataSourceMetadata); - this.dataSourceMetadataStorage.patchDataSourceMetadata(dataSourceData); } else { throw new UnsupportedOperationException( "Not allowed to update default datasource :" + DEFAULT_DATASOURCE_NAME); diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceActionTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceActionTest.java new file mode 100644 index 0000000000..d8fc2a8346 --- /dev/null +++ b/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceActionTest.java @@ -0,0 +1,79 @@ +package org.opensearch.sql.datasources.transport; + +import static org.mockito.Mockito.*; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.*; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.core.action.ActionListener; +import org.opensearch.sql.datasources.model.transport.PatchDataSourceActionRequest; +import org.opensearch.sql.datasources.model.transport.PatchDataSourceActionResponse; +import org.opensearch.sql.datasources.service.DataSourceServiceImpl; +import org.opensearch.tasks.Task; +import org.opensearch.transport.TransportService; + +@ExtendWith(MockitoExtension.class) +public class TransportPatchDataSourceActionTest { + + @Mock private TransportService transportService; + @Mock private TransportPatchDataSourceAction action; + @Mock private DataSourceServiceImpl dataSourceService; + @Mock private Task task; + @Mock private ActionListener<PatchDataSourceActionResponse> actionListener; + + @Captor + private ArgumentCaptor<PatchDataSourceActionResponse> patchDataSourceActionResponseArgumentCaptor; + + @Captor private ArgumentCaptor<Exception> exceptionArgumentCaptor; + + @BeforeEach + public void setUp() { + action = + new TransportPatchDataSourceAction( + transportService, new ActionFilters(new HashSet<>()), dataSourceService); + } + + @Test + public void testDoExecute() { + Map<String, Object> dataSourceData = new HashMap<>(); + dataSourceData.put(NAME_FIELD, "test_datasource"); + dataSourceData.put(DESCRIPTION_FIELD, "test"); + + PatchDataSourceActionRequest request = new PatchDataSourceActionRequest(dataSourceData); + + action.doExecute(task, request, actionListener); + verify(dataSourceService, times(1)).patchDataSource(dataSourceData); + Mockito.verify(actionListener) + .onResponse(patchDataSourceActionResponseArgumentCaptor.capture()); + PatchDataSourceActionResponse patchDataSourceActionResponse = + patchDataSourceActionResponseArgumentCaptor.getValue(); + String responseAsJson = "\"Created DataSource with name test_datasource\""; + Assertions.assertEquals(responseAsJson, patchDataSourceActionResponse.getResult()); + } + + @Test + public void testDoExecuteWithException() { + Map<String, Object> dataSourceData = new HashMap<>(); + dataSourceData.put(NAME_FIELD, "test_datasource"); + dataSourceData.put(DESCRIPTION_FIELD, "test"); + doThrow(new RuntimeException("Error")).when(dataSourceService).patchDataSource(dataSourceData); + PatchDataSourceActionRequest request = new PatchDataSourceActionRequest(dataSourceData); + action.doExecute(task, request, actionListener); + verify(dataSourceService, times(1)).patchDataSource(dataSourceData); + Mockito.verify(actionListener).onFailure(exceptionArgumentCaptor.capture()); + Exception exception = exceptionArgumentCaptor.getValue(); + Assertions.assertTrue(exception instanceof RuntimeException); + Assertions.assertEquals("Error", exception.getMessage()); + } +} 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 087629a1f1..c22bb83189 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 @@ -5,6 +5,8 @@ package org.opensearch.sql.datasource; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.DESCRIPTION_FIELD; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; import static org.opensearch.sql.legacy.TestUtils.getResponseBody; import com.google.common.collect.ImmutableList; @@ -15,7 +17,9 @@ import java.io.IOException; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import lombok.SneakyThrows; import org.junit.AfterClass; import org.junit.Assert; @@ -132,6 +136,31 @@ public void updateDataSourceAPITest() { Assert.assertEquals( "https://randomtest.com:9090", dataSourceMetadata.getProperties().get("prometheus.uri")); Assert.assertEquals("", dataSourceMetadata.getDescription()); + + // patch datasource + Map<String, Object> updateDS = + new HashMap<>(Map.of(NAME_FIELD, "update_prometheus", DESCRIPTION_FIELD, "test")); + Request patchRequest = getPatchDataSourceRequest(updateDS); + Response patchResponse = client().performRequest(updateRequest); + Assert.assertEquals(200, patchResponse.getStatusLine().getStatusCode()); + String patchResponseString = getResponseBody(updateResponse); + Assert.assertEquals("\"Updated DataSource with name update_prometheus\"", updateResponseString); + + // Datasource is not immediately updated. so introducing a sleep of 2s. + Thread.sleep(2000); + + // get datasource to validate the modification. + // get datasource + Request getRequestAfterPatch = getFetchDataSourceRequest("update_prometheus"); + Response getResponseAfterPatch = client().performRequest(getRequest); + Assert.assertEquals(200, getResponse.getStatusLine().getStatusCode()); + String getResponseStringAfterPatch = getResponseBody(getResponse); + DataSourceMetadata dataSourceMetadataAfterPatch = + new Gson().fromJson(getResponseString, DataSourceMetadata.class); + Assert.assertEquals( + "https://randomtest.com:9090", + dataSourceMetadataAfterPatch.getProperties().get("prometheus.uri")); + Assert.assertEquals("test", dataSourceMetadataAfterPatch.getDescription()); } @SneakyThrows diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 8335ada5a7..058182f123 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -49,6 +49,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Locale; +import java.util.Map; import javax.management.MBeanServerInvocationHandler; import javax.management.ObjectName; import javax.management.remote.JMXConnector; @@ -488,6 +489,15 @@ protected static Request getUpdateDataSourceRequest(DataSourceMetadata dataSourc return request; } + protected static Request getPatchDataSourceRequest(Map<String, Object> dataSourceData) { + Request request = new Request("PATCH", "/_plugins/_query/_datasources"); + request.setJsonEntity(new Gson().toJson(dataSourceData)); + RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder(); + restOptionsBuilder.addHeader("Content-Type", "application/json"); + request.setOptions(restOptionsBuilder); + return request; + } + protected static Request getFetchDataSourceRequest(String name) { Request request = new Request("GET", "/_plugins/_query/_datasources" + "/" + name); if (StringUtils.isEmpty(name)) { From e246a2379b1ef665ba157b8064d6ea9078817417 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Thu, 12 Oct 2023 14:56:42 -0400 Subject: [PATCH 07/21] Add back line Signed-off-by: Derek Ho <dxho@amazon.com> --- .../sql/datasources/service/DataSourceServiceImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java index 664a675708..d959146b97 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/service/DataSourceServiceImpl.java @@ -86,6 +86,7 @@ public void createDataSource(DataSourceMetadata metadata) { public void updateDataSource(DataSourceMetadata dataSourceMetadata) { validateDataSourceMetaData(dataSourceMetadata); if (!dataSourceMetadata.getName().equals(DEFAULT_DATASOURCE_NAME)) { + this.dataSourceLoaderCache.getOrLoadDataSource(dataSourceMetadata); this.dataSourceMetadataStorage.updateDataSourceMetadata(dataSourceMetadata); } else { throw new UnsupportedOperationException( From e86794a18eff92228f75f1f9de70feb7571d1dce Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Thu, 12 Oct 2023 15:00:17 -0400 Subject: [PATCH 08/21] fix build issue Signed-off-by: Derek Ho <dxho@amazon.com> --- .../java/org/opensearch/sql/analysis/AnalyzerTestBase.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java index 508567582b..569cdd96f8 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTestBase.java @@ -231,6 +231,9 @@ public DataSource getDataSource(String dataSourceName) { @Override public void updateDataSource(DataSourceMetadata dataSourceMetadata) {} + @Override + public void patchDataSource(Map<String, Object> dataSourceData) {} + @Override public void deleteDataSource(String dataSourceName) {} From 9384f8cbc881899d6561b00e74907e77a337aaf3 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Thu, 12 Oct 2023 15:20:45 -0400 Subject: [PATCH 09/21] Fix tests and add in rst Signed-off-by: Derek Ho <dxho@amazon.com> --- .../transport/TransportPatchDataSourceActionTest.java | 2 +- docs/user/ppl/admin/datasources.rst | 11 +++++++++++ .../opensearch/sql/datasource/DataSourceAPIsIT.java | 10 +++++----- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceActionTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceActionTest.java index d8fc2a8346..5e1e7df112 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceActionTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceActionTest.java @@ -58,7 +58,7 @@ public void testDoExecute() { .onResponse(patchDataSourceActionResponseArgumentCaptor.capture()); PatchDataSourceActionResponse patchDataSourceActionResponse = patchDataSourceActionResponseArgumentCaptor.getValue(); - String responseAsJson = "\"Created DataSource with name test_datasource\""; + String responseAsJson = "\"Updated DataSource with name test_datasource\""; Assertions.assertEquals(responseAsJson, patchDataSourceActionResponse.getResult()); } diff --git a/docs/user/ppl/admin/datasources.rst b/docs/user/ppl/admin/datasources.rst index 3682153b9d..bf37cc196d 100644 --- a/docs/user/ppl/admin/datasources.rst +++ b/docs/user/ppl/admin/datasources.rst @@ -93,6 +93,17 @@ we can remove authorization and other details in case of security disabled domai "allowedRoles" : ["prometheus_access"] } +* Datasource modification PATCH API ("_plugins/_query/_datasources") :: + + PATCH https://localhost:9200/_plugins/_query/_datasources + content-type: application/json + Authorization: Basic {{username}} {{password}} + + { + "name" : "my_prometheus", + "allowedRoles" : ["all_access"] + } + * Datasource Read GET API("_plugins/_query/_datasources/{{dataSourceName}}" :: GET https://localhost:9200/_plugins/_query/_datasources/my_prometheus 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 c22bb83189..2799282f7b 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 @@ -141,10 +141,10 @@ public void updateDataSourceAPITest() { Map<String, Object> updateDS = new HashMap<>(Map.of(NAME_FIELD, "update_prometheus", DESCRIPTION_FIELD, "test")); Request patchRequest = getPatchDataSourceRequest(updateDS); - Response patchResponse = client().performRequest(updateRequest); + Response patchResponse = client().performRequest(patchRequest); Assert.assertEquals(200, patchResponse.getStatusLine().getStatusCode()); String patchResponseString = getResponseBody(updateResponse); - Assert.assertEquals("\"Updated DataSource with name update_prometheus\"", updateResponseString); + Assert.assertEquals("\"Updated DataSource with name update_prometheus\"", patchResponseString); // Datasource is not immediately updated. so introducing a sleep of 2s. Thread.sleep(2000); @@ -152,11 +152,11 @@ public void updateDataSourceAPITest() { // get datasource to validate the modification. // get datasource Request getRequestAfterPatch = getFetchDataSourceRequest("update_prometheus"); - Response getResponseAfterPatch = client().performRequest(getRequest); - Assert.assertEquals(200, getResponse.getStatusLine().getStatusCode()); + Response getResponseAfterPatch = client().performRequest(getRequestAfterPatch); + Assert.assertEquals(200, getResponseAfterPatch.getStatusLine().getStatusCode()); String getResponseStringAfterPatch = getResponseBody(getResponse); DataSourceMetadata dataSourceMetadataAfterPatch = - new Gson().fromJson(getResponseString, DataSourceMetadata.class); + new Gson().fromJson(getResponseStringAfterPatch, DataSourceMetadata.class); Assert.assertEquals( "https://randomtest.com:9090", dataSourceMetadataAfterPatch.getProperties().get("prometheus.uri")); From fa130d7800e22117d604d50fc81f846d41f856d6 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Thu, 12 Oct 2023 16:27:19 -0400 Subject: [PATCH 10/21] Register patch Signed-off-by: Derek Ho <dxho@amazon.com> --- .../datasources/transport/TransportPatchDataSourceAction.java | 2 +- plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java index c2e6ba6b78..8156bd8a3e 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java @@ -33,7 +33,7 @@ public class TransportPatchDataSourceAction private DataSourceService dataSourceService; /** - * TransportUpdateDataSourceAction action for updating datasource. + * TransportPatchDataSourceAction action for updating datasource. * * @param transportService transportService. * @param actionFilters actionFilters. diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index d2c8c6ebb7..b7046c867a 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -178,6 +178,10 @@ public List<RestHandler> getRestHandlers( new ActionType<>( TransportUpdateDataSourceAction.NAME, UpdateDataSourceActionResponse::new), TransportUpdateDataSourceAction.class), + new ActionHandler<>( + new ActionType<>( + TransportPatchDataSourceAction.NAME, PatchDataSourceActionResponse::new), + TransportPatchDataSourceAction.class), new ActionHandler<>( new ActionType<>( TransportDeleteDataSourceAction.NAME, DeleteDataSourceActionResponse::new), From aa673a256637181a924a9ff5dd67d416e6ad7f9a Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Thu, 12 Oct 2023 16:31:50 -0400 Subject: [PATCH 11/21] Add imports Signed-off-by: Derek Ho <dxho@amazon.com> --- .../main/java/org/opensearch/sql/plugin/SQLPlugin.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index b7046c867a..7b49e04a41 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -58,18 +58,12 @@ import org.opensearch.sql.datasources.auth.DataSourceUserAuthorizationHelperImpl; import org.opensearch.sql.datasources.encryptor.EncryptorImpl; import org.opensearch.sql.datasources.glue.GlueDataSourceFactory; -import org.opensearch.sql.datasources.model.transport.CreateDataSourceActionResponse; -import org.opensearch.sql.datasources.model.transport.DeleteDataSourceActionResponse; -import org.opensearch.sql.datasources.model.transport.GetDataSourceActionResponse; -import org.opensearch.sql.datasources.model.transport.UpdateDataSourceActionResponse; +import org.opensearch.sql.datasources.model.transport.*; import org.opensearch.sql.datasources.rest.RestDataSourceQueryAction; import org.opensearch.sql.datasources.service.DataSourceMetadataStorage; import org.opensearch.sql.datasources.service.DataSourceServiceImpl; import org.opensearch.sql.datasources.storage.OpenSearchDataSourceMetadataStorage; -import org.opensearch.sql.datasources.transport.TransportCreateDataSourceAction; -import org.opensearch.sql.datasources.transport.TransportDeleteDataSourceAction; -import org.opensearch.sql.datasources.transport.TransportGetDataSourceAction; -import org.opensearch.sql.datasources.transport.TransportUpdateDataSourceAction; +import org.opensearch.sql.datasources.transport.*; import org.opensearch.sql.legacy.esdomain.LocalClusterState; import org.opensearch.sql.legacy.executor.AsyncRestExecutor; import org.opensearch.sql.legacy.metrics.Metrics; From b179c49a43e800c53dbd1795662904c75e3b2773 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Thu, 12 Oct 2023 16:39:38 -0400 Subject: [PATCH 12/21] Patch Signed-off-by: Derek Ho <dxho@amazon.com> --- .../datasources/transport/TransportPatchDataSourceAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java index 8156bd8a3e..303e905cec 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/transport/TransportPatchDataSourceAction.java @@ -26,7 +26,7 @@ public class TransportPatchDataSourceAction extends HandledTransportAction<PatchDataSourceActionRequest, PatchDataSourceActionResponse> { - public static final String NAME = "cluster:admin/opensearch/ql/datasources/update"; + public static final String NAME = "cluster:admin/opensearch/ql/datasources/patch"; public static final ActionType<PatchDataSourceActionResponse> ACTION_TYPE = new ActionType<>(NAME, PatchDataSourceActionResponse::new); From f336064a8c9ad1d348f52897f7cc923c03bfbb44 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Fri, 13 Oct 2023 09:46:41 -0400 Subject: [PATCH 13/21] Fix integration test Signed-off-by: Derek Ho <dxho@amazon.com> --- .../java/org/opensearch/sql/datasource/DataSourceAPIsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2799282f7b..bdefdfc928 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 @@ -143,7 +143,7 @@ public void updateDataSourceAPITest() { Request patchRequest = getPatchDataSourceRequest(updateDS); Response patchResponse = client().performRequest(patchRequest); Assert.assertEquals(200, patchResponse.getStatusLine().getStatusCode()); - String patchResponseString = getResponseBody(updateResponse); + String patchResponseString = getResponseBody(patchResponse); Assert.assertEquals("\"Updated DataSource with name update_prometheus\"", patchResponseString); // Datasource is not immediately updated. so introducing a sleep of 2s. From 8ca5e037bb2bd5801209e1746b7fd9833068eda1 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Fri, 13 Oct 2023 09:47:36 -0400 Subject: [PATCH 14/21] Update IT Signed-off-by: Derek Ho <dxho@amazon.com> --- .../java/org/opensearch/sql/datasource/DataSourceAPIsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 bdefdfc928..3c4803ea88 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 @@ -154,7 +154,7 @@ public void updateDataSourceAPITest() { Request getRequestAfterPatch = getFetchDataSourceRequest("update_prometheus"); Response getResponseAfterPatch = client().performRequest(getRequestAfterPatch); Assert.assertEquals(200, getResponseAfterPatch.getStatusLine().getStatusCode()); - String getResponseStringAfterPatch = getResponseBody(getResponse); + String getResponseStringAfterPatch = getResponseBody(getResponseAfterPatch); DataSourceMetadata dataSourceMetadataAfterPatch = new Gson().fromJson(getResponseStringAfterPatch, DataSourceMetadata.class); Assert.assertEquals( From 0ac557a19f0168ed6c4a13136b805becc9e30e72 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Fri, 13 Oct 2023 10:41:34 -0400 Subject: [PATCH 15/21] Add tests Signed-off-by: Derek Ho <dxho@amazon.com> --- .../utils/XContentParserUtils.java | 19 +++++++++++ .../service/DataSourceServiceImplTest.java | 11 +++++++ .../utils/XContentParserUtilsTest.java | 32 +++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java index 1df8a069f7..f42a0801c5 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java @@ -162,6 +162,25 @@ public static DataSourceMetadata toDataSourceMetadata(String json) throws IOExce } } + /** + * Converts json string to Map. + * + * @param json jsonstring. + * @return DataSourceData + * @throws IOException IOException. + */ + public static Map<String, Object> toMap(String json) throws IOException { + try (XContentParser parser = + XContentType.JSON + .xContent() + .createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + json)) { + return toMap(parser); + } + } + /** * Converts DataSourceMetadata to XContentBuilder. * diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java index c8312e6013..1976ea38c0 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java @@ -18,6 +18,8 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.DESCRIPTION_FIELD; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; import com.google.common.collect.ImmutableMap; import java.util.ArrayList; @@ -264,6 +266,7 @@ void testGetDataSourceMetadataSetWithDefaultDatasource() { @Test void testUpdateDataSourceSuccessCase() { + doNothing().when(dataSourceUserAuthorizationHelper).authorizeDataSource(any()); DataSourceMetadata dataSourceMetadata = metadata("testDS", DataSourceType.OPENSEARCH, Collections.emptyList(), ImmutableMap.of()); @@ -289,6 +292,14 @@ void testUpdateDefaultDataSource() { unsupportedOperationException.getMessage()); } + @Test + void testPatchDataSourceSuccessCase() { + Map<String, Object> dataSourceData = new HashMap<>(Map.of(NAME_FIELD, "test", DESCRIPTION_FIELD, "test")); + + dataSourceService.patchDataSource(dataSourceData); + verify(dataSourceMetadataStorage, times(1)).updateDataSourceMetadata(any()); + } + @Test void testDeleteDatasource() { dataSourceService.deleteDataSource("testDS"); diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java index d134293456..a3271e8ee0 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java @@ -1,8 +1,11 @@ package org.opensearch.sql.datasources.utils; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.*; import com.google.gson.Gson; + +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -52,6 +55,19 @@ public void testToDataSourceMetadataFromJson() { Assertions.assertEquals("prometheus_access", retrievedMetadata.getAllowedRoles().get(0)); } + @SneakyThrows + @Test + public void testToMapFromJson() { + Map<String, Object> dataSourceData = new HashMap<>(Map.of(NAME_FIELD, "test_DS", DESCRIPTION_FIELD, "test", ALLOWED_ROLES_FIELD, new ArrayList<>(), PROPERTIES_FIELD, Map.of(), RESULT_INDEX_FIELD, "")); + Gson gson = new Gson(); + String json = gson.toJson(dataSourceData); + + Map<String, Object> parsedData = XContentParserUtils.toMap(json); + + Assertions.assertEquals(parsedData, dataSourceData); + Assertions.assertEquals("test", parsedData.get(DESCRIPTION_FIELD)); + } + @SneakyThrows @Test public void testToDataSourceMetadataFromJsonWithoutName() { @@ -71,6 +87,22 @@ public void testToDataSourceMetadataFromJsonWithoutName() { Assertions.assertEquals("name and connector are required fields.", exception.getMessage()); } + @SneakyThrows + @Test + public void testToMapFromJsonWithoutName() { + Map<String, Object> dataSourceData = new HashMap<>(Map.of(DESCRIPTION_FIELD, "test")); + Gson gson = new Gson(); + String json = gson.toJson(dataSourceData); + + IllegalArgumentException exception = + assertThrows( + IllegalArgumentException.class, + () -> { + XContentParserUtils.toMap(json); + }); + Assertions.assertEquals("Name is a required field.", exception.getMessage()); + } + @SneakyThrows @Test public void testToDataSourceMetadataFromJsonWithoutConnector() { From 1051d5068a3bb1a6d9202e89282c97a387c5ee11 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Fri, 13 Oct 2023 11:02:09 -0400 Subject: [PATCH 16/21] Fix test Signed-off-by: Derek Ho <dxho@amazon.com> --- .../service/DataSourceServiceImplTest.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java index 1976ea38c0..14047976b0 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java @@ -294,7 +294,16 @@ void testUpdateDefaultDataSource() { @Test void testPatchDataSourceSuccessCase() { - Map<String, Object> dataSourceData = new HashMap<>(Map.of(NAME_FIELD, "test", DESCRIPTION_FIELD, "test")); + // Tests that patch underlying implementation is to call update + Map<String, Object> dataSourceData = new HashMap<>(Map.of(NAME_FIELD, "testDS", DESCRIPTION_FIELD, "test")); + DataSourceMetadata getData = metadata( + "testDS", + DataSourceType.OPENSEARCH, + Collections.emptyList(), + ImmutableMap.of()); + when(dataSourceMetadataStorage.getDataSourceMetadata("testDS")) + .thenReturn( + Optional.ofNullable(getData)); dataSourceService.patchDataSource(dataSourceData); verify(dataSourceMetadataStorage, times(1)).updateDataSourceMetadata(any()); From af4c9242c1d6ca18e1938a199edcbd952857765f Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Fri, 13 Oct 2023 11:23:38 -0400 Subject: [PATCH 17/21] Fix tests and increase code cov Signed-off-by: Derek Ho <dxho@amazon.com> --- .../OpenSearchDataSourceMetadataStorage.java | 12 --------- .../utils/XContentParserUtils.java | 12 ++++----- .../service/DataSourceServiceImplTest.java | 15 ++++------- .../utils/XContentParserUtilsTest.java | 27 ++++++++++++++----- 4 files changed, 31 insertions(+), 35 deletions(-) diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java b/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java index 8707fc5ddd..6659e54342 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/storage/OpenSearchDataSourceMetadataStorage.java @@ -7,8 +7,6 @@ package org.opensearch.sql.datasources.storage; -import static org.opensearch.sql.datasources.utils.XContentParserUtils.PROPERTIES_FIELD; - import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -265,16 +263,6 @@ private DataSourceMetadata encryptDecryptAuthenticationData( return dataSourceMetadata; } - // Encrypt and Decrypt irrespective of auth type.If properties name ends in username, password, - // secret_key and access_key. - private Map<String, Object> encryptDecryptAuthenticationData( - Map<String, Object> dataSourceData, Boolean isEncryption) { - Map<String, String> propertiesMap = (Map<String, String>) dataSourceData.get(PROPERTIES_FIELD); - handleBasicAuthPropertiesEncryptionDecryption(propertiesMap, isEncryption); - handleSigV4PropertiesEncryptionDecryption(propertiesMap, isEncryption); - return dataSourceData; - } - private void handleBasicAuthPropertiesEncryptionDecryption( Map<String, String> propertiesMap, Boolean isEncryption) { ArrayList<String> list = new ArrayList<>(); diff --git a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java index f42a0801c5..6af2a5a761 100644 --- a/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java +++ b/datasources/src/main/java/org/opensearch/sql/datasources/utils/XContentParserUtils.java @@ -171,12 +171,12 @@ public static DataSourceMetadata toDataSourceMetadata(String json) throws IOExce */ public static Map<String, Object> toMap(String json) throws IOException { try (XContentParser parser = - XContentType.JSON - .xContent() - .createParser( - NamedXContentRegistry.EMPTY, - DeprecationHandler.THROW_UNSUPPORTED_OPERATION, - json)) { + XContentType.JSON + .xContent() + .createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + json)) { return toMap(parser); } } diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java index 14047976b0..ec2dddb9cd 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java @@ -266,8 +266,6 @@ void testGetDataSourceMetadataSetWithDefaultDatasource() { @Test void testUpdateDataSourceSuccessCase() { - doNothing().when(dataSourceUserAuthorizationHelper).authorizeDataSource(any()); - DataSourceMetadata dataSourceMetadata = metadata("testDS", DataSourceType.OPENSEARCH, Collections.emptyList(), ImmutableMap.of()); dataSourceService.updateDataSource(dataSourceMetadata); @@ -295,15 +293,12 @@ void testUpdateDefaultDataSource() { @Test void testPatchDataSourceSuccessCase() { // Tests that patch underlying implementation is to call update - Map<String, Object> dataSourceData = new HashMap<>(Map.of(NAME_FIELD, "testDS", DESCRIPTION_FIELD, "test")); - DataSourceMetadata getData = metadata( - "testDS", - DataSourceType.OPENSEARCH, - Collections.emptyList(), - ImmutableMap.of()); + Map<String, Object> dataSourceData = + new HashMap<>(Map.of(NAME_FIELD, "testDS", DESCRIPTION_FIELD, "test")); + DataSourceMetadata getData = + metadata("testDS", DataSourceType.OPENSEARCH, Collections.emptyList(), ImmutableMap.of()); when(dataSourceMetadataStorage.getDataSourceMetadata("testDS")) - .thenReturn( - Optional.ofNullable(getData)); + .thenReturn(Optional.ofNullable(getData)); dataSourceService.patchDataSource(dataSourceData); verify(dataSourceMetadataStorage, times(1)).updateDataSourceMetadata(any()); diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java index a3271e8ee0..70df3196bb 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java @@ -4,7 +4,6 @@ import static org.opensearch.sql.datasources.utils.XContentParserUtils.*; import com.google.gson.Gson; - import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -58,7 +57,21 @@ public void testToDataSourceMetadataFromJson() { @SneakyThrows @Test public void testToMapFromJson() { - Map<String, Object> dataSourceData = new HashMap<>(Map.of(NAME_FIELD, "test_DS", DESCRIPTION_FIELD, "test", ALLOWED_ROLES_FIELD, new ArrayList<>(), PROPERTIES_FIELD, Map.of(), RESULT_INDEX_FIELD, "")); + Map<String, Object> dataSourceData = + new HashMap<>( + Map.of( + NAME_FIELD, + "test_DS", + DESCRIPTION_FIELD, + "test", + ALLOWED_ROLES_FIELD, + new ArrayList<>(), + PROPERTIES_FIELD, + Map.of(), + CONNECTOR_FIELD, + "PROMETHEUS", + RESULT_INDEX_FIELD, + "")); Gson gson = new Gson(); String json = gson.toJson(dataSourceData); @@ -95,11 +108,11 @@ public void testToMapFromJsonWithoutName() { String json = gson.toJson(dataSourceData); IllegalArgumentException exception = - assertThrows( - IllegalArgumentException.class, - () -> { - XContentParserUtils.toMap(json); - }); + assertThrows( + IllegalArgumentException.class, + () -> { + XContentParserUtils.toMap(json); + }); Assertions.assertEquals("Name is a required field.", exception.getMessage()); } From 970b5bf06190f430c713ab1598fc9ff4444302d8 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Fri, 13 Oct 2023 11:27:06 -0400 Subject: [PATCH 18/21] Add more coverage to impl Signed-off-by: Derek Ho <dxho@amazon.com> --- .../service/DataSourceServiceImplTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java index ec2dddb9cd..0448f72db0 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java @@ -290,6 +290,19 @@ void testUpdateDefaultDataSource() { unsupportedOperationException.getMessage()); } + @Test + void testPatchDefaultDataSource() { + Map<String, Object> dataSourceData = + Map.of(NAME_FIELD, DEFAULT_DATASOURCE_NAME, DESCRIPTION_FIELD, "test"); + UnsupportedOperationException unsupportedOperationException = + assertThrows( + UnsupportedOperationException.class, + () -> dataSourceService.patchDataSource(dataSourceData)); + assertEquals( + "Not allowed to update default datasource :" + DEFAULT_DATASOURCE_NAME, + unsupportedOperationException.getMessage()); + } + @Test void testPatchDataSourceSuccessCase() { // Tests that patch underlying implementation is to call update From 721c64fc0958c064f85dfb870318de963bf43aad Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Fri, 13 Oct 2023 12:00:03 -0400 Subject: [PATCH 19/21] Fix test and jacoco passing Signed-off-by: Derek Ho <dxho@amazon.com> --- .../service/DataSourceServiceImplTest.java | 18 +++++- .../utils/XContentParserUtilsTest.java | 60 ++++++++++++++----- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java index 0448f72db0..03c6de359f 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/service/DataSourceServiceImplTest.java @@ -18,8 +18,7 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; -import static org.opensearch.sql.datasources.utils.XContentParserUtils.DESCRIPTION_FIELD; -import static org.opensearch.sql.datasources.utils.XContentParserUtils.NAME_FIELD; +import static org.opensearch.sql.datasources.utils.XContentParserUtils.*; import com.google.common.collect.ImmutableMap; import java.util.ArrayList; @@ -307,7 +306,20 @@ void testPatchDefaultDataSource() { void testPatchDataSourceSuccessCase() { // Tests that patch underlying implementation is to call update Map<String, Object> dataSourceData = - new HashMap<>(Map.of(NAME_FIELD, "testDS", DESCRIPTION_FIELD, "test")); + new HashMap<>( + Map.of( + NAME_FIELD, + "testDS", + DESCRIPTION_FIELD, + "test", + CONNECTOR_FIELD, + "PROMETHEUS", + ALLOWED_ROLES_FIELD, + new ArrayList<>(), + PROPERTIES_FIELD, + Map.of(), + RESULT_INDEX_FIELD, + "")); DataSourceMetadata getData = metadata("testDS", DataSourceType.OPENSEARCH, Collections.emptyList(), ImmutableMap.of()); when(dataSourceMetadataStorage.getDataSourceMetadata("testDS")) diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java index 70df3196bb..42d0e0fea6 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java @@ -58,26 +58,39 @@ public void testToDataSourceMetadataFromJson() { @Test public void testToMapFromJson() { Map<String, Object> dataSourceData = - new HashMap<>( - Map.of( - NAME_FIELD, - "test_DS", - DESCRIPTION_FIELD, - "test", - ALLOWED_ROLES_FIELD, - new ArrayList<>(), - PROPERTIES_FIELD, - Map.of(), - CONNECTOR_FIELD, - "PROMETHEUS", - RESULT_INDEX_FIELD, - "")); + Map.of( + NAME_FIELD, + "test_DS", + DESCRIPTION_FIELD, + "test", + ALLOWED_ROLES_FIELD, + new ArrayList<>(), + PROPERTIES_FIELD, + Map.of(), + CONNECTOR_FIELD, + "PROMETHEUS", + RESULT_INDEX_FIELD, + ""); + + Map<String, Object> dataSourceDataConnectorRemoved = + Map.of( + NAME_FIELD, + "test_DS", + DESCRIPTION_FIELD, + "test", + ALLOWED_ROLES_FIELD, + new ArrayList<>(), + PROPERTIES_FIELD, + Map.of(), + RESULT_INDEX_FIELD, + ""); + Gson gson = new Gson(); String json = gson.toJson(dataSourceData); Map<String, Object> parsedData = XContentParserUtils.toMap(json); - Assertions.assertEquals(parsedData, dataSourceData); + Assertions.assertEquals(parsedData, dataSourceDataConnectorRemoved); Assertions.assertEquals("test", parsedData.get(DESCRIPTION_FIELD)); } @@ -151,4 +164,21 @@ public void testToDataSourceMetadataFromJsonUsingUnknownObject() { }); Assertions.assertEquals("Unknown field: test", exception.getMessage()); } + + @SneakyThrows + @Test + public void testToMapFromJsonUsingUnknownObject() { + HashMap<String, String> hashMap = new HashMap<>(); + hashMap.put("test", "test"); + Gson gson = new Gson(); + String json = gson.toJson(hashMap); + + IllegalArgumentException exception = + assertThrows( + IllegalArgumentException.class, + () -> { + XContentParserUtils.toMap(json); + }); + Assertions.assertEquals("Unknown field: test", exception.getMessage()); + } } From b98f0b3c1d503401b33265232be4b127599a341e Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Fri, 13 Oct 2023 13:53:00 -0400 Subject: [PATCH 20/21] Test fix Signed-off-by: Derek Ho <dxho@amazon.com> --- .../sql/datasources/utils/XContentParserUtilsTest.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java b/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java index 42d0e0fea6..e1e442d12b 100644 --- a/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java +++ b/datasources/src/test/java/org/opensearch/sql/datasources/utils/XContentParserUtilsTest.java @@ -4,7 +4,6 @@ import static org.opensearch.sql.datasources.utils.XContentParserUtils.*; import com.google.gson.Gson; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -64,9 +63,9 @@ public void testToMapFromJson() { DESCRIPTION_FIELD, "test", ALLOWED_ROLES_FIELD, - new ArrayList<>(), + List.of("all_access"), PROPERTIES_FIELD, - Map.of(), + Map.of("prometheus.uri", "localhost:9090"), CONNECTOR_FIELD, "PROMETHEUS", RESULT_INDEX_FIELD, @@ -79,9 +78,9 @@ public void testToMapFromJson() { DESCRIPTION_FIELD, "test", ALLOWED_ROLES_FIELD, - new ArrayList<>(), + List.of("all_access"), PROPERTIES_FIELD, - Map.of(), + Map.of("prometheus.uri", "localhost:9090"), RESULT_INDEX_FIELD, ""); From 87f0605414480b577d297b3966e92c7c9df27248 Mon Sep 17 00:00:00 2001 From: Derek Ho <dxho@amazon.com> Date: Tue, 17 Oct 2023 11:49:09 -0400 Subject: [PATCH 21/21] Add docs Signed-off-by: Derek Ho <dxho@amazon.com> --- docs/user/ppl/admin/datasources.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/user/ppl/admin/datasources.rst b/docs/user/ppl/admin/datasources.rst index bf37cc196d..31378f6cc4 100644 --- a/docs/user/ppl/admin/datasources.rst +++ b/docs/user/ppl/admin/datasources.rst @@ -104,6 +104,8 @@ we can remove authorization and other details in case of security disabled domai "allowedRoles" : ["all_access"] } + **Name is required and must exist. Connector cannot be modified and will be ignored.** + * Datasource Read GET API("_plugins/_query/_datasources/{{dataSourceName}}" :: GET https://localhost:9200/_plugins/_query/_datasources/my_prometheus @@ -125,6 +127,7 @@ Each of the datasource configuration management apis are controlled by following * cluster:admin/opensearch/datasources/create [Create POST API] * cluster:admin/opensearch/datasources/read [Get GET API] * cluster:admin/opensearch/datasources/update [Update PUT API] +* cluster:admin/opensearch/datasources/patch [Update PATCH API] * cluster:admin/opensearch/datasources/delete [Delete DELETE API] Only users mapped with roles having above actions are authorized to execute datasource management apis.