Don't try to write non-writeable properties.

Original Pull Request #2249
Closes #2230

(cherry picked from commit acf02a1dc9e427b1d72bd949553e70f51539bd4d)

# Conflicts:
#	src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java
This commit is contained in:
Peter-Josef Meisch 2022-08-04 08:04:19 +02:00
parent be70a398be
commit 3c6d96e49f
7 changed files with 200 additions and 74 deletions

View File

@ -478,7 +478,7 @@ public abstract class AbstractElasticsearchTemplate implements ElasticsearchOper
ElasticsearchPersistentProperty idProperty = persistentEntity.getIdProperty();
// Only deal with text because ES generated Ids are strings!
if (indexedObjectInformation.getId() != null && idProperty != null
if (indexedObjectInformation.getId() != null && idProperty != null && idProperty.isWritable()
&& idProperty.getType().isAssignableFrom(String.class)) {
propertyAccessor.setProperty(idProperty, indexedObjectInformation.getId());
}

View File

@ -259,7 +259,7 @@ abstract public class AbstractReactiveElasticsearchTemplate
ElasticsearchPersistentProperty idProperty = persistentEntity.getIdProperty();
// Only deal with text because ES generated Ids are strings!
if (indexedObjectInformation.getId() != null && idProperty != null
if (indexedObjectInformation.getId() != null && idProperty != null && idProperty.isWritable()
&& idProperty.getType().isAssignableFrom(String.class)) {
propertyAccessor.setProperty(idProperty, indexedObjectInformation.getId());
}

View File

@ -15,12 +15,6 @@
*/
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;
@ -53,7 +47,16 @@ 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.*;
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.util.ClassTypeInformation;
import org.springframework.data.util.TypeInformation;
import org.springframework.format.datetime.DateFormatterRegistrar;
@ -63,6 +66,12 @@ 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}.
@ -83,12 +92,15 @@ import org.springframework.util.ObjectUtils;
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<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext;
private final MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty>
mappingContext;
private final GenericConversionService conversionService;
private CustomConversions conversions = new ElasticsearchCustomConversions(Collections.emptyList());
private final SpELContext spELContext = new SpELContext(new MapAccessor());
@ -130,8 +142,8 @@ public class MappingElasticsearchConverter
}
/**
* Set the {@link CustomConversions} to be applied during the mapping process. <br />
* Conversions are registered after {@link #afterPropertiesSet() bean initialization}.
* Set the {@link CustomConversions} to be applied during the mapping process. <br /> Conversions are registered
* after {@link #afterPropertiesSet() bean initialization}.
*
* @param conversions must not be {@literal null}.
*/
@ -157,7 +169,8 @@ public class MappingElasticsearchConverter
@Override
public <R> R read(Class<R> 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);
}
@ -175,7 +188,8 @@ public class MappingElasticsearchConverter
*/
private static class Base {
protected final MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext;
protected final MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty>
mappingContext;
protected final ElasticsearchTypeMapper typeMapper;
protected final GenericConversionService conversionService;
protected final CustomConversions conversions;
@ -183,7 +197,8 @@ public class MappingElasticsearchConverter
private Base(
MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext,
GenericConversionService conversionService, CustomConversions conversions, ElasticsearchTypeMapper typeMapper) {
GenericConversionService conversionService, CustomConversions conversions,
ElasticsearchTypeMapper typeMapper) {
this.mappingContext = mappingContext;
this.conversionService = conversionService;
this.conversions = conversions;
@ -202,7 +217,8 @@ public class MappingElasticsearchConverter
public Reader(
MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext,
GenericConversionService conversionService, CustomConversions conversions, ElasticsearchTypeMapper typeMapper,
GenericConversionService conversionService, CustomConversions conversions,
ElasticsearchTypeMapper typeMapper,
SpELContext spELContext, EntityInstantiators instantiators) {
super(mappingContext, conversionService, conversions, typeMapper);
@ -292,7 +308,8 @@ public class MappingElasticsearchConverter
map.put(key, read(defaultedValueType, (Map<String, Object>) value));
} else if (value instanceof List) {
map.put(key,
readCollectionOrArray(valueType != null ? valueType : ClassTypeInformation.LIST, (List<Object>) value));
readCollectionOrArray(valueType != null ? valueType : ClassTypeInformation.LIST,
(List<Object>) value));
} else {
map.put(key, getPotentiallyConvertedSimpleRead(value, rawValueType));
}
@ -323,7 +340,8 @@ public class MappingElasticsearchConverter
return instance;
}
ElasticsearchPropertyValueProvider valueProvider = new ElasticsearchPropertyValueProvider(accessor, evaluator);
ElasticsearchPropertyValueProvider valueProvider =
new ElasticsearchPropertyValueProvider(accessor, evaluator);
R result = readProperties(targetEntity, instance, valueProvider);
if (source instanceof Document) {
@ -333,7 +351,8 @@ public class MappingElasticsearchConverter
PersistentPropertyAccessor<R> propertyAccessor = new ConvertingPropertyAccessor<>(
targetEntity.getPropertyAccessor(result), conversionService);
// Only deal with String because ES generated Ids are strings !
if (idProperty != null && idProperty.getType().isAssignableFrom(String.class)) {
if (idProperty != null && idProperty.isWritable() &&
idProperty.getType().isAssignableFrom(String.class)) {
propertyAccessor.setProperty(idProperty, document.getId());
}
}
@ -352,7 +371,8 @@ 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);
}
@ -375,10 +395,12 @@ public class MappingElasticsearchConverter
// TODO: Support for non-static inner classes via ObjectPath
// noinspection ConstantConditions
PersistentEntityParameterValueProvider<ElasticsearchPersistentProperty> parameterProvider = new PersistentEntityParameterValueProvider<>(
PersistentEntityParameterValueProvider<ElasticsearchPersistentProperty> parameterProvider =
new PersistentEntityParameterValueProvider<>(
entity, provider, null);
return new ConverterAwareSpELExpressionParameterValueProvider(evaluator, conversionService, parameterProvider);
return new ConverterAwareSpELExpressionParameterValueProvider(evaluator, conversionService,
parameterProvider);
}
private boolean isAssignedSeqNo(long seqNo) {
@ -392,12 +414,13 @@ public class MappingElasticsearchConverter
protected <R> R readProperties(ElasticsearchPersistentEntity<?> entity, R instance,
ElasticsearchPropertyValueProvider valueProvider) {
PersistentPropertyAccessor<R> accessor = new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance),
PersistentPropertyAccessor<R> accessor =
new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance),
conversionService);
for (ElasticsearchPersistentProperty prop : entity) {
if (entity.isConstructorArgument(prop) || !prop.isReadable()) {
if (entity.isConstructorArgument(prop) || !prop.isReadable() || !prop.isWritable()) {
continue;
}
@ -462,7 +485,8 @@ public class MappingElasticsearchConverter
}
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
@ -639,7 +663,8 @@ public class MappingElasticsearchConverter
* @param delegate must not be {@literal null}.
*/
public ConverterAwareSpELExpressionParameterValueProvider(SpELExpressionEvaluator evaluator,
ConversionService conversionService, ParameterValueProvider<ElasticsearchPersistentProperty> delegate) {
ConversionService conversionService,
ParameterValueProvider<ElasticsearchPersistentProperty> delegate) {
super(evaluator, conversionService, delegate);
}
@ -649,7 +674,8 @@ public class MappingElasticsearchConverter
* @see org.springframework.data.mapping.model.SpELExpressionParameterValueProvider#potentiallyConvertSpelValue(java.lang.Object, org.springframework.data.mapping.PreferredConstructor.Parameter)
*/
@Override
protected <T> T potentiallyConvertSpelValue(Object object, Parameter<T, ElasticsearchPersistentProperty> parameter) {
protected <T> T potentiallyConvertSpelValue(Object object,
Parameter<T, ElasticsearchPersistentProperty> parameter) {
return readValue(object, parameter.getType());
}
}
@ -675,7 +701,8 @@ public class MappingElasticsearchConverter
public Writer(
MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext,
GenericConversionService conversionService, CustomConversions conversions, ElasticsearchTypeMapper typeMapper) {
GenericConversionService conversionService, CustomConversions conversions,
ElasticsearchTypeMapper typeMapper) {
super(mappingContext, conversionService, conversions, typeMapper);
}
@ -760,7 +787,8 @@ public class MappingElasticsearchConverter
}
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);
@ -809,7 +837,8 @@ 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<String, Object> document = Document.create();
TypeInformation<?> valueTypeInfo = propertyType.isMap() ? propertyType.getMapValueType()
@ -961,9 +990,9 @@ 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}.
@ -1015,9 +1044,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
*/
@ -1057,7 +1086,8 @@ 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());
@ -1076,7 +1106,8 @@ public class MappingElasticsearchConverter
* @param property must not be {@literal null}.
*/
protected List<Object> createCollection(Collection<?> collection, ElasticsearchPersistentProperty property) {
return writeCollectionInternal(collection, property.getTypeInformation(), new ArrayList<>(collection.size()));
return writeCollectionInternal(collection, property.getTypeInformation(),
new ArrayList<>(collection.size()));
}
/**

View File

@ -45,7 +45,9 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.DataAccessException;
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.dao.OptimisticLockingFailureException;
import org.springframework.data.annotation.AccessType;
import org.springframework.data.annotation.Id;
import org.springframework.data.annotation.ReadOnlyProperty;
import org.springframework.data.annotation.Version;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
@ -3718,6 +3720,21 @@ public abstract class ElasticsearchIntegrationTests implements NewElasticsearchC
}
}
@Test // #2230
@DisplayName("should work with readonly id")
void shouldWorkWithReadonlyId() {
ReadonlyIdEntity entity = new ReadonlyIdEntity();
entity.setPart1("foo");
entity.setPart2("bar");
operations.save(entity);
ReadonlyIdEntity readEntity = operations.get(entity.getId(), ReadonlyIdEntity.class);
assertThat(readEntity.getPart1()).isEqualTo(entity.getPart1());
assertThat(readEntity.getPart2()).isEqualTo(entity.getPart2());
}
@Document(indexName = "#{@indexNameProvider.indexName()}")
private static class SampleEntityUUIDKeyed {
@Nullable
@ -4463,5 +4480,36 @@ public abstract class ElasticsearchIntegrationTests implements NewElasticsearchC
+ '}';
}
}
@Document(indexName = "#{@indexNameProvider.indexName()}-readonly-id")
static class ReadonlyIdEntity {
@Field(type = FieldType.Keyword) private String part1;
@Field(type = FieldType.Keyword) private String part2;
@Id
@ReadOnlyProperty
@AccessType(AccessType.Type.PROPERTY)
public String getId() {
return part1 + '-' + part2;
}
public String getPart1() {
return part1;
}
public void setPart1(String part1) {
this.part1 = part1;
}
public String getPart2() {
return part2;
}
public void setPart2(String part2) {
this.part2 = part2;
}
}
// endregion
}

View File

@ -49,7 +49,9 @@ import org.skyscreamer.jsonassert.JSONAssert;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.DataAccessException;
import org.springframework.dao.OptimisticLockingFailureException;
import org.springframework.data.annotation.AccessType;
import org.springframework.data.annotation.Id;
import org.springframework.data.annotation.ReadOnlyProperty;
import org.springframework.data.annotation.Version;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
@ -1156,6 +1158,23 @@ public abstract class ReactiveElasticsearchIntegrationTests {
.verifyComplete();
}
@Test // #2230
@DisplayName("should work with readonly id")
void shouldWorkWithReadonlyId() {
ReadonlyIdEntity entity = new ReadonlyIdEntity();
entity.setPart1("foo");
entity.setPart2("bar");
operations.save(entity).block();
operations.get(entity.getId(), ReadonlyIdEntity.class) //
.as(StepVerifier::create) //
.assertNext(readEntity -> { //
assertThat(readEntity.getPart1()).isEqualTo(entity.getPart1()); //
assertThat(readEntity.getPart2()).isEqualTo(entity.getPart2()); //
}).verifyComplete();
}
// endregion
// region Helper functions
@ -1489,5 +1508,35 @@ public abstract class ReactiveElasticsearchIntegrationTests {
+ seqNoPrimaryTerm + '}';
}
}
@Document(indexName = "#{@indexNameProvider.indexName()}-readonly-id")
static class ReadonlyIdEntity {
@Field(type = FieldType.Keyword) private String part1;
@Field(type = FieldType.Keyword) private String part2;
@Id
@ReadOnlyProperty
@AccessType(AccessType.Type.PROPERTY)
public String getId() {
return part1 + '-' + part2;
}
public String getPart1() {
return part1;
}
public void setPart1(String part1) {
this.part1 = part1;
}
public String getPart2() {
return part2;
}
public void setPart2(String part2) {
this.part2 = part2;
}
}
// endregion
}

View File

@ -241,7 +241,7 @@ abstract class CallbackIntegrationTests {
@Id private String id;
@Nullable private String text;
@ReadOnlyProperty
// @ReadOnlyProperty
@Nullable private String className;
@Nullable

View File

@ -27,7 +27,6 @@ import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.annotation.Id;
import org.springframework.data.annotation.ReadOnlyProperty;
import org.springframework.data.elasticsearch.annotations.Document;
import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations;
import org.springframework.data.elasticsearch.core.ReactiveIndexOperations;
@ -119,7 +118,6 @@ public abstract class ReactiveCallbackIntegrationTests {
@Id private String id;
private String text;
@ReadOnlyProperty
@Nullable private String className;
public SampleEntity(String id, String text) {