diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f5f4c12..58ad7254 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) * Added APIs and components to implement running scheduled experiments ([#220](https://github.com/opensearch-project/search-relevance/pull/220)) ### Enhancements +* Support for adding description in Search Configuration ([#293](https://github.com/opensearch-project/search-relevance/pull/293)) ### Bug Fixes diff --git a/src/main/java/org/opensearch/searchrelevance/dao/SearchConfigurationDao.java b/src/main/java/org/opensearch/searchrelevance/dao/SearchConfigurationDao.java index a909c325..b920932c 100644 --- a/src/main/java/org/opensearch/searchrelevance/dao/SearchConfigurationDao.java +++ b/src/main/java/org/opensearch/searchrelevance/dao/SearchConfigurationDao.java @@ -186,7 +186,8 @@ private SearchConfiguration convertToSearchConfiguration(SearchResponse response (String) source.get(SearchConfiguration.TIME_STAMP), (String) source.get(SearchConfiguration.INDEX), (String) source.get(SearchConfiguration.QUERY), - (String) source.get(SearchConfiguration.SEARCH_PIPELINE) + (String) source.get(SearchConfiguration.SEARCH_PIPELINE), + (String) source.get(SearchConfiguration.DESCRIPTION) ); } } diff --git a/src/main/java/org/opensearch/searchrelevance/executors/ExperimentRunningManager.java b/src/main/java/org/opensearch/searchrelevance/executors/ExperimentRunningManager.java index 17e16b84..ce5399a2 100644 --- a/src/main/java/org/opensearch/searchrelevance/executors/ExperimentRunningManager.java +++ b/src/main/java/org/opensearch/searchrelevance/executors/ExperimentRunningManager.java @@ -98,7 +98,7 @@ public class ExperimentRunningManager { /** * Starts the experiment by setting up cancellation callback for the cancellation token and also retrieves the queryset. * - * One thing to be careful about is that only at most scheduled run for a given scheduled experiment run id + * One thing to be careful about is that only at most scheduled run for a given scheduled experiment run id * should be in the system at all times. * @param experimentId - the id of the experiment to be run * @param request - required parameters for placing a request to start an experiment @@ -329,7 +329,8 @@ private SearchConfiguration convertToSearchConfiguration(SearchResponse response (String) source.get("timestamp"), (String) source.get("index"), (String) source.get("query"), - (String) source.get("searchPipeline") + (String) source.get("searchPipeline"), + (String) source.get("description") ); } diff --git a/src/main/java/org/opensearch/searchrelevance/model/SearchConfiguration.java b/src/main/java/org/opensearch/searchrelevance/model/SearchConfiguration.java index 84e3d982..c4b577c2 100644 --- a/src/main/java/org/opensearch/searchrelevance/model/SearchConfiguration.java +++ b/src/main/java/org/opensearch/searchrelevance/model/SearchConfiguration.java @@ -18,6 +18,7 @@ public class SearchConfiguration implements ToXContentObject { public static final String ID = "id"; public static final String NAME = "name"; + public static final String DESCRIPTION = "description"; public static final String TIME_STAMP = "timestamp"; public static final String INDEX = "index"; @@ -29,18 +30,28 @@ public class SearchConfiguration implements ToXContentObject { */ private final String id; private final String name; + private final String description; private final String timestamp; private final String index; private final String query; private final String searchPipeline; - public SearchConfiguration(String id, String name, String timestamp, String index, String query, String searchPipeline) { + public SearchConfiguration( + String id, + String name, + String timestamp, + String index, + String query, + String searchPipeline, + String description + ) { this.id = id; this.name = name; this.timestamp = timestamp; this.index = index; this.query = query; this.searchPipeline = searchPipeline; + this.description = description; } @Override @@ -52,6 +63,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws xContentBuilder.field(INDEX, this.index.trim()); xContentBuilder.field(QUERY, this.query.trim()); xContentBuilder.field(SEARCH_PIPELINE, this.searchPipeline == null ? "" : this.searchPipeline.trim()); + xContentBuilder.field(DESCRIPTION, this.description == null ? "" : this.description.trim()); return xContentBuilder.endObject(); } diff --git a/src/main/java/org/opensearch/searchrelevance/rest/RestPutSearchConfigurationAction.java b/src/main/java/org/opensearch/searchrelevance/rest/RestPutSearchConfigurationAction.java index b6e305e5..a0159f5f 100644 --- a/src/main/java/org/opensearch/searchrelevance/rest/RestPutSearchConfigurationAction.java +++ b/src/main/java/org/opensearch/searchrelevance/rest/RestPutSearchConfigurationAction.java @@ -9,6 +9,7 @@ import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.PUT; +import static org.opensearch.searchrelevance.common.PluginConstants.DESCRIPTION; import static org.opensearch.searchrelevance.common.PluginConstants.INDEX; import static org.opensearch.searchrelevance.common.PluginConstants.NAME; import static org.opensearch.searchrelevance.common.PluginConstants.QUERY; @@ -72,11 +73,28 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli new BytesRestResponse(RestStatus.BAD_REQUEST, "Invalid name: " + nameValidation.getErrorMessage()) ); } + + String description = (String) source.get(DESCRIPTION); + if (description != null) { + TextValidationUtil.ValidationResult descriptionValidation = TextValidationUtil.validateDescription(description); + if (!descriptionValidation.isValid()) { + return channel -> channel.sendResponse( + new BytesRestResponse(RestStatus.BAD_REQUEST, "Invalid description: " + descriptionValidation.getErrorMessage()) + ); + } + } + String index = (String) source.get(INDEX); String queryBody = (String) source.get(QUERY); String searchPipeline = (String) source.getOrDefault(SEARCH_PIPELINE, ""); - PutSearchConfigurationRequest createRequest = new PutSearchConfigurationRequest(name, index, queryBody, searchPipeline); + PutSearchConfigurationRequest createRequest = new PutSearchConfigurationRequest( + name, + index, + queryBody, + searchPipeline, + description + ); return channel -> client.execute(PutSearchConfigurationAction.INSTANCE, createRequest, new ActionListener() { @Override diff --git a/src/main/java/org/opensearch/searchrelevance/transport/searchConfiguration/PutSearchConfigurationRequest.java b/src/main/java/org/opensearch/searchrelevance/transport/searchConfiguration/PutSearchConfigurationRequest.java index 0b5c5031..501f9a94 100644 --- a/src/main/java/org/opensearch/searchrelevance/transport/searchConfiguration/PutSearchConfigurationRequest.java +++ b/src/main/java/org/opensearch/searchrelevance/transport/searchConfiguration/PutSearchConfigurationRequest.java @@ -19,12 +19,14 @@ public class PutSearchConfigurationRequest extends ActionRequest { private final String index; private final String queryBody; private final String searchPipeline; + private final String description; - public PutSearchConfigurationRequest(String name, String index, String queryBody, String searchPipeline) { + public PutSearchConfigurationRequest(String name, String index, String queryBody, String searchPipeline, String description) { this.name = name; this.index = index; this.queryBody = queryBody; this.searchPipeline = searchPipeline; + this.description = description; } public PutSearchConfigurationRequest(StreamInput in) throws IOException { @@ -33,6 +35,7 @@ public PutSearchConfigurationRequest(StreamInput in) throws IOException { this.index = in.readString(); this.queryBody = in.readString(); this.searchPipeline = in.readOptionalString(); + this.description = in.readOptionalString(); } @Override @@ -42,6 +45,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(index); out.writeString(queryBody); out.writeOptionalString(searchPipeline); + out.writeOptionalString(description); } public String getName() { @@ -60,6 +64,10 @@ public String getSearchPipeline() { return searchPipeline; } + public String getDescription() { + return description; + } + @Override public ActionRequestValidationException validate() { return null; diff --git a/src/main/java/org/opensearch/searchrelevance/transport/searchConfiguration/PutSearchConfigurationTransportAction.java b/src/main/java/org/opensearch/searchrelevance/transport/searchConfiguration/PutSearchConfigurationTransportAction.java index 2739068c..33cff71d 100644 --- a/src/main/java/org/opensearch/searchrelevance/transport/searchConfiguration/PutSearchConfigurationTransportAction.java +++ b/src/main/java/org/opensearch/searchrelevance/transport/searchConfiguration/PutSearchConfigurationTransportAction.java @@ -55,6 +55,8 @@ protected void doExecute(Task task, PutSearchConfigurationRequest request, Actio listener.onFailure(new SearchRelevanceException("Name cannot be null or empty. Request: " + request, RestStatus.BAD_REQUEST)); return; } + String description = request.getDescription(); + String index = request.getIndex(); String queryBody = request.getQueryBody(); String searchPipeline = request.getSearchPipeline(); @@ -62,7 +64,15 @@ protected void doExecute(Task task, PutSearchConfigurationRequest request, Actio StepListener createIndexStep = new StepListener<>(); searchConfigurationDao.createIndexIfAbsent(createIndexStep); createIndexStep.whenComplete(v -> { - SearchConfiguration searchConfiguration = new SearchConfiguration(id, name, timestamp, index, queryBody, searchPipeline); + SearchConfiguration searchConfiguration = new SearchConfiguration( + id, + name, + timestamp, + index, + queryBody, + searchPipeline, + description + ); searchConfigurationDao.putSearchConfiguration(searchConfiguration, listener); }, listener::onFailure); } diff --git a/src/test/java/org/opensearch/searchrelevance/action/searchConfiguration/PutSearchConfigurationActionTests.java b/src/test/java/org/opensearch/searchrelevance/action/searchConfiguration/PutSearchConfigurationActionTests.java index 474ddd03..2ae9e6f2 100644 --- a/src/test/java/org/opensearch/searchrelevance/action/searchConfiguration/PutSearchConfigurationActionTests.java +++ b/src/test/java/org/opensearch/searchrelevance/action/searchConfiguration/PutSearchConfigurationActionTests.java @@ -17,7 +17,13 @@ public class PutSearchConfigurationActionTests extends OpenSearchTestCase { public void testStreams() throws IOException { - PutSearchConfigurationRequest request = new PutSearchConfigurationRequest("base_line", "sample_id", "{\"match_all\":{}}", "n/a"); + PutSearchConfigurationRequest request = new PutSearchConfigurationRequest( + "base_line", + "sample_id", + "{\"match_all\":{}}", + "n/a", + "sample description" + ); BytesStreamOutput output = new BytesStreamOutput(); request.writeTo(output); StreamInput in = StreamInput.wrap(output.bytes().toBytesRef().bytes); @@ -26,10 +32,17 @@ public void testStreams() throws IOException { assertEquals("sample_id", serialized.getIndex()); assertEquals("{\"match_all\":{}}", serialized.getQueryBody()); assertEquals("n/a", serialized.getSearchPipeline()); + assertEquals("sample description", serialized.getDescription()); } public void testRequestValidation() { - PutSearchConfigurationRequest request = new PutSearchConfigurationRequest("base_line", "sample_id", "{\"match_all\":{}}", "n/a"); + PutSearchConfigurationRequest request = new PutSearchConfigurationRequest( + "base_line", + "sample_id", + "{\"match_all\":{}}", + "n/a", + "sample description" + ); assertNull(request.validate()); } } diff --git a/src/test/java/org/opensearch/searchrelevance/action/searchConfiguration/SearchConfigurationIT.java b/src/test/java/org/opensearch/searchrelevance/action/searchConfiguration/SearchConfigurationIT.java index ee1a8ca4..422bba0d 100644 --- a/src/test/java/org/opensearch/searchrelevance/action/searchConfiguration/SearchConfigurationIT.java +++ b/src/test/java/org/opensearch/searchrelevance/action/searchConfiguration/SearchConfigurationIT.java @@ -68,6 +68,7 @@ public void testMainActions_whenCreateReadDeleteSearchConfig_thenSuccessful() { assertEquals("test_index", source.get("index")); assertEquals("{\"query\": {\"match_all\": {}}}", source.get("query")); assertEquals("test_pipeline", source.get("searchPipeline")); + assertEquals("sample description", source.get("description")); Response deleteSearchConfigResponse = makeRequest( client(), diff --git a/src/test/java/org/opensearch/searchrelevance/executors/ExperimentRunningManagerTests.java b/src/test/java/org/opensearch/searchrelevance/executors/ExperimentRunningManagerTests.java index 3b2685e4..3824b81a 100644 --- a/src/test/java/org/opensearch/searchrelevance/executors/ExperimentRunningManagerTests.java +++ b/src/test/java/org/opensearch/searchrelevance/executors/ExperimentRunningManagerTests.java @@ -187,7 +187,7 @@ public void testConcurrentFutureMapUpdates() { } public void testStartExperimentRunReject() { - // Make sure that only at most one experiment run for a given scheduled experiment run id + // Make sure that only at most one experiment run for a given scheduled experiment run id // can be scheduled at a given time // Also makes sure the entry in the mapping futures is removed when the cancellation token is cancelled. PutExperimentRequest request = createExperimentRequest(); diff --git a/src/test/resources/searchconfig/CreateSearchConfiguration.json b/src/test/resources/searchconfig/CreateSearchConfiguration.json index 9a71c835..6445d2a4 100644 --- a/src/test/resources/searchconfig/CreateSearchConfiguration.json +++ b/src/test/resources/searchconfig/CreateSearchConfiguration.json @@ -2,5 +2,6 @@ "name": "test_search_config", "index": "test_index", "query": "{\"query\": {\"match_all\": {}}}", - "searchPipeline": "test_pipeline" + "searchPipeline": "test_pipeline", + "description": "sample description" }