From a647fec00b196c27dc475c81118fbb200f562957 Mon Sep 17 00:00:00 2001 From: Amit Galitzky Date: Fri, 2 Dec 2022 00:22:40 +0000 Subject: [PATCH 1/4] adding includes and excludes from source Signed-off-by: Amit Galitzky --- .../ad/rest/AbstractSearchAction.java | 7 ++++++- .../ad/rest/RestSearchAnomalyResultAction.java | 2 +- .../opensearch/ad/util/RestHandlerUtils.java | 17 +++++++++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java b/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java index 3bba68bcd..cc3d64b20 100644 --- a/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java +++ b/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java @@ -82,7 +82,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.parseXContent(request.contentOrSourceParamParser()); - searchSourceBuilder.fetchSource(getSourceContext(request)); + searchSourceBuilder.fetchSource(getSourceContext(request, searchSourceBuilder)); searchSourceBuilder.seqNoAndPrimaryTerm(true).version(true); SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder).indices(this.index); return channel -> client.execute(actionType, searchRequest, search(channel)); @@ -113,6 +113,8 @@ public RestResponse buildResponse(SearchResponse response) throws Exception { return new BytesRestResponse(RestStatus.REQUEST_TIMEOUT, response.toString()); } + System.out.println("response before: " + response); + if (clazz == AnomalyDetector.class) { for (SearchHit hit : response.getHits()) { XContentParser parser = XContentType.JSON @@ -131,6 +133,9 @@ public RestResponse buildResponse(SearchResponse response) throws Exception { } } + System.out.println("response after: " + response); + + return new BytesRestResponse(RestStatus.OK, response.toXContent(channel.newBuilder(), EMPTY_PARAMS)); } }; diff --git a/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java b/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java index 64771181a..f8468bd15 100644 --- a/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java +++ b/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java @@ -70,7 +70,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.parseXContent(request.contentOrSourceParamParser()); - searchSourceBuilder.fetchSource(getSourceContext(request)); + searchSourceBuilder.fetchSource(getSourceContext(request, searchSourceBuilder)); searchSourceBuilder.seqNoAndPrimaryTerm(true).version(true); SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder).indices(this.index); diff --git a/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java b/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java index c11918653..03848f28d 100644 --- a/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java +++ b/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Set; +import org.apache.commons.lang.ArrayUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.OpenSearchStatusException; @@ -43,6 +44,7 @@ import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestStatus; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.search.fetch.subphase.FetchSourceContext; import com.google.common.base.Throwables; @@ -90,11 +92,22 @@ private RestHandlerUtils() {} * If the request came from the client then we exclude the UI Metadata from the search result. * * @param request rest request + * @param searchSourceBuilder an instance of the searchSourceBuolder to fetch _source field * @return instance of {@link org.opensearch.search.fetch.subphase.FetchSourceContext} */ - public static FetchSourceContext getSourceContext(RestRequest request) { + public static FetchSourceContext getSourceContext(RestRequest request, SearchSourceBuilder searchSourceBuilder) { String userAgent = Strings.coalesceToEmpty(request.header("User-Agent")); - if (!userAgent.contains(OPENSEARCH_DASHBOARDS_USER_AGENT)) { + + // if excludes is empty, just add include + // if excludes isn't empty and opensearch_Dashboard_user agent false then uiMetadata to exclude + if (searchSourceBuilder.fetchSource() != null) { + if (userAgent.contains(OPENSEARCH_DASHBOARDS_USER_AGENT)) { + return new FetchSourceContext(true, searchSourceBuilder.fetchSource().includes(), searchSourceBuilder.fetchSource().excludes()); + } else { + String[] newArray = (String[]) ArrayUtils.addAll(searchSourceBuilder.fetchSource().excludes(), UI_METADATA_EXCLUDE); + return new FetchSourceContext(true, searchSourceBuilder.fetchSource().includes(), newArray); + } + } else if (!userAgent.contains(OPENSEARCH_DASHBOARDS_USER_AGENT)) { return new FetchSourceContext(true, Strings.EMPTY_ARRAY, UI_METADATA_EXCLUDE); } else { return null; From fc67d8fcbfa0285cedf90cf73fff7d493adb1be4 Mon Sep 17 00:00:00 2001 From: Amit Galitzky Date: Mon, 5 Dec 2022 18:37:10 +0000 Subject: [PATCH 2/4] adding tests Signed-off-by: Amit Galitzky --- .../opensearch/ad/model/AnomalyDetector.java | 2 +- .../ad/rest/AbstractSearchAction.java | 28 +++++------- .../rest/RestSearchAnomalyResultAction.java | 4 +- .../opensearch/ad/util/RestHandlerUtils.java | 38 ++++++++++++---- .../ad/util/RestHandlerUtilsTests.java | 44 +++++++++++++++++-- 5 files changed, 84 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/opensearch/ad/model/AnomalyDetector.java b/src/main/java/org/opensearch/ad/model/AnomalyDetector.java index 2c069d703..da64b30ea 100644 --- a/src/main/java/org/opensearch/ad/model/AnomalyDetector.java +++ b/src/main/java/org/opensearch/ad/model/AnomalyDetector.java @@ -139,7 +139,7 @@ public class AnomalyDetector implements Writeable, ToXContentObject { * @param detectionInterval detecting interval * @param windowDelay max delay window for realtime data * @param shingleSize number of the most recent time intervals to form a shingled data point - * @param uiMetadata metadata used by Kibana + * @param uiMetadata metadata used by OpenSearch-Dashboards * @param schemaVersion anomaly detector index mapping version * @param lastUpdateTime detector's last update time * @param categoryFields a list of partition fields diff --git a/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java b/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java index cc3d64b20..e68bcb17a 100644 --- a/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java +++ b/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java @@ -12,6 +12,7 @@ package org.opensearch.ad.rest; import static org.opensearch.ad.util.RestHandlerUtils.getSourceContext; +import static org.opensearch.ad.util.RestHandlerUtils.getSourceContextWithSearchSource; import static org.opensearch.common.xcontent.ToXContent.EMPTY_PARAMS; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; @@ -28,11 +29,9 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.ad.constant.CommonErrorMessages; import org.opensearch.ad.model.AnomalyDetector; -import org.opensearch.ad.rest.handler.AnomalyDetectorFunction; import org.opensearch.ad.settings.EnabledSetting; import org.opensearch.client.node.NodeClient; import org.opensearch.common.bytes.BytesReference; -import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.ToXContentObject; import org.opensearch.common.xcontent.XContentBuilder; @@ -82,21 +81,20 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.parseXContent(request.contentOrSourceParamParser()); - searchSourceBuilder.fetchSource(getSourceContext(request, searchSourceBuilder)); + // Currently, if we are searching for detectors we don't consider the given _source because + // we want to keep the same current order of response which is something we have decided too + // in our initial release when excluding UI_Metadata if request didn't originate from OpenSearch-Dashboards + // ref-link: https://github.com/elastic/elasticsearch/issues/17639 + if (clazz != AnomalyDetector.class) { + searchSourceBuilder.fetchSource(getSourceContextWithSearchSource(request, searchSourceBuilder)); + } else { + searchSourceBuilder.fetchSource(getSourceContext(request)); + } searchSourceBuilder.seqNoAndPrimaryTerm(true).version(true); SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder).indices(this.index); return channel -> client.execute(actionType, searchRequest, search(channel)); } - protected void executeWithAdmin(NodeClient client, AnomalyDetectorFunction function, RestChannel channel) { - try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { - function.execute(); - } catch (Exception e) { - logger.error("Failed to execute with admin", e); - onFailure(channel, e); - } - } - protected void onFailure(RestChannel channel, Exception e) { try { channel.sendResponse(new BytesRestResponse(channel, e)); @@ -113,8 +111,6 @@ public RestResponse buildResponse(SearchResponse response) throws Exception { return new BytesRestResponse(RestStatus.REQUEST_TIMEOUT, response.toString()); } - System.out.println("response before: " + response); - if (clazz == AnomalyDetector.class) { for (SearchHit hit : response.getHits()) { XContentParser parser = XContentType.JSON @@ -127,15 +123,13 @@ public RestResponse buildResponse(SearchResponse response) throws Exception { ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); // write back id and version to anomaly detector object + // re-orders Anomaly Detector response JSON to original order after excluding UI_metadata ToXContentObject xContentObject = AnomalyDetector.parse(parser, hit.getId(), hit.getVersion()); XContentBuilder builder = xContentObject.toXContent(jsonBuilder(), EMPTY_PARAMS); hit.sourceRef(BytesReference.bytes(builder)); } } - System.out.println("response after: " + response); - - return new BytesRestResponse(RestStatus.OK, response.toXContent(channel.newBuilder(), EMPTY_PARAMS)); } }; diff --git a/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java b/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java index f8468bd15..893deb2e6 100644 --- a/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java +++ b/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java @@ -13,7 +13,7 @@ import static org.opensearch.ad.indices.AnomalyDetectionIndices.ALL_AD_RESULTS_INDEX_PATTERN; import static org.opensearch.ad.util.RestHandlerUtils.RESULT_INDEX; -import static org.opensearch.ad.util.RestHandlerUtils.getSourceContext; +import static org.opensearch.ad.util.RestHandlerUtils.getSourceContextWithSearchSource; import java.io.IOException; import java.util.Locale; @@ -70,7 +70,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.parseXContent(request.contentOrSourceParamParser()); - searchSourceBuilder.fetchSource(getSourceContext(request, searchSourceBuilder)); + searchSourceBuilder.fetchSource(getSourceContextWithSearchSource(request, searchSourceBuilder)); searchSourceBuilder.seqNoAndPrimaryTerm(true).version(true); SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder).indices(this.index); diff --git a/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java b/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java index 03848f28d..9803c4ea9 100644 --- a/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java +++ b/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java @@ -82,27 +82,47 @@ public final class RestHandlerUtils { public static final String VALIDATE = "_validate"; public static final ToXContent.MapParams XCONTENT_WITH_TYPE = new ToXContent.MapParams(ImmutableMap.of("with_type", "true")); - private static final String OPENSEARCH_DASHBOARDS_USER_AGENT = "OpenSearch Dashboards"; - private static final String[] UI_METADATA_EXCLUDE = new String[] { AnomalyDetector.UI_METADATA_FIELD }; + public static final String OPENSEARCH_DASHBOARDS_USER_AGENT = "OpenSearch Dashboards"; + public static final String[] UI_METADATA_EXCLUDE = new String[] { AnomalyDetector.UI_METADATA_FIELD }; private RestHandlerUtils() {} /** - * Checks to see if the request came from Kibana, if so we want to return the UI Metadata from the document. + * Checks to see if the request came from OpenSearch-Dashboards, if so we want to return the UI Metadata from the document. * If the request came from the client then we exclude the UI Metadata from the search result. - * + * We don't take into account the given _source field in this case + * @param request rest request + * @return instance of {@link org.opensearch.search.fetch.subphase.FetchSourceContext} + */ + public static FetchSourceContext getSourceContext(RestRequest request) { + String userAgent = Strings.coalesceToEmpty(request.header("User-Agent")); + if (!userAgent.contains(OPENSEARCH_DASHBOARDS_USER_AGENT)) { + return new FetchSourceContext(true, Strings.EMPTY_ARRAY, UI_METADATA_EXCLUDE); + } else { + return null; + } + } + + /** + * Checks to see if the request came from OpenSearch-Dashboards, if so we want to return the UI Metadata from the document. + * If the request came from the client then we exclude the UI Metadata from the search result. + * We also take into account the given `_source` field and respect the correct fields to be returned. * @param request rest request - * @param searchSourceBuilder an instance of the searchSourceBuolder to fetch _source field + * @param searchSourceBuilder an instance of the searchSourceBuilder to fetch _source field * @return instance of {@link org.opensearch.search.fetch.subphase.FetchSourceContext} */ - public static FetchSourceContext getSourceContext(RestRequest request, SearchSourceBuilder searchSourceBuilder) { + public static FetchSourceContext getSourceContextWithSearchSource(RestRequest request, SearchSourceBuilder searchSourceBuilder) { String userAgent = Strings.coalesceToEmpty(request.header("User-Agent")); - // if excludes is empty, just add include - // if excludes isn't empty and opensearch_Dashboard_user agent false then uiMetadata to exclude + // If there is a _source given in request than we either add UI_Metadata to exclude or not depending on if request + // is from OpenSearch-Dashboards, if no _source field then we either exclude UI_metadata or return nothing at all. if (searchSourceBuilder.fetchSource() != null) { if (userAgent.contains(OPENSEARCH_DASHBOARDS_USER_AGENT)) { - return new FetchSourceContext(true, searchSourceBuilder.fetchSource().includes(), searchSourceBuilder.fetchSource().excludes()); + return new FetchSourceContext( + true, + searchSourceBuilder.fetchSource().includes(), + searchSourceBuilder.fetchSource().excludes() + ); } else { String[] newArray = (String[]) ArrayUtils.addAll(searchSourceBuilder.fetchSource().excludes(), UI_METADATA_EXCLUDE); return new FetchSourceContext(true, searchSourceBuilder.fetchSource().includes(), newArray); diff --git a/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java b/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java index 0e37109ef..525da2c68 100644 --- a/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java +++ b/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java @@ -13,6 +13,8 @@ import static org.opensearch.ad.TestHelpers.builder; import static org.opensearch.ad.TestHelpers.randomFeature; +import static org.opensearch.ad.util.RestHandlerUtils.OPENSEARCH_DASHBOARDS_USER_AGENT; +import static org.opensearch.ad.util.RestHandlerUtils.UI_METADATA_EXCLUDE; import java.io.IOException; @@ -24,6 +26,7 @@ import org.opensearch.common.xcontent.XContentParser; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.search.fetch.subphase.FetchSourceContext; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.rest.FakeRestChannel; @@ -37,16 +40,51 @@ public class RestHandlerUtilsTests extends OpenSearchTestCase { public void testGetSourceContext() { RestRequest request = new FakeRestRequest(); FetchSourceContext context = RestHandlerUtils.getSourceContext(request); - assertArrayEquals(new String[] { "ui_metadata" }, context.excludes()); + assertArrayEquals(UI_METADATA_EXCLUDE, context.excludes()); } - public void testGetSourceContextForKibana() { + public void testGetSourceContextFromOpenSearchDashboards() { FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); - builder.withHeaders(ImmutableMap.of("User-Agent", ImmutableList.of("OpenSearch Dashboards", randomAlphaOfLength(10)))); + builder.withHeaders(ImmutableMap.of("User-Agent", ImmutableList.of(OPENSEARCH_DASHBOARDS_USER_AGENT, randomAlphaOfLength(10)))); FetchSourceContext context = RestHandlerUtils.getSourceContext(builder.build()); assertNull(context); } + public void testGetSourceContextWithSearchSourceFromOpenSearchDashboardEmptyExcludes() { + FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); + builder.withHeaders(ImmutableMap.of("User-Agent", ImmutableList.of(OPENSEARCH_DASHBOARDS_USER_AGENT, randomAlphaOfLength(10)))); + SearchSourceBuilder testSearchSourceBuilder = new SearchSourceBuilder(); + testSearchSourceBuilder.fetchSource(new String[] { "a" }, new String[0]); + FetchSourceContext sourceContext = RestHandlerUtils.getSourceContextWithSearchSource(builder.build(), testSearchSourceBuilder); + assertArrayEquals(new String[] { "a" }, sourceContext.includes()); + assertEquals(0, sourceContext.excludes().length); + assertEquals(1, sourceContext.includes().length); + } + + public void testGetSourceContextWithSearchSourceFromClientWithExcludes() { + FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); + SearchSourceBuilder testSearchSourceBuilder = new SearchSourceBuilder(); + testSearchSourceBuilder.fetchSource(new String[] { "a" }, new String[] { "b" }); + FetchSourceContext sourceContext = RestHandlerUtils.getSourceContextWithSearchSource(builder.build(), testSearchSourceBuilder); + assertEquals(sourceContext.excludes().length, 2); + } + + public void testGetSourceContextWithSearchSourceFromClientWithoutSource() { + FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); + SearchSourceBuilder testSearchSourceBuilder = new SearchSourceBuilder(); + FetchSourceContext sourceContext = RestHandlerUtils.getSourceContextWithSearchSource(builder.build(), testSearchSourceBuilder); + assertEquals(sourceContext.excludes().length, 1); + assertEquals(sourceContext.includes().length, 0); + } + + public void testGetSourceContextWithSearchSourceOpenSearchDashboardWithoutSources() { + FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); + builder.withHeaders(ImmutableMap.of("User-Agent", ImmutableList.of(OPENSEARCH_DASHBOARDS_USER_AGENT, randomAlphaOfLength(10)))); + SearchSourceBuilder testSearchSourceBuilder = new SearchSourceBuilder(); + FetchSourceContext sourceContext = RestHandlerUtils.getSourceContextWithSearchSource(builder.build(), testSearchSourceBuilder); + assertNull(sourceContext); + } + public void testCreateXContentParser() throws IOException { RestRequest request = new FakeRestRequest(); RestChannel channel = new FakeRestChannel(request, false, 1); From 5805d5b2b4f3fdb2afa25d448bc7c32c7b800a74 Mon Sep 17 00:00:00 2001 From: Amit Galitzky Date: Tue, 6 Dec 2022 20:48:50 +0000 Subject: [PATCH 3/4] enforce _source in detector search, allow json re-order Signed-off-by: Amit Galitzky --- .../ad/rest/AbstractSearchAction.java | 32 ++----------------- .../rest/RestSearchAnomalyResultAction.java | 4 +-- .../opensearch/ad/util/RestHandlerUtils.java | 24 +++----------- .../ad/util/RestHandlerUtilsTests.java | 29 +++++------------ 4 files changed, 17 insertions(+), 72 deletions(-) diff --git a/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java b/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java index e68bcb17a..f49fbe730 100644 --- a/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java +++ b/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java @@ -12,7 +12,6 @@ package org.opensearch.ad.rest; import static org.opensearch.ad.util.RestHandlerUtils.getSourceContext; -import static org.opensearch.ad.util.RestHandlerUtils.getSourceContextWithSearchSource; import static org.opensearch.common.xcontent.ToXContent.EMPTY_PARAMS; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; @@ -81,15 +80,10 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.parseXContent(request.contentOrSourceParamParser()); - // Currently, if we are searching for detectors we don't consider the given _source because - // we want to keep the same current order of response which is something we have decided too - // in our initial release when excluding UI_Metadata if request didn't originate from OpenSearch-Dashboards + // order of response will be re-arranged everytime we use `_source`, we sometimes do this + // even if user doesn't give this field as we exclude ui_metadata if request isn't from OSD // ref-link: https://github.com/elastic/elasticsearch/issues/17639 - if (clazz != AnomalyDetector.class) { - searchSourceBuilder.fetchSource(getSourceContextWithSearchSource(request, searchSourceBuilder)); - } else { - searchSourceBuilder.fetchSource(getSourceContext(request)); - } + searchSourceBuilder.fetchSource(getSourceContext(request, searchSourceBuilder)); searchSourceBuilder.seqNoAndPrimaryTerm(true).version(true); SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder).indices(this.index); return channel -> client.execute(actionType, searchRequest, search(channel)); @@ -110,26 +104,6 @@ public RestResponse buildResponse(SearchResponse response) throws Exception { if (response.isTimedOut()) { return new BytesRestResponse(RestStatus.REQUEST_TIMEOUT, response.toString()); } - - if (clazz == AnomalyDetector.class) { - for (SearchHit hit : response.getHits()) { - XContentParser parser = XContentType.JSON - .xContent() - .createParser( - channel.request().getXContentRegistry(), - LoggingDeprecationHandler.INSTANCE, - hit.getSourceAsString() - ); - ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); - - // write back id and version to anomaly detector object - // re-orders Anomaly Detector response JSON to original order after excluding UI_metadata - ToXContentObject xContentObject = AnomalyDetector.parse(parser, hit.getId(), hit.getVersion()); - XContentBuilder builder = xContentObject.toXContent(jsonBuilder(), EMPTY_PARAMS); - hit.sourceRef(BytesReference.bytes(builder)); - } - } - return new BytesRestResponse(RestStatus.OK, response.toXContent(channel.newBuilder(), EMPTY_PARAMS)); } }; diff --git a/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java b/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java index 893deb2e6..f8468bd15 100644 --- a/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java +++ b/src/main/java/org/opensearch/ad/rest/RestSearchAnomalyResultAction.java @@ -13,7 +13,7 @@ import static org.opensearch.ad.indices.AnomalyDetectionIndices.ALL_AD_RESULTS_INDEX_PATTERN; import static org.opensearch.ad.util.RestHandlerUtils.RESULT_INDEX; -import static org.opensearch.ad.util.RestHandlerUtils.getSourceContextWithSearchSource; +import static org.opensearch.ad.util.RestHandlerUtils.getSourceContext; import java.io.IOException; import java.util.Locale; @@ -70,7 +70,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); searchSourceBuilder.parseXContent(request.contentOrSourceParamParser()); - searchSourceBuilder.fetchSource(getSourceContextWithSearchSource(request, searchSourceBuilder)); + searchSourceBuilder.fetchSource(getSourceContext(request, searchSourceBuilder)); searchSourceBuilder.seqNoAndPrimaryTerm(true).version(true); SearchRequest searchRequest = new SearchRequest().source(searchSourceBuilder).indices(this.index); diff --git a/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java b/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java index 9803c4ea9..d82f402f9 100644 --- a/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java +++ b/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java @@ -87,22 +87,6 @@ public final class RestHandlerUtils { private RestHandlerUtils() {} - /** - * Checks to see if the request came from OpenSearch-Dashboards, if so we want to return the UI Metadata from the document. - * If the request came from the client then we exclude the UI Metadata from the search result. - * We don't take into account the given _source field in this case - * @param request rest request - * @return instance of {@link org.opensearch.search.fetch.subphase.FetchSourceContext} - */ - public static FetchSourceContext getSourceContext(RestRequest request) { - String userAgent = Strings.coalesceToEmpty(request.header("User-Agent")); - if (!userAgent.contains(OPENSEARCH_DASHBOARDS_USER_AGENT)) { - return new FetchSourceContext(true, Strings.EMPTY_ARRAY, UI_METADATA_EXCLUDE); - } else { - return null; - } - } - /** * Checks to see if the request came from OpenSearch-Dashboards, if so we want to return the UI Metadata from the document. * If the request came from the client then we exclude the UI Metadata from the search result. @@ -111,7 +95,7 @@ public static FetchSourceContext getSourceContext(RestRequest request) { * @param searchSourceBuilder an instance of the searchSourceBuilder to fetch _source field * @return instance of {@link org.opensearch.search.fetch.subphase.FetchSourceContext} */ - public static FetchSourceContext getSourceContextWithSearchSource(RestRequest request, SearchSourceBuilder searchSourceBuilder) { + public static FetchSourceContext getSourceContext(RestRequest request, SearchSourceBuilder searchSourceBuilder) { String userAgent = Strings.coalesceToEmpty(request.header("User-Agent")); // If there is a _source given in request than we either add UI_Metadata to exclude or not depending on if request @@ -119,9 +103,9 @@ public static FetchSourceContext getSourceContextWithSearchSource(RestRequest re if (searchSourceBuilder.fetchSource() != null) { if (userAgent.contains(OPENSEARCH_DASHBOARDS_USER_AGENT)) { return new FetchSourceContext( - true, - searchSourceBuilder.fetchSource().includes(), - searchSourceBuilder.fetchSource().excludes() + true, + searchSourceBuilder.fetchSource().includes(), + searchSourceBuilder.fetchSource().excludes() ); } else { String[] newArray = (String[]) ArrayUtils.addAll(searchSourceBuilder.fetchSource().excludes(), UI_METADATA_EXCLUDE); diff --git a/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java b/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java index 525da2c68..5a2dc0bee 100644 --- a/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java +++ b/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java @@ -37,51 +37,38 @@ public class RestHandlerUtilsTests extends OpenSearchTestCase { - public void testGetSourceContext() { - RestRequest request = new FakeRestRequest(); - FetchSourceContext context = RestHandlerUtils.getSourceContext(request); - assertArrayEquals(UI_METADATA_EXCLUDE, context.excludes()); - } - - public void testGetSourceContextFromOpenSearchDashboards() { - FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); - builder.withHeaders(ImmutableMap.of("User-Agent", ImmutableList.of(OPENSEARCH_DASHBOARDS_USER_AGENT, randomAlphaOfLength(10)))); - FetchSourceContext context = RestHandlerUtils.getSourceContext(builder.build()); - assertNull(context); - } - - public void testGetSourceContextWithSearchSourceFromOpenSearchDashboardEmptyExcludes() { + public void testGetSourceContextFromOpenSearchDashboardEmptyExcludes() { FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); builder.withHeaders(ImmutableMap.of("User-Agent", ImmutableList.of(OPENSEARCH_DASHBOARDS_USER_AGENT, randomAlphaOfLength(10)))); SearchSourceBuilder testSearchSourceBuilder = new SearchSourceBuilder(); testSearchSourceBuilder.fetchSource(new String[] { "a" }, new String[0]); - FetchSourceContext sourceContext = RestHandlerUtils.getSourceContextWithSearchSource(builder.build(), testSearchSourceBuilder); + FetchSourceContext sourceContext = RestHandlerUtils.getSourceContext(builder.build(), testSearchSourceBuilder); assertArrayEquals(new String[] { "a" }, sourceContext.includes()); assertEquals(0, sourceContext.excludes().length); assertEquals(1, sourceContext.includes().length); } - public void testGetSourceContextWithSearchSourceFromClientWithExcludes() { + public void testGetSourceContextFromClientWithExcludes() { FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); SearchSourceBuilder testSearchSourceBuilder = new SearchSourceBuilder(); testSearchSourceBuilder.fetchSource(new String[] { "a" }, new String[] { "b" }); - FetchSourceContext sourceContext = RestHandlerUtils.getSourceContextWithSearchSource(builder.build(), testSearchSourceBuilder); + FetchSourceContext sourceContext = RestHandlerUtils.getSourceContext(builder.build(), testSearchSourceBuilder); assertEquals(sourceContext.excludes().length, 2); } - public void testGetSourceContextWithSearchSourceFromClientWithoutSource() { + public void testGetSourceContextFromClientWithoutSource() { FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); SearchSourceBuilder testSearchSourceBuilder = new SearchSourceBuilder(); - FetchSourceContext sourceContext = RestHandlerUtils.getSourceContextWithSearchSource(builder.build(), testSearchSourceBuilder); + FetchSourceContext sourceContext = RestHandlerUtils.getSourceContext(builder.build(), testSearchSourceBuilder); assertEquals(sourceContext.excludes().length, 1); assertEquals(sourceContext.includes().length, 0); } - public void testGetSourceContextWithSearchSourceOpenSearchDashboardWithoutSources() { + public void testGetSourceContextOpenSearchDashboardWithoutSources() { FakeRestRequest.Builder builder = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY); builder.withHeaders(ImmutableMap.of("User-Agent", ImmutableList.of(OPENSEARCH_DASHBOARDS_USER_AGENT, randomAlphaOfLength(10)))); SearchSourceBuilder testSearchSourceBuilder = new SearchSourceBuilder(); - FetchSourceContext sourceContext = RestHandlerUtils.getSourceContextWithSearchSource(builder.build(), testSearchSourceBuilder); + FetchSourceContext sourceContext = RestHandlerUtils.getSourceContext(builder.build(), testSearchSourceBuilder); assertNull(sourceContext); } From 6b307419ac138e19aabbf2bff4ad7fbf7aedecf1 Mon Sep 17 00:00:00 2001 From: Amit Galitzky Date: Tue, 6 Dec 2022 20:50:15 +0000 Subject: [PATCH 4/4] formatting Signed-off-by: Amit Galitzky --- .../org/opensearch/ad/rest/AbstractSearchAction.java | 9 --------- .../java/org/opensearch/ad/util/RestHandlerUtils.java | 6 +++--- .../org/opensearch/ad/util/RestHandlerUtilsTests.java | 1 - 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java b/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java index f49fbe730..07c55e6df 100644 --- a/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java +++ b/src/main/java/org/opensearch/ad/rest/AbstractSearchAction.java @@ -13,8 +13,6 @@ import static org.opensearch.ad.util.RestHandlerUtils.getSourceContext; import static org.opensearch.common.xcontent.ToXContent.EMPTY_PARAMS; -import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; -import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; import java.io.IOException; import java.util.ArrayList; @@ -27,15 +25,9 @@ import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchResponse; import org.opensearch.ad.constant.CommonErrorMessages; -import org.opensearch.ad.model.AnomalyDetector; import org.opensearch.ad.settings.EnabledSetting; import org.opensearch.client.node.NodeClient; -import org.opensearch.common.bytes.BytesReference; -import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.ToXContentObject; -import org.opensearch.common.xcontent.XContentBuilder; -import org.opensearch.common.xcontent.XContentParser; -import org.opensearch.common.xcontent.XContentType; import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; @@ -43,7 +35,6 @@ import org.opensearch.rest.RestResponse; import org.opensearch.rest.RestStatus; import org.opensearch.rest.action.RestResponseListener; -import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; /** diff --git a/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java b/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java index d82f402f9..36dc76a28 100644 --- a/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java +++ b/src/main/java/org/opensearch/ad/util/RestHandlerUtils.java @@ -103,9 +103,9 @@ public static FetchSourceContext getSourceContext(RestRequest request, SearchSou if (searchSourceBuilder.fetchSource() != null) { if (userAgent.contains(OPENSEARCH_DASHBOARDS_USER_AGENT)) { return new FetchSourceContext( - true, - searchSourceBuilder.fetchSource().includes(), - searchSourceBuilder.fetchSource().excludes() + true, + searchSourceBuilder.fetchSource().includes(), + searchSourceBuilder.fetchSource().excludes() ); } else { String[] newArray = (String[]) ArrayUtils.addAll(searchSourceBuilder.fetchSource().excludes(), UI_METADATA_EXCLUDE); diff --git a/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java b/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java index 5a2dc0bee..08089a11f 100644 --- a/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java +++ b/src/test/java/org/opensearch/ad/util/RestHandlerUtilsTests.java @@ -14,7 +14,6 @@ import static org.opensearch.ad.TestHelpers.builder; import static org.opensearch.ad.TestHelpers.randomFeature; import static org.opensearch.ad.util.RestHandlerUtils.OPENSEARCH_DASHBOARDS_USER_AGENT; -import static org.opensearch.ad.util.RestHandlerUtils.UI_METADATA_EXCLUDE; import java.io.IOException;