diff --git a/src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java b/src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java index 03d7e1ef9..d398d7ee1 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.elasticsearch.core.geo.GeoBox; @@ -859,6 +860,7 @@ private List toCollection(Object... values) { // endregion + // region equals/hashcode @Override public boolean equals(Object o) { if (this == o) @@ -874,6 +876,8 @@ public boolean equals(Object o) { return false; if (!Objects.equals(field, criteria.field)) return false; + if (!criteriaChain.filter(this).equals(criteria.criteriaChain.filter(criteria))) + return false; if (!queryCriteriaEntries.equals(criteria.queryCriteriaEntries)) return false; if (!filterCriteriaEntries.equals(criteria.filterCriteriaEntries)) @@ -886,11 +890,16 @@ public int hashCode() { int result = field != null ? field.hashCode() : 0; result = 31 * result + (boost != +0.0f ? Float.floatToIntBits(boost) : 0); result = 31 * result + (negating ? 1 : 0); + // the criteriaChain contains "this" object, so we need to filter it out + // to avoid a stackoverflow here, because the hashcode implementation + // uses the element's hashcodes + result = 31 * result + criteriaChain.filter(this).hashCode(); result = 31 * result + queryCriteriaEntries.hashCode(); result = 31 * result + filterCriteriaEntries.hashCode(); result = 31 * result + subCriteria.hashCode(); return result; } + // endregion @Override public String toString() { @@ -938,7 +947,17 @@ public Operator getOperator() { * * @since 4.1 */ - public static class CriteriaChain extends LinkedList {} + public static class CriteriaChain extends LinkedList { + /** + * return a copy of this list with the given element filtered out. + * + * @param criteria the element to filter + * @return the filtered list + */ + List filter(Criteria criteria) { + return this.stream().filter(c -> c != criteria).collect(Collectors.toList()); + } + } /** * Operator to join the entries of the criteria chain diff --git a/src/test/java/org/springframework/data/elasticsearch/client/elc/CriteriaQueryMappingUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/client/elc/CriteriaQueryMappingUnitTests.java index 88f984614..e53a33df6 100644 --- a/src/test/java/org/springframework/data/elasticsearch/client/elc/CriteriaQueryMappingUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/client/elc/CriteriaQueryMappingUnitTests.java @@ -15,6 +15,7 @@ */ package org.springframework.data.elasticsearch.client.elc; +import static org.assertj.core.api.Assertions.*; import static org.skyscreamer.jsonassert.JSONAssert.*; import static org.springframework.data.elasticsearch.client.elc.JsonUtils.*; @@ -22,6 +23,7 @@ import co.elastic.clients.json.jackson.JacksonJsonpMapper; import java.time.LocalDate; +import java.util.ArrayList; import java.util.Collections; import java.util.Date; import java.util.List; @@ -445,6 +447,108 @@ void shouldMapNamesInSourceStoredFields() { softly.assertAll(); } + // the following test failed because of a wrong implementation in Criteria + // equals and hscode methods. + @Test // #3083 + @DisplayName("should map correct subcriteria") + void shouldMapCorrectSubcriteria() throws JSONException { + Criteria criteria = new Criteria("first").is("hello"); + + List criterias = new ArrayList<>(); + criterias.add(new Criteria().or("second").exists()); + + List subCriterias = new ArrayList<>(); + subCriterias.add(new Criteria("third").exists() + .and(new Criteria("fourth").is("ciao"))); + subCriterias.add(new Criteria("third").exists() + .and(new Criteria("fourth").is("hi"))); + + Criteria result = Criteria.or(); + + for (Criteria c : criterias) { + result = result.or(c); + } + + for (Criteria c : subCriterias) { + result = result.subCriteria(c); + } + criteria = criteria.subCriteria(result); + CriteriaQuery criteriaQuery = new CriteriaQuery(criteria); + + String expected = """ + { + "bool": { + "must": [ + { + "query_string": { + "default_operator": "and", + "fields": [ + "first" + ], + "query": "hello" + } + }, + { + "bool": { + "should": [ + { + "exists": { + "field": "second" + } + }, + { + "bool": { + "must": [ + { + "exists": { + "field": "third" + } + }, + { + "query_string": { + "default_operator": "and", + "fields": [ + "fourth" + ], + "query": "ciao" + } + } + ] + } + }, + { + "bool": { + "must": [ + { + "exists": { + "field": "third" + } + }, + { + "query_string": { + "default_operator": "and", + "fields": [ + "fourth" + ], + "query": "hi" + } + } + ] + } + } + ] + } + } + ] + } + } + """; + + mappingElasticsearchConverter.updateQuery(criteriaQuery, Person.class); + var queryString = queryToJson(CriteriaQueryProcessor.createQuery(criteriaQuery.getCriteria()), mapper); + + assertEquals(expected, queryString, false); + } // endregion // region helper functions