Use FormattingConversionService in JSON Patch binding.

We no pipe the Spring MVC ConversionService into the JSON Patch path binding. That usually is a FormattingConversionService at runtime and also supports the conversion of dates.

The ConversionServices is configured into the BindContext(Factory) we use for binding. The context then exposes the EvaluationContext set up with it.

Fixes #2233.
This commit is contained in:
Oliver Drotbohm
2023-02-21 18:40:31 +01:00
parent fdb9c660ee
commit a71ffd65c7
9 changed files with 80 additions and 17 deletions

View File

@@ -30,6 +30,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.mongodb.core.convert.MongoCustomConversions;
import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
@@ -76,7 +77,8 @@ class JsonPatchHandlerUnitTests {
PersistentEntities entities = new PersistentEntities(Arrays.asList(context));
Associations associations = new Associations(mappings, mock(RepositoryRestConfiguration.class));
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities);
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities,
DefaultConversionService.getSharedInstance());
this.handler = new JsonPatchHandler(factory, new DomainObjectReader(entities, associations));

View File

@@ -500,7 +500,7 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon
PluginRegistry<EntityLookup<?>, Class<?>> lookups = PluginRegistry.of(getEntityLookups());
DomainObjectReader reader = new DomainObjectReader(entities, associationLinks);
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities);
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities, defaultConversionService);
return new PersistentEntityResourceHandlerMethodArgumentResolver(defaultMessageConverters,
repoRequestArgumentResolver, backendIdHandlerMethodArgumentResolver,

View File

@@ -17,9 +17,12 @@ package org.springframework.data.rest.webmvc.json;
import java.util.Optional;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.mapping.PersistentProperty;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.rest.webmvc.json.patch.BindContext;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.spel.support.SimpleEvaluationContext;
import org.springframework.util.Assert;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -34,20 +37,24 @@ class JacksonBindContext implements BindContext {
private final PersistentEntities entities;
private final ObjectMapper mapper;
private final EvaluationContext context;
/**
* Creates a new {@link JacksonBindContext} for the given {@link PersistentEntities} and {@link ObjectMapper}.
*
* @param entities must not be {@literal null}.
* @param conversionService must not be {@literal null}.
* @param mapper must not be {@literal null}.
*/
public JacksonBindContext(PersistentEntities entities, ObjectMapper mapper) {
public JacksonBindContext(PersistentEntities entities, ConversionService conversionService, ObjectMapper mapper) {
Assert.notNull(entities, "PersistentEntities must not be null");
Assert.notNull(mapper, "ObjectMapper must not be null");
Assert.notNull(conversionService, "ConversionService must not be null!");
this.entities = entities;
this.mapper = mapper;
this.context = SimpleEvaluationContext.forReadWriteDataBinding().withConversionService(conversionService).build();
}
@Override
@@ -66,6 +73,15 @@ class JacksonBindContext implements BindContext {
.filter(it -> it.isWritableField(segment)), segment);
}
/*
* (non-Javadoc)
* @see org.springframework.data.rest.webmvc.json.patch.BindContext#getEvaluationContext()
*/
@Override
public EvaluationContext getEvaluationContext() {
return context;
}
private static Optional<String> getProperty(Optional<MappedProperties> properties, String segment) {
return properties.map(it -> it.getPersistentProperty(segment))

View File

@@ -15,6 +15,7 @@
*/
package org.springframework.data.rest.webmvc.json;
import org.springframework.core.convert.ConversionService;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.rest.webmvc.json.patch.BindContext;
import org.springframework.util.Assert;
@@ -29,21 +30,23 @@ import com.fasterxml.jackson.databind.ObjectMapper;
public class PersistentEntitiesBindContextFactory implements BindContextFactory {
private final PersistentEntities entities;
private final ConversionService conversionService;
/**
* Creates a new {@link PersistentEntitiesBindContextFactory} for the given {@link PersistentEntities}.
*
* @param entities must not be {@literal null}.
*/
public PersistentEntitiesBindContextFactory(PersistentEntities entities) {
public PersistentEntitiesBindContextFactory(PersistentEntities entities, ConversionService conversionService) {
Assert.notNull(entities, "PersistentEntities must not be null!");
this.entities = entities;
this.conversionService = conversionService;
}
@Override
public BindContext getBindContextFor(ObjectMapper mapper) {
return new JacksonBindContext(entities, mapper);
return new JacksonBindContext(entities, conversionService, mapper);
}
}

View File

@@ -17,6 +17,8 @@ package org.springframework.data.rest.webmvc.json.patch;
import java.util.Optional;
import org.springframework.expression.EvaluationContext;
/**
* Contextual mapping for he translation of JSON Pointer segments into property references on persistent types.
*
@@ -41,4 +43,12 @@ public interface BindContext {
* @return will never be {@literal null}.
*/
Optional<String> getReadableProperty(String segment, Class<?> type);
/**
* Returns the {@link EvaluationContext} to be used for evaluating the underlying SpEL expression.
*
* @return will never be {@literal null}.
* @since 3.7.9
*/
EvaluationContext getEvaluationContext();
}

View File

@@ -35,7 +35,6 @@ import org.springframework.expression.ExpressionException;
import org.springframework.expression.spel.SpelEvaluationException;
import org.springframework.expression.spel.SpelMessage;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.SimpleEvaluationContext;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
@@ -142,7 +141,7 @@ class SpelPath {
return READ_PATHS.computeIfAbsent(CacheKey.of(type, this, context),
key -> {
String mapped = new JsonPointerMapping(context).forRead(key.path.path, type);
return new TypedSpelPath(mapped, key.type);
return new TypedSpelPath(mapped, key.type, context.getEvaluationContext());
});
}
@@ -160,7 +159,7 @@ class SpelPath {
return WRITE_PATHS.computeIfAbsent(CacheKey.of(type, this, context),
key -> {
String mapped = new JsonPointerMapping(context).forWrite(key.path.path, type);
return new TypedSpelPath(mapped, key.type);
return new TypedSpelPath(mapped, key.type, context.getEvaluationContext());
});
}
@@ -253,17 +252,18 @@ class SpelPath {
private static final String INVALID_PATH_REFERENCE = "Invalid path reference %s on type %s";
private static final String INVALID_COLLECTION_INDEX = "Invalid collection index %s for collection of size %s; Use '…/-' or the collection's actual size as index to append to it";
private static final EvaluationContext CONTEXT = SimpleEvaluationContext.forReadWriteDataBinding().build();
private final Expression expression;
private final Class<?> type;
private final EvaluationContext context;
private TypedSpelPath(String path, Class<?> type) {
private TypedSpelPath(String path, Class<?> type, EvaluationContext context) {
super(path);
this.type = type;
this.expression = toSpel(path, type);
this.context = context;
}
/**
@@ -278,7 +278,7 @@ class SpelPath {
Assert.notNull(target, "Target must not be null!");
try {
return (T) expression.getValue(CONTEXT, target);
return (T) expression.getValue(context, target);
} catch (ExpressionException o_O) {
throw new PatchException("Unable to get value from target", o_O);
}
@@ -294,7 +294,7 @@ class SpelPath {
Assert.notNull(target, "Target must not be null!");
expression.setValue(CONTEXT, target, value);
expression.setValue(context, target, value);
}
/**
@@ -326,7 +326,7 @@ class SpelPath {
try {
return expression.getValueType(CONTEXT, root);
return expression.getValueType(context, root);
} catch (SpelEvaluationException o_O) {
@@ -460,11 +460,11 @@ class SpelPath {
}
private TypedSpelPath getParent() {
return new TypedSpelPath(path.substring(0, path.lastIndexOf('/')), type);
return new TypedSpelPath(path.substring(0, path.lastIndexOf('/')), type, context);
}
private TypeDescriptor getTypeDescriptor(Object target) {
return expression.getValueTypeDescriptor(CONTEXT, target);
return expression.getValueTypeDescriptor(context, target);
}
private Integer getTargetListIndex() {

View File

@@ -20,6 +20,7 @@ import java.util.Collection;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.rest.webmvc.json.BindContextFactory;
@@ -45,7 +46,8 @@ public class JsonPointerMappingTests {
context.getPersistentEntity(Sample.class);
PersistentEntities entities = new PersistentEntities(Arrays.asList(context));
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities);
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities,
DefaultConversionService.getSharedInstance());
ObjectMapper mapper = new ObjectMapper();
this.verifier = new JsonPointerMapping(factory.getBindContextFor(mapper));

View File

@@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.*;
import lombok.Data;
import lombok.Getter;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -33,6 +34,9 @@ import org.springframework.data.rest.webmvc.json.BindContextFactory;
import org.springframework.data.rest.webmvc.json.PersistentEntitiesBindContextFactory;
import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath;
import org.springframework.data.rest.webmvc.json.patch.SpelPath.WritingOperations;
import org.springframework.format.annotation.DateTimeFormat;
import org.springframework.format.annotation.DateTimeFormat.ISO;
import org.springframework.format.support.DefaultFormattingConversionService;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
@@ -57,7 +61,8 @@ class SpelPathUnitTests {
context.getPersistentEntity(Person.class);
PersistentEntities entities = new PersistentEntities(Arrays.asList(context));
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities);
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities,
new DefaultFormattingConversionService());
this.context = factory.getBindContextFor(new ObjectMapper());
}
@@ -167,6 +172,18 @@ class SpelPathUnitTests {
assertThat(path.getExpressionString()).isEqualTo("renamed");
}
@Test // #2233
void bindsDatesProperly() {
Person person = new Person();
SpelPath.untyped("/birthday")
.bindForWrite(Person.class, context)
.setValue(person, "2000-01-01");
assertThat(person.birthday).isEqualTo(LocalDate.of(2000, 1, 1));
}
// DATAREST-1338
@Data
@@ -175,6 +192,7 @@ class SpelPathUnitTests {
@JsonIgnore String hiddenProperty;
@Getter(onMethod = @__(@JsonIgnore)) String hiddenGetter;
@JsonProperty("demaner") String renamed;
@DateTimeFormat(iso = ISO.DATE) LocalDate birthday;
}
@Data

View File

@@ -17,6 +17,9 @@ package org.springframework.data.rest.webmvc.json.patch;
import java.util.Optional;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.spel.support.SimpleEvaluationContext;
public class TestPropertyPathContext implements BindContext {
public static final BindContext INSTANCE = new TestPropertyPathContext();
@@ -38,4 +41,13 @@ public class TestPropertyPathContext implements BindContext {
public Optional<String> getWritableProperty(String segment, Class<?> type) {
return Optional.of(segment);
}
/*
* (non-Javadoc)
* @see org.springframework.data.rest.webmvc.json.patch.BindContext#getEvaluationContext()
*/
@Override
public EvaluationContext getEvaluationContext() {
return SimpleEvaluationContext.forReadWriteDataBinding().build();
}
}