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.

Author: odrotbohm

Diff for: spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java

@@ -30,6 +30,7 @@
 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 @@ void setUp() {
 
 		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));
 

Diff for: spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java

@@ -496,7 +496,7 @@ public PersistentEntityResourceHandlerMethodArgumentResolver persistentEntityArg
 
 		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,

Diff for: spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonBindContext.java

@@ -17,9 +17,12 @@
 
 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 @@ public Optional<String> getWritableProperty(String segment, Class<?> type) {
 				.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))

Diff for: spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/PersistentEntitiesBindContextFactory.java

@@ -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 @@
 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);
 	}
 }

Diff for: spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/BindContext.java

@@ -17,6 +17,8 @@
 
 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();
 }

Diff for: spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java

@@ -35,7 +35,6 @@
 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;
@@ -130,7 +129,7 @@ public ReadingOperations bindForRead(Class<?> type, BindContext context) {
 			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());
 					});
 		}
 
@@ -148,7 +147,7 @@ public WritingOperations bindForWrite(Class<?> type, BindContext context) {
 			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());
 					});
 		}
 
@@ -233,17 +232,18 @@ static class TypedSpelPath extends SpelPath implements ReadingOperations, Writin
 
 		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;
 		}
 
 		/**
@@ -258,7 +258,7 @@ public <T> T getValue(Object target) {
 			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);
 			}
@@ -274,7 +274,7 @@ public void setValue(Object target, @Nullable Object value) {
 
 			Assert.notNull(target, "Target must not be null");
 
-			expression.setValue(CONTEXT, target, value);
+			expression.setValue(context, target, value);
 		}
 
 		/**
@@ -306,7 +306,7 @@ public Class<?> getType(Object root) {
 
 			try {
 
-				return expression.getValueType(CONTEXT, root);
+				return expression.getValueType(context, root);
 
 			} catch (SpelEvaluationException o_O) {
 
@@ -436,11 +436,11 @@ public String toString() {
 		}
 
 		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() {

Diff for: spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPointerMappingTests.java

@@ -20,6 +20,7 @@
 
 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 @@ void setUp() {
 		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));

Diff for: spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java

@@ -20,6 +20,7 @@
 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,7 @@
 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.support.DefaultFormattingConversionService;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
@@ -57,7 +59,8 @@ void setUp() {
 		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 +170,18 @@ void mapsRenamedProperty() {
 		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 +190,7 @@ static class Person {
 		@JsonIgnore String hiddenProperty;
 		@Getter(onMethod = @__(@JsonIgnore)) String hiddenGetter;
 		@JsonProperty("demaner") String renamed;
+		LocalDate birthday;
 	}
 
 	@Data

Diff for: spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/TestPropertyPathContext.java

@@ -17,6 +17,9 @@
 
 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 Optional<String> getReadableProperty(String segment, Class<?> type) {
 	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();
+	}
 }