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 34356f3e3..d17fab144 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 @@ -15,6 +15,8 @@ */ package org.springframework.data.rest.webmvc.json.patch; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath; + /** * Operation to add a new value to the given "path". Will throw a {@link PatchException} if the path is invalid or if * the given value is not assignable to the given path. @@ -30,12 +32,12 @@ 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. */ - private AddOperation(SpelPath path, Object value) { + private AddOperation(UntypedSpelPath path, Object value) { super("add", path, value); } public static AddOperation of(String path, Object value) { - return new AddOperation(SpelPath.of(path), value); + return new AddOperation(SpelPath.untyped(path), value); } /* @@ -58,6 +60,6 @@ class AddOperation extends PatchOperation { return super.evaluateValueFromTarget(targetObject, entityType); } - return evaluate(path.getLeafType(entityType)); + return evaluate(path.bindTo(entityType).getLeafType()); } } 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 e1150c064..9e1f74db8 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 @@ -17,6 +17,8 @@ package org.springframework.data.rest.webmvc.json.patch; import lombok.RequiredArgsConstructor; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath; + /** *

* Operation to copy a value from the given "from" path to the given "path". Will throw a {@link PatchException} if @@ -41,7 +43,7 @@ import lombok.RequiredArgsConstructor; */ class CopyOperation extends PatchOperation { - private final SpelPath from; + private final UntypedSpelPath from; /** * Constructs the copy operation @@ -49,7 +51,7 @@ class CopyOperation extends PatchOperation { * @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(SpelPath path, SpelPath from) { + public CopyOperation(UntypedSpelPath path, UntypedSpelPath from) { super("copy", path); @@ -66,7 +68,7 @@ class CopyOperation extends PatchOperation { private final String from; CopyOperation to(String to) { - return new CopyOperation(SpelPath.of(to), SpelPath.of(from)); + return new CopyOperation(SpelPath.untyped(to), SpelPath.untyped(from)); } } 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 08ed3d385..b0933bbc2 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 @@ -18,6 +18,8 @@ package org.springframework.data.rest.webmvc.json.patch; import lombok.AccessLevel; import lombok.RequiredArgsConstructor; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath; + /** *

* Operation that moves a value from the given "from" path to the given "path". Will throw a {@link PatchException} if @@ -35,7 +37,7 @@ import lombok.RequiredArgsConstructor; */ class MoveOperation extends PatchOperation { - private final SpelPath from; + private final UntypedSpelPath from; /** * Constructs the move operation. @@ -43,7 +45,7 @@ class MoveOperation extends PatchOperation { * @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') */ - private MoveOperation(SpelPath path, SpelPath from) { + private MoveOperation(UntypedSpelPath path, UntypedSpelPath from) { super("move", path); @@ -60,7 +62,7 @@ class MoveOperation extends PatchOperation { private final String from; public MoveOperation to(String to) { - return new MoveOperation(SpelPath.of(to), SpelPath.of(from)); + return new MoveOperation(SpelPath.untyped(to), SpelPath.untyped(from)); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/Patch.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/Patch.java index 349548fa3..a589ed88a 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/Patch.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/patch/Patch.java @@ -15,8 +15,11 @@ */ package org.springframework.data.rest.webmvc.json.patch; +import java.util.Iterator; import java.util.List; +import org.springframework.data.util.Streamable; + /** *

* Represents a Patch. @@ -29,7 +32,7 @@ import java.util.List; * @author Craig Walls * @author Oliver Gierke */ -public class Patch { +public class Patch implements Streamable { private final List operations; @@ -44,6 +47,13 @@ public class Patch { return operations.size(); } + /** + * Returns all underlying {@link PatchOperation}s. + * + * @return + * @deprecated since 3.2, prefer streaming via {@link #stream()}. + */ + @Deprecated public List getOperations() { return operations; } @@ -85,4 +95,13 @@ public class Patch { return in; } + + /* + * (non-Javadoc) + * @see java.lang.Iterable#iterator() + */ + @Override + public Iterator iterator() { + return operations.iterator(); + } } 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 4f6ed4da5..3e6a72d39 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 @@ -18,6 +18,8 @@ package org.springframework.data.rest.webmvc.json.patch; import lombok.NonNull; import lombok.RequiredArgsConstructor; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath; + /** * Abstract base class representing and providing support methods for patch operations. * @@ -29,7 +31,7 @@ import lombok.RequiredArgsConstructor; public abstract class PatchOperation { protected final @NonNull String op; - protected final @NonNull SpelPath path; + protected final @NonNull UntypedSpelPath path; protected final Object value; /** @@ -38,7 +40,7 @@ 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, SpelPath path) { + public PatchOperation(String op, UntypedSpelPath path) { this(op, path, null); } 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 983717546..01150eb54 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 @@ -15,6 +15,8 @@ */ package org.springframework.data.rest.webmvc.json.patch; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath; + /** * Operation that removes the value at the given path. Will throw a {@link PatchException} if the given path isn't valid * or if the path is non-nullable. @@ -29,12 +31,12 @@ class RemoveOperation extends PatchOperation { * * @param path The path of the value to be removed. (e.g., '/foo/bar/4') */ - private RemoveOperation(SpelPath path) { + private RemoveOperation(UntypedSpelPath path) { super("remove", path); } public static RemoveOperation valueAt(String path) { - return new RemoveOperation(SpelPath.of(path)); + return new RemoveOperation(SpelPath.untyped(path)); } /* 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 6587092a4..8e1ed29fd 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 @@ -18,6 +18,8 @@ package org.springframework.data.rest.webmvc.json.patch; import lombok.AccessLevel; import lombok.RequiredArgsConstructor; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath; + /** * Operation that replaces the value at the given path with a new value. * @@ -32,7 +34,7 @@ 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. */ - private ReplaceOperation(SpelPath path, Object value) { + private ReplaceOperation(UntypedSpelPath path, Object value) { super("replace", path, value); } @@ -46,7 +48,7 @@ class ReplaceOperation extends PatchOperation { private final String path; public ReplaceOperation with(Object value) { - return new ReplaceOperation(SpelPath.of(path), value); + return new ReplaceOperation(SpelPath.untyped(path), value); } } 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 index 4df29867a..fb9ae68ad 100644 --- 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 @@ -26,12 +26,14 @@ import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; +import java.util.stream.Stream; 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.data.util.ClassTypeInformation; +import org.springframework.data.util.TypeInformation; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionException; @@ -39,9 +41,11 @@ import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.expression.spel.SpelMessage; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.SimpleEvaluationContext; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.ConcurrentReferenceHashMap; +import org.springframework.util.StringUtils; /** * Value object to represent a SpEL-backed patch path. @@ -53,53 +57,28 @@ 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); + private static final Map UNTYPED_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. + * Returns a {@link UntypedSpelPath} for the given source. * * @param source must not be {@literal null}. * @return */ - public static SpelPath of(String source) { - return PATHS.computeIfAbsent(source, SpelPath::new); + public static UntypedSpelPath untyped(String source) { + return UNTYPED_PATHS.computeIfAbsent(source, UntypedSpelPath::new); } /** - * Returns a {@link TypedSpelPath} binding the expression to the given type. + * Returns a {@link TypedSpelPath} for the given source and type. * - * @param type must not be {@literal null}. + * @param source 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); + public static TypedSpelPath typed(String source, Class type) { + return untyped(source).bindTo(type); } /** @@ -111,10 +90,6 @@ class SpelPath { return path.endsWith("-"); } - private SpelPath getParent() { - return SpelPath.of(path.substring(0, path.lastIndexOf('/'))); - } - /* * (non-Javadoc) * @see java.lang.Object#toString() @@ -135,7 +110,7 @@ class SpelPath { return true; } - if (!(SpelPath.class.isInstance(obj))) { + if (!SpelPath.class.isInstance(obj)) { return false; } @@ -153,54 +128,24 @@ class SpelPath { return path.hashCode(); } - private static String pathToSpEL(String path) { - return pathNodesToSpEL(path.split("\\/")); - } + static class UntypedSpelPath extends SpelPath { - 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); - } + private UntypedSpelPath(String path) { + super(path); } - String spel = spelBuilder.toString(); + /** + * Returns a {@link TypedSpelPath} binding the expression to the given type. + * + * @param type must not be {@literal null}. + * @return + */ + public TypedSpelPath bindTo(Class type) { - if (spel.length() == 0) { - spel = "#this"; + Assert.notNull(type, "Type must not be null!"); + + return TypedSpelPath.of(this, type); } - - return spel; } /** @@ -211,26 +156,26 @@ class SpelPath { @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 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 TYPED_PATHS = new ConcurrentReferenceHashMap<>(32); private static final EvaluationContext CONTEXT = SimpleEvaluationContext.forReadWriteDataBinding().build(); + private final Expression expression; private final Class type; @Value(staticConstructor = "of") private static class CacheKey { Class type; - SpelPath path; + UntypedSpelPath path; } - private TypedSpelPath(SpelPath path, Class type) { + private TypedSpelPath(UntypedSpelPath path, Class type) { - super(path.path, path.expression); - - verifyPath(path.path, type); + super(path.path); this.type = type; + this.expression = toSpel(path.path, type); } /** @@ -240,7 +185,7 @@ class SpelPath { * @param type must not be {@literal null}. * @return */ - public static TypedSpelPath of(SpelPath path, Class type) { + public static TypedSpelPath of(UntypedSpelPath path, Class type) { Assert.notNull(path, "Path must not be null!"); Assert.notNull(type, "Type must not be null!"); @@ -272,13 +217,30 @@ class SpelPath { * @param target must not be {@literal null}. * @param value can be {@literal null}. */ - public void setValue(Object target, Object value) { + public void setValue(Object target, @Nullable Object value) { Assert.notNull(target, "Target must not be null!"); expression.setValue(CONTEXT, target, value); } + /** + * Returns the type of the leaf property of the path. + * + * @return will never be {@literal null}. + */ + public Class getLeafType() { + + return TypedSpelPath.verifyPath(path, type) // + .map(PropertyPath::getLeafProperty) // + .> map(PropertyPath::getType) // + .orElse(type); + } + + public String getExpressionString() { + return expression.getExpressionString(); + } + /** * Returns the type of the expression target based on the given root. * @@ -316,8 +278,7 @@ class SpelPath { * @param source the source object to look the value up from, must not be {@literal null}. * @return */ - - public void copyFrom(SpelPath path, Object source) { + public void copyFrom(UntypedSpelPath path, Object source) { Assert.notNull(path, "Source path must not be null!"); Assert.notNull(source, "Source value must not be null!"); @@ -333,7 +294,7 @@ class SpelPath { * @param source the source object to look the value up from, must not be {@literal null}. * @return */ - public void moveFrom(SpelPath path, Object source) { + public void moveFrom(UntypedSpelPath path, Object source) { Assert.notNull(path, "Source path must not be null!"); Assert.notNull(source, "Source value must not be null!"); @@ -413,8 +374,20 @@ class SpelPath { } } + /* + * (non-Javadoc) + * @see org.springframework.data.rest.webmvc.json.patch.SpelPath#toString() + */ + @Override + public String toString() { + return String.format("%s on %s -> %s", path, type.getName(), getExpressionString()); + } + private TypedSpelPath getParent() { - return TypedSpelPath.of(super.getParent(), type); + + return SpelPath // + .untyped(path.substring(0, path.lastIndexOf('/'))) // + .bindTo(type); } private TypeDescriptor getTypeDescriptor(Object target) { @@ -449,20 +422,153 @@ class SpelPath { 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(".")); + // Remove leading digits + String segmentSource = path.replaceAll("^/\\d+", ""); - if (pathSource.isEmpty()) { - return Optional.empty(); - } + Stream segments = Arrays.stream(segmentSource.split("/"))// + .filter(it -> !it.equals("-")) // no "last element"s + .filter(it -> !it.isEmpty()); try { - return Optional.of(PropertyPath.from(pathSource, type)); + + return segments.reduce(Optional. empty(), // + (current, next) -> Optional.of(createOrSkip(current, next, type)), // + (l, r) -> r) // + .map(SkippedPropertyPath::getPath); + } catch (PropertyReferenceException o_O) { - throw new PatchException(String.format(INVALID_PATH_REFERENCE, pathSource, type, path), o_O); + throw new PatchException(String.format(INVALID_PATH_REFERENCE, o_O.getPropertyName(), type), o_O); + } + } + + @Value + @RequiredArgsConstructor(access = AccessLevel.PRIVATE, staticName = "of") + private static class SkippedPropertyPath { + + PropertyPath path; + boolean skipped; + + public static SkippedPropertyPath of(String segment, Class type) { + return of(PropertyPath.from(segment, type), false); + } + + public SkippedPropertyPath nested(String segment) { + + if (skipped) { + return SkippedPropertyPath.of(path.nested(segment), false); + } + + TypeInformation typeInformation = path.getTypeInformation(); + + return typeInformation.isMap() || typeInformation.isCollectionLike() // + ? SkippedPropertyPath.of(path, true) // + : SkippedPropertyPath.of(path.nested(segment), false); + } + } + + private static Expression toSpel(String path, Class type) { + + String expression = Arrays.stream(path.split("/"))// + .filter(it -> !it.isEmpty()) // + .reduce(Optional. empty(), // + (current, next) -> Optional.of(nextOrCreate(current, next, type)), // + (l, r) -> r) // + .map(it -> it.getExpression()) // + .orElse("#this"); + + return SPEL_EXPRESSION_PARSER.parseExpression(expression); + } + + private static SpelExpressionBuilder nextOrCreate(Optional current, String next, + Class type) { + + return current // + .map(it -> it.next(next)) // + .orElseGet(() -> SpelExpressionBuilder.of(type).next(next)); + } + + private static SkippedPropertyPath createOrSkip(Optional current, String next, Class type) { + + return current // + .map(it -> it.nested(next)) // + .orElseGet(() -> SkippedPropertyPath.of(next, type)); + } + + @Value + private static class SpelExpressionBuilder { + + private static final TypeInformation STRING_TYPE = ClassTypeInformation.from(String.class); + + private final @Nullable PropertyPath basePath; + private final Class type; + private final String spelSegment; + private final boolean skipped; + + public String getExpression() { + return StringUtils.hasText(spelSegment) ? spelSegment : null; + } + + public static SpelExpressionBuilder of(Class type) { + return new SpelExpressionBuilder(null, type, "", false); + } + + private SpelExpressionBuilder skipWith(String segment) { + return new SpelExpressionBuilder(basePath, type, spelSegment.concat(segment), true); + } + + private SpelExpressionBuilder nested(String segment) { + + String segmentBase = StringUtils.hasText(spelSegment) // + ? spelSegment.concat(".") // + : spelSegment; + + try { + + PropertyPath path = basePath == null // + ? PropertyPath.from(segment, type) // + : basePath.nested(segment); + + return new SpelExpressionBuilder(path, type, segmentBase.concat(segment), false); + + } catch (PropertyReferenceException o_O) { + throw new PatchException(String.format(INVALID_PATH_REFERENCE, o_O.getPropertyName(), type), o_O); + } + } + + public SpelExpressionBuilder next(String segment) { + + if (basePath == null) { + + if (APPEND_CHARACTER.equals(segment)) { + return skipWith("$[true]"); + } + + if (segment.matches("\\d+")) { + return skipWith(String.format("[%s]", segment)); + } + + return nested(segment); + } + + if (skipped) { + return nested(segment); + } + + TypeInformation typeInformation = basePath.getTypeInformation(); + + if (typeInformation.isMap()) { + + TypeInformation componentType = typeInformation.getComponentType(); + String keyExpression = STRING_TYPE.equals(componentType) ? String.format("'%s'", segment) : segment; + + return skipWith(String.format("[%s]", keyExpression)); + } + + if (typeInformation.isCollectionLike()) { + return skipWith(APPEND_CHARACTER.equals(segment) ? "$[true]" : String.format("[%s]", segment)); + } + + return nested(segment); } } } 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 4436325aa..d856f4a56 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 @@ -21,6 +21,7 @@ import lombok.RequiredArgsConstructor; import java.math.BigDecimal; import java.math.BigInteger; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath; import org.springframework.util.ObjectUtils; /** @@ -43,7 +44,7 @@ class TestOperation extends PatchOperation { * @param path The path to test. (e.g., '/foo/bar/4') * @param value The value to test the path against. */ - private TestOperation(SpelPath path, Object value) { + private TestOperation(UntypedSpelPath path, Object value) { super("test", path, value); } @@ -57,7 +58,7 @@ class TestOperation extends PatchOperation { private final String path; public TestOperation hasValue(Object value) { - return new TestOperation(SpelPath.of(path), value); + return new TestOperation(SpelPath.untyped(path), value); } } 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 034ac1d0e..4ae778bb7 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 @@ -15,13 +15,17 @@ */ package org.springframework.data.rest.webmvc.json.patch; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.*; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.junit.Test; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; public class ReplaceOperationTests { @@ -82,4 +86,30 @@ public class ReplaceOperationTests { assertNotNull(todo.getType().getValue()); assertTrue(todo.getType().getValue().equals("new")); } + + @Test // DATAREST-1338 + public void replacesMapValueCorrectly() throws Exception { + + Book book = new Book(); + book.characters = new HashMap<>(); + book.characters.put("protagonist", "Pinco"); + + ReplaceOperation.valueAt("/characters/protagonist") // + .with(prepareValue("\"Pallo\"")) // + .perform(book, Book.class); + + assertThat(book.characters.get("protagonist")).isEqualTo("Pallo"); + } + + private static Object prepareValue(String json) throws Exception { + + ObjectMapper mapper = new ObjectMapper(); + JsonNode node = mapper.readTree(json); + return new JsonLateObjectEvaluator(mapper, node); + } + + // DATAREST-1338 + class Book { + public Map characters; + } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java index 625575e1f..c21d67461 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/patch/SpelPathUnitTests.java @@ -20,9 +20,11 @@ import static org.junit.Assert.*; import java.util.ArrayList; import java.util.List; +import java.util.Map; import org.junit.Test; import org.springframework.data.rest.webmvc.json.patch.SpelPath.TypedSpelPath; +import org.springframework.data.rest.webmvc.json.patch.SpelPath.UntypedSpelPath; /** * Unit tests for {@link SpelPath}. @@ -34,34 +36,34 @@ public class SpelPathUnitTests { @Test public void listIndex() { - SpelPath expr = SpelPath.of("/1/description"); + UntypedSpelPath expr = SpelPath.untyped("/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.bindTo(Todo.class).getValue(todos)); + assertEquals("B", expr.bindTo(Todo.class).getValue(todos)); } @Test public void accessesLastCollectionElementWithDash() { - SpelPath expr = SpelPath.of("/-/description"); + UntypedSpelPath expr = SpelPath.untyped("/-/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.bindTo(Todo.class).getValue(todos)); + assertEquals("C", expr.bindTo(Todo.class).getValue(todos)); } @Test // DATAREST-1152 public void cachesSpelPath() { - SpelPath left = SpelPath.of("/description"); - SpelPath right = SpelPath.of("/description"); + UntypedSpelPath left = SpelPath.untyped("/description"); + UntypedSpelPath right = SpelPath.untyped("/description"); assertSame(left, right); } @@ -69,7 +71,7 @@ public class SpelPathUnitTests { @Test // DATAREST-1152 public void cachesTypedSpelPath() { - SpelPath source = SpelPath.of("/description"); + UntypedSpelPath source = SpelPath.untyped("/description"); TypedSpelPath left = source.bindTo(Todo.class); TypedSpelPath right = source.bindTo(Todo.class); @@ -78,6 +80,35 @@ public class SpelPathUnitTests { @Test // DATAREST-1274 public void supportsMultiDigitCollectionIndex() { - assertThat(SpelPath.of("/11/description").getLeafType(Todo.class)).isEqualTo(String.class); + assertThat(SpelPath.untyped("/11/description").bindTo(Todo.class).getLeafType()).isEqualTo(String.class); + } + + @Test // DATAREST-1338 + public void handlesStringMapKeysInPathExpressions() { + + TypedSpelPath path = SpelPath.untyped("people/Dave/name").bindTo(MapWrapper.class); + + assertThat(path.getExpressionString()).isEqualTo("people['Dave'].name"); + assertThat(path.getLeafType()).isEqualTo(String.class); + } + + @Test // DATAREST-1338 + public void handlesIntegerMapKeysInPathExpressions() { + + TypedSpelPath path = SpelPath.untyped("peopleByInt/0/name").bindTo(MapWrapper.class); + + assertThat(path.getExpressionString()).isEqualTo("peopleByInt[0].name"); + assertThat(path.getLeafType()).isEqualTo(String.class); + } + + // DATAREST-1338 + + static class Person { + String name; + } + + static class MapWrapper { + Map people; + Map peopleByInt; } }