From 346c5cce580fc876162435c613cedcd3f62313e4 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 (cherry picked from commit 86634ceb38dbc9d1a3985171a4bf12a6d5912612) --- .../MappingElasticsearchConverter.java | 251 ++++---- ...appingElasticsearchConverterUnitTests.java | 547 ++++++++++++++++-- 2 files changed, 640 insertions(+), 158 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 d03c3056a..393c623fd 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 @@ -15,6 +15,12 @@ */ package org.springframework.data.elasticsearch.core.convert; +import java.time.temporal.TemporalAccessor; +import java.util.*; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeansException; @@ -47,16 +53,7 @@ import org.springframework.data.mapping.Parameter; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mapping.SimplePropertyHandler; import org.springframework.data.mapping.context.MappingContext; -import org.springframework.data.mapping.model.ConvertingPropertyAccessor; -import org.springframework.data.mapping.model.DefaultSpELExpressionEvaluator; -import org.springframework.data.mapping.model.EntityInstantiator; -import org.springframework.data.mapping.model.EntityInstantiators; -import org.springframework.data.mapping.model.ParameterValueProvider; -import org.springframework.data.mapping.model.PersistentEntityParameterValueProvider; -import org.springframework.data.mapping.model.PropertyValueProvider; -import org.springframework.data.mapping.model.SpELContext; -import org.springframework.data.mapping.model.SpELExpressionEvaluator; -import org.springframework.data.mapping.model.SpELExpressionParameterValueProvider; +import org.springframework.data.mapping.model.*; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.format.datetime.DateFormatterRegistrar; @@ -66,12 +63,6 @@ import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; -import java.time.temporal.TemporalAccessor; -import java.util.*; -import java.util.Map.Entry; -import java.util.concurrent.ConcurrentHashMap; -import java.util.stream.Collectors; - /** * Elasticsearch specific {@link org.springframework.data.convert.EntityConverter} implementation based on domain type * {@link ElasticsearchPersistentEntity metadata}. @@ -92,15 +83,12 @@ import java.util.stream.Collectors; public class MappingElasticsearchConverter implements ElasticsearchConverter, ApplicationContextAware, InitializingBean { - private static final String INCOMPATIBLE_TYPES = - "Cannot convert %1$s of type %2$s into an instance of %3$s! Implement a custom Converter<%2$s, %3$s> and register it with the CustomConversions."; - private static final String INVALID_TYPE_TO_READ = - "Expected to read Document %s into type %s but didn't find a PersistentEntity for the latter!"; + private static final String INCOMPATIBLE_TYPES = "Cannot convert %1$s of type %2$s into an instance of %3$s! Implement a custom Converter<%2$s, %3$s> and register it with the CustomConversions."; + private static final String INVALID_TYPE_TO_READ = "Expected to read Document %s into type %s but didn't find a PersistentEntity for the latter!"; private static final Log LOGGER = LogFactory.getLog(MappingElasticsearchConverter.class); - private final MappingContext, ElasticsearchPersistentProperty> - mappingContext; + private final MappingContext, ElasticsearchPersistentProperty> mappingContext; private final GenericConversionService conversionService; private CustomConversions conversions = new ElasticsearchCustomConversions(Collections.emptyList()); private final SpELContext spELContext = new SpELContext(new MapAccessor()); @@ -142,8 +130,8 @@ public class MappingElasticsearchConverter } /** - * Set the {@link CustomConversions} to be applied during the mapping process.
Conversions are registered - * after {@link #afterPropertiesSet() bean initialization}. + * Set the {@link CustomConversions} to be applied during the mapping process.
+ * Conversions are registered after {@link #afterPropertiesSet() bean initialization}. * * @param conversions must not be {@literal null}. */ @@ -169,8 +157,7 @@ public class MappingElasticsearchConverter @Override public R read(Class type, Document source) { - Reader reader = - new Reader(mappingContext, conversionService, conversions, typeMapper, spELContext, instantiators); + Reader reader = new Reader(mappingContext, conversionService, conversions, typeMapper, spELContext, instantiators); return reader.read(type, source); } @@ -188,8 +175,7 @@ public class MappingElasticsearchConverter */ private static class Base { - protected final MappingContext, ElasticsearchPersistentProperty> - mappingContext; + protected final MappingContext, ElasticsearchPersistentProperty> mappingContext; protected final ElasticsearchTypeMapper typeMapper; protected final GenericConversionService conversionService; protected final CustomConversions conversions; @@ -197,8 +183,7 @@ public class MappingElasticsearchConverter private Base( MappingContext, ElasticsearchPersistentProperty> mappingContext, - GenericConversionService conversionService, CustomConversions conversions, - ElasticsearchTypeMapper typeMapper) { + GenericConversionService conversionService, CustomConversions conversions, ElasticsearchTypeMapper typeMapper) { this.mappingContext = mappingContext; this.conversionService = conversionService; this.conversions = conversions; @@ -207,7 +192,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 { @@ -217,8 +202,7 @@ public class MappingElasticsearchConverter public Reader( MappingContext, ElasticsearchPersistentProperty> mappingContext, - GenericConversionService conversionService, CustomConversions conversions, - ElasticsearchTypeMapper typeMapper, + GenericConversionService conversionService, CustomConversions conversions, ElasticsearchTypeMapper typeMapper, SpELContext spELContext, EntityInstantiators instantiators) { super(mappingContext, conversionService, conversions, typeMapper); @@ -227,10 +211,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 = ClassTypeInformation.from((Class) ClassUtils.getUserClass(type)); - R r = read(typeHint, source); + TypeInformation typeInformation = ClassTypeInformation.from((Class) ClassUtils.getUserClass(type)); + R r = read(typeInformation, source); if (r == null) { throw new ConversionException("could not convert into object of class " + type); @@ -241,11 +232,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)) { @@ -263,8 +254,8 @@ public class MappingElasticsearchConverter if (typeToUse.equals(ClassTypeInformation.OBJECT)) { return (R) source; } - // Retrieve persistent entity info + // Retrieve persistent entity info ElasticsearchPersistentEntity entity = mappingContext.getPersistentEntity(typeToUse); if (entity == null) { @@ -308,8 +299,7 @@ public class MappingElasticsearchConverter map.put(key, read(defaultedValueType, (Map) value)); } else if (value instanceof List) { map.put(key, - readCollectionOrArray(valueType != null ? valueType : ClassTypeInformation.LIST, - (List) value)); + readCollectionOrArray(valueType != null ? valueType : ClassTypeInformation.LIST, (List) value)); } else { map.put(key, getPotentiallyConvertedSimpleRead(value, rawValueType)); } @@ -329,19 +319,18 @@ public class MappingElasticsearchConverter ParameterValueProvider propertyValueProvider = creatorMetadata != null && creatorMetadata.hasParameters() ? getParameterProvider(entity, accessor, evaluator) - : NoOpParameterValueProvider.INSTANCE; + : NoOpParameterValueProvider.INSTANCE; EntityInstantiator instantiator = instantiators.getInstantiatorFor(targetEntity); - @SuppressWarnings({"unchecked"}) + @SuppressWarnings({ "unchecked" }) R instance = (R) instantiator.createInstance(targetEntity, propertyValueProvider); if (!targetEntity.requiresPropertyPopulation()) { return instance; } - ElasticsearchPropertyValueProvider valueProvider = - new ElasticsearchPropertyValueProvider(accessor, evaluator); + ElasticsearchPropertyValueProvider valueProvider = new ElasticsearchPropertyValueProvider(accessor, evaluator); R result = readProperties(targetEntity, instance, valueProvider); if (source instanceof Document) { @@ -351,8 +340,7 @@ public class MappingElasticsearchConverter PersistentPropertyAccessor propertyAccessor = new ConvertingPropertyAccessor<>( targetEntity.getPropertyAccessor(result), conversionService); // Only deal with String because ES generated Ids are strings ! - if (idProperty != null && idProperty.isWritable() && - idProperty.getType().isAssignableFrom(String.class)) { + if (idProperty != null && idProperty.isWritable() && idProperty.getType().isAssignableFrom(String.class)) { propertyAccessor.setProperty(idProperty, document.getId()); } } @@ -371,8 +359,7 @@ public class MappingElasticsearchConverter if (targetEntity.hasSeqNoPrimaryTermProperty() && document.hasSeqNo() && document.hasPrimaryTerm()) { if (isAssignedSeqNo(document.getSeqNo()) && isAssignedPrimaryTerm(document.getPrimaryTerm())) { - SeqNoPrimaryTerm seqNoPrimaryTerm = - new SeqNoPrimaryTerm(document.getSeqNo(), document.getPrimaryTerm()); + SeqNoPrimaryTerm seqNoPrimaryTerm = new SeqNoPrimaryTerm(document.getSeqNo(), document.getPrimaryTerm()); ElasticsearchPersistentProperty property = targetEntity.getRequiredSeqNoPrimaryTermProperty(); targetEntity.getPropertyAccessor(result).setProperty(property, seqNoPrimaryTerm); } @@ -385,7 +372,6 @@ public class MappingElasticsearchConverter } return result; - } private ParameterValueProvider getParameterProvider( @@ -395,12 +381,10 @@ public class MappingElasticsearchConverter // TODO: Support for non-static inner classes via ObjectPath // noinspection ConstantConditions - PersistentEntityParameterValueProvider parameterProvider = - new PersistentEntityParameterValueProvider<>( - entity, provider, null); + PersistentEntityParameterValueProvider parameterProvider = new PersistentEntityParameterValueProvider<>( + entity, provider, null); - return new ConverterAwareSpELExpressionParameterValueProvider(evaluator, conversionService, - parameterProvider); + return new ConverterAwareSpELExpressionParameterValueProvider(evaluator, conversionService, parameterProvider); } private boolean isAssignedSeqNo(long seqNo) { @@ -412,11 +396,10 @@ public class MappingElasticsearchConverter } protected R readProperties(ElasticsearchPersistentEntity entity, R instance, - ElasticsearchPropertyValueProvider valueProvider) { + ElasticsearchPropertyValueProvider valueProvider) { - PersistentPropertyAccessor accessor = - new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance), - conversionService); + PersistentPropertyAccessor accessor = new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance), + conversionService); for (ElasticsearchPersistentProperty prop : entity) { @@ -435,7 +418,7 @@ public class MappingElasticsearchConverter @Nullable protected R readValue(@Nullable Object value, ElasticsearchPersistentProperty property, - TypeInformation type) { + TypeInformation type) { if (value == null) { return null; @@ -478,15 +461,46 @@ 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(ClassTypeInformation.from(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()); + PropertyValueConverter propertyValueConverter = Objects.requireNonNull(property.getPropertyValueConverter()); if (source instanceof String[]) { // convert to a List @@ -513,7 +527,7 @@ public class MappingElasticsearchConverter * Reads the given {@link Collection} into a collection of the given {@link TypeInformation}. * * @param targetType must not be {@literal null}. - * @param source must not be {@literal null}. + * @param source must not be {@literal null}. * @return the converted {@link Collection} or array, will never be {@literal null}. */ @SuppressWarnings("unchecked") @@ -567,7 +581,7 @@ public class MappingElasticsearchConverter return getPotentiallyConvertedSimpleRead(value, targetType.getType()); } - @SuppressWarnings({"unchecked", "rawtypes"}) + @SuppressWarnings({ "unchecked", "rawtypes" }) @Nullable private Object getPotentiallyConvertedSimpleRead(@Nullable Object value, @Nullable Class target) { @@ -587,7 +601,7 @@ public class MappingElasticsearchConverter } private void populateScriptFields(ElasticsearchPersistentEntity entity, T result, - SearchDocument searchDocument) { + SearchDocument searchDocument) { Map> fields = searchDocument.getFields(); entity.doWithProperties((SimplePropertyHandler) property -> { if (property.isAnnotationPresent(ScriptedField.class) && fields.containsKey(property.getName())) { @@ -605,7 +619,7 @@ public class MappingElasticsearchConverter * Compute the type to use by checking the given entity against the store type; */ private ElasticsearchPersistentEntity computeClosestEntity(ElasticsearchPersistentEntity entity, - Map source) { + Map source) { TypeInformation typeToUse = typeMapper.readType(source); @@ -658,13 +672,12 @@ public class MappingElasticsearchConverter /** * Creates a new {@link ConverterAwareSpELExpressionParameterValueProvider}. * - * @param evaluator must not be {@literal null}. + * @param evaluator must not be {@literal null}. * @param conversionService must not be {@literal null}. - * @param delegate must not be {@literal null}. + * @param delegate must not be {@literal null}. */ public ConverterAwareSpELExpressionParameterValueProvider(SpELExpressionEvaluator evaluator, - ConversionService conversionService, - ParameterValueProvider delegate) { + ConversionService conversionService, ParameterValueProvider delegate) { super(evaluator, conversionService, delegate); } @@ -675,7 +688,7 @@ public class MappingElasticsearchConverter */ @Override protected T potentiallyConvertSpelValue(Object object, - Parameter parameter) { + Parameter parameter) { return readValue(object, parameter.getType()); } } @@ -701,8 +714,7 @@ public class MappingElasticsearchConverter public Writer( MappingContext, ElasticsearchPersistentProperty> mappingContext, - GenericConversionService conversionService, CustomConversions conversions, - ElasticsearchTypeMapper typeMapper) { + GenericConversionService conversionService, CustomConversions conversions, ElasticsearchTypeMapper typeMapper) { super(mappingContext, conversionService, conversions, typeMapper); } @@ -733,13 +745,13 @@ public class MappingElasticsearchConverter /** * Internal write conversion method which should be used for nested invocations. * - * @param source the object to write - * @param sink the write destination + * @param source the object to write + * @param sink the write destination * @param typeInformation type information for the source */ @SuppressWarnings("unchecked") private void writeInternal(@Nullable Object source, Map sink, - @Nullable TypeInformation typeInformation) { + @Nullable TypeInformation typeInformation) { if (null == source) { return; @@ -776,19 +788,18 @@ public class MappingElasticsearchConverter * Internal write conversion method which should be used for nested invocations. * * @param source the object to write - * @param sink the write destination + * @param sink the write destination * @param entity entity for the source */ private void writeInternal(@Nullable Object source, Map sink, - @Nullable ElasticsearchPersistentEntity entity) { + @Nullable ElasticsearchPersistentEntity entity) { if (source == null) { return; } if (null == entity) { - throw new MappingException( - "No mapping metadata found for entity of type " + source.getClass().getName()); + throw new MappingException("No mapping metadata found for entity of type " + source.getClass().getName()); } PersistentPropertyAccessor accessor = entity.getPropertyAccessor(source); @@ -818,12 +829,12 @@ public class MappingElasticsearchConverter /** * Writes the given {@link Map} to the given {@link Document} considering the given {@link TypeInformation}. * - * @param source must not be {@literal null}. - * @param sink must not be {@literal null}. + * @param source must not be {@literal null}. + * @param sink must not be {@literal null}. * @param propertyType must not be {@literal null}. */ private Map writeMapInternal(Map source, Map sink, - TypeInformation propertyType) { + TypeInformation propertyType) { for (Map.Entry entry : source.entrySet()) { @@ -837,8 +848,7 @@ public class MappingElasticsearchConverter sink.put(simpleKey, getPotentiallyConvertedSimpleWrite(value, Object.class)); } else if (value instanceof Collection || value.getClass().isArray()) { sink.put(simpleKey, - writeCollectionInternal(asCollection(value), propertyType.getMapValueType(), - new ArrayList<>())); + writeCollectionInternal(asCollection(value), propertyType.getMapValueType(), new ArrayList<>())); } else { Map document = Document.create(); TypeInformation valueTypeInfo = propertyType.isMap() ? propertyType.getMapValueType() @@ -859,12 +869,12 @@ public class MappingElasticsearchConverter * Populates the given {@link Collection sink} with converted values from the given {@link Collection source}. * * @param source the collection to create a {@link Collection} for, must not be {@literal null}. - * @param type the {@link TypeInformation} to consider or {@literal null} if unknown. - * @param sink the {@link Collection} to write to. + * @param type the {@link TypeInformation} to consider or {@literal null} if unknown. + * @param sink the {@link Collection} to write to. */ @SuppressWarnings("unchecked") private List writeCollectionInternal(Collection source, @Nullable TypeInformation type, - Collection sink) { + Collection sink) { TypeInformation componentType = null; @@ -894,7 +904,7 @@ public class MappingElasticsearchConverter } private void writeProperties(ElasticsearchPersistentEntity entity, PersistentPropertyAccessor accessor, - MapValueAccessor sink) { + MapValueAccessor sink) { for (ElasticsearchPersistentProperty property : entity) { @@ -990,16 +1000,16 @@ public class MappingElasticsearchConverter } /** - * Adds custom typeInformation information to the given {@link Map} if necessary. That is if the value is not - * the same as the one given. This is usually the case if you store a subtype of the actual declared - * typeInformation of the property. + * Adds custom typeInformation information to the given {@link Map} if necessary. That is if the value is not the + * same as the one given. This is usually the case if you store a subtype of the actual declared typeInformation of + * the property. * * @param source must not be {@literal null}. - * @param sink must not be {@literal null}. - * @param type type to compare to + * @param sink must not be {@literal null}. + * @param type type to compare to */ private void addCustomTypeKeyIfNecessary(Object source, Map sink, - @Nullable TypeInformation type) { + @Nullable TypeInformation type) { if (!writeTypeHints) { return; @@ -1044,9 +1054,9 @@ public class MappingElasticsearchConverter } /** - * Checks whether we have a custom conversion registered for the given value into an arbitrary simple - * Elasticsearch type. Returns the converted value if so. If not, we perform special enum handling or simply - * return the value as is. + * Checks whether we have a custom conversion registered for the given value into an arbitrary simple Elasticsearch + * type. Returns the converted value if so. If not, we perform special enum handling or simply return the value as + * is. * * @param value value to convert */ @@ -1086,8 +1096,7 @@ public class MappingElasticsearchConverter } private Object propertyConverterWrite(ElasticsearchPersistentProperty property, Object value) { - PropertyValueConverter propertyValueConverter = - Objects.requireNonNull(property.getPropertyValueConverter()); + PropertyValueConverter propertyValueConverter = Objects.requireNonNull(property.getPropertyValueConverter()); if (value instanceof List) { value = ((List) value).stream().map(propertyValueConverter::write).collect(Collectors.toList()); @@ -1103,17 +1112,16 @@ public class MappingElasticsearchConverter * Writes the given {@link Collection} using the given {@link ElasticsearchPersistentProperty} information. * * @param collection must not be {@literal null}. - * @param property must not be {@literal null}. + * @param property must not be {@literal null}. */ protected List createCollection(Collection collection, ElasticsearchPersistentProperty property) { - return writeCollectionInternal(collection, property.getTypeInformation(), - new ArrayList<>(collection.size())); + return writeCollectionInternal(collection, property.getTypeInformation(), new ArrayList<>(collection.size())); } /** * Writes the given {@link Map} using the given {@link ElasticsearchPersistentProperty} information. * - * @param map must not {@literal null}. + * @param map must not {@literal null}. * @param property must not be {@literal null}. */ protected Map createMap(Map map, ElasticsearchPersistentProperty property) { @@ -1148,17 +1156,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); @@ -1183,12 +1192,12 @@ public class MappingElasticsearchConverter if (sourceFilter.getIncludes() != null) { includes = updateFieldNames(Arrays.asList(sourceFilter.getIncludes()), persistentEntity) - .toArray(new String[]{}); + .toArray(new String[] {}); } if (sourceFilter.getExcludes() != null) { excludes = updateFieldNames(Arrays.asList(sourceFilter.getExcludes()), persistentEntity) - .toArray(new String[]{}); + .toArray(new String[] {}); } query.addSourceFilter(new FetchSourceFilter(includes, excludes)); @@ -1196,14 +1205,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 +1229,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 d669d6443..2655a523b 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 @@ -33,7 +33,10 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +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; @@ -623,7 +626,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(target.address).isEqualTo(bigBunsCafe); } - @Test // DATAES-716 + @Test + // DATAES-716 void shouldWriteLocalDate() throws JSONException { Person person = new Person(); person.setId("4711"); @@ -665,7 +669,8 @@ public class MappingElasticsearchConverterUnitTests { assertEquals(expected, json, false); } - @Test // DATAES-716 + @Test + // DATAES-716 void shouldReadLocalDate() { Document document = Document.create(); document.put("id", "4711"); @@ -695,7 +700,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(entity.getDates()).hasSize(2).containsExactly(LocalDate.of(2020, 9, 15), LocalDate.of(2019, 5, 1)); } - @Test // DATAES-763 + @Test + // DATAES-763 void writeEntityWithMapDataType() { Notification notification = new Notification(); @@ -712,7 +718,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(document).isEqualTo(notificationAsMap); } - @Test // DATAES-763 + @Test + // DATAES-763 void readEntityWithMapDataType() { Document document = Document.create(); @@ -729,7 +736,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(notification.params.get("content")).isNull(); } - @Test // DATAES-795 + @Test + // DATAES-795 void readGenericMapWithSimpleTypes() { Map mapWithSimpleValues = new HashMap<>(); mapWithSimpleValues.put("int", 1); @@ -743,7 +751,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(wrapper.getSchemaLessObject()).isEqualTo(mapWithSimpleValues); } - @Test // DATAES-797 + @Test + // DATAES-797 void readGenericListWithMaps() { Map simpleMap = new HashMap<>(); simpleMap.put("int", 1); @@ -761,7 +770,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(wrapper.getSchemaLessObject()).isEqualTo(mapWithSimpleList); } - @Test // DATAES-799 + @Test + // DATAES-799 void shouldNotWriteSeqNoPrimaryTermProperty() { EntityWithSeqNoPrimaryTerm entity = new EntityWithSeqNoPrimaryTerm(); entity.seqNoPrimaryTerm = new SeqNoPrimaryTerm(1L, 2L); @@ -772,7 +782,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(document).doesNotContainKey("seqNoPrimaryTerm"); } - @Test // DATAES-799 + @Test + // DATAES-799 void shouldNotReadSeqNoPrimaryTermProperty() { Document document = Document.create().append("seqNoPrimaryTerm", emptyMap()); @@ -781,7 +792,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(entity.seqNoPrimaryTerm).isNull(); } - @Test // DATAES-845 + @Test + // DATAES-845 void shouldWriteCollectionsWithNullValues() throws JSONException { EntityWithListProperty entity = new EntityWithListProperty(); entity.setId("42"); @@ -798,7 +810,8 @@ public class MappingElasticsearchConverterUnitTests { assertEquals(expected, json, false); } - @Test // DATAES-857 + @Test + // DATAES-857 void shouldWriteEntityWithListOfGeoPoints() throws JSONException { GeoPointListEntity entity = new GeoPointListEntity(); @@ -827,7 +840,8 @@ public class MappingElasticsearchConverterUnitTests { assertEquals(expected, json, false); } - @Test // DATAES-857 + @Test + // DATAES-857 void shouldReadEntityWithListOfGeoPoints() { String json = "{\n" + // @@ -852,7 +866,8 @@ public class MappingElasticsearchConverterUnitTests { assertThat(entity.locations).containsExactly(new GeoPoint(12.34, 23.45), new GeoPoint(34.56, 45.67)); } - @Test // DATAES-865 + @Test + // DATAES-865 void shouldWriteEntityWithMapAsObject() throws JSONException { Map map = new LinkedHashMap<>(); @@ -1509,6 +1524,305 @@ public class MappingElasticsearchConverterUnitTests { mappingElasticsearchConverter.updateQuery(query, EntityWithCustomValueConverters.class); } + @Test // #2280 + @DisplayName("should read a single String into a List property") + void shouldReadASingleStringIntoAListProperty() { + + String json = "{\n" + // + " \"stringList\": \"foo\"\n" + // + "}\n"; // + Document source = Document.parse(json); + + EntityWithCollections 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() { + + String json = "{\n" + // + " \"stringList\": [\"foo\", \"bar\"]\n" + // + "}\n"; // + Document source = Document.parse(json); + + EntityWithCollections 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() { + + String json = "{\n" + // + " \"stringSet\": \"foo\"\n" + // + "}\n"; // + Document source = Document.parse(json); + + EntityWithCollections 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() { + + String json = "{\n" + // + " \"stringSet\": [\n" + // + " \"foo\",\n" + // + " \"bar\"\n" + // + " ]\n" + // + "}\n"; // + Document source = Document.parse(json); + + EntityWithCollections 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() { + + String json = "{\n" + // + " \"childrenList\": {\n" + // + " \"name\": \"child\"\n" + // + " }\n" + // + "}\n"; // + Document source = Document.parse(json); + + EntityWithCollections 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() { + + String json = " {\n" + // + " \"childrenList\": [\n" + // + " {\n" + // + " \"name\": \"child1\"\n" + // + " },\n" + // + " {\n" + // + " \"name\": \"child2\"\n" + // + " }\n" + // + " ]\n" + // + " }\n"; // + Document source = Document.parse(json); + + EntityWithCollections 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() { + + String json = "{\n" + // + " \"childrenSet\": {\n" + // + " \"name\": \"child\"\n" + // + " }\n" + // + "}\n"; // + Document source = Document.parse(json); + + EntityWithCollections 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() { + + String json = "{\n" + // + " \"childrenSet\": [\n" + // + " {\n" + // + " \"name\": \"child1\"\n" + // + " },\n" + // + " {\n" + // + " \"name\": \"child2\"\n" + // + " }\n" + // + " ]\n" + // + "}\n"; // + Document source = Document.parse(json); + + EntityWithCollections 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() { + + String json = "{\n" + // + " \"stringList\": \"foo\"\n" + // + "}\n"; // + Document source = Document.parse(json); + + ImmutableEntityWithCollections 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() { + + String json = "{\n" + // + " \"stringList\": [\n" + // + " \"foo\",\n" + // + " \"bar\"\n" + // + " ]\n" + // + "}\n"; // + Document source = Document.parse(json); + + ImmutableEntityWithCollections 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() { + + String json = "{\n" + // + " \"stringSet\": \"foo\"\n" + // + "}\n"; // + Document source = Document.parse(json); + + ImmutableEntityWithCollections 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() { + + String json = "{\n" + // + " \"stringSet\": [\n" + // + " \"foo\",\n" + // + " \"bar\"\n" + // + " ]\n" + // + "}\n"; // + Document source = Document.parse(json); + + ImmutableEntityWithCollections 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() { + + String json = "{\n" + // + " \"childrenList\": {\n" + // + " \"name\": \"child\"\n" + // + " }\n" + // + "}\n"; // + Document source = Document.parse(json); + + ImmutableEntityWithCollections 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() { + + String json = "{\n" + // + " \"childrenList\": [\n" + // + " {\n" + // + " \"name\": \"child1\"\n" + // + " },\n" + // + " {\n" + // + " \"name\": \"child2\"\n" + // + " }\n" + // + " ]\n" + // + "}\n"; // + Document source = Document.parse(json); + + ImmutableEntityWithCollections 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() { + + String json = "{\n" + // + " \"childrenSet\": {\n" + // + " \"name\": \"child\"\n" + // + " }\n" + // + "}\n"; // + Document source = Document.parse(json); + + ImmutableEntityWithCollections 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() { + + String json = "{\n" + // + " \"childrenSet\": [\n" + // + " {\n" + // + " \"name\": \"child1\"\n" + // + " },\n" + // + " {\n" + // + " \"name\": \"child2\"\n" + // + " }\n" + // + " ]\n" + // + "}\n"; // + Document source = Document.parse(json); + + ImmutableEntityWithCollections 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(); @@ -1678,34 +1992,46 @@ public class MappingElasticsearchConverterUnitTests { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (o == null || getClass() != o.getClass()) + } + if (o == null || getClass() != o.getClass()) { return false; + } Person person = (Person) o; - if (id != null ? !id.equals(person.id) : person.id != null) + if (id != null ? !id.equals(person.id) : person.id != null) { return false; - if (name != null ? !name.equals(person.name) : person.name != null) + } + if (name != null ? !name.equals(person.name) : person.name != null) { return false; - if (firstName != null ? !firstName.equals(person.firstName) : person.firstName != null) + } + if (firstName != null ? !firstName.equals(person.firstName) : person.firstName != null) { return false; - if (lastName != null ? !lastName.equals(person.lastName) : person.lastName != null) + } + if (lastName != null ? !lastName.equals(person.lastName) : person.lastName != null) { return false; - if (birthDate != null ? !birthDate.equals(person.birthDate) : person.birthDate != null) + } + if (birthDate != null ? !birthDate.equals(person.birthDate) : person.birthDate != null) { return false; - if (gender != person.gender) + } + if (gender != person.gender) { return false; - if (address != null ? !address.equals(person.address) : person.address != null) + } + if (address != null ? !address.equals(person.address) : person.address != null) { return false; - if (coWorkers != null ? !coWorkers.equals(person.coWorkers) : person.coWorkers != null) + } + if (coWorkers != null ? !coWorkers.equals(person.coWorkers) : person.coWorkers != null) { return false; - if (inventoryList != null ? !inventoryList.equals(person.inventoryList) : person.inventoryList != null) + } + if (inventoryList != null ? !inventoryList.equals(person.inventoryList) : person.inventoryList != null) { return false; + } if (shippingAddresses != null ? !shippingAddresses.equals(person.shippingAddresses) - : person.shippingAddresses != null) + : person.shippingAddresses != null) { return false; + } return inventoryMap != null ? inventoryMap.equals(person.inventoryMap) : person.inventoryMap == null; } @@ -1792,15 +2118,18 @@ public class MappingElasticsearchConverterUnitTests { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (o == null || getClass() != o.getClass()) + } + if (o == null || getClass() != o.getClass()) { return false; + } Gun gun = (Gun) o; - if (shotsPerMagazine != gun.shotsPerMagazine) + if (shotsPerMagazine != gun.shotsPerMagazine) { return false; + } return label.equals(gun.label); } @@ -1826,10 +2155,12 @@ public class MappingElasticsearchConverterUnitTests { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (!(o instanceof Grenade)) + } + if (!(o instanceof Grenade)) { return false; + } Grenade grenade = (Grenade) o; @@ -1862,17 +2193,21 @@ public class MappingElasticsearchConverterUnitTests { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (!(o instanceof Rifle)) + } + if (!(o instanceof Rifle)) { return false; + } Rifle rifle = (Rifle) o; - if (Double.compare(rifle.weight, weight) != 0) + if (Double.compare(rifle.weight, weight) != 0) { return false; - if (maxShotsPerMagazine != rifle.maxShotsPerMagazine) + } + if (maxShotsPerMagazine != rifle.maxShotsPerMagazine) { return false; + } return label.equals(rifle.label); } @@ -1903,10 +2238,12 @@ public class MappingElasticsearchConverterUnitTests { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (!(o instanceof ShotGun)) + } + if (!(o instanceof ShotGun)) { return false; + } ShotGun shotGun = (ShotGun) o; @@ -1953,17 +2290,21 @@ public class MappingElasticsearchConverterUnitTests { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (!(o instanceof Address)) + } + if (!(o instanceof Address)) { return false; + } Address address = (Address) o; - if (location != null ? !location.equals(address.location) : address.location != null) + if (location != null ? !location.equals(address.location) : address.location != null) { return false; - if (street != null ? !street.equals(address.street) : address.street != null) + } + if (street != null ? !street.equals(address.street) : address.street != null) { return false; + } return city != null ? city.equals(address.city) : address.city == null; } @@ -1990,10 +2331,12 @@ public class MappingElasticsearchConverterUnitTests { @Override public boolean equals(Object o) { - if (this == o) + if (this == o) { return true; - if (!(o instanceof Place)) + } + if (!(o instanceof Place)) { return false; + } Place place = (Place) o; @@ -2462,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) {