From f66af53480be1713f22664ba1dc40057aa58c985 Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Wed, 9 Feb 2022 22:45:38 +0100 Subject: [PATCH] Fix exception on property conversion with criteria exists/empty/non-empty. Original Pull Request #2081 Closes #2080 (cherry picked from commit 32fa7391c42a64710dc073bb38cbf4611d9ac5aa) --- .../MappingElasticsearchConverter.java | 19 +++++++++------ .../elasticsearch/core/query/Criteria.java | 23 +++++++++++++++---- ...appingElasticsearchConverterUnitTests.java | 15 +++++++++++- 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java index ca26df40e..8fbba40c6 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java @@ -1256,14 +1256,19 @@ public class MappingElasticsearchConverter PropertyValueConverter propertyValueConverter = Objects .requireNonNull(persistentProperty.getPropertyValueConverter()); criteria.getQueryCriteriaEntries().forEach(criteriaEntry -> { - Object value = criteriaEntry.getValue(); - if (value.getClass().isArray()) { - Object[] objects = (Object[]) value; - for (int i = 0; i < objects.length; i++) { - objects[i] = propertyValueConverter.write(objects[i]); + + if (criteriaEntry.getKey().hasValue()) { + Object value = criteriaEntry.getValue(); + + if (value.getClass().isArray()) { + Object[] objects = (Object[]) value; + + for (int i = 0; i < objects.length; i++) { + objects[i] = propertyValueConverter.write(objects[i]); + } + } else { + criteriaEntry.setValue(propertyValueConverter.write(value)); } - } else { - criteriaEntry.setValue(propertyValueConverter.write(value)); } }); } 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 145c5d070..88cdecb83 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 @@ -954,7 +954,23 @@ public class Criteria { /** * @since 4.3 */ - NOT_EMPTY + NOT_EMPTY; + + /** + * @return true if this key does not have an associated value + * @since 4.4 + */ + public boolean hasNoValue() { + return this == OperationKey.EXISTS || this == OperationKey.EMPTY || this == OperationKey.NOT_EMPTY; + } + + /** + * @return true if this key does have an associated value + * @since 4.4 + */ + public boolean hasValue() { + return !hasNoValue(); + } } /** @@ -967,9 +983,8 @@ public class Criteria { protected CriteriaEntry(OperationKey key) { - boolean keyIsValid = key == OperationKey.EXISTS || key == OperationKey.EMPTY || key == OperationKey.NOT_EMPTY; - Assert.isTrue(keyIsValid, - "key must be OperationKey.EXISTS, OperationKey.EMPTY or OperationKey.EMPTY for this call"); + Assert.isTrue(key.hasNoValue(), + "key must be OperationKey.EXISTS, OperationKey.EMPTY or OperationKey.NOT_EMPTY for this call"); this.key = key; } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverterUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverterUnitTests.java index 23e52bee3..7d388c394 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverterUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverterUnitTests.java @@ -66,6 +66,9 @@ import org.springframework.data.elasticsearch.core.geo.GeoJsonPolygon; import org.springframework.data.elasticsearch.core.geo.GeoPoint; import org.springframework.data.elasticsearch.core.mapping.PropertyValueConverter; import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext; +import org.springframework.data.elasticsearch.core.query.Criteria; +import org.springframework.data.elasticsearch.core.query.CriteriaQuery; +import org.springframework.data.elasticsearch.core.query.Query; import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm; import org.springframework.data.geo.Box; import org.springframework.data.geo.Circle; @@ -1496,6 +1499,16 @@ public class MappingElasticsearchConverterUnitTests { assertThat(entity.getDontConvert()).isEqualTo("Monty Python's Flying Circus"); } + @Test // #2080 + @DisplayName("should not try to call property converter on updating criteria exists") + void shouldNotTryToCallPropertyConverterOnUpdatingCriteriaExists() { + + // don't care if the query makes no sense, we just add all criteria without values + Query query = new CriteriaQuery(Criteria.where("fieldWithClassBasedConverter").exists().empty().notEmpty()); + + mappingElasticsearchConverter.updateQuery(query, EntityWithCustomValueConverters.class); + } + private Map writeToMap(Object source) { Document sink = Document.create(); @@ -2429,6 +2442,7 @@ public class MappingElasticsearchConverterUnitTests { return reverse(value); } } + // endregion private static String reverse(Object o) { @@ -2436,5 +2450,4 @@ public class MappingElasticsearchConverterUnitTests { return new StringBuilder().append(o.toString()).reverse().toString(); } - // endregion }