Improve JSON Patch implementation.

Refactor JSON Patch application implementation to improve the property detection for which values are supposed to be set.

Fixes #2177.
This commit is contained in:
Oliver Drotbohm
2022-09-01 11:09:03 -05:00
parent 5a4923a849
commit 2ad081f75b
35 changed files with 893 additions and 197 deletions

View File

@@ -19,6 +19,8 @@ import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import static org.springframework.data.rest.tests.mongodb.TestUtils.*;
import lombok.Data;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -36,21 +38,28 @@ import org.springframework.data.rest.core.mapping.ResourceMappings;
import org.springframework.data.rest.tests.mongodb.Address;
import org.springframework.data.rest.tests.mongodb.User;
import org.springframework.data.rest.webmvc.RestMediaTypes;
import org.springframework.data.rest.webmvc.json.BindContextFactory;
import org.springframework.data.rest.webmvc.json.DomainObjectReader;
import org.springframework.data.rest.webmvc.json.PersistentEntitiesBindContextFactory;
import org.springframework.data.rest.webmvc.json.patch.PatchException;
import org.springframework.data.rest.webmvc.mapping.Associations;
import org.springframework.http.converter.HttpMessageNotReadableException;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.databind.ObjectMapper;
/**
* Unit tests for {@link JsonPatchHandler}.
*
* @author Oliver Gierke
* @author Mark Paluch
*/
@ExtendWith(MockitoExtension.class)
class JsonPatchHandlerUnitTests {
JsonPatchHandler handler;
ObjectMapper mapper = new ObjectMapper();
User user;
@Mock ResourceMappings mappings;
@@ -63,12 +72,13 @@ class JsonPatchHandlerUnitTests {
MongoMappingContext context = new MongoMappingContext();
context.setSimpleTypeHolder(conversions.getSimpleTypeHolder());
context.getPersistentEntity(User.class);
context.getPersistentEntity(WithIgnoredProperties.class);
PersistentEntities entities = new PersistentEntities(Arrays.asList(context));
Associations associations = new Associations(mappings, mock(RepositoryRestConfiguration.class));
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities);
this.handler = new JsonPatchHandler(new ObjectMapper(), new DomainObjectReader(entities, associations));
this.handler = new JsonPatchHandler(factory, new DomainObjectReader(entities, associations));
Address address = new Address();
address.street = "Foo";
@@ -86,7 +96,7 @@ class JsonPatchHandlerUnitTests {
String input = "[{ \"op\": \"replace\", \"path\": \"/address/zipCode\", \"value\": \"ZIP\" },"
+ "{ \"op\": \"remove\", \"path\": \"/lastname\" }]";
User result = handler.applyPatch(asStream(input), user);
User result = handler.applyPatch(asStream(input), user, mapper);
assertThat(result.lastname).isNull();
assertThat(result.address.zipCode).isEqualTo("ZIP");
@@ -97,7 +107,7 @@ class JsonPatchHandlerUnitTests {
String input = "{ \"address\" : { \"zipCode\" : \"ZIP\"}, \"lastname\" : null }";
User result = handler.applyMergePatch(asStream(input), user);
User result = handler.applyMergePatch(asStream(input), user, mapper);
assertThat(result.lastname).isNull();
assertThat(result.address.zipCode).isEqualTo("ZIP");
@@ -119,7 +129,7 @@ class JsonPatchHandlerUnitTests {
String input = "[{ \"op\": \"remove\", \"path\": \"/colleagues/0\" }]";
handler.applyPatch(asStream(input), user);
handler.applyPatch(asStream(input), user, mapper);
assertThat(user.colleagues).hasSize(1);
assertThat(user.colleagues.get(0).firstname).isEqualTo(christoph.firstname);
@@ -129,7 +139,49 @@ class JsonPatchHandlerUnitTests {
void hintsToMediaTypeIfBodyCantBeRead() throws Exception {
assertThatExceptionOfType(HttpMessageNotReadableException.class)
.isThrownBy(() -> handler.applyPatch(asStream("{ \"foo\" : \"bar\" }"), new User()))
.isThrownBy(() -> handler.applyPatch(asStream("{ \"foo\" : \"bar\" }"), new User(), mapper))
.withMessageContaining(RestMediaTypes.JSON_PATCH_JSON.toString());
}
@Test
void skipsReplaceConditionally() throws Exception {
WithIgnoredProperties object = new WithIgnoredProperties();
assertThatExceptionOfType(PatchException.class).isThrownBy(() -> {
handler.applyPatch(asStream("[{ \"op\": \"replace\", \"path\": \"/password\", \"value\": \"hello\" }]"), object,
mapper);
});
WithIgnoredProperties result = handler
.applyPatch(asStream("[{ \"op\": \"replace\", \"path\": \"/name\", \"value\": \"hello\" }]"), object, mapper);
assertThat(result.name).isEqualTo("hello");
}
@Test
void skipsCopyConditionally() throws Exception {
WithIgnoredProperties object = new WithIgnoredProperties();
object.setName("hello");
assertThatExceptionOfType(PatchException.class).isThrownBy(() -> {
handler.applyPatch(asStream("[{ \"op\": \"copy\", \"path\": \"/password\", \"from\": \"/name\" }]"), object,
mapper);
});
WithIgnoredProperties result = handler
.applyPatch(asStream("[{ \"op\": \"copy\", \"path\": \"/lastname\", \"from\": \"/name\" }]"), object, mapper);
assertThat(result.lastname).isEqualTo("hello");
}
@JsonIgnoreProperties("password")
@Data
static class WithIgnoredProperties {
String name, lastname, password;
@JsonIgnore String ssn;
}
}

View File

@@ -19,14 +19,15 @@ import java.io.InputStream;
import org.springframework.data.rest.webmvc.IncomingRequest;
import org.springframework.data.rest.webmvc.RestMediaTypes;
import org.springframework.data.rest.webmvc.json.BindContextFactory;
import org.springframework.data.rest.webmvc.json.DomainObjectReader;
import org.springframework.data.rest.webmvc.json.patch.BindContext;
import org.springframework.data.rest.webmvc.json.patch.JsonPatchPatchConverter;
import org.springframework.data.rest.webmvc.json.patch.Patch;
import org.springframework.data.rest.webmvc.util.InputStreamHttpInputMessage;
import org.springframework.http.converter.HttpMessageNotReadableException;
import org.springframework.util.Assert;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -44,26 +45,23 @@ import com.fasterxml.jackson.databind.node.ObjectNode;
*/
class JsonPatchHandler {
private final ObjectMapper mapper;
private final ObjectMapper sourceMapper;
private final BindContextFactory factory;
private final DomainObjectReader reader;
/**
* Creates a new {@link JsonPatchHandler} with the given {@link ObjectMapper} and {@link DomainObjectReader}.
* Creates a new {@link JsonPatchHandler} with the given {@link JacksonBindContextFactory} and
* {@link DomainObjectReader}.
*
* @param mapper must not be {@literal null}.
* @param factory must not be {@literal null}.
* @param reader must not be {@literal null}.
*/
public JsonPatchHandler(ObjectMapper mapper, DomainObjectReader reader) {
public JsonPatchHandler(BindContextFactory factory, DomainObjectReader reader) {
Assert.notNull(mapper, "ObjectMapper must not be null!");
Assert.notNull(reader, "DomainObjectReader must not be null!");
Assert.notNull(factory, "BindContextFactory must not be null");
Assert.notNull(reader, "DomainObjectReader must not be null");
this.mapper = mapper;
this.factory = factory;
this.reader = reader;
this.sourceMapper = mapper.copy();
this.sourceMapper.setSerializationInclusion(Include.NON_NULL);
}
/**
@@ -74,29 +72,33 @@ class JsonPatchHandler {
* @return
* @throws Exception
*/
public <T> T apply(IncomingRequest request, T target) throws Exception {
public <T> T apply(IncomingRequest request, T target, ObjectMapper mapper) throws Exception {
Assert.notNull(request, "Request must not be null!");
Assert.isTrue(request.isPatchRequest(), "Cannot handle non-PATCH request!");
Assert.notNull(target, "Target must not be null!");
if (request.isJsonPatchRequest()) {
return applyPatch(request.getBody(), target);
return applyPatch(request.getBody(), target, mapper);
} else {
return applyMergePatch(request.getBody(), target);
return applyMergePatch(request.getBody(), target, mapper);
}
}
@SuppressWarnings("unchecked")
<T> T applyPatch(InputStream source, T target) throws Exception {
return getPatchOperations(source).apply(target, (Class<T>) target.getClass());
<T> T applyPatch(InputStream source, T target, ObjectMapper mapper) throws Exception {
Class<?> type = target.getClass();
BindContext context = factory.getBindContextFor(mapper);
return getPatchOperations(source, mapper, context).apply(target, (Class<T>) target.getClass());
}
<T> T applyMergePatch(InputStream source, T existingObject) throws Exception {
<T> T applyMergePatch(InputStream source, T existingObject, ObjectMapper mapper) throws Exception {
return reader.read(source, existingObject, mapper);
}
<T> T applyPut(ObjectNode source, T existingObject) throws Exception {
<T> T applyPut(ObjectNode source, T existingObject, ObjectMapper mapper) throws Exception {
return reader.readPut(source, existingObject, mapper);
}
@@ -104,13 +106,14 @@ class JsonPatchHandler {
* Returns all {@link JsonPatchOperation}s to be applied.
*
* @param source must not be {@literal null}.
* @param mapper must not be {@literal null}.
* @return
* @throws HttpMessageNotReadableException in case the payload can't be read.
*/
private Patch getPatchOperations(InputStream source) {
private Patch getPatchOperations(InputStream source, ObjectMapper mapper, BindContext context) {
try {
return new JsonPatchPatchConverter(mapper).convert(mapper.readTree(source));
return new JsonPatchPatchConverter(mapper, context).convert(mapper.readTree(source));
} catch (Exception o_O) {
throw new HttpMessageNotReadableException(
String.format("Could not read PATCH operations! Expected %s!", RestMediaTypes.JSON_PATCH_JSON), o_O,

View File

@@ -36,6 +36,7 @@ import org.springframework.data.rest.webmvc.PersistentEntityResource;
import org.springframework.data.rest.webmvc.PersistentEntityResource.Builder;
import org.springframework.data.rest.webmvc.ResourceNotFoundException;
import org.springframework.data.rest.webmvc.RootResourceInformation;
import org.springframework.data.rest.webmvc.json.BindContextFactory;
import org.springframework.data.rest.webmvc.json.DomainObjectReader;
import org.springframework.data.rest.webmvc.support.BackendIdHandlerMethodArgumentResolver;
import org.springframework.http.MediaType;
@@ -69,15 +70,15 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha
private final List<HttpMessageConverter<?>> messageConverters;
private final RootResourceInformationHandlerMethodArgumentResolver resourceInformationResolver;
private final BackendIdHandlerMethodArgumentResolver idResolver;
private final DomainObjectReader reader;
private final PluginRegistry<EntityLookup<?>, Class<?>> lookups;
private final ConversionService conversionService = new DefaultConversionService();
private final JsonPatchHandler jsonPatchHandler;
public PersistentEntityResourceHandlerMethodArgumentResolver(
List<HttpMessageConverter<?>> messageConverters,
RootResourceInformationHandlerMethodArgumentResolver resourceInformationResolver,
BackendIdHandlerMethodArgumentResolver idResolver, DomainObjectReader reader,
PluginRegistry<EntityLookup<?>, Class<?>> lookups) {
PluginRegistry<EntityLookup<?>, Class<?>> lookups, BindContextFactory factory) {
Assert.notNull(messageConverters, "HttpMessageConverters must not be null!");
Assert.notNull(resourceInformationResolver, "RootResourceInformation resolver must not be null!");
@@ -88,8 +89,8 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha
this.messageConverters = messageConverters;
this.resourceInformationResolver = resourceInformationResolver;
this.idResolver = idResolver;
this.reader = reader;
this.lookups = lookups;
this.jsonPatchHandler = new JsonPatchHandler(mapper -> factory.getBindContextFor(mapper), reader);
}
/*
@@ -210,8 +211,7 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha
try {
JsonPatchHandler handler = new JsonPatchHandler(mapper, reader);
return handler.apply(request, existingObject);
return jsonPatchHandler.apply(request, existingObject, mapper);
} catch (Exception o_O) {
@@ -228,10 +228,9 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha
try {
JsonPatchHandler handler = new JsonPatchHandler(mapper, reader);
JsonNode jsonNode = mapper.readTree(request.getBody());
return handler.applyPut((ObjectNode) jsonNode, existingObject);
return jsonPatchHandler.applyPut((ObjectNode) jsonNode, existingObject, mapper);
} catch (Exception o_O) {
throw new HttpMessageNotReadableException(String.format(ERROR_MESSAGE, existingObject.getClass()), o_O,

View File

@@ -495,13 +495,15 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon
@Qualifier("defaultMessageConverters") List<HttpMessageConverter<?>> defaultMessageConverters,
RootResourceInformationHandlerMethodArgumentResolver repoRequestArgumentResolver, Associations associationLinks,
BackendIdHandlerMethodArgumentResolver backendIdHandlerMethodArgumentResolver,
PersistentEntities persistentEntities) {
PersistentEntities entities) {
PluginRegistry<EntityLookup<?>, Class<?>> lookups = PluginRegistry.of(getEntityLookups());
DomainObjectReader reader = new DomainObjectReader(entities, associationLinks);
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities);
return new PersistentEntityResourceHandlerMethodArgumentResolver(defaultMessageConverters,
repoRequestArgumentResolver, backendIdHandlerMethodArgumentResolver,
new DomainObjectReader(persistentEntities, associationLinks), lookups);
reader, lookups, factory);
}
/**

View File

@@ -0,0 +1,36 @@
/*
* Copyright 2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.rest.webmvc.json;
import org.springframework.data.rest.webmvc.json.patch.BindContext;
import com.fasterxml.jackson.databind.ObjectMapper;
/**
* Factory to create {@link BindContext} instances.
*
* @author Oliver Drotbohm
*/
public interface BindContextFactory {
/**
* Creates a {@link BindContext} for the given {@link ObjectMapper}.
*
* @param mapper must not be {@literal null}.
* @return will never be {@literal null}.
*/
BindContext getBindContextFor(ObjectMapper mapper);
}

View File

@@ -246,7 +246,7 @@ public class DomainObjectReader {
JsonNode child = entry.getValue();
String fieldName = entry.getKey();
if (!mappedProperties.isWritableProperty(fieldName)) {
if (!mappedProperties.isWritableField(fieldName)) {
i.remove();
continue;

View File

@@ -0,0 +1,74 @@
/*
* Copyright 2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.rest.webmvc.json;
import java.util.Optional;
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.util.Assert;
import com.fasterxml.jackson.databind.ObjectMapper;
/**
* A {@link BindContext} that uses a Jackson {@link ObjectMapper} to inspect its metadata to decide whether segments are
* exposed or not.
*
* @author Oliver Drotbohm
*/
class JacksonBindContext implements BindContext {
private final PersistentEntities entities;
private final ObjectMapper mapper;
/**
* Creates a new {@link JacksonBindContext} for the given {@link PersistentEntities} and {@link ObjectMapper}.
*
* @param entities must not be {@literal null}.
* @param mapper must not be {@literal null}.
*/
public JacksonBindContext(PersistentEntities entities, ObjectMapper mapper) {
Assert.notNull(entities, "PersistentEntities must not be null");
Assert.notNull(mapper, "ObjectMapper must not be null");
this.entities = entities;
this.mapper = mapper;
}
@Override
public Optional<String> getReadableProperty(String segment, Class<?> type) {
return getProperty(entities.getPersistentEntity(type)
.map(it -> MappedProperties.forSerialization(it, mapper))
.filter(it -> it.isReadableField(segment)), segment);
}
@Override
public Optional<String> getWritableProperty(String segment, Class<?> type) {
return getProperty(entities.getPersistentEntity(type)
.map(it -> MappedProperties.forDeserialization(it, mapper))
.filter(it -> it.isWritableField(segment)), segment);
}
private static Optional<String> getProperty(Optional<MappedProperties> properties, String segment) {
return properties.map(it -> it.getPersistentProperty(segment))
.map(PersistentProperty::getName);
}
}

View File

@@ -26,9 +26,11 @@ import java.util.Set;
import org.springframework.data.mapping.PersistentEntity;
import org.springframework.data.mapping.PersistentProperty;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -83,7 +85,15 @@ class MappedProperties {
// collection of ignored properties in the first place. See
// https://github.com/FasterXML/jackson-databind/issues/2531
this.ignoredPropertyNames = description.getIgnoredPropertyNames();
this.ignoredPropertyNames = new HashSet<>(description.getIgnoredPropertyNames());
JsonIgnoreProperties annotation = entity.findAnnotation(JsonIgnoreProperties.class);
if (annotation != null) {
for (String property : annotation.value()) {
ignoredPropertyNames.add(property);
}
}
for (BeanPropertyDefinition property : description.findProperties()) {
@@ -172,6 +182,7 @@ class MappedProperties {
* @param fieldName must not be empty or {@literal null}.
* @return the {@link PersistentProperty} backing the field with the field name.
*/
@Nullable
public PersistentProperty<?> getPersistentProperty(String fieldName) {
Assert.hasText(fieldName, "Field name must not be null or empty!");
@@ -229,7 +240,31 @@ class MappedProperties {
* @param name must not be {@literal null} or empty.
* @return
*/
public boolean isWritableProperty(String name) {
public boolean isWritableField(String name) {
Assert.hasText(name, "Property name must not be null or empty");
if (ignoredPropertyNames.contains(name)) {
return false;
}
PersistentProperty<?> property = fieldNameToProperty.get(name);
return property != null ? property.isWritable() : anySetterFound;
}
public boolean isReadableField(String name) {
Assert.hasText(name, "Property name must not be null or empty");
if (ignoredPropertyNames.contains(name)) {
return false;
}
return fieldNameToProperty.get(name) != null;
}
public boolean isExposedProperty(String name) {
Assert.hasText(name, "Property name must not be null or empty!");

View File

@@ -0,0 +1,49 @@
/*
* Copyright 2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.rest.webmvc.json;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.rest.webmvc.json.patch.BindContext;
import org.springframework.util.Assert;
import com.fasterxml.jackson.databind.ObjectMapper;
/**
* A {@link BindContextFactory} based on {@link PersistentEntities}.
*
* @author Oliver Drotbohm
*/
public class PersistentEntitiesBindContextFactory implements BindContextFactory {
private final PersistentEntities entities;
/**
* Creates a new {@link PersistentEntitiesBindContextFactory} for the given {@link PersistentEntities}.
*
* @param entities must not be {@literal null}.
*/
public PersistentEntitiesBindContextFactory(PersistentEntities entities) {
Assert.notNull(entities, "PersistentEntities must not be null!");
this.entities = entities;
}
@Override
public BindContext getBindContextFor(ObjectMapper mapper) {
return new JacksonBindContext(entities, mapper);
}
}

View File

@@ -42,11 +42,12 @@ class AddOperation extends PatchOperation {
/*
* (non-Javadoc)
* @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class)
* @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class, org.springframework.data.rest.webmvc.json.PropertyFilter)
*/
@Override
void perform(Object targetObject, Class<?> type) {
path.bindTo(type).addValue(targetObject, evaluateValueFromTarget(targetObject, type));
void perform(Object target, Class<?> type, BindContext context) {
path.bindForWrite(type, context).addValue(target, evaluateValueFromTarget(target, type, context));
}
/*
@@ -54,12 +55,12 @@ class AddOperation extends PatchOperation {
* @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#evaluateValueFromTarget(java.lang.Object, java.lang.Class)
*/
@Override
protected Object evaluateValueFromTarget(Object targetObject, Class<?> entityType) {
protected Object evaluateValueFromTarget(Object targetObject, Class<?> entityType, BindContext context) {
if (!path.isAppend()) {
return super.evaluateValueFromTarget(targetObject, entityType);
return super.evaluateValueFromTarget(targetObject, entityType, context);
}
return evaluate(path.bindTo(entityType).getLeafType());
return evaluate(path.bindForWrite(entityType, context).getLeafType());
}
}

View File

@@ -0,0 +1,44 @@
/*
* Copyright 2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.rest.webmvc.json.patch;
import java.util.Optional;
/**
* Contextual mapping for he translation of JSON Pointer segments into property references on persistent types.
*
* @author Oliver Drotbohm
*/
public interface BindContext {
/**
* Returns the name of the writable property for the given JSON pointer segment.
*
* @param segment must not be {@literal null} or empty.
* @param type must not be {@literal null}.
* @return will never be {@literal null}.
*/
Optional<String> getWritableProperty(String segment, Class<?> type);
/**
* Return the name of the readable property for the given JSON pointer segment.
*
* @param segment must not be {@literal null} or empty.
* @param type must not be {@literal null}.
* @return will never be {@literal null}.
*/
Optional<String> getReadableProperty(String segment, Class<?> type);
}

View File

@@ -82,7 +82,9 @@ class CopyOperation extends PatchOperation {
* @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class)
*/
@Override
void perform(Object target, Class<?> type) {
path.bindTo(type).copyFrom(from, target);
void perform(Object target, Class<?> type, BindContext context) {
path.bindForWrite(type, context) //
.copyFrom(from, target, context);
}
}

View File

@@ -36,12 +36,14 @@ import com.fasterxml.jackson.databind.node.ArrayNode;
public class JsonPatchPatchConverter implements PatchConverter<JsonNode> {
private final ObjectMapper mapper;
private final BindContext context;
public JsonPatchPatchConverter(ObjectMapper mapper) {
public JsonPatchPatchConverter(ObjectMapper mapper, BindContext context) {
Assert.notNull(mapper, "ObjectMapper must not be null!");
this.mapper = mapper;
this.context = context;
}
/**
@@ -87,7 +89,7 @@ public class JsonPatchPatchConverter implements PatchConverter<JsonNode> {
}
}
return new Patch(ops);
return new Patch(ops, context);
}
private Object valueFromJsonNode(String path, JsonNode valueNode) {

View File

@@ -0,0 +1,127 @@
/*
* Copyright 2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.rest.webmvc.json.patch;
import lombok.RequiredArgsConstructor;
import java.util.Optional;
import java.util.function.BiFunction;
import org.springframework.data.mapping.PropertyPath;
import org.springframework.data.mapping.PropertyReferenceException;
import org.springframework.data.util.ClassTypeInformation;
import org.springframework.data.util.TypeInformation;
import org.springframework.util.StringUtils;
/**
* @author Oliver Drotbohm
*/
@RequiredArgsConstructor
class JsonPointerMapping {
private final BiFunction<String, Class<?>, Optional<String>> reader, writer;
public JsonPointerMapping(BindContext context) {
this.reader = context::getReadableProperty;
this.writer = context::getWritableProperty;
}
/**
* Maps the given JSON Pointer to the given type to ultimately read the attribute pointed to.
*
* @param pointer must not be {@literal null}.
* @param type must not be {@literal null}.
* @return a JSON Pointer with the segments translated into the matching property references.
*/
public String forRead(String pointer, Class<?> type) {
return verify(pointer, type, reader, "readable");
}
/**
* Maps the given JSON Pointer to the given type to ultimately write the attribute pointed to.
*
* @param pointer must not be {@literal null}.
* @param type must not be {@literal null}.
* @return a JSON Pointer with the segments translated into the matching property references.
*/
public String forWrite(String pointer, Class<?> type) {
return verify(pointer, type, writer, "writable");
}
private String verify(String pointer, Class<?> type, BiFunction<String, Class<?>, Optional<String>> filter,
String qualifier) {
String[] strings = pointer.split("/");
if (strings.length == 0) {
return pointer;
}
PropertyPath base = null;
StringBuilder result = new StringBuilder();
TypeInformation<?> currentType = ClassTypeInformation.from(type);
for (int i = 0; i < strings.length; i++) {
String segment = strings[i];
if (!StringUtils.hasText(segment)) {
continue;
}
if (currentType != null && currentType.isMap()) {
result.append("/").append(segment);
currentType = currentType.getActualType();
continue;
}
if (segment.equals("-") || segment.matches("\\d+")) {
result.append("/").append(segment);
currentType = currentType.getActualType();
continue;
}
TypeInformation<?> rejectType = currentType;
// Use given filter for final segment, reader otherwise
String property = (i == strings.length - 1 ? filter : reader) //
.apply(segment, currentType.getType()) //
.orElseThrow(() -> reject(segment, rejectType, pointer, qualifier));
try {
base = base == null ? PropertyPath.from(property, type) : base.nested(segment);
} catch (PropertyReferenceException o_O) {
throw reject(segment, rejectType, pointer, qualifier);
}
currentType = base.getTypeInformation();
result.append("/").append(property);
}
return result.toString();
}
private static PatchException reject(String segment, TypeInformation<?> type, String pointer, String qualifier) {
return new PatchException(
String.format("Couldn't find %s property for pointer segment %s on %s in %s", qualifier, segment,
type.getType(), pointer));
}
}

View File

@@ -75,7 +75,7 @@ class MoveOperation extends PatchOperation {
* @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class)
*/
@Override
void perform(Object target, Class<?> type) {
path.bindTo(type).moveFrom(from, target);
void perform(Object target, Class<?> type, BindContext context) {
path.bindForWrite(type, context).moveFrom(from, target, context);
}
}

View File

@@ -35,9 +35,12 @@ import org.springframework.data.util.Streamable;
public class Patch implements Streamable<PatchOperation> {
private final List<PatchOperation> operations;
private final BindContext context;
public Patch(List<PatchOperation> operations, BindContext context) {
public Patch(List<PatchOperation> operations) {
this.operations = operations;
this.context = context;
}
/**
@@ -60,7 +63,7 @@ public class Patch implements Streamable<PatchOperation> {
public <T> T apply(T in, Class<T> type) throws PatchException {
for (PatchOperation operation : operations) {
operation.perform(in, type);
operation.perform(in, type, context);
}
return in;
@@ -79,7 +82,7 @@ public class Patch implements Streamable<PatchOperation> {
public <T> List<T> apply(List<T> in, Class<T> type) throws PatchException {
for (PatchOperation operation : operations) {
operation.perform(in, type);
operation.perform(in, type, context);
}
return in;

View File

@@ -59,8 +59,8 @@ public abstract class PatchOperation {
* @return the result of late-value evaluation if the value is a {@link LateObjectEvaluator}; the value itself
* otherwise.
*/
protected Object evaluateValueFromTarget(Object targetObject, Class<?> entityType) {
return evaluate(path.bindTo(entityType).getType(targetObject));
protected Object evaluateValueFromTarget(Object targetObject, Class<?> entityType, BindContext context) {
return evaluate(path.bindForRead(entityType, context).getType(targetObject));
}
protected final Object evaluate(Class<?> type) {
@@ -73,5 +73,5 @@ public abstract class PatchOperation {
* @param target the target of the operation, must not be {@literal null}.
* @param type must not be {@literal null}.
*/
abstract void perform(Object target, Class<?> type);
abstract void perform(Object target, Class<?> type, BindContext context);
}

View File

@@ -44,7 +44,7 @@ class RemoveOperation extends PatchOperation {
* @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class)
*/
@Override
void perform(Object target, Class<?> type) {
path.bindTo(type).removeFrom(target);
void perform(Object target, Class<?> type, BindContext context) {
path.bindForWrite(type, context).removeFrom(target);
}
}

View File

@@ -39,7 +39,6 @@ class ReplaceOperation extends PatchOperation {
return new ReplaceOperationBuilder(path);
}
static class ReplaceOperationBuilder {
private final String path;
@@ -58,7 +57,7 @@ class ReplaceOperation extends PatchOperation {
* @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class)
*/
@Override
void perform(Object target, Class<?> type) {
path.bindTo(type).setValue(target, evaluateValueFromTarget(target, type));
void perform(Object target, Class<?> type, BindContext context) {
path.bindForWrite(type, context).setValue(target, evaluateValueFromTarget(target, type, context));
}
}

View File

@@ -42,10 +42,14 @@ import org.springframework.util.CollectionUtils;
import org.springframework.util.ConcurrentReferenceHashMap;
import org.springframework.util.StringUtils;
import com.fasterxml.jackson.databind.ObjectMapper;
/**
* Value object to represent a SpEL-backed patch path.
*
* @author Oliver Gierke
* @author Mark Paluch
* @author Greg Turnquist
*/
class SpelPath {
@@ -53,6 +57,8 @@ class SpelPath {
private static final String APPEND_CHARACTER = "-";
private static final Map<String, UntypedSpelPath> UNTYPED_PATHS = new ConcurrentReferenceHashMap<>(32);
private static final ObjectMapper objectMapper = new ObjectMapper();
protected final String path;
private SpelPath(String path) {
@@ -62,10 +68,6 @@ class SpelPath {
this.path = path;
}
public String getPath() {
return this.path;
}
/**
* Returns a {@link UntypedSpelPath} for the given source.
*
@@ -76,16 +78,6 @@ class SpelPath {
return UNTYPED_PATHS.computeIfAbsent(source, UntypedSpelPath::new);
}
/**
* Returns a {@link TypedSpelPath} for the given source and type.
*
* @param source must not be {@literal null}.
* @return
*/
public static TypedSpelPath typed(String source, Class<?> type) {
return untyped(source).bindTo(type);
}
/**
* Returns whether the current path represents an append path, i.e. is supposed to append to collection.
*
@@ -135,55 +127,61 @@ class SpelPath {
static class UntypedSpelPath extends SpelPath {
private static final Map<CacheKey, TypedSpelPath> READ_PATHS = new ConcurrentReferenceHashMap<>(256);
private static final Map<CacheKey, TypedSpelPath> WRITE_PATHS = new ConcurrentReferenceHashMap<>(256);
private UntypedSpelPath(String path) {
super(path);
}
public ReadingOperations bindForRead(Class<?> type, BindContext context) {
Assert.notNull(path, "Path must not be null");
Assert.notNull(type, "Type must not be null");
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);
});
}
/**
* Returns a {@link TypedSpelPath} binding the expression to the given type.
*
* @param type must not be {@literal null}.
* @return
*/
public TypedSpelPath bindTo(Class<?> type) {
public WritingOperations bindForWrite(Class<?> type, BindContext context) {
Assert.notNull(type, "Type must not be null!");
Assert.notNull(context, "BindContext must not be null");
Assert.notNull(type, "Type must not be null");
return TypedSpelPath.of(this, type);
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);
});
}
}
/**
* A {@link SpelPath} that has typing information tied to it.
*
* @author Oliver Gierke
*/
static class TypedSpelPath extends 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 Map<CacheKey, TypedSpelPath> TYPED_PATHS = new ConcurrentReferenceHashMap<>(32);
private static final EvaluationContext CONTEXT = SimpleEvaluationContext.forReadWriteDataBinding().build();
private final Expression expression;
private final Class<?> type;
private static final class CacheKey {
private final Class<?> type;
private final UntypedSpelPath path;
private final BindContext context;
private CacheKey(Class<?> type, UntypedSpelPath path) {
private CacheKey(Class<?> type, UntypedSpelPath path, BindContext context) {
Assert.notNull(type, "Type must not be null!");
Assert.notNull(path, "UntypedSpelPath must not be null!");
this.type = type;
this.path = path;
this.context = context;
}
public static CacheKey of(final Class<?> type, final UntypedSpelPath path) {
return new CacheKey(type, path);
public static CacheKey of(Class<?> type, UntypedSpelPath path, BindContext context) {
return new CacheKey(type, path, context);
}
/*
@@ -204,7 +202,8 @@ class SpelPath {
CacheKey that = (CacheKey) o;
return Objects.equals(type, that.type) //
&& Objects.equals(path, that.path);
&& Objects.equals(path, that.path) //
&& Objects.equals(context, that.context);
}
/*
@@ -213,40 +212,58 @@ class SpelPath {
*/
@Override
public int hashCode() {
return Objects.hash(type, path);
}
/*
* (non-Javadoc)
* @see java.lang.Object#toString()
*/
@Override
public java.lang.String toString() {
return "SpelPath.TypedSpelPath.CacheKey(type=" + type + ", path=" + path + ")";
return Objects.hash(type, path, context);
}
}
}
private TypedSpelPath(UntypedSpelPath path, Class<?> type) {
interface CommonOperations {
super(path.path);
String getExpressionString();
}
interface ReadingOperations extends CommonOperations {
<T> T getValue(Object target);
Class<?> getType(Object root);
}
interface WritingOperations extends CommonOperations {
Class<?> getLeafType();
Object removeFrom(Object target);
void addValue(Object target, Object value);
void setValue(Object target, @Nullable Object value);
void copyFrom(UntypedSpelPath path, Object source, BindContext context);
void moveFrom(UntypedSpelPath path, Object source, BindContext context);
}
/**
* A {@link SpelPath} that has typing information tied to it.
*
* @author Oliver Gierke
*/
static class TypedSpelPath extends SpelPath implements ReadingOperations, WritingOperations {
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 TypedSpelPath(String path, Class<?> type) {
super(path);
this.type = type;
this.expression = toSpel(path.path, type);
}
/**
* Returns the {@link TypedSpelPath} for the given {@link SpelPath} and type.
*
* @param path must not be {@literal null}.
* @param type must not be {@literal null}.
* @return
*/
public static TypedSpelPath of(UntypedSpelPath path, Class<?> type) {
Assert.notNull(path, "Path must not be null!");
Assert.notNull(type, "Type must not be null!");
return TYPED_PATHS.computeIfAbsent(CacheKey.of(type, path), key -> new TypedSpelPath(key.path, key.type));
this.expression = toSpel(path, type);
}
/**
@@ -334,12 +351,12 @@ class SpelPath {
* @param source the source object to look the value up from, must not be {@literal null}.
* @return
*/
public void copyFrom(UntypedSpelPath path, Object source) {
public void copyFrom(UntypedSpelPath path, Object source, BindContext context) {
Assert.notNull(path, "Source path must not be null!");
Assert.notNull(source, "Source value must not be null!");
addValue(source, path.bindTo(type).getValue(source));
addValue(source, path.bindForRead(type, context).getValue(source));
}
/**
@@ -350,12 +367,15 @@ class SpelPath {
* @param source the source object to look the value up from, must not be {@literal null}.
* @return
*/
public void moveFrom(UntypedSpelPath path, Object source) {
public void moveFrom(UntypedSpelPath path, Object source, BindContext context) {
Assert.notNull(path, "Source path must not be null!");
Assert.notNull(source, "Source value must not be null!");
addValue(source, path.bindTo(type).removeFrom(source));
// Verify we are allowed to read the source
path.bindForRead(type, context);
addValue(source, path.bindForWrite(type, context).removeFrom(source));
}
/**
@@ -377,7 +397,7 @@ class SpelPath {
setValue(target, null);
return value;
} catch (SpelEvaluationException o_O) {
throw new PatchException("Path '" + path + "' is not nullable.", o_O);
throw new PatchException("Path '" + path + "' is not nullable", o_O);
}
} else {
@@ -440,10 +460,7 @@ class SpelPath {
}
private TypedSpelPath getParent() {
return SpelPath //
.untyped(path.substring(0, path.lastIndexOf('/'))) //
.bindTo(type);
return new TypedSpelPath(path.substring(0, path.lastIndexOf('/')), type);
}
private TypeDescriptor getTypeDescriptor(Object target) {
@@ -658,6 +675,8 @@ class SpelPath {
? spelSegment.concat(".") //
: spelSegment;
Class<?> currentType = basePath == null ? type : basePath.getLeafType();
try {
PropertyPath path = basePath == null //

View File

@@ -71,10 +71,10 @@ class TestOperation extends PatchOperation {
* @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class)
*/
@Override
void perform(Object target, Class<?> type) {
void perform(Object target, Class<?> type, BindContext context) {
Object expected = normalizeIfNumber(evaluateValueFromTarget(target, type));
Object actual = normalizeIfNumber(path.bindTo(type).getValue(target));
Object expected = normalizeIfNumber(evaluateValueFromTarget(target, type, context));
Object actual = normalizeIfNumber(path.bindForRead(type, context).getValue(target));
if (!ObjectUtils.nullSafeEquals(expected, actual)) {
throw new PatchException("Test against path '" + path + "' failed.");

View File

@@ -0,0 +1,17 @@
/*
* Copyright 2013-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
@org.springframework.lang.NonNullApi
package org.springframework.data.rest.webmvc.json.patch;

View File

@@ -36,6 +36,8 @@ import org.springframework.data.rest.core.support.EntityLookup;
import org.springframework.data.rest.webmvc.PersistentEntityResource;
import org.springframework.data.rest.webmvc.RootResourceInformation;
import org.springframework.data.rest.webmvc.json.DomainObjectReader;
import org.springframework.data.rest.webmvc.json.BindContextFactory;
import org.springframework.data.rest.webmvc.json.patch.TestPropertyPathContext;
import org.springframework.data.rest.webmvc.support.BackendIdHandlerMethodArgumentResolver;
import org.springframework.http.HttpInputMessage;
import org.springframework.http.MediaType;
@@ -55,6 +57,8 @@ import org.springframework.web.method.support.ModelAndViewContainer;
*/
class PersistentEntityResourceHandlerMethodArgumentResolverUnitTests {
private static final BindContextFactory FACTORY = mapper -> TestPropertyPathContext.INSTANCE;
HttpMessageConverter<?> converter;
RootResourceInformationHandlerMethodArgumentResolver rootResourceResolver;
BackendIdHandlerMethodArgumentResolver backendIdResolver;
@@ -79,7 +83,7 @@ class PersistentEntityResourceHandlerMethodArgumentResolverUnitTests {
PersistentEntityResourceHandlerMethodArgumentResolver argumentResolver = new PersistentEntityResourceHandlerMethodArgumentResolver(
Arrays.<HttpMessageConverter<?>> asList(converter), rootResourceResolver, backendIdResolver, reader,
PluginRegistry.empty());
PluginRegistry.empty(), FACTORY);
HttpServletRequest request = new MockHttpServletRequest("PUT", "/foo/4711");
@@ -103,7 +107,7 @@ class PersistentEntityResourceHandlerMethodArgumentResolverUnitTests {
PersistentEntityResourceHandlerMethodArgumentResolver argumentResolver = new PersistentEntityResourceHandlerMethodArgumentResolver(
Arrays.<HttpMessageConverter<?>> asList(converter), rootResourceResolver, backendIdResolver, reader,
PluginRegistry.of(Arrays.asList(lookup)));
PluginRegistry.of(Arrays.asList(lookup)), FACTORY);
HttpServletRequest request = new MockHttpServletRequest("PUT", "/foo/someName");

View File

@@ -25,6 +25,7 @@ import org.springframework.data.mapping.PersistentEntity;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonProperty.Access;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -91,7 +92,7 @@ class MappedPropertiesUnitTests {
MappedProperties properties = MappedProperties.forDeserialization(entity, mapper);
assertThat(properties.isWritableProperty("anotherReadOnlyProperty")).isFalse();
assertThat(properties.isWritableField("anotherReadOnlyProperty")).isFalse();
assertThat(properties.getPersistentProperty("readOnlyProperty")).isNull();
properties = MappedProperties.forSerialization(entity, mapper);
@@ -107,12 +108,12 @@ class MappedPropertiesUnitTests {
MappedProperties properties = MappedProperties.forDeserialization(entity, mapper);
assertThat(properties.isWritableProperty("someProperty")).isTrue();
assertThat(properties.isWritableProperty("readOnlyProperty")).isFalse();
assertThat(properties.isWritableProperty("anotherReadOnlyProperty")).isFalse();
assertThat(properties.isWritableField("someProperty")).isTrue();
assertThat(properties.isWritableField("readOnlyProperty")).isFalse();
assertThat(properties.isWritableField("anotherReadOnlyProperty")).isFalse();
// Due to @JsonAnySetter
assertThat(properties.isWritableProperty("someRandomProperty")).isTrue();
assertThat(properties.isWritableField("someRandomProperty")).isTrue();
}
@Test // #2130
@@ -120,6 +121,13 @@ class MappedPropertiesUnitTests {
assertThat(properties.getIgnoredProperties()).contains("notExposedByJackson");
}
@Test
void ignoresTypeLevelProperties() {
assertThat(properties.getIgnoredProperties()).contains("typeLevelIgnored");
}
@JsonIgnoreProperties("typeLevelIgnored")
static class Sample {
public @Transient String notExposedBySpringData;
@@ -128,6 +136,7 @@ class MappedPropertiesUnitTests {
public @JsonProperty("email") String emailAddress;
public @JsonProperty(access = Access.READ_ONLY) String readOnlyProperty;
public @ReadOnlyProperty String anotherReadOnlyProperty;
public String typeLevelIgnored;
}
static class SampleWithJsonAnySetter {

View File

@@ -40,7 +40,7 @@ class AddOperationUnitTests {
todos.add(new Todo(3L, "C", false));
AddOperation add = AddOperation.of("/1/complete", true);
add.perform(todos, Todo.class);
add.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).isComplete()).isTrue();
}
@@ -54,7 +54,7 @@ class AddOperationUnitTests {
todos.add(new Todo(3L, "C", false));
AddOperation add = AddOperation.of("/1/description", "BBB");
add.perform(todos, Todo.class);
add.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).getDescription()).isEqualTo("BBB");
}
@@ -68,7 +68,7 @@ class AddOperationUnitTests {
todos.add(new Todo(3L, "C", false));
AddOperation add = AddOperation.of("/1", new Todo(null, "D", true));
add.perform(todos, Todo.class);
add.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(4);
assertThat(todos.get(0).getDescription()).isEqualTo("A");
@@ -86,7 +86,7 @@ class AddOperationUnitTests {
Todo todo = new Todo(1L, "description", false);
AddOperation.of("/items/-", "Some text.").perform(todo, Todo.class);
AddOperation.of("/items/-", "Some text.").perform(todo, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todo.getItems().get(0)).isEqualTo("Some text.");
}
@@ -100,7 +100,7 @@ class AddOperationUnitTests {
JsonNode node = mapper.readTree("\"Some text.\"");
JsonLateObjectEvaluator evaluator = new JsonLateObjectEvaluator(mapper, node);
AddOperation.of("/items/-", evaluator).perform(todo, Todo.class);
AddOperation.of("/items/-", evaluator).perform(todo, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todo.getItems().get(0)).isEqualTo("Some text.");
}
@@ -110,7 +110,7 @@ class AddOperationUnitTests {
Todo todo = new Todo(1L, "description", false);
AddOperation.of("/uninitialized/-", "Text").perform(todo, Todo.class);
AddOperation.of("/uninitialized/-", "Text").perform(todo, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todo.getUninitialized()).containsExactly("Text");
}
@@ -122,7 +122,7 @@ class AddOperationUnitTests {
todos.add(new Todo(1L, "A", false));
Todo todo = new Todo(2L, "B", true);
AddOperation.of("/1", todo).perform(todos, Todo.class);
AddOperation.of("/1", todo).perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos).element(1).isEqualTo(todo);
}
@@ -134,7 +134,8 @@ class AddOperationUnitTests {
todos.add(new Todo(1L, "A", false));
assertThatExceptionOfType(PatchException.class) //
.isThrownBy(() -> AddOperation.of("/2", new Todo(2L, "B", true)).perform(todos, Todo.class)) //
.isThrownBy(() -> AddOperation.of("/2", new Todo(2L, "B", true)).perform(todos, Todo.class,
TestPropertyPathContext.INSTANCE)) //
.withMessageContaining("index") //
.withMessageContaining("2") //
.withMessageContaining("1");
@@ -152,7 +153,8 @@ class AddOperationUnitTests {
TodoListWrapper outer = new TodoListWrapper(todoList);
Todo newTodo = new Todo(3L, "C", false);
AddOperation.of("/todoList/todos/-", newTodo).perform(outer, TodoListWrapper.class);
AddOperation.of("/todoList/todos/-", newTodo).perform(outer, TodoListWrapper.class,
TestPropertyPathContext.INSTANCE);
assertThat(outer.todoList.getTodos()).containsExactly(todos.get(0), todos.get(1), newTodo);
}

View File

@@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.*;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import org.junit.jupiter.api.Test;
@@ -33,7 +34,7 @@ class CopyOperationUnitTests {
todos.add(new Todo(3L, "C", false));
CopyOperation copy = CopyOperation.from("/0/complete").to("/1/complete");
copy.perform(todos, Todo.class);
copy.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).isComplete()).isTrue();
}
@@ -47,7 +48,7 @@ class CopyOperationUnitTests {
todos.add(new Todo(3L, "C", false));
CopyOperation copy = CopyOperation.from("/0/description").to("/1/description");
copy.perform(todos, Todo.class);
copy.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).getDescription()).isEqualTo("A");
}
@@ -61,7 +62,7 @@ class CopyOperationUnitTests {
todos.add(new Todo(3L, "C", false));
CopyOperation copy = CopyOperation.from("/0/complete").to("/1/description");
copy.perform(todos, Todo.class);
copy.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).getDescription()).isEqualTo("true");
}
@@ -75,7 +76,7 @@ class CopyOperationUnitTests {
todos.add(new Todo(3L, "C", false));
CopyOperation copy = CopyOperation.from("/1").to("/0");
copy.perform(todos, Todo.class);
copy.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(4);
assertThat(todos.get(0).getId().longValue()).isEqualTo(2L); // NOTE: This could be problematic if you try to save it
@@ -94,7 +95,7 @@ class CopyOperationUnitTests {
todos.add(new Todo(3L, "C", false));
CopyOperation copy = CopyOperation.from("/0").to("/2");
copy.perform(todos, Todo.class);
copy.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(4);
assertThat(todos.get(2).getId().longValue()).isEqualTo(1L); // NOTE: This could be problematic if you try to save it
@@ -113,7 +114,7 @@ class CopyOperationUnitTests {
todos.add(new Todo(3L, "C", false));
CopyOperation copy = CopyOperation.from("/0").to("/3");
copy.perform(todos, Todo.class);
copy.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(4);
assertThat(todos.get(3).getId().longValue()).isEqualTo(1L); // NOTE: This could be problematic if you try to save it
@@ -132,7 +133,7 @@ class CopyOperationUnitTests {
todos.add(new Todo(3L, "C", false));
CopyOperation copy = CopyOperation.from("/0").to("/-");
copy.perform(todos, Todo.class);
copy.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(4);
assertThat(todos.get(3)).isEqualTo(new Todo(1L, "A", true)); // NOTE: This could be problematic if you try to save
@@ -148,10 +149,48 @@ class CopyOperationUnitTests {
todos.add(new Todo(3L, "C", false));
CopyOperation copy = CopyOperation.from("/-").to("/0");
copy.perform(todos, Todo.class);
copy.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(4);
assertThat(todos.get(0)).isEqualTo(new Todo(3L, "C", false)); // NOTE: This could be problematic if you try to save
// it to a DB because there'll be duplicate IDs
}
@Test
void rejectsCopyingFromHiddenProperty() {
BindContext context = new TestPropertyPathContext() {
@Override
public Optional<String> getReadableProperty(String segment, Class<?> type) {
return Optional.of(segment).filter(it -> !"description".equals(it));
}
};
CopyOperation operation = CopyOperation.from("/description").to("/description");
Todo target = new Todo(1L, "Description", false);
assertThatExceptionOfType(PatchException.class)
.isThrownBy(() -> operation.perform(target, Todo.class, context))
.withMessageContaining("readable property");
}
@Test
void rejectsCopyingToHiddenProperty() {
BindContext context = new TestPropertyPathContext() {
@Override
public Optional<String> getWritableProperty(String segment, Class<?> type) {
return Optional.of(segment).filter(it -> !"description".equals(it));
}
};
CopyOperation operation = CopyOperation.from("/description").to("/description");
Todo target = new Todo(1L, "Description", false);
assertThatExceptionOfType(PatchException.class)
.isThrownBy(() -> operation.perform(target, Todo.class, context))
.withMessageContaining("writable property");
}
}

View File

@@ -161,7 +161,7 @@ class JsonPatchUnitTests {
ClassPathResource resource = new ClassPathResource(jsonPatchFile, getClass());
JsonNode node = new ObjectMapper().readValue(resource.getInputStream(), JsonNode.class);
Patch patch = new JsonPatchPatchConverter(new ObjectMapper()).convert(node);
Patch patch = new JsonPatchPatchConverter(new ObjectMapper(), TestPropertyPathContext.INSTANCE).convert(node);
return patch;
}

View File

@@ -0,0 +1,69 @@
/*
* Copyright 2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.rest.webmvc.json.patch;
import java.util.Arrays;
import java.util.Collection;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.rest.webmvc.json.BindContextFactory;
import org.springframework.data.rest.webmvc.json.PersistentEntitiesBindContextFactory;
import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
import com.fasterxml.jackson.databind.ObjectMapper;
/**
* Unit tests for {@link JsonPointerMapping}.
*
* @author Oliver Drotbohm
*/
public class JsonPointerMappingTests {
JsonPointerMapping verifier;
@BeforeEach
void setUp() {
KeyValueMappingContext<?, ?> context = new KeyValueMappingContext<>();
context.getPersistentEntity(Sample.class);
PersistentEntities entities = new PersistentEntities(Arrays.asList(context));
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities);
ObjectMapper mapper = new ObjectMapper();
this.verifier = new JsonPointerMapping(factory.getBindContextFor(mapper));
}
@Test
void verifiesSimpleProperty() {
verifier.forRead("/firstname", Sample.class);
}
@Test
void verifiesPathIntoCollection() {
verifier.forRead("/collection/27/firstname", Sample.class);
}
@JsonAutoDetect(fieldVisibility = Visibility.ANY)
static class Sample {
String firstname;
Collection<Sample> collection;
}
}

View File

@@ -35,8 +35,8 @@ class MoveOperationUnitTests {
MoveOperation move = MoveOperation.from("/0/complete").to("/1/complete");
assertThatExceptionOfType(PatchException.class)
.isThrownBy(() -> move.perform(todos, Todo.class))
.withMessage("Path '/0/complete' is not nullable.");
.isThrownBy(() -> move.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE))
.withMessage("Path '/0/complete' is not nullable");
assertThat(todos.get(1).isComplete()).isFalse();
}
@@ -50,7 +50,7 @@ class MoveOperationUnitTests {
todos.add(new Todo(3L, "C", false));
MoveOperation move = MoveOperation.from("/0/description").to("/1/description");
move.perform(todos, Todo.class);
move.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).getDescription()).isEqualTo("A");
}
@@ -66,8 +66,8 @@ class MoveOperationUnitTests {
MoveOperation move = MoveOperation.from("/0/complete").to("/1/description");
assertThatExceptionOfType(PatchException.class)
.isThrownBy(() -> move.perform(todos, Todo.class))
.withMessage("Path '/0/complete' is not nullable.");
.isThrownBy(() -> move.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE))
.withMessage("Path '/0/complete' is not nullable");
assertThat(todos.get(1).getDescription()).isEqualTo("B");
}
@@ -89,7 +89,7 @@ class MoveOperationUnitTests {
todos.add(new Todo(3L, "C", false));
MoveOperation move = MoveOperation.from("/1").to("/0");
move.perform(todos, Todo.class);
move.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(3);
assertThat(todos.get(0).getId().longValue()).isEqualTo(2L);
@@ -106,7 +106,7 @@ class MoveOperationUnitTests {
todos.add(new Todo(3L, "C", false));
MoveOperation move = MoveOperation.from("/0").to("/2");
move.perform(todos, Todo.class);
move.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(3);
assertThat(todos.get(2).getId().longValue()).isEqualTo(1L);
@@ -123,7 +123,7 @@ class MoveOperationUnitTests {
todos.add(new Todo(3L, "C", false));
MoveOperation move = MoveOperation.from("/0").to("/2");
move.perform(todos, Todo.class);
move.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(3);
assertThat(todos.get(2).getId().longValue()).isEqualTo(1L);
@@ -147,7 +147,7 @@ class MoveOperationUnitTests {
expected.add(new Todo(4L, "E", false));
MoveOperation move = MoveOperation.from("/-").to("/1");
move.perform(todos, Todo.class);
move.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos).isEqualTo(expected);
}
@@ -168,7 +168,7 @@ class MoveOperationUnitTests {
expected.add(new Todo(2L, "G", false));
MoveOperation move = MoveOperation.from("/1").to("/-");
move.perform(todos, Todo.class);
move.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos).isEqualTo(expected);
}

View File

@@ -53,7 +53,7 @@ class PatchOperationUnitTests {
Todo todo = new Todo(1L, "A", false);
assertThatExceptionOfType(PatchException.class) //
.isThrownBy(() -> it.perform(todo, Todo.class));
.isThrownBy(() -> it.perform(todo, Todo.class, TestPropertyPathContext.INSTANCE));
});
}
}

View File

@@ -32,7 +32,7 @@ class RemoveOperationTests {
todos.add(new Todo(2L, "B", false));
todos.add(new Todo(3L, "C", false));
RemoveOperation.valueAt("/1/description").perform(todos, Todo.class);
RemoveOperation.valueAt("/1/description").perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).getDescription()).isNull();
}
@@ -45,7 +45,7 @@ class RemoveOperationTests {
todos.add(new Todo(2L, "B", false));
todos.add(new Todo(3L, "C", false));
RemoveOperation.valueAt("/1").perform(todos, Todo.class);
RemoveOperation.valueAt("/1").perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.size()).isEqualTo(2);
assertThat(todos.get(0).getDescription()).isEqualTo("A");

View File

@@ -38,7 +38,7 @@ class ReplaceOperationTests {
todos.add(new Todo(3L, "C", false));
ReplaceOperation replace = ReplaceOperation.valueAt("/1/complete").with(true);
replace.perform(todos, Todo.class);
replace.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).isComplete()).isTrue();
}
@@ -52,7 +52,7 @@ class ReplaceOperationTests {
todos.add(new Todo(3L, "C", false));
ReplaceOperation replace = ReplaceOperation.valueAt("/1/description").with("BBB");
replace.perform(todos, Todo.class);
replace.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).getDescription()).isEqualTo("BBB");
}
@@ -66,7 +66,7 @@ class ReplaceOperationTests {
todos.add(new Todo(3L, "C", false));
ReplaceOperation replace = ReplaceOperation.valueAt("/1/description").with(22);
replace.perform(todos, Todo.class);
replace.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todos.get(1).getDescription()).isEqualTo("22");
}
@@ -79,7 +79,7 @@ class ReplaceOperationTests {
ObjectMapper mapper = new ObjectMapper();
ReplaceOperation replace = ReplaceOperation.valueAt("/type")
.with(new JsonLateObjectEvaluator(mapper, mapper.readTree("{ \"value\" : \"new\" }")));
replace.perform(todo, Todo.class);
replace.perform(todo, Todo.class, TestPropertyPathContext.INSTANCE);
assertThat(todo.getType()).isNotNull();
assertThat(todo.getType().getValue()).isNotNull();
@@ -95,7 +95,7 @@ class ReplaceOperationTests {
ReplaceOperation.valueAt("/characters/protagonist") //
.with(prepareValue("\"Pallo\"")) //
.perform(book, Book.class);
.perform(book, Book.class, TestPropertyPathContext.INSTANCE);
assertThat(book.characters.get("protagonist")).isEqualTo("Pallo");
}

View File

@@ -17,21 +17,51 @@ package org.springframework.data.rest.webmvc.json.patch;
import static org.assertj.core.api.Assertions.*;
import lombok.Data;
import lombok.Getter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.data.rest.webmvc.json.patch.SpelPath.TypedSpelPath;
import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
import org.springframework.data.mapping.context.PersistentEntities;
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 com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
/**
* Unit tests for {@link SpelPath}.
*
* @author Oliver Gierke
* @author Greg Turnquist
*/
class SpelPathUnitTests {
BindContext context;
@BeforeEach
void setUp() {
KeyValueMappingContext<?, ?> context = new KeyValueMappingContext<>();
context.getPersistentEntity(MapWrapper.class);
context.getPersistentEntity(Todo.class);
context.getPersistentEntity(Person.class);
PersistentEntities entities = new PersistentEntities(Arrays.asList(context));
BindContextFactory factory = new PersistentEntitiesBindContextFactory(entities);
this.context = factory.getBindContextFor(new ObjectMapper());
}
@Test
void listIndex() {
@@ -42,7 +72,7 @@ class SpelPathUnitTests {
todos.add(new Todo(2L, "B", false));
todos.add(new Todo(3L, "C", false));
Object value = expr.bindTo(Todo.class).getValue(todos);
Object value = expr.bindForRead(Todo.class, context).getValue(todos);
assertThat(value).isEqualTo("B");
}
@@ -57,7 +87,7 @@ class SpelPathUnitTests {
todos.add(new Todo(2L, "B", false));
todos.add(new Todo(3L, "C", false));
Object value = expr.bindTo(Todo.class).getValue(todos);
Object value = expr.bindForRead(Todo.class, context).getValue(todos);
assertThat(value).isEqualTo("C");
}
@@ -75,21 +105,22 @@ class SpelPathUnitTests {
void cachesTypedSpelPath() {
UntypedSpelPath source = SpelPath.untyped("/description");
TypedSpelPath left = source.bindTo(Todo.class);
TypedSpelPath right = source.bindTo(Todo.class);
WritingOperations left = source.bindForWrite(Todo.class, context);
WritingOperations right = source.bindForWrite(Todo.class, context);
assertThat(left).isSameAs(right);
}
@Test // DATAREST-1274
void supportsMultiDigitCollectionIndex() {
assertThat(SpelPath.untyped("/11/description").bindTo(Todo.class).getLeafType()).isEqualTo(String.class);
assertThat(SpelPath.untyped("/11/description").bindForWrite(Todo.class, context).getLeafType())
.isEqualTo(String.class);
}
@Test // DATAREST-1338
void handlesStringMapKeysInPathExpressions() {
TypedSpelPath path = SpelPath.untyped("people/Dave/name").bindTo(MapWrapper.class);
WritingOperations path = SpelPath.untyped("people/Dave/name").bindForWrite(MapWrapper.class, context);
assertThat(path.getExpressionString()).isEqualTo("people['Dave'].name");
assertThat(path.getLeafType()).isEqualTo(String.class);
@@ -98,18 +129,55 @@ class SpelPathUnitTests {
@Test // DATAREST-1338
void handlesIntegerMapKeysInPathExpressions() {
TypedSpelPath path = SpelPath.untyped("peopleByInt/0/name").bindTo(MapWrapper.class);
WritingOperations path = SpelPath.untyped("peopleByInt/0/name").bindForWrite(MapWrapper.class, context);
assertThat(path.getExpressionString()).isEqualTo("peopleByInt[0].name");
assertThat(path.getLeafType()).isEqualTo(String.class);
}
// DATAREST-1338
@Test
void failsAccessingPropertyIgnoredByJackson() {
static class Person {
String name;
String path = "peopleByInt/0/hiddenProperty";
assertThatExceptionOfType(PatchException.class) //
.isThrownBy(() -> SpelPath.untyped(path).bindForWrite(MapWrapper.class, context)) //
.withMessageContaining("hiddenProperty") //
.withMessageContaining(Person.class.getName()) //
.withMessageContaining(path); //
}
@Test
void failsAccessingGetterIgnoredByJackson() {
String path = "peopleByInt/0/hiddenGetter";
assertThatExceptionOfType(PatchException.class) //
.isThrownBy(() -> SpelPath.untyped(path).bindForWrite(MapWrapper.class, context)) //
.withMessageContaining("hiddenGetter") //
.withMessageContaining(Person.class.getName()) //
.withMessageContaining(path); //
}
@Test
void mapsRenamedProperty() {
WritingOperations path = SpelPath.untyped("demaner").bindForWrite(Person.class, context);
assertThat(path.getExpressionString()).isEqualTo("renamed");
}
// DATAREST-1338
@Data
static class Person {
String name;
@JsonIgnore String hiddenProperty;
@Getter(onMethod = @__(@JsonIgnore)) String hiddenGetter;
@JsonProperty("demaner") String renamed;
}
@Data
static class MapWrapper {
Map<String, Person> people;
Map<Integer, Person> peopleByInt;

View File

@@ -33,10 +33,10 @@ class TestOperationUnitTests {
todos.add(new Todo(3L, "C", false));
TestOperation test = TestOperation.whetherValueAt("/0/complete").hasValue(false);
test.perform(todos, Todo.class);
test.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
TestOperation test2 = TestOperation.whetherValueAt("/1/complete").hasValue(true);
test2.perform(todos, Todo.class);
test2.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
}
@@ -51,7 +51,7 @@ class TestOperationUnitTests {
TestOperation test = TestOperation.whetherValueAt("/0/complete").hasValue(true);
assertThatExceptionOfType(PatchException.class) //
.isThrownBy(() -> test.perform(todos, Todo.class));
.isThrownBy(() -> test.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE));
}
@Test
@@ -63,6 +63,6 @@ class TestOperationUnitTests {
todos.add(new Todo(3L, "C", false));
TestOperation test = TestOperation.whetherValueAt("/1").hasValue(new Todo(2L, "B", true));
test.perform(todos, Todo.class);
test.perform(todos, Todo.class, TestPropertyPathContext.INSTANCE);
}
}

View File

@@ -0,0 +1,41 @@
/*
* Copyright 2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.rest.webmvc.json.patch;
import java.util.Optional;
public class TestPropertyPathContext implements BindContext {
public static final BindContext INSTANCE = new TestPropertyPathContext();
/*
* (non-Javadoc)
* @see org.springframework.data.rest.webmvc.json.patch.BindContext#getReadableProperty(java.lang.String, java.lang.Class)
*/
@Override
public Optional<String> getReadableProperty(String segment, Class<?> type) {
return Optional.of(segment);
}
/*
* (non-Javadoc)
* @see org.springframework.data.rest.webmvc.json.patch.BindContext#getWritableProperty(java.lang.String, java.lang.Class)
*/
@Override
public Optional<String> getWritableProperty(String segment, Class<?> type) {
return Optional.of(segment);
}
}