From 32fa7391c42a64710dc073bb38cbf4611d9ac5aa 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 --- .../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 5b6c9773f..81480bcae 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 @@ -1248,14 +1248,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 ca0c2e162..535983eeb 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 9990e5637..d669d6443 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(); @@ -2449,6 +2462,7 @@ public class MappingElasticsearchConverterUnitTests { return reverse(value); } } + // endregion private static String reverse(Object o) { @@ -2456,5 +2470,4 @@ public class MappingElasticsearchConverterUnitTests { return new StringBuilder().append(o.toString()).reverse().toString(); } - // endregion }