From 120eed02ee648adb2607f33487aa837da204314c Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Sat, 13 Mar 2021 17:50:33 +0100 Subject: [PATCH] Polishing. --- .../elasticsearch/core/RequestFactory.java | 14 +- .../core/query/AbstractQuery.java | 6 + .../core/query/NativeSearchQueryBuilder.java | 5 +- .../data/elasticsearch/core/query/Query.java | 11 +- .../core/query/RescorerQuery.java | 20 ++- .../core/ElasticsearchTemplateTests.java | 2 +- .../core/RequestFactoryTests.java | 130 +++++------------- 7 files changed, 71 insertions(+), 117 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java b/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java index 3be0ee0ef..f8c3145d0 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java @@ -355,7 +355,8 @@ class RequestFactory { // region index management - public CreateIndexRequest createIndexRequest(IndexCoordinates index, @Nullable Document settings, @Nullable Document mapping) { + public CreateIndexRequest createIndexRequest(IndexCoordinates index, @Nullable Document settings, + @Nullable Document mapping) { CreateIndexRequest request = new CreateIndexRequest(index.getIndexName()); if (settings != null && !settings.isEmpty()) { @@ -1053,8 +1054,7 @@ class RequestFactory { sourceBuilder.searchAfter(query.getSearchAfter().toArray()); } - query.getRescorerQueries().forEach(rescorer -> sourceBuilder.addRescorer( - getQueryRescorerBuilder(rescorer))); + query.getRescorerQueries().forEach(rescorer -> sourceBuilder.addRescorer(getQueryRescorerBuilder(rescorer))); request.source(sourceBuilder); return request; @@ -1142,8 +1142,7 @@ class RequestFactory { searchRequestBuilder.searchAfter(query.getSearchAfter().toArray()); } - query.getRescorerQueries().forEach(rescorer -> searchRequestBuilder.addRescorer( - getQueryRescorerBuilder(rescorer))); + query.getRescorerQueries().forEach(rescorer -> searchRequestBuilder.addRescorer(getQueryRescorerBuilder(rescorer))); return searchRequestBuilder; } @@ -1272,7 +1271,10 @@ class RequestFactory { private QueryRescorerBuilder getQueryRescorerBuilder(RescorerQuery rescorerQuery) { - QueryRescorerBuilder builder = new QueryRescorerBuilder(Objects.requireNonNull(getQuery(rescorerQuery.getQuery()))); + QueryBuilder queryBuilder = getQuery(rescorerQuery.getQuery()); + Assert.notNull("queryBuilder", "Could not build query for rescorerQuery"); + + QueryRescorerBuilder builder = new QueryRescorerBuilder(queryBuilder); if (rescorerQuery.getScoreMode() != ScoreMode.Default) { builder.setScoreMode(QueryRescoreMode.valueOf(rescorerQuery.getScoreMode().name())); diff --git a/src/main/java/org/springframework/data/elasticsearch/core/query/AbstractQuery.java b/src/main/java/org/springframework/data/elasticsearch/core/query/AbstractQuery.java index 252638ef5..8384d757a 100755 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/AbstractQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/query/AbstractQuery.java @@ -300,11 +300,17 @@ abstract class AbstractQuery implements Query { @Override public void addRescorerQuery(RescorerQuery rescorerQuery) { + + Assert.notNull(rescorerQuery, "rescorerQuery must not be null"); + this.rescorerQueries.add(rescorerQuery); } @Override public void setRescorerQueries(List rescorerQueryList) { + + Assert.notNull(rescorerQueries, "rescorerQueries must not be null"); + this.rescorerQueries.clear(); this.rescorerQueries.addAll(rescorerQueryList); } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/query/NativeSearchQueryBuilder.java b/src/main/java/org/springframework/data/elasticsearch/core/query/NativeSearchQueryBuilder.java index 7a74f41bf..6d8852e35 100755 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/NativeSearchQueryBuilder.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/query/NativeSearchQueryBuilder.java @@ -20,7 +20,6 @@ import static org.springframework.util.CollectionUtils.*; import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.stream.Collectors; import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.support.IndicesOptions; @@ -29,7 +28,6 @@ import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; import org.elasticsearch.search.collapse.CollapseBuilder; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; -import org.elasticsearch.search.rescore.QueryRescorerBuilder; import org.elasticsearch.search.sort.SortBuilder; import org.springframework.data.domain.Pageable; import org.springframework.lang.Nullable; @@ -265,8 +263,7 @@ public class NativeSearchQueryBuilder { } if (!isEmpty(rescorerQueries)) { - nativeSearchQuery.setRescorerQueries( - rescorerQueries); + nativeSearchQuery.setRescorerQueries(rescorerQueries); } return nativeSearchQuery; diff --git a/src/main/java/org/springframework/data/elasticsearch/core/query/Query.java b/src/main/java/org/springframework/data/elasticsearch/core/query/Query.java index ee926d5dd..25aeb0bf0 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/Query.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/query/Query.java @@ -314,9 +314,9 @@ public interface Query { List getSearchAfter(); /** - * Sets the {@link RescorerQuery}. + * Adds a {@link RescorerQuery}. * - * @param rescorerQuery the query to add to the list of rescorer queries + * @param rescorerQuery the query to add to the list of rescorer queries, must not be {@literal null} * @since 4.2 */ void addRescorerQuery(RescorerQuery rescorerQuery); @@ -324,11 +324,16 @@ public interface Query { /** * Sets the {@link RescorerQuery}. * - * @param rescorerQueryList list of rescorer queries set + * @param rescorerQueryList list of rescorer queries set, must not be {@literal null}. * @since 4.2 */ void setRescorerQueries(List rescorerQueryList); + /** + * get the list of {@link RescorerQuery}s + * + * @since 4.2 + */ default List getRescorerQueries() { return Collections.emptyList(); } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/query/RescorerQuery.java b/src/main/java/org/springframework/data/elasticsearch/core/query/RescorerQuery.java index b8be4b9a6..0d25b17f8 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/RescorerQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/query/RescorerQuery.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021 the original author or authors. + * Copyright 2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,9 +15,8 @@ */ package org.springframework.data.elasticsearch.core.query; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.search.rescore.QueryRescorerBuilder; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * Implementation of RescorerQuery to be used for rescoring filtered search results. @@ -34,6 +33,9 @@ public class RescorerQuery { @Nullable private Float rescoreQueryWeight; public RescorerQuery(Query query) { + + Assert.notNull(query, "query must not be null"); + this.query = query; } @@ -61,6 +63,9 @@ public class RescorerQuery { } public RescorerQuery withScoreMode(ScoreMode scoreMode) { + + Assert.notNull(scoreMode, "scoreMode must not be null"); + this.scoreMode = scoreMode; return this; } @@ -80,15 +85,8 @@ public class RescorerQuery { return this; } - - public enum ScoreMode { - Default, - Avg, - Max, - Min, - Total, - Multiply + Default, Avg, Max, Min, Total, Multiply } } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateTests.java b/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateTests.java index 915cab29b..68fd8085b 100755 --- a/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateTests.java @@ -3132,7 +3132,7 @@ public abstract class ElasticsearchTemplateTests { assertThat(highlightField.get(1)).contains("message"); } - @Test + @Test // #1686 void shouldRunRescoreQueryInSearchQuery() { IndexCoordinates index = IndexCoordinates.of("test-index-rescore-entity-template"); diff --git a/src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java b/src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java index 9aaf7cd31..b06b7edbf 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java @@ -45,12 +45,10 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.index.query.MatchPhraseQueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder; import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder.FilterFunctionBuilder; import org.elasticsearch.index.query.functionscore.GaussDecayFunctionBuilder; -import org.elasticsearch.index.query.functionscore.ScriptScoreQueryBuilder; import org.json.JSONException; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; @@ -522,37 +520,24 @@ class RequestFactoryTests { return XContentHelper.toXContent(request, XContentType.JSON, true).utf8ToString(); } - @Test + @Test // #1686 void shouldBuildSearchWithRescorerQuery() throws JSONException { CriteriaQuery query = new CriteriaQuery(new Criteria("lastName").is("Smith")); - RescorerQuery rescorerQuery = new RescorerQuery( new NativeSearchQueryBuilder() // - .withQuery( - QueryBuilders.functionScoreQuery(new FunctionScoreQueryBuilder.FilterFunctionBuilder[]{ + RescorerQuery rescorerQuery = new RescorerQuery(new NativeSearchQueryBuilder() // + .withQuery(QueryBuilders + .functionScoreQuery(new FunctionScoreQueryBuilder.FilterFunctionBuilder[] { new FilterFunctionBuilder(QueryBuilders.existsQuery("someField"), - new GaussDecayFunctionBuilder("someField", 0, 100000.0, null, 0.683) - .setWeight(5.022317f)), + new GaussDecayFunctionBuilder("someField", 0, 100000.0, null, 0.683).setWeight(5.022317f)), new FilterFunctionBuilder(QueryBuilders.existsQuery("anotherField"), new GaussDecayFunctionBuilder("anotherField", "202102", "31536000s", null, 0.683) - .setWeight(4.170836f))}) - .scoreMode(FunctionScoreQuery.ScoreMode.SUM) - .maxBoost(50.0f) - .boostMode(CombineFunction.AVG) - .boost(1.5f)) - .build() - ) - .withWindowSize(50) - .withQueryWeight(2.0f) - .withRescoreQueryWeight(5.0f) - .withScoreMode(ScoreMode.Multiply); + .setWeight(4.170836f)) }) + .scoreMode(FunctionScoreQuery.ScoreMode.SUM).maxBoost(50.0f).boostMode(CombineFunction.AVG).boost(1.5f)) + .build()).withWindowSize(50).withQueryWeight(2.0f).withRescoreQueryWeight(5.0f) + .withScoreMode(ScoreMode.Multiply); RescorerQuery anotherRescorerQuery = new RescorerQuery(new NativeSearchQueryBuilder() // - .withQuery( - QueryBuilders.matchPhraseQuery("message", "the quick brown").slop(2)) - .build() - ) - .withWindowSize(100) - .withQueryWeight(0.7f) - .withRescoreQueryWeight(1.2f); + .withQuery(QueryBuilders.matchPhraseQuery("message", "the quick brown").slop(2)).build()).withWindowSize(100) + .withQueryWeight(0.7f).withRescoreQueryWeight(1.2f); query.addRescorerQuery(rescorerQuery); query.addRescorerQuery(anotherRescorerQuery); @@ -574,79 +559,40 @@ class RequestFactoryTests { " ]" + // " }" + // " }," + // - " \"rescore\": [{\n" - + " \"window_size\" : 100,\n" - + " \"query\" : {\n" - + " \"rescore_query\" : {\n" - + " \"match_phrase\" : {\n" - + " \"message\" : {\n" - + " \"query\" : \"the quick brown\",\n" - + " \"slop\" : 2\n" - + " }\n" - + " }\n" - + " },\n" - + " \"query_weight\" : 0.7,\n" - + " \"rescore_query_weight\" : 1.2\n" - + " }\n" - + " }," - + " {\n" - + " \"window_size\": 50,\n" - + " \"query\": {\n" - + " \"rescore_query\": {\n" - + " \"function_score\": {\n" - + " \"query\": {\n" - + " \"match_all\": {\n" - + " \"boost\": 1.0\n" - + " }\n" - + " },\n" - + " \"functions\": [\n" - + " {\n" - + " \"filter\": {\n" + " \"rescore\": [{\n" + " \"window_size\" : 100,\n" + " \"query\" : {\n" + + " \"rescore_query\" : {\n" + " \"match_phrase\" : {\n" + " \"message\" : {\n" + + " \"query\" : \"the quick brown\",\n" + " \"slop\" : 2\n" + + " }\n" + " }\n" + " },\n" + " \"query_weight\" : 0.7,\n" + + " \"rescore_query_weight\" : 1.2\n" + " }\n" + " }," + " {\n" + " \"window_size\": 50,\n" + + " \"query\": {\n" + " \"rescore_query\": {\n" + " \"function_score\": {\n" + + " \"query\": {\n" + " \"match_all\": {\n" + + " \"boost\": 1.0\n" + " }\n" + + " },\n" + " \"functions\": [\n" + + " {\n" + " \"filter\": {\n" + " \"exists\": {\n" + " \"field\": \"someField\",\n" - + " \"boost\": 1.0\n" - + " }\n" - + " },\n" - + " \"weight\": 5.022317,\n" - + " \"gauss\": {\n" - + " \"someField\": {\n" + + " \"boost\": 1.0\n" + " }\n" + + " },\n" + " \"weight\": 5.022317,\n" + + " \"gauss\": {\n" + " \"someField\": {\n" + " \"origin\": 0.0,\n" + " \"scale\": 100000.0,\n" - + " \"decay\": 0.683\n" - + " },\n" - + " \"multi_value_mode\": \"MIN\"\n" - + " }\n" - + " },\n" - + " {\n" - + " \"filter\": {\n" - + " \"exists\": {\n" + + " \"decay\": 0.683\n" + " },\n" + + " \"multi_value_mode\": \"MIN\"\n" + " }\n" + + " },\n" + " {\n" + + " \"filter\": {\n" + " \"exists\": {\n" + " \"field\": \"anotherField\",\n" - + " \"boost\": 1.0\n" - + " }\n" - + " },\n" - + " \"weight\": 4.170836,\n" - + " \"gauss\": {\n" - + " \"anotherField\": {\n" + + " \"boost\": 1.0\n" + " }\n" + + " },\n" + " \"weight\": 4.170836,\n" + + " \"gauss\": {\n" + " \"anotherField\": {\n" + " \"origin\": \"202102\",\n" + " \"scale\": \"31536000s\",\n" - + " \"decay\": 0.683\n" - + " },\n" - + " \"multi_value_mode\": \"MIN\"\n" - + " }\n" - + " }\n" - + " ],\n" - + " \"score_mode\": \"sum\",\n" - + " \"boost_mode\": \"avg\",\n" - + " \"max_boost\": 50.0,\n" - + " \"boost\": 1.5\n" - + " }\n" - + " },\n" - + " \"query_weight\": 2.0," - + " \"rescore_query_weight\": 5.0," - + " \"score_mode\": \"multiply\"" - + " }\n" - + " }\n" - + " ]\n" + + " \"decay\": 0.683\n" + " },\n" + + " \"multi_value_mode\": \"MIN\"\n" + " }\n" + + " }\n" + " ],\n" + + " \"score_mode\": \"sum\",\n" + " \"boost_mode\": \"avg\",\n" + + " \"max_boost\": 50.0,\n" + " \"boost\": 1.5\n" + + " }\n" + " },\n" + " \"query_weight\": 2.0," + + " \"rescore_query_weight\": 5.0," + " \"score_mode\": \"multiply\"" + " }\n" + " }\n" + " ]\n" + '}'; String searchRequest = requestFactory.searchRequest(query, Person.class, IndexCoordinates.of("persons")).source()