From 86634ceb38dbc9d1a3985171a4bf12a6d5912612 Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Mon, 29 Aug 2022 20:14:11 +0200 Subject: [PATCH] Fix mapping of property values into a collection. When reading from Elasticsearch into a property of type Collection (List or Set) the MappingElasticsearchConverter now can read both from the returned JSON: an array of T objects - will put the objects in a corresponding collection a single T object will put the single object into a corrsponding colletcion This is implemented and tested for both: entities where the properties have setters and immutable classes that only provide an all-args constructor. Original Pull Request #2282 Closes #2280 --- .../MappingElasticsearchConverter.java | 85 +++- ...appingElasticsearchConverterUnitTests.java | 455 ++++++++++++++++++ 2 files changed, 521 insertions(+), 19 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 da5d8f37e..6f10508c8 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 @@ -201,7 +201,7 @@ public class MappingElasticsearchConverter } /** - * Class to do the actual writing. The methods originally were in the MappingElasticsearchConverter class, but are + * Class to do the actual reading. The methods originally were in the MappingElasticsearchConverter class, but are * refactored to allow for keeping state during the conversion of an object. */ private static class Reader extends Base { @@ -220,10 +220,17 @@ public class MappingElasticsearchConverter } @SuppressWarnings("unchecked") + /** + * Reads the given source into the given type. + * + * @param type they type to convert the given source to. + * @param source the source to create an object of the given type from. + * @return the object that was read + */ R read(Class type, Document source) { - TypeInformation typeHint = TypeInformation.of((Class) ClassUtils.getUserClass(type)); - R r = read(typeHint, source); + TypeInformation typeInformation = TypeInformation.of((Class) ClassUtils.getUserClass(type)); + R r = read(typeInformation, source); if (r == null) { throw new ConversionException("could not convert into object of class " + type); @@ -234,11 +241,11 @@ public class MappingElasticsearchConverter @Nullable @SuppressWarnings("unchecked") - private R read(TypeInformation type, Map source) { + private R read(TypeInformation typeInformation, Map source) { Assert.notNull(source, "Source must not be null!"); - TypeInformation typeToUse = typeMapper.readType(source, type); + TypeInformation typeToUse = typeMapper.readType(source, typeInformation); Class rawType = typeToUse.getType(); if (conversions.hasCustomReadTarget(source.getClass(), rawType)) { @@ -256,8 +263,8 @@ public class MappingElasticsearchConverter if (typeToUse.equals(TypeInformation.OBJECT)) { return (R) source; } - // Retrieve persistent entity info + // Retrieve persistent entity info ElasticsearchPersistentEntity entity = mappingContext.getPersistentEntity(typeToUse); if (entity == null) { @@ -372,7 +379,6 @@ public class MappingElasticsearchConverter } return result; - } private ParameterValueProvider getParameterProvider( @@ -462,12 +468,44 @@ public class MappingElasticsearchConverter } else if (value.getClass().isArray()) { return (T) readCollectionOrArray(type, Arrays.asList((Object[]) value)); } else if (value instanceof Map) { + + TypeInformation collectionComponentType = getCollectionComponentType(type); + if (collectionComponentType != null) { + Object o = read(collectionComponentType, (Map) value); + return getCollectionWithSingleElement(type, collectionComponentType, o); + } return (T) read(type, (Map) value); } else { + + TypeInformation collectionComponentType = getCollectionComponentType(type); + if (collectionComponentType != null + && collectionComponentType.isAssignableFrom(TypeInformation.of(value.getClass()))) { + Object o = getPotentiallyConvertedSimpleRead(value, collectionComponentType); + return getCollectionWithSingleElement(type, collectionComponentType, o); + } + return (T) getPotentiallyConvertedSimpleRead(value, rawType); } } + @SuppressWarnings("unchecked") + private static T getCollectionWithSingleElement(TypeInformation collectionType, + TypeInformation componentType, Object element) { + Collection collection = CollectionFactory.createCollection(collectionType.getType(), + componentType.getType(), 1); + collection.add(element); + return (T) collection; + } + + /** + * @param type the type to check + * @return true if type is a collectoin, null otherwise, + */ + @Nullable + TypeInformation getCollectionComponentType(TypeInformation type) { + return type.isCollectionLike() ? type.getComponentType() : null; + } + private Object propertyConverterRead(ElasticsearchPersistentProperty property, Object source) { PropertyValueConverter propertyValueConverter = Objects.requireNonNull(property.getPropertyValueConverter()); @@ -1148,17 +1186,18 @@ public class MappingElasticsearchConverter Assert.notNull(query, "query must not be null"); - if (domainClass != null) { + if (domainClass == null) { + return; + } - updateFieldsAndSourceFilter(query, domainClass); + updatePropertiesInFieldsAndSourceFilter(query, domainClass); - if (query instanceof CriteriaQuery) { - updateCriteriaQuery((CriteriaQuery) query, domainClass); - } + if (query instanceof CriteriaQuery) { + updatePropertiesInCriteriaQuery((CriteriaQuery) query, domainClass); } } - private void updateFieldsAndSourceFilter(Query query, Class domainClass) { + private void updatePropertiesInFieldsAndSourceFilter(Query query, Class domainClass) { ElasticsearchPersistentEntity persistentEntity = mappingContext.getPersistentEntity(domainClass); @@ -1196,14 +1235,22 @@ public class MappingElasticsearchConverter } } - private List updateFieldNames(List fields, ElasticsearchPersistentEntity persistentEntity) { - return fields.stream().map(fieldName -> { + /** + * relaces the fieldName with the property name of a property of the persistentEntity with the corresponding + * fieldname. If no such property exists, the original fieldName is kept. + * + * @param fieldNames list of fieldnames + * @param persistentEntity the persistent entity to check + * @return an updated list of field names + */ + private List updateFieldNames(List fieldNames, ElasticsearchPersistentEntity persistentEntity) { + return fieldNames.stream().map(fieldName -> { ElasticsearchPersistentProperty persistentProperty = persistentEntity.getPersistentProperty(fieldName); return persistentProperty != null ? persistentProperty.getFieldName() : fieldName; }).collect(Collectors.toList()); } - private void updateCriteriaQuery(CriteriaQuery criteriaQuery, Class domainClass) { + private void updatePropertiesInCriteriaQuery(CriteriaQuery criteriaQuery, Class domainClass) { Assert.notNull(criteriaQuery, "criteriaQuery must not be null"); Assert.notNull(domainClass, "domainClass must not be null"); @@ -1212,17 +1259,17 @@ public class MappingElasticsearchConverter if (persistentEntity != null) { for (Criteria chainedCriteria : criteriaQuery.getCriteria().getCriteriaChain()) { - updateCriteria(chainedCriteria, persistentEntity); + updatePropertiesInCriteria(chainedCriteria, persistentEntity); } for (Criteria subCriteria : criteriaQuery.getCriteria().getSubCriteria()) { for (Criteria chainedCriteria : subCriteria.getCriteriaChain()) { - updateCriteria(chainedCriteria, persistentEntity); + updatePropertiesInCriteria(chainedCriteria, persistentEntity); } } } } - private void updateCriteria(Criteria criteria, ElasticsearchPersistentEntity persistentEntity) { + private void updatePropertiesInCriteria(Criteria criteria, ElasticsearchPersistentEntity persistentEntity) { Field field = criteria.getField(); 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 6b4e67424..951be4dc7 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 @@ -34,7 +34,10 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import org.intellij.lang.annotations.Language; import org.json.JSONException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -1531,6 +1534,336 @@ public class MappingElasticsearchConverterUnitTests { mappingElasticsearchConverter.updateQuery(query, EntityWithCustomValueConverters.class); } + @Test // #2280 + @DisplayName("should read a single String into a List property") + void shouldReadASingleStringIntoAListProperty() { + + @Language("JSON") + var json = """ + { + "stringList": "foo" + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(EntityWithCollections.class, source); + + assertThat(entity.getStringList()).containsExactly("foo"); + } + + @Test // #2280 + @DisplayName("should read a String array into a List property") + void shouldReadAStringArrayIntoAListProperty() { + + @Language("JSON") + var json = """ + { + "stringList": ["foo", "bar"] + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(EntityWithCollections.class, source); + + assertThat(entity.getStringList()).containsExactly("foo", "bar"); + } + + @Test // #2280 + @DisplayName("should read a single String into a Set property") + void shouldReadASingleStringIntoASetProperty() { + + @Language("JSON") + var json = """ + { + "stringSet": "foo" + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(EntityWithCollections.class, source); + + assertThat(entity.getStringSet()).containsExactly("foo"); + } + + @Test // #2280 + @DisplayName("should read a String array into a Set property") + void shouldReadAStringArrayIntoASetProperty() { + + @Language("JSON") + var json = """ + { + "stringSet": ["foo", "bar"] + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(EntityWithCollections.class, source); + + assertThat(entity.getStringSet()).containsExactly("foo", "bar"); + } + + @Test // #2280 + @DisplayName("should read a single object into a List property") + void shouldReadASingleObjectIntoAListProperty() { + + @Language("JSON") + var json = """ + { + "childrenList": { + "name": "child" + } + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(EntityWithCollections.class, source); + + assertThat(entity.getChildrenList()).hasSize(1); + // noinspection ConstantConditions + assertThat(entity.getChildrenList().get(0).getName()).isEqualTo("child"); + } + + @Test // #2280 + @DisplayName("should read an object array into a List property") + void shouldReadAnObjectArrayIntoAListProperty() { + + @Language("JSON") + var json = """ + { + "childrenList": [ + { + "name": "child1" + }, + { + "name": "child2" + } + ] + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(EntityWithCollections.class, source); + + assertThat(entity.getChildrenList()).hasSize(2); + // noinspection ConstantConditions + assertThat(entity.getChildrenList().get(0).getName()).isEqualTo("child1"); + assertThat(entity.getChildrenList().get(1).getName()).isEqualTo("child2"); + } + + @Test // #2280 + @DisplayName("should read a single object into a Set property") + void shouldReadASingleObjectIntoASetProperty() { + + @Language("JSON") + var json = """ + { + "childrenSet": { + "name": "child" + } + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(EntityWithCollections.class, source); + + assertThat(entity.getChildrenSet()).hasSize(1); + // noinspection ConstantConditions + assertThat(entity.getChildrenSet().iterator().next().getName()).isEqualTo("child"); + } + + @Test // #2280 + @DisplayName("should read an object array into a Set property") + void shouldReadAnObjectArrayIntoASetProperty() { + + @Language("JSON") + var json = """ + { + "childrenSet": [ + { + "name": "child1" + }, + { + "name": "child2" + } + ] + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(EntityWithCollections.class, source); + + assertThat(entity.getChildrenSet()).hasSize(2); + // noinspection ConstantConditions + List names = entity.getChildrenSet().stream().map(EntityWithCollections.Child::getName) + .collect(Collectors.toList()); + assertThat(names).containsExactlyInAnyOrder("child1", "child2"); + } + + @Test // #2280 + @DisplayName("should read a single String into a List property immutable") + void shouldReadASingleStringIntoAListPropertyImmutable() { + + @Language("JSON") + var json = """ + { + "stringList": "foo" + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(ImmutableEntityWithCollections.class, source); + + assertThat(entity.getStringList()).containsExactly("foo"); + } + + @Test // #2280 + @DisplayName("should read a String array into a List property immutable") + void shouldReadAStringArrayIntoAListPropertyImmutable() { + + @Language("JSON") + var json = """ + { + "stringList": ["foo", "bar"] + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(ImmutableEntityWithCollections.class, source); + + assertThat(entity.getStringList()).containsExactly("foo", "bar"); + } + + @Test // #2280 + @DisplayName("should read a single String into a Set property immutable") + void shouldReadASingleStringIntoASetPropertyImmutable() { + + @Language("JSON") + var json = """ + { + "stringSet": "foo" + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(ImmutableEntityWithCollections.class, source); + + assertThat(entity.getStringSet()).containsExactly("foo"); + } + + @Test // #2280 + @DisplayName("should read a String array into a Set property immutable") + void shouldReadAStringArrayIntoASetPropertyImmutable() { + + @Language("JSON") + var json = """ + { + "stringSet": ["foo", "bar"] + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(ImmutableEntityWithCollections.class, source); + + assertThat(entity.getStringSet()).containsExactly("foo", "bar"); + } + + @Test // #2280 + @DisplayName("should read a single object into a List property immutable") + void shouldReadASingleObjectIntoAListPropertyImmutable() { + + @Language("JSON") + var json = """ + { + "childrenList": { + "name": "child" + } + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(ImmutableEntityWithCollections.class, source); + + assertThat(entity.getChildrenList()).hasSize(1); + // noinspection ConstantConditions + assertThat(entity.getChildrenList().get(0).getName()).isEqualTo("child"); + } + + @Test // #2280 + @DisplayName("should read an object array into a List property immutable") + void shouldReadAnObjectArrayIntoAListPropertyImmutable() { + + @Language("JSON") + var json = """ + { + "childrenList": [ + { + "name": "child1" + }, + { + "name": "child2" + } + ] + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(ImmutableEntityWithCollections.class, source); + + assertThat(entity.getChildrenList()).hasSize(2); + // noinspection ConstantConditions + assertThat(entity.getChildrenList().get(0).getName()).isEqualTo("child1"); + assertThat(entity.getChildrenList().get(1).getName()).isEqualTo("child2"); + } + + @Test // #2280 + @DisplayName("should read a single object into a Set property immutable") + void shouldReadASingleObjectIntoASetPropertyImmutable() { + + @Language("JSON") + var json = """ + { + "childrenSet": { + "name": "child" + } + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(ImmutableEntityWithCollections.class, source); + + assertThat(entity.getChildrenSet()).hasSize(1); + // noinspection ConstantConditions + assertThat(entity.getChildrenSet().iterator().next().getName()).isEqualTo("child"); + } + + @Test // #2280 + @DisplayName("should read an object array into a Set property immutable") + void shouldReadAnObjectArrayIntoASetPropertyImmutable() { + + @Language("JSON") + var json = """ + { + "childrenSet": [ + { + "name": "child1" + }, + { + "name": "child2" + } + ] + } + """; + Document source = Document.parse(json); + + var entity = mappingElasticsearchConverter.read(ImmutableEntityWithCollections.class, source); + + assertThat(entity.getChildrenSet()).hasSize(2); + // noinspection ConstantConditions + List names = entity.getChildrenSet().stream().map(ImmutableEntityWithCollections.Child::getName) + .collect(Collectors.toList()); + assertThat(names).containsExactlyInAnyOrder("child1", "child2"); + } + private Map writeToMap(Object source) { Document sink = Document.create(); @@ -2472,6 +2805,128 @@ public class MappingElasticsearchConverterUnitTests { return reverse(value); } } + + private static class EntityWithCollections { + @Field(type = FieldType.Keyword) + @Nullable private List stringList; + + @Field(type = FieldType.Keyword) + @Nullable private Set stringSet; + + @Field(type = FieldType.Object) + @Nullable private List childrenList; + + @Field(type = FieldType.Object) + @Nullable private Set childrenSet; + + @Nullable + public List getStringList() { + return stringList; + } + + public void setStringList(@Nullable List stringList) { + this.stringList = stringList; + } + + @Nullable + public Set getStringSet() { + return stringSet; + } + + public void setStringSet(@Nullable Set stringSet) { + this.stringSet = stringSet; + } + + @Nullable + public List getChildrenList() { + return childrenList; + } + + public void setChildrenList(@Nullable List childrenList) { + this.childrenList = childrenList; + } + + @Nullable + public Set getChildrenSet() { + return childrenSet; + } + + public void setChildrenSet(@Nullable Set childrenSet) { + this.childrenSet = childrenSet; + } + + public static class Child { + + @Field(type = FieldType.Keyword) + @Nullable private String name; + + @Nullable + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + } + + private static final class ImmutableEntityWithCollections { + @Field(type = FieldType.Keyword) + @Nullable private List stringList; + + @Field(type = FieldType.Keyword) + @Nullable private Set stringSet; + + @Field(type = FieldType.Object) + @Nullable private List childrenList; + + @Field(type = FieldType.Object) + @Nullable private Set childrenSet; + + public ImmutableEntityWithCollections(@Nullable List stringList, @Nullable Set stringSet, + @Nullable List childrenList, @Nullable Set childrenSet) { + this.stringList = stringList; + this.stringSet = stringSet; + this.childrenList = childrenList; + this.childrenSet = childrenSet; + } + + @Nullable + public List getStringList() { + return stringList; + } + + @Nullable + public Set getStringSet() { + return stringSet; + } + + @Nullable + public List getChildrenList() { + return childrenList; + } + + @Nullable + public Set getChildrenSet() { + return childrenSet; + } + + public static class Child { + + @Field(type = FieldType.Keyword) + @Nullable private String name; + + public Child(@Nullable String name) { + this.name = name; + } + + @Nullable + public String getName() { + return name; + } + } + } // endregion private static String reverse(Object o) {