From 63582d8f15e64d8f2938f3d1eb71b7575ca3a9ec Mon Sep 17 00:00:00 2001 From: Mahita Mahesh Date: Thu, 10 Nov 2022 09:56:09 -0500 Subject: [PATCH] Bug fix: Fix NPE when parsing field names in query parser. * Update gitignore for Intellij Signed-off-by: Mahita Mahesh --- .gitignore | 4 ++- .../actionfilter/SearchActionFilter.java | 4 +-- .../preprocess/QueryParser.java | 18 +++++++++++-- .../preprocess/QueryParserTests.java | 26 +++++++++++++++++++ 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index fc343a0..f82106c 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,8 @@ gradle-app.setting # JDT-specific (Eclipse Java Development Tools) .classpath +# Intellij files +.idea/ # Compiled class file *.class @@ -86,4 +88,4 @@ Icon .AppleDesktop Network Trash Folder Temporary Items -.apdisk \ No newline at end of file +.apdisk diff --git a/src/main/java/org/opensearch/search/relevance/actionfilter/SearchActionFilter.java b/src/main/java/org/opensearch/search/relevance/actionfilter/SearchActionFilter.java index 92ba5ac..ca1e6ef 100644 --- a/src/main/java/org/opensearch/search/relevance/actionfilter/SearchActionFilter.java +++ b/src/main/java/org/opensearch/search/relevance/actionfilter/SearchActionFilter.java @@ -251,8 +251,8 @@ public void onResponse(final Response response) { logger.info("Re-ranking overhead time: {}ms", tookInMillis - searchResponse.getTook().getMillis()); } catch (final Exception e) { - logger.error("Failed to parse search response.", e); - throw new OpenSearchException("Failed to parse a search response.", e); + logger.error("Result transformer operations failed.", e); + throw new OpenSearchException("Result transformer operations failed.", e); } } diff --git a/src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParser.java b/src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParser.java index 49dfb7c..5964038 100644 --- a/src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParser.java +++ b/src/main/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParser.java @@ -22,15 +22,25 @@ public class QueryParser { private static final Logger logger = LogManager.getLogger(QueryParser.class); + private static final String BODY_FIELD_REQUIRED_ERROR_MESSAGE = + "Property [" + BODY_FIELD + "] must be specified"; private static final String FILED_MISMATCH_ERROR_MESSAGE = "Mismatch: Field configured in %s property [%s] is not present in query fields [%s]. Will not apply Kendra Intelligent Ranking."; + private static final String QUERY_PARSER_RESULT_LOG = + "Kendra Intelligent Ranker query parser fields for query type [%s]: bodyField: %s, titleField: %s"; + private static final String QUERY_PARSER_RESULT_LOG_WITHOUT_TITLE = + "Kendra Intelligent Ranker query parser fields for query type [%s]: bodyField: %s"; public QueryParserResult parse( final QueryBuilder query, final List bodyFieldSetting, final List titleFieldSetting) { - final String bodyFieldFromSetting = bodyFieldSetting.isEmpty() ? null : bodyFieldSetting.get(0); - final String titleFieldFromSetting = titleFieldSetting.isEmpty() ? null : titleFieldSetting.get(0); + if (bodyFieldSetting == null || bodyFieldSetting.isEmpty()) { + throw new IllegalArgumentException(BODY_FIELD_REQUIRED_ERROR_MESSAGE); + } + + final String bodyFieldFromSetting = bodyFieldSetting.get(0); + final String titleFieldFromSetting = (titleFieldSetting == null || titleFieldSetting.isEmpty()) ? null : titleFieldSetting.get(0); QueryParserResult result = null; if (query instanceof MatchQueryBuilder) { @@ -51,6 +61,8 @@ private QueryParserResult parseMatchQuery(MatchQueryBuilder matchQuery, String b bodyFieldFromSetting, matchQuery.fieldName())); } else { result = new QueryParserResult(matchQuery.value().toString(), bodyFieldFromSetting); + logger.info(String.format(Locale.ENGLISH, QUERY_PARSER_RESULT_LOG_WITHOUT_TITLE, + matchQuery.NAME, result.bodyFieldName)); } return result; } @@ -76,6 +88,8 @@ private QueryParserResult parseMultiMatchQuery(MultiMatchQueryBuilder multiMatch } else { final String titleFieldToUse = configuredTitleFieldPresentInQuery ? titleFieldFromSetting : null; result = new QueryParserResult(multiMatchQuery.value().toString(), bodyFieldFromSetting, titleFieldToUse); + logger.info(String.format(Locale.ENGLISH, QUERY_PARSER_RESULT_LOG, + multiMatchQuery.NAME, result.bodyFieldName, result.titleFieldName)); } return result; } diff --git a/src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParserTests.java b/src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParserTests.java index af27553..5d9b9a4 100644 --- a/src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParserTests.java +++ b/src/test/java/org/opensearch/search/relevance/transformer/kendraintelligentranking/preprocess/QueryParserTests.java @@ -24,6 +24,14 @@ public class QueryParserTests extends OpenSearchTestCase { private static final String INVALID_FIELD = "invalidField"; private QueryParser queryParser = new QueryParser(); + public void testParse_BodyFieldNull_ThrowsException() { + assertThrows(IllegalArgumentException.class, () -> queryParser.parse( + new MatchQueryBuilder(TEST_BODY_FIELD, TEST_QUERY_TEXT), + null, + null) + ); + } + public void testParse_Match_RequestFieldMatchesSetting() { QueryParser.QueryParserResult queryParserResult = queryParser.parse( new MatchQueryBuilder(TEST_BODY_FIELD, TEST_QUERY_TEXT), @@ -33,6 +41,15 @@ public void testParse_Match_RequestFieldMatchesSetting() { performQueryParserResultAssertions(queryParserResult, TEST_QUERY_TEXT, TEST_BODY_FIELD, null); } + public void testParse_Match_TitleFieldSettingNull() { + QueryParser.QueryParserResult queryParserResult = queryParser.parse( + new MatchQueryBuilder(TEST_BODY_FIELD, TEST_QUERY_TEXT), + Arrays.asList(TEST_BODY_FIELD), + null); + + performQueryParserResultAssertions(queryParserResult, TEST_QUERY_TEXT, TEST_BODY_FIELD, null); + } + public void testParse_Match_RequestFieldDoesNotMatchSetting() { QueryParser.QueryParserResult queryParserResult = queryParser.parse( new MatchQueryBuilder(TEST_BODY_FIELD, TEST_QUERY_TEXT), @@ -60,6 +77,15 @@ public void testParse_MultiMatch_OnlyBodyFieldMatchesSetting() { performQueryParserResultAssertions(queryParserResult, TEST_QUERY_TEXT, TEST_BODY_FIELD, null); } + public void testParse_MultiMatch_TitleFieldSettingNull() { + QueryParser.QueryParserResult queryParserResult = queryParser.parse( + new MultiMatchQueryBuilder(TEST_QUERY_TEXT, TEST_BODY_FIELD, TEST_TITLE_FIELD), + Arrays.asList(TEST_BODY_FIELD), + null); + + performQueryParserResultAssertions(queryParserResult, TEST_QUERY_TEXT, TEST_BODY_FIELD, null); + } + public void testParse_MultiMatch_RequestFieldMatchesSetting_WithFieldBoosting() { MultiMatchQueryBuilder multiMatchQueryBuilder = new MultiMatchQueryBuilder(TEST_QUERY_TEXT, TEST_TITLE_FIELD, TEST_BODY_FIELD); multiMatchQueryBuilder.field(TEST_BODY_FIELD, 1.5f);