diff --git a/pom.xml b/pom.xml index 03a5c6cc..42a3a6bf 100644 --- a/pom.xml +++ b/pom.xml @@ -192,7 +192,7 @@ au.org.aodn stacmodel - 1.0.3 + 1.0.4 diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/model/enumeration/CQLFields.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/model/enumeration/CQLFields.java index 84520908..839973a5 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/model/enumeration/CQLFields.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/model/enumeration/CQLFields.java @@ -467,6 +467,33 @@ public static SortOptions getDatasetGroupPrioritySort(List candidates) { .build(); } + /** + * Every word of the literal must appear somewhere across title, description and the vocab fields, e.g. for + * "coastal wave data" the word "coastal" may come from an organisation vocab, "wave" from the title and + * "data" from the description. + * cross_fields is term-centric, so the words may come from different fields. best_fields (fuzzy_title and + * fuzzy_desc) is field-centric, its operator AND needs all words in one field. Elastic does not allow fuzziness + * for cross_fields, typos are still handled by fuzzy_title and fuzzy_desc. + * Each word scores its best field plus 0.1 times the others, the same tie_breaker the field and synonyms + * multi_match queries use, applied per word across these fields. + * The .synonyms sub-fields use another analyzer so they would form a separate group that must match all words + * on its own, acronyms are left to fuzzy_title and fuzzy_desc. Credits is long free text that names many + * organisations, so it is left out to avoid noise. + */ + public static Query getAllWordsAcrossFieldsQuery(String literal) { + return MultiMatchQuery.of(m -> m + .type(TextQueryType.CrossFields) + .fields( + StacBasicField.Title.searchField + "^2", + StacBasicField.Description.searchField, + StacBasicField.ParameterVocabs.searchField, + StacBasicField.PlatformVocabs.searchField, + StacBasicField.OrganisationVocabs.searchField) + .tieBreaker(0.1) + .operator(Operator.And) + .query(literal))._toQuery(); + } + @Override public Query getPropertyEqualToQuery(String literal) { if (getOverridePropertyEqualsToQuery() == null) { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearch.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearch.java index 4ad60a9c..37c07f85 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearch.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/ElasticSearch.java @@ -437,25 +437,52 @@ private static List createKeywordShouldClauses(List keywords) { // If search text with double quote, remove quotee, otherwise keeps same String term = isExact ? t.substring(1, t.length() - 1) : t; + List clauses = new ArrayList<>(); if (isExact) { // Match phrase in original title and description, not use fuzzy fields - should.add(CQLFields.title.getPropertyEqualToQuery(term)); - should.add(CQLFields.description.getPropertyEqualToQuery(term)); + clauses.add(CQLFields.title.getPropertyEqualToQuery(term)); + clauses.add(CQLFields.description.getPropertyEqualToQuery(term)); } else { - should.add(CQLFields.fuzzy_title.getPropertyEqualToQuery(term)); - should.add(CQLFields.fuzzy_desc.getPropertyEqualToQuery(term)); + clauses.add(CQLFields.fuzzy_title.getPropertyEqualToQuery(term)); + clauses.add(CQLFields.fuzzy_desc.getPropertyEqualToQuery(term)); + } + clauses.add(CQLFields.parameter_vocabs.getPropertyEqualToQuery(term)); + clauses.add(CQLFields.organisation_vocabs.getPropertyEqualToQuery(term)); + clauses.add(CQLFields.platform_vocabs.getPropertyEqualToQuery(term)); + clauses.add(CQLFields.id.getPropertyEqualToQuery(term)); + // credit_contains is a phrase match, the same for exact and non-exact keywords + clauses.add(CQLFields.credit_contains.getPropertyEqualToQuery(term)); + if (!isExact && hasSeveralWords(term)) { + should.add(bestOfClausesOrAllWordsAcrossFields(term, clauses)); + } + else { + should.addAll(clauses); } - should.add(CQLFields.parameter_vocabs.getPropertyEqualToQuery(term)); - should.add(CQLFields.organisation_vocabs.getPropertyEqualToQuery(term)); - should.add(CQLFields.platform_vocabs.getPropertyEqualToQuery(term)); - should.add(CQLFields.id.getPropertyEqualToQuery(term)); - // credit_contains uses match query by default, exact match is not applied here - should.add(CQLFields.credit_contains.getPropertyEqualToQuery(term)); } return should; } + /** + * For a keyword whose words may be spread across fields, e.g. "coastal wave data" where "coastal" is only in an + * organisation vocab. A record matches if either the keyword's clauses or the all words across fields query + * matches, and keeps the higher of the two scores. The clauses are summed in a bool exactly as when they are added + * one by one, so a record never scores below what the clauses alone give, and each clause is evaluated once. + */ + private static Query bestOfClausesOrAllWordsAcrossFields(String term, List clauses) { + Query keywordClauses = BoolQuery.of(b -> b.should(clauses))._toQuery(); + return DisMaxQuery.of(d -> d + .queries(keywordClauses, CQLFields.getAllWordsAcrossFieldsQuery(term)))._toQuery(); + } + + /** + * A single word found in title, description or a vocab is already matched by fuzzy_title, fuzzy_desc or the vocab + * phrase clause, so matching it across fields cannot find more records. + */ + private static boolean hasSeveralWords(String term) { + return term.trim().split("\\s+").length > 1; + } + /** * Dataset-group candidate values for the priority sort, expanded from the search keywords with the same exact/quoted handling as the should clauses. */ diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/common/RestAdminApiTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/common/RestAdminApiTest.java index 20d993ba..192b7d09 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/common/RestAdminApiTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/common/RestAdminApiTest.java @@ -199,6 +199,33 @@ public void explainSimplifiedFormatReportsMultiWordMatches() throws IOException } } + @Test + public void explainSimplifiedFormatReportsWordsAcrossFields() throws IOException { + // 'queensland' is only in an organisation vocab and 'current velocity' is in the title, so only the + // all words across fields fallback matches, and each word is reported against the field it came from + insertRecordsWithExplicitIds("ae86e2f5-eaaf-459e-a405-e654d85adb9c.json"); + + URI simpleUri = explainUri("q", "queensland current velocity", "format", "simple"); + + ResponseEntity response = testRestTemplate.getForEntity(simpleUri, JsonNode.class); + + assertTrue(response.getStatusCode().is2xxSuccessful()); + JsonNode hit = Objects.requireNonNull(response.getBody()).path("hits").path(0); + assertEquals("ae86e2f5-eaaf-459e-a405-e654d85adb9c", hit.path("id").asText()); + assertTrue(hit.path("es_relevance").asDouble() > 0.0, "the fallback scores the record"); + + List reported = StreamSupport.stream(hit.path("matched_terms").spliterator(), false) + .map(term -> term.path("field").asText() + ":" + term.path("term").asText()) + .toList(); + assertTrue(reported.contains("summaries.organisation_vocabs:queensland"), + "'queensland' must be reported for the organisation vocab, got: " + reported); + // the best of the fields is reported, both words are in the title and may be in the description too + for (String word : List.of("current", "velocity")) { + assertTrue(reported.contains("title:" + word) || reported.contains("description:" + word), + "'" + word + "' must be reported, got: " + reported); + } + } + @Test public void explainSimplifiedFormatReportsTheDatasetGroupPriorityInTheSortValues() throws IOException { // the record sits in the "imos" dataset group and its credits name IMOS, so the search recalls it on the text and the group then ranks it diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/common/RestApiTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/common/RestApiTest.java index d150a9ec..e47c6b52 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/common/RestApiTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/common/RestApiTest.java @@ -133,6 +133,36 @@ public void verifyApiCollectionsQueryOnText2() throws IOException { collections.getBody().getCollections().get(1).getId(), "Correct UUID - 9fdb1eee-bc28-43a9-88c5-972324784837"); } + /** + * The words of one keyword may be spread across title, description and the vocab fields. + * ae86e2f5-eaaf-459e-a405-e654d85adb9c - 'queensland' is only in an organisation vocab, 'current velocity' is in the title + * 9fdb1eee-bc28-43a9-88c5-972324784837 - has none of the words + * + * @throws IOException - IO Exception + */ + @Test + public void verifyApiCollectionsQueryWordsAcrossFields() throws IOException { + super.insertJsonToElasticRecordIndex( + "ae86e2f5-eaaf-459e-a405-e654d85adb9c.json", + "9fdb1eee-bc28-43a9-88c5-972324784837.json" + ); + + // No single field holds all words, only the all words across fields fallback finds it + ResponseEntity collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=queensland current velocity", ExtendedCollections.class); + assertEquals(1, Objects.requireNonNull(collections.getBody()).getTotal(), "Only 1 hit"); + assertEquals( + "ae86e2f5-eaaf-459e-a405-e654d85adb9c", + collections.getBody().getCollections().get(0).getId(), + "Correct UUID - ae86e2f5-eaaf-459e-a405-e654d85adb9c"); + + // Every word must be found somewhere, 'zebra' is in no field + collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=queensland current zebra", ExtendedCollections.class); + assertEquals(0, Objects.requireNonNull(collections.getBody()).getTotal(), "No hit when a word is missing"); + + // A quoted keyword is an exact phrase search, the fallback does not apply + collections = testRestTemplate.getForEntity(getBasePath() + "/collections?q=\"queensland current velocity\"", ExtendedCollections.class); + assertEquals(0, Objects.requireNonNull(collections.getBody()).getTotal(), "No hit for a quoted keyword"); + } /** * The datetime field after xxx/.. xxx/ etc. It uses CQL internally so no need to test Before After During in CQL */ diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/util/ExplainSimplifierTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/util/ExplainSimplifierTest.java index 5510da71..6cc765cc 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/core/util/ExplainSimplifierTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/util/ExplainSimplifierTest.java @@ -122,6 +122,23 @@ public void disMaxReportsHighestChildWhenNoChildEqualsParent() { assertEquals("title", terms.get(0).getField()); } + @Test + public void disMaxOfKeywordClausesAndAcrossFieldsReportsOnlyTheWinningGroup() { + // a multi word keyword: dis_max of the summed keyword clauses and the cross_fields match, each a "sum of:" group + List terms = collectDisMax("max of:", 12.0f, + detail(8.0f, "sum of:", + detail(5.0f, "weight(description:wave in 1) [BM25], result of:"), + detail(3.0f, "weight(description:data in 1) [BM25], result of:")), + detail(12.0f, "sum of:", + detail(9.0f, "weight(title:wave in 1) [BM25], result of:"), + detail(2.0f, "weight(summaries.organisation_vocabs:coastal in 1) [BM25], result of:"), + detail(1.0f, "weight(description:data in 1) [BM25], result of:"))); + + assertEquals(List.of("title:wave", "summaries.organisation_vocabs:coastal", "description:data"), + terms.stream().map(term -> term.getField() + ":" + term.getTerm()).toList(), + "only the terms of the higher scoring group are reported"); + } + @Test public void disMaxWithTieBreakerReportsOnlyTheWinner() { // best_fields multi_match with tie_breaker 0.1: 36.3 + 0.1 * 12.1 diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/service/ElasticSearchTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/service/ElasticSearchTest.java index d05f624b..621ed90c 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/service/ElasticSearchTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/service/ElasticSearchTest.java @@ -5,6 +5,8 @@ import co.elastic.clients.elasticsearch.ElasticsearchClient; import co.elastic.clients.elasticsearch._types.FieldValue; import co.elastic.clients.elasticsearch._types.SortOptions; +import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery; +import co.elastic.clients.elasticsearch._types.query_dsl.DisMaxQuery; import co.elastic.clients.elasticsearch._types.query_dsl.MultiMatchQuery; import co.elastic.clients.elasticsearch._types.query_dsl.Operator; import co.elastic.clients.elasticsearch._types.query_dsl.Query; @@ -17,6 +19,7 @@ import org.junit.jupiter.api.Test; import java.util.List; import java.util.function.Supplier; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -41,7 +44,9 @@ public void searchByParametersWithDoubleQuote() throws Exception { CQLCrsType.EPSG4326); assertEquals(7, capturingSearch.should.size(), - "Exact match should produce 7 queries (title + description + other fields, no dataset_group)"); + "Exact match should produce 7 queries (title + description + other fields, no dataset_group, no fallback)"); + assertTrue(capturingSearch.should.stream().noneMatch(Query::isDisMax), + "the all words across fields match is not used for a quoted keyword"); MultiMatchQuery title = capturingSearch.should.get(0).multiMatch(); assertEquals(TextQueryType.Phrase, title.type(), "Title query should be a phrase multi_match"); @@ -71,9 +76,12 @@ public void searchByParametersWithoutDoubleQuote() throws Exception { "-score,-rank", CQLCrsType.EPSG4326); - assertEquals(7, capturingSearch.should.size(), "Fuzzy match should produce 7 queries"); + assertEquals(1, capturingSearch.should.size(), + "a multi word keyword is one dis_max of its clauses and the all words across fields match"); + List clauses = assertBestOfClausesOrAllWordsAcrossFields(capturingSearch.should.get(0), "ocean temperature"); + assertEquals(7, clauses.size(), "the same 7 clauses as a single word keyword"); - MultiMatchQuery title = capturingSearch.should.get(0).multiMatch(); + MultiMatchQuery title = clauses.get(0).multiMatch(); assertEquals(TextQueryType.BestFields, title.type(), "fuzzy_title should be a best_fields multi_match"); assertEquals(List.of("title^2", "title.synonyms^2"), title.fields()); assertEquals(0.1, title.tieBreaker()); @@ -81,7 +89,7 @@ public void searchByParametersWithoutDoubleQuote() throws Exception { assertEquals(4, title.prefixLength()); assertEquals(Operator.And, title.operator()); - MultiMatchQuery description = capturingSearch.should.get(1).multiMatch(); + MultiMatchQuery description = clauses.get(1).multiMatch(); assertEquals(TextQueryType.BestFields, description.type(), "fuzzy_desc should be a best_fields multi_match"); assertEquals(List.of("description", "description.synonyms"), description.fields()); assertEquals(0.1, description.tieBreaker()); @@ -94,12 +102,85 @@ public void searchByParametersWithoutDoubleQuote() throws Exception { "dataset_group priority sort should be the first sort key"); } + @Test + public void searchByParametersSingleWordHasNoAcrossFieldsMatch() throws Exception { + CapturingElasticSearch capturingSearch = new CapturingElasticSearch(mockClient); + + capturingSearch.searchByParameters( + List.of("temperature"), + null, + null, + "-score,-rank", + CQLCrsType.EPSG4326); + + assertEquals(7, capturingSearch.should.size(), "a single word keyword adds its 7 clauses one by one"); + assertTrue(capturingSearch.should.stream().noneMatch(Query::isDisMax), + "a single word cannot be spread across fields"); + assertEquals(TextQueryType.BestFields, capturingSearch.should.get(0).multiMatch().type()); + } + + @Test + public void searchByParametersAcrossFieldsMatchHoldsOnlyItsOwnKeywordClauses() throws Exception { + CapturingElasticSearch capturingSearch = new CapturingElasticSearch(mockClient); + + capturingSearch.searchByParameters( + List.of("ocean temperature", "wave"), + null, + null, + "-score,-rank", + CQLCrsType.EPSG4326); + + assertEquals(8, capturingSearch.should.size(), "1 dis_max for 'ocean temperature' and 7 clauses for 'wave'"); + List clauses = assertBestOfClausesOrAllWordsAcrossFields(capturingSearch.should.get(0), "ocean temperature"); + assertEquals(7, clauses.size()); + assertTrue(clauses.stream().noneMatch(q -> q.toString().contains("wave")), + "the dis_max must not hold the clauses of another keyword"); + assertTrue(capturingSearch.should.subList(1, 8).stream().noneMatch(Query::isDisMax)); + } + + /** + * A multi word keyword is a dis_max of a bool summing its clauses, as when added one by one, and the + * cross_fields match over title, description and the vocab fields. The record keeps the higher score. + * @return the keyword clauses inside the bool + */ + private static List assertBestOfClausesOrAllWordsAcrossFields(Query query, String keyword) { + assertTrue(query.isDisMax(), "should be a dis_max"); + DisMaxQuery disMax = query.disMax(); + assertNull(disMax.tieBreaker(), "only the higher of the two scores counts"); + assertEquals(2, disMax.queries().size()); + + assertTrue(disMax.queries().get(0).isBool(), "the keyword clauses are summed in a bool"); + BoolQuery clauses = disMax.queries().get(0).bool(); + assertTrue(clauses.must().isEmpty() && clauses.mustNot().isEmpty() && clauses.filter().isEmpty(), + "the clauses are only should clauses, scored as before"); + + MultiMatchQuery crossFields = disMax.queries().get(1).multiMatch(); + assertEquals(TextQueryType.CrossFields, crossFields.type(), "words may come from different fields"); + assertEquals(Operator.And, crossFields.operator(), "every word must be found"); + assertEquals(List.of( + "title^2", + "description", + "summaries.parameter_vocabs", + "summaries.platform_vocabs", + "summaries.organisation_vocabs"), + crossFields.fields()); + assertNull(crossFields.fuzziness(), "fuzziness is not allowed for cross_fields"); + assertEquals(0.1, crossFields.tieBreaker(), "best field per word plus 0.1 times the others"); + assertEquals(keyword, crossFields.query()); + + return clauses.should(); + } + /** * The synonyms sub-fields must only be scored inside the title/description multi_match, a separate clause on them - * would score plain words twice + * would score plain words twice. The clauses inside a dis_max are checked the same way. */ private static void assertSynonymsOnlyInsideMultiMatch(List should) { should.stream() + .flatMap(q -> q.isDisMax() + ? q.disMax().queries().stream() + .flatMap(child -> child.isBool() ? child.bool().should().stream() : Stream.of(child)) + : Stream.of(q)) .filter(q -> !q.isMultiMatch()) .forEach(q -> assertFalse(q.toString().contains(".synonyms"), "unexpected clause on a synonyms field: " + q)); @@ -165,8 +246,11 @@ public void explainByParametersUsesScriptScoreRequestForKeywords() throws Except "title-only _source is lightweight so the larger search_after batch is used"); assertNotNull(capturingSearch.explainRequest.query()); assertTrue(capturingSearch.explainRequest.query().isScriptScore()); - assertEquals(7, capturingSearch.explainRequest.query().scriptScore() - .query().bool().should().size()); + List should = capturingSearch.explainRequest.query().scriptScore().query().bool().should(); + assertEquals(1, should.size(), + "a multi word keyword is one dis_max of its clauses and the all words across fields match"); + assertEquals(7, assertBestOfClausesOrAllWordsAcrossFields(should.get(0), "ocean temperature").size(), + "the same 7 clauses as the search request"); assertNotNull(capturingSearch.explainRequest.source()); assertTrue(capturingSearch.explainRequest.source().isFilter()); assertFalse(capturingSearch.explainRequest.source().filter().includes().isEmpty()); @@ -363,6 +447,7 @@ private CapturingElasticSearch(ElasticsearchClient client) { this.searchAfterSplitRegex = "\\|\\|"; } + @Override protected SearchResult searchCollectionBy( List queries,