diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java index 11c79d9b2..811072901 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/AddOperation.java @@ -30,17 +30,21 @@ class AddOperation extends PatchOperation { * @param path The path where the value will be added. (e.g., '/foo/bar/4') * @param value The value to add. */ - public AddOperation(String path, Object value) { + private AddOperation(SpelPath path, Object value) { super("add", path, value); } + public static AddOperation of(String path, Object value) { + return new AddOperation(SpelPath.of(path), value); + } + /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) */ @Override - void doPerform(Object targetObject, Class type) { - addValue(targetObject, evaluateValueFromTarget(targetObject, type)); + void perform(Object targetObject, Class type) { + path.bindTo(type).addValue(targetObject, evaluateValueFromTarget(targetObject, type)); } /* @@ -48,12 +52,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) { - if (!path.endsWith("-")) { + if (!path.isAppend()) { return super.evaluateValueFromTarget(targetObject, entityType); } - return evaluate(verifyPath(entityType).> map(it -> it.getType()).orElse(entityType)); + return evaluate(path.getLeafType(entityType)); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/CopyOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/CopyOperation.java index f432e59ae..af4c9b9b2 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/CopyOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/CopyOperation.java @@ -15,7 +15,7 @@ */ package org.springframework.data.rest.webmvc.json.patch; -import static org.springframework.data.rest.webmvc.json.patch.PathToSpEL.*; +import lombok.RequiredArgsConstructor; /** *

@@ -39,7 +39,9 @@ import static org.springframework.data.rest.webmvc.json.patch.PathToSpEL.*; * @author Craig Walls * @author Oliver Gierke */ -class CopyOperation extends FromOperation { +class CopyOperation extends PatchOperation { + + private final SpelPath from; /** * Constructs the copy operation @@ -47,16 +49,33 @@ class CopyOperation extends FromOperation { * @param path The path to copy the source value to. (e.g., '/foo/bar/4') * @param from The source path from which a value will be copied. (e.g., '/foo/bar/5') */ - public CopyOperation(String path, String from) { - super("copy", path, from); + public CopyOperation(SpelPath path, SpelPath from) { + + super("copy", path); + + this.from = from; + } + + public static CopyOperationBuilder from(String from) { + return new CopyOperationBuilder(from); + } + + @RequiredArgsConstructor + static class CopyOperationBuilder { + + private final String from; + + CopyOperation to(String to) { + return new CopyOperation(SpelPath.of(to), SpelPath.of(from)); + } } /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) */ @Override - void doPerform(Object target, Class type) { - addValue(target, pathToExpression(getFrom()).getValue(target)); + void perform(Object target, Class type) { + path.bindTo(type).copyFrom(from, target); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/FromOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/FromOperation.java deleted file mode 100644 index 5fffe7dcd..000000000 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/FromOperation.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2014-2016 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 - * - * http://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; - -/** - * Abstract base class for operations requiring a source property, such as "copy" and "move". (e.g., copy from - * here to there. - * - * @author Craig Walls - * @author Oliver Gierke - */ -abstract class FromOperation extends PatchOperation { - - private final String from; - - /** - * Constructs the operation - * - * @param op The name of the operation to perform. (e.g., 'copy') - * @param path The operation's target path. (e.g., '/foo/bar/4') - * @param from The operation's source path. (e.g., '/foo/bar/5') - */ - public FromOperation(String op, String path, String from) { - super(op, path); - this.from = from; - } - - public String getFrom() { - return from; - } -} diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonLateObjectEvaluator.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonLateObjectEvaluator.java index 889c01a61..29de507f6 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonLateObjectEvaluator.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonLateObjectEvaluator.java @@ -39,12 +39,12 @@ class JsonLateObjectEvaluator implements LateObjectEvaluator { * @see org.springframework.data.rest.webmvc.json.patch.LateObjectEvaluator#evaluate(java.lang.Class) */ @Override - public Object evaluate(Class type) { + public Object evaluate(Class type) { try { return mapper.readValue(valueNode.traverse(), type); - } catch (Exception e) { - throw new PatchException(String.format("Could not read %s into %s!", valueNode, type), e); + } catch (Exception o_O) { + throw new PatchException(String.format("Could not read %s into %s!", valueNode, type), o_O); } } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchPatchConverter.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchPatchConverter.java index 2ab981972..4d51eeb26 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchPatchConverter.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchPatchConverter.java @@ -25,8 +25,6 @@ import java.util.List; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.JsonNodeFactory; -import com.fasterxml.jackson.databind.node.ObjectNode; /** * Convert {@link JsonNode}s containing JSON Patch to/from {@link Patch} objects. @@ -68,17 +66,17 @@ public class JsonPatchPatchConverter implements PatchConverter { String from = opNode.has("from") ? opNode.get("from").textValue() : null; if (opType.equals("test")) { - ops.add(new TestOperation(path, value)); + ops.add(TestOperation.whetherValueAt(path).hasValue(value)); } else if (opType.equals("replace")) { - ops.add(new ReplaceOperation(path, value)); + ops.add(ReplaceOperation.valueAt(path).with(value)); } else if (opType.equals("remove")) { - ops.add(new RemoveOperation(path)); + ops.add(RemoveOperation.valueAt(path)); } else if (opType.equals("add")) { - ops.add(new AddOperation(path, value)); + ops.add(AddOperation.of(path, value)); } else if (opType.equals("copy")) { - ops.add(new CopyOperation(path, from)); + ops.add(CopyOperation.from(from).to(path)); } else if (opType.equals("move")) { - ops.add(new MoveOperation(path, from)); + ops.add(MoveOperation.from(from).to(path)); } else { throw new PatchException("Unrecognized operation type: " + opType); } @@ -87,42 +85,6 @@ public class JsonPatchPatchConverter implements PatchConverter { return new Patch(ops); } - /** - * Renders a {@link Patch} as a {@link JsonNode}. - * - * @param patch the patch - * @return a {@link JsonNode} containing JSON Patch. - */ - public JsonNode convert(Patch patch) { - - List operations = patch.getOperations(); - JsonNodeFactory nodeFactory = JsonNodeFactory.instance; - ArrayNode patchNode = nodeFactory.arrayNode(); - - for (PatchOperation operation : operations) { - - ObjectNode opNode = nodeFactory.objectNode(); - opNode.set("op", nodeFactory.textNode(operation.getOp())); - opNode.set("path", nodeFactory.textNode(operation.getPath())); - - if (operation instanceof FromOperation) { - - FromOperation fromOp = (FromOperation) operation; - opNode.set("from", nodeFactory.textNode(fromOp.getFrom())); - } - - Object value = operation.getValue(); - - if (value != null) { - opNode.set("value", mapper.valueToTree(value)); - } - - patchNode.add(opNode); - } - - return patchNode; - } - private Object valueFromJsonNode(String path, JsonNode valueNode) { if (valueNode == null || valueNode.isNull()) { diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/LateObjectEvaluator.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/LateObjectEvaluator.java index 652310f7c..6efc251de 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/LateObjectEvaluator.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/LateObjectEvaluator.java @@ -32,7 +32,7 @@ package org.springframework.data.rest.webmvc.json.patch; * * @author Craig Walls */ -public interface LateObjectEvaluator { +interface LateObjectEvaluator { - Object evaluate(Class type); + Object evaluate(Class type); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/MoveOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/MoveOperation.java index 3359fd3a4..5ad076e15 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/MoveOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/MoveOperation.java @@ -15,6 +15,9 @@ */ package org.springframework.data.rest.webmvc.json.patch; +import lombok.AccessLevel; +import lombok.RequiredArgsConstructor; + /** *

* Operation that moves a value from the given "from" path to the given "path". Will throw a {@link PatchException} if @@ -30,7 +33,9 @@ package org.springframework.data.rest.webmvc.json.patch; * @author Craig Walls * @author Oliver Gierke */ -class MoveOperation extends FromOperation { +class MoveOperation extends PatchOperation { + + private final SpelPath from; /** * Constructs the move operation. @@ -38,16 +43,33 @@ class MoveOperation extends FromOperation { * @param path The path to move the source value to. (e.g., '/foo/bar/4') * @param from The source path from which a value will be moved. (e.g., '/foo/bar/5') */ - public MoveOperation(String path, String from) { - super("move", path, from); + private MoveOperation(SpelPath path, SpelPath from) { + + super("move", path); + + this.from = from; + } + + static MoveOperationBuilder from(String from) { + return new MoveOperationBuilder(from); + } + + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) + static class MoveOperationBuilder { + + private final String from; + + public MoveOperation to(String to) { + return new MoveOperation(SpelPath.of(to), SpelPath.of(from)); + } } /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) */ @Override - void doPerform(Object target, Class type) { - addValue(target, popValueAtPath(target, getFrom())); + void perform(Object target, Class type) { + path.bindTo(type).moveFrom(from, target); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchConverter.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchConverter.java index 4b1ebdc56..71994eae5 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchConverter.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchConverter.java @@ -41,12 +41,4 @@ public interface PatchConverter { * @return the {@link Patch} object that the document represents. */ Patch convert(T patchRepresentation); - - /** - * Convert a {@link Patch} to a representation object. - * - * @param patch the {@link Patch} to convert. - * @return the patch representation object. - */ - T convert(Patch patch); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java index bbad0b199..8f26e085f 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PatchOperation.java @@ -15,21 +15,8 @@ */ package org.springframework.data.rest.webmvc.json.patch; -import static org.springframework.data.rest.webmvc.json.patch.PathToSpEL.*; - -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Optional; -import java.util.stream.Collectors; - -import org.springframework.core.CollectionFactory; -import org.springframework.core.convert.TypeDescriptor; -import org.springframework.data.mapping.PropertyPath; -import org.springframework.data.mapping.PropertyReferenceException; -import org.springframework.expression.Expression; -import org.springframework.expression.ExpressionException; -import org.springframework.expression.spel.SpelEvaluationException; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; /** * Abstract base class representing and providing support methods for patch operations. @@ -38,14 +25,12 @@ import org.springframework.expression.spel.SpelEvaluationException; * @author Mathias Düsterhöft * @author Oliver Gierke */ +@RequiredArgsConstructor public abstract class PatchOperation { - private static final String INVALID_PATH_REFERENCE = "Invalid path reference %s on type %s (from source %s)!"; - - protected final String op; - protected final String path; + protected final @NonNull String op; + protected final @NonNull SpelPath path; protected final Object value; - protected final Expression spelExpression; /** * Constructs the operation. @@ -53,140 +38,10 @@ public abstract class PatchOperation { * @param op the operation name. (e.g., 'move') * @param path the path to perform the operation on. (e.g., '/1/description') */ - public PatchOperation(String op, String path) { + public PatchOperation(String op, SpelPath path) { this(op, path, null); } - /** - * Constructs the operation. - * - * @param op the operation name. (e.g., 'move') - * @param path the path to perform the operation on. (e.g., '/1/description') - * @param value the value to apply in the operation. Could be an actual value or an implementation of - * {@link LateObjectEvaluator}. - */ - public PatchOperation(String op, String path, Object value) { - - this.op = op; - this.path = path; - this.value = value; - this.spelExpression = pathToExpression(path); - } - - /** - * @return the operation name - */ - public String getOp() { - return op; - } - - /** - * @return the operation path - */ - public String getPath() { - return path; - } - - /** - * @return the operation's value (or {@link LateObjectEvaluator}) - */ - public Object getValue() { - return value; - } - - /** - * Pops a value from the given path. - * - * @param target the target from which to pop a value. - * @param removePath the path from which to pop a value. Must be a list. - * @return the value popped from the list - */ - protected Object popValueAtPath(Object target, String removePath) { - - Integer listIndex = targetListIndex(removePath); - Expression expression = pathToExpression(removePath); - Object value = expression.getValue(target); - - if (listIndex == null) { - - try { - expression.setValue(target, null); - return value; - } catch (SpelEvaluationException o_O) { - throw new PatchException("Path '" + removePath + "' is not nullable.", o_O); - } - - } else { - - Expression parentExpression = pathToParentExpression(removePath); - List list = (List) parentExpression.getValue(target); - list.remove(listIndex >= 0 ? listIndex.intValue() : list.size() - 1); - return value; - } - } - - /** - * Adds a value to the operation's path. If the path references a list index, the value is added to the list at the - * given index. If the path references an object property, the property is set to the value. - * - * @param target The target object. - * @param value The value to add. - */ - @SuppressWarnings({ "unchecked", "null" }) - protected void addValue(Object target, Object value) { - - Expression parentExpression = pathToParentExpression(path); - Object parent = parentExpression != null ? parentExpression.getValue(target) : null; - Integer listIndex = targetListIndex(path); - - if (parent == null || !(parent instanceof List) || listIndex == null) { - - TypeDescriptor descriptor = parentExpression.getValueTypeDescriptor(target); - - // Set as new collection if necessary - if (descriptor.isCollection() && !Collection.class.isInstance(value)) { - - Collection collection = CollectionFactory.createCollection(descriptor.getType(), 1); - collection.add(value); - - parentExpression.setValue(target, collection); - - } else { - spelExpression.setValue(target, value); - } - - } else { - - List list = (List) parentExpression.getValue(target); - list.add(listIndex >= 0 ? listIndex.intValue() : list.size(), value); - } - } - - /** - * Sets a value to the operation's path. - * - * @param target The target object. - * @param value The value to set. - */ - protected void setValueOnTarget(Object target, Object value) { - spelExpression.setValue(target, value); - } - - /** - * Retrieves a value from the operation's path. - * - * @param target the target object. - * @return the value at the path on the given target object. - */ - protected Object getValueFromTarget(Object target) { - - try { - return spelExpression.getValue(target); - } catch (ExpressionException e) { - throw new PatchException("Unable to get value from target", e); - } - } - /** * Performs late-value evaluation on the operation value if the value is a {@link LateObjectEvaluator}. * @@ -196,77 +51,19 @@ 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) { - - verifyPath(entityType); - - return evaluate(spelExpression.getValueType(targetObject)); + protected Object evaluateValueFromTarget(Object targetObject, Class entityType) { + return evaluate(path.bindTo(entityType).getType(targetObject)); } - protected final Object evaluate(Class type) { + protected final Object evaluate(Class type) { return value instanceof LateObjectEvaluator ? ((LateObjectEvaluator) value).evaluate(type) : value; } - /** - * Verifies that the current path is available on the given type. - * - * @param type must not be {@literal null}. - * @return the {@link PropertyPath} representing the path. Empty if the path only consists of index lookups or append - * characters. - */ - protected final Optional verifyPath(Class type) { - - String pathSource = Arrays.stream(path.split("/"))// - .filter(it -> !it.matches("\\d")) // no digits - .filter(it -> !it.equals("-")) // no "last element"s - .filter(it -> !it.isEmpty()) // - .collect(Collectors.joining(".")); - - if (pathSource.isEmpty()) { - return Optional.empty(); - } - - try { - return Optional.of(PropertyPath.from(pathSource, type)); - } catch (PropertyReferenceException o_O) { - throw new PatchException(String.format(INVALID_PATH_REFERENCE, pathSource, type, path), o_O); - } - } - /** * Perform the operation in the given target object. * * @param target the target of the operation, must not be {@literal null}. * @param type must not be {@literal null}. */ - final void perform(Object target, Class type) { - - verifyPath(type); - - doPerform(target, type); - } - - /** - * Implements the actually application of the operation. - * - * @param target must not be {@literal null}. - * @param type must not be {@literal null}. - */ - abstract void doPerform(Object target, Class type); - - private Integer targetListIndex(String path) { - - String[] pathNodes = path.split("\\/"); - String lastNode = pathNodes[pathNodes.length - 1]; - - if (APPEND_CHARACTERS.contains(lastNode)) { - return -1; - } - - try { - return Integer.parseInt(lastNode); - } catch (NumberFormatException e) { - return null; - } - } + abstract void perform(Object target, Class type); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PathToSpEL.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PathToSpEL.java deleted file mode 100644 index e8a20d648..000000000 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/PathToSpEL.java +++ /dev/null @@ -1,119 +0,0 @@ -/* - * Copyright 2014-2017 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 - * - * http://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.List; - -import org.springframework.expression.Expression; -import org.springframework.expression.spel.standard.SpelExpressionParser; - -/** - * Utilities for converting patch paths to/from SpEL expressions. For example, "/foo/bars/1/baz" becomes - * "foo.bars[1].baz". - * - * @author Craig Walls - * @author Oliver Gierke - */ -public class PathToSpEL { - - private static final SpelExpressionParser SPEL_EXPRESSION_PARSER = new SpelExpressionParser(); - static final List APPEND_CHARACTERS = Arrays.asList("-"); - - /** - * Converts a patch path to an {@link Expression}. - * - * @param path the patch path to convert. - * @return an {@link Expression} - */ - public static Expression pathToExpression(String path) { - return SPEL_EXPRESSION_PARSER.parseExpression(pathToSpEL(path)); - } - - /** - * Convenience method to convert a SpEL String to an {@link Expression}. - * - * @param spel the SpEL expression as a String - * @return an {@link Expression} - */ - public static Expression spelToExpression(String spel) { - return SPEL_EXPRESSION_PARSER.parseExpression(spel); - } - - /** - * Produces an expression targeting the parent of the object that the given path targets. - * - * @param path the path to find a parent expression for. - * @return an {@link Expression} targeting the parent of the object specified by path. - */ - public static Expression pathToParentExpression(String path) { - return spelToExpression(pathNodesToSpEL(copyOf(path.split("\\/"), path.split("\\/").length - 1))); - } - - private static String pathToSpEL(String path) { - return pathNodesToSpEL(path.split("\\/")); - } - - private static String pathNodesToSpEL(String[] pathNodes) { - StringBuilder spelBuilder = new StringBuilder(); - - for (int i = 0; i < pathNodes.length; i++) { - - String pathNode = pathNodes[i]; - - if (pathNode.length() == 0) { - continue; - } - - if (APPEND_CHARACTERS.contains(pathNode)) { - - if (spelBuilder.length() > 0) { - spelBuilder.append("."); - } - - spelBuilder.append("$[true]"); - continue; - } - - try { - - int index = Integer.parseInt(pathNode); - spelBuilder.append('[').append(index).append(']'); - - } catch (NumberFormatException e) { - - if (spelBuilder.length() > 0) { - spelBuilder.append('.'); - } - - spelBuilder.append(pathNode); - } - } - - String spel = spelBuilder.toString(); - - if (spel.length() == 0) { - spel = "#this"; - } - - return spel; - } - - @SuppressWarnings("unchecked") - private static T[] copyOf(T[] original, int newLength) { - return (T[]) Arrays.copyOf(original, newLength, original.getClass()); - } -} diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperation.java index 90a3bf98a..b48ce13f0 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperation.java @@ -29,16 +29,20 @@ class RemoveOperation extends PatchOperation { * * @param path The path of the value to be removed. (e.g., '/foo/bar/4') */ - public RemoveOperation(String path) { + private RemoveOperation(SpelPath path) { super("remove", path); } + public static RemoveOperation valueAt(String path) { + return new RemoveOperation(SpelPath.of(path)); + } + /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) */ @Override - void doPerform(Object target, Class type) { - popValueAtPath(target, path); + void perform(Object target, Class type) { + path.bindTo(type).removeFrom(target); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperation.java index fcfbfac03..3debd7f32 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperation.java @@ -15,6 +15,9 @@ */ package org.springframework.data.rest.webmvc.json.patch; +import lombok.AccessLevel; +import lombok.RequiredArgsConstructor; + /** * Operation that replaces the value at the given path with a new value. * @@ -29,16 +32,30 @@ class ReplaceOperation extends PatchOperation { * @param path The path whose value is to be replaced. (e.g., '/foo/bar/4') * @param value The value that will replace the current path value. */ - public ReplaceOperation(String path, Object value) { + private ReplaceOperation(SpelPath path, Object value) { super("replace", path, value); } + public static ReplaceOperationBuilder valueAt(String path) { + return new ReplaceOperationBuilder(path); + } + + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) + static class ReplaceOperationBuilder { + + private final String path; + + public ReplaceOperation with(Object value) { + return new ReplaceOperation(SpelPath.of(path), value); + } + } + /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) */ @Override - void doPerform(Object target, Class type) { - setValueOnTarget(target, evaluateValueFromTarget(target, type)); + void perform(Object target, Class type) { + path.bindTo(type).setValue(target, evaluateValueFromTarget(target, type)); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java new file mode 100644 index 000000000..acfa32781 --- /dev/null +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/SpelPath.java @@ -0,0 +1,441 @@ +/* + * Copyright 2017 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 + * + * http://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.AccessLevel; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import lombok.Value; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +import org.springframework.core.CollectionFactory; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.data.mapping.PropertyPath; +import org.springframework.data.mapping.PropertyReferenceException; +import org.springframework.expression.Expression; +import org.springframework.expression.ExpressionException; +import org.springframework.expression.spel.SpelEvaluationException; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.util.Assert; +import org.springframework.util.ConcurrentReferenceHashMap; + +/** + * Value object to represent a SpEL-backed patch path. + * + * @author Oliver Gierke + */ +@RequiredArgsConstructor(access = AccessLevel.PRIVATE) +class SpelPath { + + private static final SpelExpressionParser SPEL_EXPRESSION_PARSER = new SpelExpressionParser(); + private static final String APPEND_CHARACTER = "-"; + private static final Map PATHS = new ConcurrentReferenceHashMap<>(32); + + protected final @Getter String path; + protected final Expression expression; + + private SpelPath(String path) { + + Assert.notNull(path, "Path must not be null!"); + + this.path = path; + this.expression = SPEL_EXPRESSION_PARSER.parseExpression(pathToSpEL(path)); + } + + /** + * Returns a {@link SpelPath} for the given source. + * + * @param source must not be {@literal null}. + * @return + */ + public static SpelPath of(String source) { + return PATHS.computeIfAbsent(source, SpelPath::new); + } + + /** + * Returns a {@link TypedSpelPath} binding the expression to the given type. + * + * @param type must not be {@literal null}. + * @return + */ + public TypedSpelPath bindTo(Class type) { + + Assert.notNull(type, "Type must not be null!"); + + return TypedSpelPath.of(this, type); + } + + /** + * Returns the leaf type of the underlying expression or the given type + * + * @param type + * @return + */ + public Class getLeafType(Class type) { + + return TypedSpelPath.verifyPath(path, type) // + .> map(it -> it.getType()) // + .orElse(type); + } + + /** + * Returns whether the current path represents an append path, i.e. is supposed to append to collection. + * + * @return + */ + public boolean isAppend() { + return path.endsWith("-"); + } + + private SpelPath getParent() { + return SpelPath.of(path.substring(0, path.lastIndexOf('/'))); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return path; + } + + /* + * (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + + if (this == obj) { + return true; + } + + if (!(SpelPath.class.isInstance(obj))) { + return false; + } + + SpelPath that = (SpelPath) obj; + + return this.path.equals(that.path); + } + + /* + * (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + return path.hashCode(); + } + + private static String pathToSpEL(String path) { + return pathNodesToSpEL(path.split("\\/")); + } + + private static String pathNodesToSpEL(String[] pathNodes) { + + StringBuilder spelBuilder = new StringBuilder(); + + for (int i = 0; i < pathNodes.length; i++) { + + String pathNode = pathNodes[i]; + + if (pathNode.length() == 0) { + continue; + } + + if (APPEND_CHARACTER.equals(pathNode)) { + + if (spelBuilder.length() > 0) { + spelBuilder.append("."); + } + + spelBuilder.append("$[true]"); + continue; + } + + try { + + int index = Integer.parseInt(pathNode); + spelBuilder.append('[').append(index).append(']'); + + } catch (NumberFormatException e) { + + if (spelBuilder.length() > 0) { + spelBuilder.append('.'); + } + + spelBuilder.append(pathNode); + } + } + + String spel = spelBuilder.toString(); + + if (spel.length() == 0) { + spel = "#this"; + } + + return spel; + } + + /** + * A {@link SpelPath} that has typing information tied to it. + * + * @author Oliver Gierke + */ + @EqualsAndHashCode(callSuper = true) + static class TypedSpelPath extends SpelPath { + + private static final String INVALID_PATH_REFERENCE = "Invalid path reference %s on type %s (from source %s)!"; + private static final Map TYPED_PATHS = new ConcurrentReferenceHashMap<>(32); + + private final Class type; + + @Value(staticConstructor = "of") + private static class CacheKey { + Class type; + SpelPath path; + } + + private TypedSpelPath(SpelPath path, Class type) { + + super(path.path, path.expression); + + verifyPath(path.path, type); + + this.type = 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(SpelPath 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)); + } + + /** + * Returns the value pointed to by the current path with the given target object. + * + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @SuppressWarnings("unchecked") + public T getValue(Object target) { + + Assert.notNull(target, "Target must not be null!"); + + try { + return (T) expression.getValue(target); + } catch (ExpressionException o_O) { + throw new PatchException("Unable to get value from target", o_O); + } + } + + /** + * Sets the given value on the given target object. + * + * @param target must not be {@literal null}. + * @param value can be {@literal null}. + */ + public void setValue(Object target, Object value) { + + Assert.notNull(target, "Target must not be null!"); + + expression.setValue(target, value); + } + + /** + * Returns the type of the expression target based on the given root. + * + * @param root must not be {@literal null}. + * @return + */ + public Class getType(Object root) { + + Assert.notNull(root, "Root object must not be null!"); + + return expression.getValueType(root); + } + + /** + * Copies the value pointed to by the given path within the given source object to the current expression target. + * + * @param path the {@link SpelPath} to look the value up from, must not be {@literal null}. + * @param source the source object to look the value up from, must not be {@literal null}. + * @return + */ + + public void copyFrom(SpelPath path, Object source) { + + 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)); + } + + /** + * Moves the value pointed to by the given path within the given source object to the current expression target and + * removes the value from its original position. + * + * @param path the {@link SpelPath} to look the value up from, must not be {@literal null}. + * @param source the source object to look the value up from, must not be {@literal null}. + * @return + */ + public void moveFrom(SpelPath path, Object source) { + + 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)); + } + + /** + * Removes the value pointed to by the current path within the given target. + * + * @param target must not be {@literal null}. + * @return the original value that was just removed. + */ + public Object removeFrom(Object target) { + + Assert.notNull(target, "Target must not be null!"); + + Integer listIndex = getTargetListIndex(); + Object value = getValue(target); + + if (listIndex == null) { + + try { + setValue(target, null); + return value; + } catch (SpelEvaluationException o_O) { + throw new PatchException("Path '" + path + "' is not nullable.", o_O); + } + + } else { + + List list = getParent().getValue(target); + list.remove(listIndex >= 0 ? listIndex.intValue() : list.size() - 1); + return value; + } + } + + /** + * Adds a value to the operation's path. If the path references a list index, the value is added to the list at the + * given index. If the path references an object property, the property is set to the value. + * + * @param target The target object. + * @param value The value to add. + */ + public void addValue(Object target, Object value) { + + TypedSpelPath parentPath = getParent(); + Object parent = parentPath.getValue(target); + + Integer listIndex = getTargetListIndex(); + + if (parent == null || !(parent instanceof List) || listIndex == null) { + + TypeDescriptor descriptor = parentPath.getTypeDescriptor(target); + + // Set as new collection if necessary + if (descriptor.isCollection() && !Collection.class.isInstance(value)) { + + Collection collection = CollectionFactory.createCollection(descriptor.getType(), 1); + collection.add(value); + + parentPath.setValue(target, collection); + + } else { + setValue(target, value); + } + + } else { + + List list = parentPath.getValue(target); + list.add(listIndex >= 0 ? listIndex.intValue() : list.size(), value); + } + } + + private TypedSpelPath getParent() { + return TypedSpelPath.of(super.getParent(), type); + } + + private TypeDescriptor getTypeDescriptor(Object target) { + return expression.getValueTypeDescriptor(target); + } + + private Integer getTargetListIndex() { + + String lastNode = path.substring(path.lastIndexOf('/') + 1); + + if (APPEND_CHARACTER.equals(lastNode)) { + return -1; + } + + try { + return Integer.parseInt(lastNode); + } catch (NumberFormatException e) { + return null; + } + } + + /** + * Verifies that the given path exists on the given type. Skips collection index parts and append characters. + * + * @param path must not be {@literal null} or empty. + * @param type must not be {@literal null}. + * @return the {@link PropertyPath} if the path could be resolved or {@link Optional#empty()} in case an empty path + * is given. + */ + private static Optional verifyPath(String path, Class type) { + + Assert.notNull(path, "Path must not be null!"); + Assert.notNull(type, "Type must not be null!"); + + String pathSource = Arrays.stream(path.split("/"))// + .filter(it -> !it.matches("\\d")) // no digits + .filter(it -> !it.equals("-")) // no "last element"s + .filter(it -> !it.isEmpty()) // + .collect(Collectors.joining(".")); + + if (pathSource.isEmpty()) { + return Optional.empty(); + } + + try { + return Optional.of(PropertyPath.from(pathSource, type)); + } catch (PropertyReferenceException o_O) { + throw new PatchException(String.format(INVALID_PATH_REFERENCE, pathSource, type, path), o_O); + } + } + } +} diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/TestOperation.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/TestOperation.java index 1d2ad3acc..a16c137c1 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/TestOperation.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/TestOperation.java @@ -15,6 +15,9 @@ */ package org.springframework.data.rest.webmvc.json.patch; +import lombok.AccessLevel; +import lombok.RequiredArgsConstructor; + import java.math.BigDecimal; import java.math.BigInteger; @@ -40,19 +43,33 @@ class TestOperation extends PatchOperation { * @param path The path to test. (e.g., '/foo/bar/4') * @param value The value to test the path against. */ - public TestOperation(String path, Object value) { + private TestOperation(SpelPath path, Object value) { super("test", path, value); } + public static TestOperationBuilder whetherValueAt(String path) { + return new TestOperationBuilder(path); + } + + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) + static class TestOperationBuilder { + + private final String path; + + public TestOperation hasValue(Object value) { + return new TestOperation(SpelPath.of(path), value); + } + } + /* * (non-Javadoc) - * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#doPerform(java.lang.Object, java.lang.Class) + * @see org.springframework.data.rest.webmvc.json.patch.PatchOperation#perform(java.lang.Object, java.lang.Class) */ @Override - void doPerform(Object target, Class type) { + void perform(Object target, Class type) { Object expected = normalizeIfNumber(evaluateValueFromTarget(target, type)); - Object actual = normalizeIfNumber(getValueFromTarget(target)); + Object actual = normalizeIfNumber(path.bindTo(type).getValue(target)); if (!ObjectUtils.nullSafeEquals(expected, actual)) { throw new PatchException("Test against path '" + path + "' failed."); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationUnitTests.java similarity index 87% rename from spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationTests.java rename to spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationUnitTests.java index bbb0fe3e4..66d9f7423 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/AddOperationUnitTests.java @@ -26,7 +26,7 @@ import org.junit.Test; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -public class AddOperationTests { +public class AddOperationUnitTests { @Test public void addBooleanPropertyValue() throws Exception { @@ -36,7 +36,7 @@ public class AddOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - AddOperation add = new AddOperation("/1/complete", true); + AddOperation add = AddOperation.of("/1/complete", true); add.perform(todos, Todo.class); assertTrue(todos.get(1).isComplete()); @@ -50,7 +50,7 @@ public class AddOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - AddOperation add = new AddOperation("/1/description", "BBB"); + AddOperation add = AddOperation.of("/1/description", "BBB"); add.perform(todos, Todo.class); assertEquals("BBB", todos.get(1).getDescription()); @@ -64,7 +64,7 @@ public class AddOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - AddOperation add = new AddOperation("/1", new Todo(null, "D", true)); + AddOperation add = AddOperation.of("/1", new Todo(null, "D", true)); add.perform(todos, Todo.class); assertEquals(4, todos.size()); @@ -83,7 +83,7 @@ public class AddOperationTests { Todo todo = new Todo(1L, "description", false); - new AddOperation("/items/-", "Some text.").perform(todo, Todo.class); + AddOperation.of("/items/-", "Some text.").perform(todo, Todo.class); assertThat(todo.getItems().get(0)).isEqualTo("Some text."); } @@ -97,7 +97,7 @@ public class AddOperationTests { JsonNode node = mapper.readTree("\"Some text.\""); JsonLateObjectEvaluator evaluator = new JsonLateObjectEvaluator(mapper, node); - new AddOperation("/items/-", evaluator).perform(todo, Todo.class); + AddOperation.of("/items/-", evaluator).perform(todo, Todo.class); assertThat(todo.getItems().get(0)).isEqualTo("Some text."); } @@ -107,7 +107,7 @@ public class AddOperationTests { Todo todo = new Todo(1L, "description", false); - new AddOperation("/uninitialized/-", "Text").perform(todo, Todo.class); + AddOperation.of("/uninitialized/-", "Text").perform(todo, Todo.class); assertThat(todo.getUninitialized()).containsExactly("Text"); } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/CopyOperationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/CopyOperationUnitTests.java similarity index 88% rename from spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/CopyOperationTests.java rename to spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/CopyOperationUnitTests.java index 61a01b4be..b72966ada 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/CopyOperationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/CopyOperationUnitTests.java @@ -22,7 +22,7 @@ import java.util.List; import org.junit.Test; -public class CopyOperationTests { +public class CopyOperationUnitTests { @Test public void copyBooleanPropertyValue() throws Exception { @@ -32,7 +32,7 @@ public class CopyOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - CopyOperation copy = new CopyOperation("/1/complete", "/0/complete"); + CopyOperation copy = CopyOperation.from("/0/complete").to("/1/complete"); copy.perform(todos, Todo.class); assertTrue(todos.get(1).isComplete()); @@ -46,7 +46,7 @@ public class CopyOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - CopyOperation copy = new CopyOperation("/1/description", "/0/description"); + CopyOperation copy = CopyOperation.from("/0/description").to("/1/description"); copy.perform(todos, Todo.class); assertEquals("A", todos.get(1).getDescription()); @@ -60,7 +60,7 @@ public class CopyOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - CopyOperation copy = new CopyOperation("/1/description", "/0/complete"); + CopyOperation copy = CopyOperation.from("/0/complete").to("/1/description"); copy.perform(todos, Todo.class); assertEquals("true", todos.get(1).getDescription()); @@ -74,7 +74,7 @@ public class CopyOperationTests { todos.add(new Todo(2L, "B", true)); todos.add(new Todo(3L, "C", false)); - CopyOperation copy = new CopyOperation("/0", "/1"); + CopyOperation copy = CopyOperation.from("/1").to("/0"); copy.perform(todos, Todo.class); assertEquals(4, todos.size()); @@ -92,7 +92,7 @@ public class CopyOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - CopyOperation copy = new CopyOperation("/2", "/0"); + CopyOperation copy = CopyOperation.from("/0").to("/2"); copy.perform(todos, Todo.class); assertEquals(4, todos.size()); @@ -110,7 +110,7 @@ public class CopyOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - CopyOperation copy = new CopyOperation("/3", "/0"); + CopyOperation copy = CopyOperation.from("/0").to("/3"); copy.perform(todos, Todo.class); assertEquals(4, todos.size()); @@ -128,7 +128,7 @@ public class CopyOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - CopyOperation copy = new CopyOperation("/-", "/0"); + CopyOperation copy = CopyOperation.from("/0").to("/-"); copy.perform(todos, Todo.class); assertEquals(4, todos.size()); @@ -144,7 +144,7 @@ public class CopyOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - CopyOperation copy = new CopyOperation("/0", "/-"); + CopyOperation copy = CopyOperation.from("/-").to("/0"); copy.perform(todos, Todo.class); assertEquals(4, todos.size()); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchUnitTests.java similarity index 99% rename from spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchTests.java rename to spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchUnitTests.java index f4057828a..21425e057 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/JsonPatchUnitTests.java @@ -43,7 +43,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; * @author Mathias Düsterhöft * @author Oliver Trosien */ -public class JsonPatchTests { +public class JsonPatchUnitTests { public @Rule ExpectedException exception = ExpectedException.none(); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/MoveOperationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/MoveOperationUnitTests.java similarity index 89% rename from spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/MoveOperationTests.java rename to spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/MoveOperationUnitTests.java index 041149c5e..e18991558 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/MoveOperationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/MoveOperationUnitTests.java @@ -22,7 +22,7 @@ import java.util.List; import org.junit.Test; -public class MoveOperationTests { +public class MoveOperationUnitTests { @Test public void moveBooleanPropertyValue() throws Exception { @@ -33,14 +33,14 @@ public class MoveOperationTests { todos.add(new Todo(3L, "C", false)); try { - MoveOperation move = new MoveOperation("/1/complete", "/0/complete"); + MoveOperation move = MoveOperation.from("/0/complete").to("/1/complete"); move.perform(todos, Todo.class); fail(); } catch (PatchException e) { assertEquals("Path '/0/complete' is not nullable.", e.getMessage()); } - assertFalse(todos.get(1).isComplete()); + assertFalse(todos.get(1).isComplete()); } @Test @@ -51,7 +51,7 @@ public class MoveOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - MoveOperation move = new MoveOperation("/1/description", "/0/description"); + MoveOperation move = MoveOperation.from("/0/description").to("/1/description"); move.perform(todos, Todo.class); assertEquals("A", todos.get(1).getDescription()); @@ -66,7 +66,7 @@ public class MoveOperationTests { todos.add(new Todo(3L, "C", false)); try { - MoveOperation move = new MoveOperation("/1/description", "/0/complete"); + MoveOperation move = MoveOperation.from("/0/complete").to("/1/description"); move.perform(todos, Todo.class); fail(); } catch (PatchException e) { @@ -91,7 +91,7 @@ public class MoveOperationTests { todos.add(new Todo(2L, "B", true)); todos.add(new Todo(3L, "C", false)); - MoveOperation move = new MoveOperation("/0", "/1"); + MoveOperation move = MoveOperation.from("/1").to("/0"); move.perform(todos, Todo.class); assertEquals(3, todos.size()); @@ -108,7 +108,7 @@ public class MoveOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - MoveOperation move = new MoveOperation("/2", "/0"); + MoveOperation move = MoveOperation.from("/0").to("/2"); move.perform(todos, Todo.class); assertEquals(3, todos.size()); @@ -125,7 +125,7 @@ public class MoveOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - MoveOperation move = new MoveOperation("/2", "/0"); + MoveOperation move = MoveOperation.from("/0").to("/2"); move.perform(todos, Todo.class); assertEquals(3, todos.size()); @@ -149,7 +149,7 @@ public class MoveOperationTests { expected.add(new Todo(3L, "C", false)); expected.add(new Todo(4L, "E", false)); - MoveOperation move = new MoveOperation("/1", "/-"); + MoveOperation move = MoveOperation.from("/-").to("/1"); move.perform(todos, Todo.class); assertEquals(expected, todos); } @@ -169,7 +169,7 @@ public class MoveOperationTests { expected.add(new Todo(4L, "E", false)); expected.add(new Todo(2L, "G", false)); - MoveOperation move = new MoveOperation("/-", "/1"); + MoveOperation move = MoveOperation.from("/1").to("/-"); move.perform(todos, Todo.class); assertEquals(expected, todos); } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PatchOperationUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PatchOperationUnitTests.java index 87a47a49a..f968dc3f4 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PatchOperationUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PatchOperationUnitTests.java @@ -17,30 +17,48 @@ package org.springframework.data.rest.webmvc.json.patch; import static org.assertj.core.api.Assertions.*; +import java.util.Arrays; + import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; /** * General unit tests for {@link PatchOperation} implementations. * * @author Oliver Gierke */ +@RunWith(Parameterized.class) public class PatchOperationUnitTests { + @Parameters + public static Iterable operations() { + + String invalidPath = "/nonExistant"; + String validPath = "/1/description"; + + return Arrays.asList( // + + AddOperation.of(invalidPath, null), // + RemoveOperation.valueAt(invalidPath), // + ReplaceOperation.valueAt(invalidPath).with(null), // + TestOperation.whetherValueAt(invalidPath).hasValue(null), // + + CopyOperation.from(invalidPath).to(validPath), // + CopyOperation.from(validPath).to(invalidPath), // + + MoveOperation.from(invalidPath).to(validPath), // + MoveOperation.from(validPath).to(invalidPath) // + ); + } + + public @Parameter(0) PatchOperation operation; + @Test // DATAREST-1137 public void invalidPathGetsRejected() { - String invalidPath = "/nonExistant"; - - verifyIllegalPath(new AddOperation(invalidPath, null)); - verifyIllegalPath(new CopyOperation(invalidPath, null)); - verifyIllegalPath(new MoveOperation(invalidPath, null)); - verifyIllegalPath(new RemoveOperation(invalidPath)); - verifyIllegalPath(new ReplaceOperation(invalidPath, null)); - verifyIllegalPath(new TestOperation(invalidPath, null)); - } - - private static void verifyIllegalPath(PatchOperation operation) { - Todo todo = new Todo(1L, "A", false); assertThatExceptionOfType(PatchException.class) // diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperationTests.java index b87a994e8..82e0137a5 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/RemoveOperationTests.java @@ -32,7 +32,7 @@ public class RemoveOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - new RemoveOperation("/1/description").perform(todos, Todo.class); + RemoveOperation.valueAt("/1/description").perform(todos, Todo.class); assertNull(todos.get(1).getDescription()); } @@ -45,7 +45,7 @@ public class RemoveOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - new RemoveOperation("/1").perform(todos, Todo.class); + RemoveOperation.valueAt("/1").perform(todos, Todo.class); assertEquals(2, todos.size()); assertEquals("A", todos.get(0).getDescription()); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperationTests.java index a978c5032..adce38e57 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/ReplaceOperationTests.java @@ -34,7 +34,7 @@ public class ReplaceOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - ReplaceOperation replace = new ReplaceOperation("/1/complete", true); + ReplaceOperation replace = ReplaceOperation.valueAt("/1/complete").with(true); replace.perform(todos, Todo.class); assertTrue(todos.get(1).isComplete()); @@ -48,7 +48,7 @@ public class ReplaceOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - ReplaceOperation replace = new ReplaceOperation("/1/description", "BBB"); + ReplaceOperation replace = ReplaceOperation.valueAt("/1/description").with("BBB"); replace.perform(todos, Todo.class); assertEquals("BBB", todos.get(1).getDescription()); @@ -62,7 +62,7 @@ public class ReplaceOperationTests { todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - ReplaceOperation replace = new ReplaceOperation("/1/description", 22); + ReplaceOperation replace = ReplaceOperation.valueAt("/1/description").with(22); replace.perform(todos, Todo.class); assertEquals("22", todos.get(1).getDescription()); @@ -74,8 +74,8 @@ public class ReplaceOperationTests { Todo todo = new Todo(1L, "A", false); ObjectMapper mapper = new ObjectMapper(); - ReplaceOperation replace = new ReplaceOperation("/type", - new JsonLateObjectEvaluator(mapper, mapper.readTree("{ \"value\" : \"new\" }"))); + ReplaceOperation replace = ReplaceOperation.valueAt("/type") + .with(new JsonLateObjectEvaluator(mapper, mapper.readTree("{ \"value\" : \"new\" }"))); replace.perform(todo, Todo.class); assertNotNull(todo.getType()); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PathToSpelTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java similarity index 61% rename from spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PathToSpelTests.java rename to spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java index 70dd0a522..3bcc6795f 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/PathToSpelTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java @@ -21,33 +21,52 @@ import java.util.ArrayList; import java.util.List; import org.junit.Test; -import org.springframework.expression.Expression; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.TypedSpelPath; -public class PathToSpelTests { +public class SpelPathUnitTests { @Test public void listIndex() { - Expression expr = PathToSpEL.pathToExpression("/1/description"); + SpelPath expr = SpelPath.of("/1/description"); List todos = new ArrayList(); todos.add(new Todo(1L, "A", false)); todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - assertEquals("B", (String) expr.getValue(todos)); + assertEquals("B", (String) expr.bindTo(Todo.class).getValue(todos)); } @Test public void accessesLastCollectionElementWithDash() { - Expression expr = PathToSpEL.pathToExpression("/-/description"); + SpelPath expr = SpelPath.of("/-/description"); List todos = new ArrayList(); todos.add(new Todo(1L, "A", false)); todos.add(new Todo(2L, "B", false)); todos.add(new Todo(3L, "C", false)); - assertEquals("C", (String) expr.getValue(todos)); + assertEquals("C", (String) expr.bindTo(Todo.class).getValue(todos)); + } + + @Test // DATAREST-1152 + public void cachesSpelPath() { + + SpelPath left = SpelPath.of("/description"); + SpelPath right = SpelPath.of("/description"); + + assertSame(left, right); + } + + @Test // DATAREST-1152 + public void cachesTypedSpelPath() { + + SpelPath source = SpelPath.of("/description"); + TypedSpelPath left = source.bindTo(Todo.class); + TypedSpelPath right = source.bindTo(Todo.class); + + assertSame(left, right); } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/TestOperationTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/TestOperationUnitTests.java similarity index 80% rename from spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/TestOperationTests.java rename to spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/TestOperationUnitTests.java index 348e9603a..b84c51a99 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/TestOperationTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/TestOperationUnitTests.java @@ -20,7 +20,7 @@ import java.util.List; import org.junit.Test; -public class TestOperationTests { +public class TestOperationUnitTests { @Test public void testPropertyValueEquals() throws Exception { @@ -30,10 +30,10 @@ public class TestOperationTests { todos.add(new Todo(2L, "B", true)); todos.add(new Todo(3L, "C", false)); - TestOperation test = new TestOperation("/0/complete", false); + TestOperation test = TestOperation.whetherValueAt("/0/complete").hasValue(false); test.perform(todos, Todo.class); - TestOperation test2 = new TestOperation("/1/complete", true); + TestOperation test2 = TestOperation.whetherValueAt("/1/complete").hasValue(true); test2.perform(todos, Todo.class); } @@ -46,7 +46,7 @@ public class TestOperationTests { todos.add(new Todo(2L, "B", true)); todos.add(new Todo(3L, "C", false)); - TestOperation test = new TestOperation("/0/complete", true); + TestOperation test = TestOperation.whetherValueAt("/0/complete").hasValue(true); test.perform(todos, Todo.class); } @@ -58,7 +58,7 @@ public class TestOperationTests { todos.add(new Todo(2L, "B", true)); todos.add(new Todo(3L, "C", false)); - TestOperation test = new TestOperation("/1", new Todo(2L, "B", true)); + TestOperation test = TestOperation.whetherValueAt("/1").hasValue(new Todo(2L, "B", true)); test.perform(todos, Todo.class); } }