Skip to content

[US642] JSON Patch Query preparations - use JsonPath instead of JsonPointer #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 27, 2022
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ buildscript {
repositories {
mavenCentral()
maven {
url "http://repo.springsource.org/plugins-release";
url "https://repo.springsource.org/plugins-release";
}
}
dependencies {
Expand Down
2 changes: 1 addition & 1 deletion project.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ project.ext.description = "JSON Patch (RFC 6902) and JSON Merge Patch (RFC 7386)
dependencies {
provided(group: "com.google.code.findbugs", name: "jsr305", version: "3.0.2");
compile(group: "com.fasterxml.jackson.core", name: "jackson-databind", version: "2.11.0");
compile(group: 'com.jayway.jsonpath', name: 'json-path', version: '2.6.0')
compile(group: "com.github.java-json-tools", name: "msg-simple", version: "1.2");

compile(group: "com.github.java-json-tools", name: "jackson-coreutils", version: "2.0");
testCompile(group: "org.testng", name: "testng", version: "7.1.0") {
exclude(group: "junit", module: "junit");
Expand Down
91 changes: 39 additions & 52 deletions src/main/java/com/github/fge/jsonpatch/AddOperation.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
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.ObjectNode;
import com.github.fge.jackson.jsonpointer.JsonPointer;
import com.github.fge.jackson.jsonpointer.ReferenceToken;
import com.github.fge.jackson.jsonpointer.TokenResolver;

import java.util.NoSuchElementException;
import com.jayway.jsonpath.DocumentContext;
import com.jayway.jsonpath.JsonPath;


/**
Expand Down Expand Up @@ -65,78 +65,65 @@
* [ 1, 2, 3 ]
* </pre>
*/
public final class AddOperation
extends PathValueOperation
{
private static final ReferenceToken LAST_ARRAY_ELEMENT
= ReferenceToken.fromRaw("-");
public final class AddOperation extends PathValueOperation {
public static final String LAST_ARRAY_ELEMENT_SYMBOL = "-";

@JsonCreator
public AddOperation(@JsonProperty("path") final JsonPointer path,
@JsonProperty("value") final JsonNode value)
{
public AddOperation(@JsonProperty("path") final String path,
@JsonProperty("value") final JsonNode value) {
super("add", path, value);
}

@Override
public JsonNode apply(final JsonNode node)
throws JsonPatchException
{
if (path.isEmpty())
public JsonNode apply(final JsonNode node) throws JsonPatchException {
if (path.isEmpty()) {
return value;

}
/*
* Check the parent node: it must exist and be a container (ie an array
* or an object) for the add operation to work.
*/
final JsonNode parentNode = path.parent().path(node);
if (parentNode.isMissingNode())
throw new JsonPatchException(BUNDLE.getMessage(
"jsonPatch.noSuchParent"));
if (!parentNode.isContainerNode())
throw new JsonPatchException(BUNDLE.getMessage(
"jsonPatch.parentNotContainer"));
return parentNode.isArray()
? addToArray(path, node)
: addToObject(path, node);
}
final int lastSlashIndex = path.lastIndexOf('/');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test with paths like
/this/is/my/path
and
/this/is/my/path/

Will both work?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First path obviously works.
Technically the second path leads to an element with name equal to empty string, inside a node with name "path", this is currently not working in json path anyway, I filed a ticket for it and it's on our channel.
Our implementation might work with it, but tests are commented out due to that bug.

final String newNodeName = path.substring(lastSlashIndex + 1);
final String pathToParent = path.substring(0, lastSlashIndex);
final String jsonPath = JsonPathParser.tmfStringToJsonPath(pathToParent);
final DocumentContext nodeContext = JsonPath.parse(node.deepCopy());

private JsonNode addToArray(final JsonPointer path, final JsonNode node)
throws JsonPatchException
{
final JsonNode ret = node.deepCopy();
final ArrayNode target = (ArrayNode) path.parent().get(ret);
final JsonNode parentNode = nodeContext.read(jsonPath);
if (parentNode == null) {
throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.noSuchParent"));
}
if (!parentNode.isContainerNode()) {
throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.parentNotContainer"));
}

final TokenResolver<JsonNode> token = Iterables.getLast(path);
return parentNode.isArray()
? addToArray(nodeContext, jsonPath, newNodeName)
: addToObject(nodeContext, jsonPath, newNodeName);
}

if (token.getToken().equals(LAST_ARRAY_ELEMENT)) {
target.add(value);
return ret;
private JsonNode addToArray(final DocumentContext node, String jsonPath, String newNodeName) throws JsonPatchException {
if (newNodeName.equals(LAST_ARRAY_ELEMENT_SYMBOL)) {
return node.add(jsonPath, value).read("$", JsonNode.class);
}

final int size = target.size();
final int size = node.read(jsonPath, JsonNode.class).size();
final int index;
try {
index = Integer.parseInt(token.toString());
index = Integer.parseInt(newNodeName);
} catch (NumberFormatException ignored) {
throw new JsonPatchException(BUNDLE.getMessage(
"jsonPatch.notAnIndex"));
throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.notAnIndex"));
}

if (index < 0 || index > size)
throw new JsonPatchException(BUNDLE.getMessage(
"jsonPatch.noSuchIndex"));
throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.noSuchIndex"));

target.insert(index, value);
return ret;
ArrayNode updatedArray = node.read(jsonPath, ArrayNode.class).insert(index, value);
return "$".equals(jsonPath) ? updatedArray : node.set(jsonPath, updatedArray).read("$", JsonNode.class);
}

private JsonNode addToObject(final JsonPointer path, final JsonNode node)
{
final TokenResolver<JsonNode> token = Iterables.getLast(path);
final JsonNode ret = node.deepCopy();
final ObjectNode target = (ObjectNode) path.parent().get(ret);
target.set(token.getToken().getRaw(), value);
return ret;
private JsonNode addToObject(final DocumentContext node, String jsonPath, String newNodeName) {
return node
.put(jsonPath, newNodeName, value)
.read("$", JsonNode.class);
}
}
24 changes: 10 additions & 14 deletions src/main/java/com/github/fge/jsonpatch/CopyOperation.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.JsonNode;
import com.github.fge.jackson.jsonpointer.JsonPointer;
import com.jayway.jsonpath.JsonPath;

/**
* JSON Patch {@code copy} operation
Expand All @@ -40,24 +40,20 @@
*
* <p>It is an error if {@code from} fails to resolve to a JSON value.</p>
*/
public final class CopyOperation
extends DualPathOperation
{
public final class CopyOperation extends DualPathOperation {

@JsonCreator
public CopyOperation(@JsonProperty("from") final JsonPointer from,
@JsonProperty("path") final JsonPointer path)
{
public CopyOperation(@JsonProperty("from") final String from, @JsonProperty("path") final String path) {
super("copy", from, path);
}

@Override
public JsonNode apply(final JsonNode node)
throws JsonPatchException
{
final JsonNode dupData = from.path(node).deepCopy();
if (dupData.isMissingNode())
throw new JsonPatchException(BUNDLE.getMessage(
"jsonPatch.noSuchPath"));
public JsonNode apply(final JsonNode node) throws JsonPatchException {
final String jsonPath = JsonPathParser.tmfStringToJsonPath(from);
final JsonNode dupData = JsonPath.parse(node.deepCopy()).read(jsonPath);
if (dupData == null) {
throw new JsonPatchException(BUNDLE.getMessage("jsonPatch.noSuchPath"));
}
return new AddOperation(path, dupData).apply(node);
}
}
29 changes: 9 additions & 20 deletions src/main/java/com/github/fge/jsonpatch/DualPathOperation.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,38 +25,30 @@
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
import com.github.fge.jackson.jsonpointer.JsonPointer;

import java.io.IOException;

/**
* Base class for JSON Patch operations taking two JSON Pointers as arguments
*/
public abstract class DualPathOperation
extends JsonPatchOperation
{
public abstract class DualPathOperation extends JsonPatchOperation {
@JsonSerialize(using = ToStringSerializer.class)
protected final JsonPointer from;
protected final String from;

/**
* Protected constructor
*
* @param op operation name
* @param op operation name
* @param from source path
* @param path destination path
*/
protected DualPathOperation(final String op, final JsonPointer from,
final JsonPointer path)
{
protected DualPathOperation(final String op, final String from, final String path) {
super(op, path);
this.from = from;
}

@Override
public final void serialize(final JsonGenerator jgen,
final SerializerProvider provider)
throws IOException, JsonProcessingException
{
public final void serialize(final JsonGenerator jgen, final SerializerProvider provider) throws IOException, JsonProcessingException {
jgen.writeStartObject();
jgen.writeStringField("op", op);
jgen.writeStringField("path", path.toString());
Expand All @@ -65,20 +57,17 @@ public final void serialize(final JsonGenerator jgen,
}

@Override
public final void serializeWithType(final JsonGenerator jgen,
final SerializerProvider provider, final TypeSerializer typeSer)
throws IOException, JsonProcessingException
{
public final void serializeWithType(final JsonGenerator jgen, final SerializerProvider provider,
final TypeSerializer typeSer) throws IOException, JsonProcessingException {
serialize(jgen, provider);
}

public final JsonPointer getFrom() {
public final String getFrom() {
return from;
}

@Override
public final String toString()
{
public final String toString() {
return "op: " + op + "; from: \"" + from + "\"; path: \"" + path + '"';
}
}
59 changes: 41 additions & 18 deletions src/main/java/com/github/fge/jsonpatch/JsonPatchOperation.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,28 @@
import com.github.fge.jackson.jsonpointer.JsonPointer;
import com.github.fge.msgsimple.bundle.MessageBundle;
import com.github.fge.msgsimple.load.MessageBundles;
import com.jayway.jsonpath.Configuration;
import com.jayway.jsonpath.Option;
import com.jayway.jsonpath.spi.json.JacksonJsonNodeJsonProvider;
import com.jayway.jsonpath.spi.json.JsonProvider;
import com.jayway.jsonpath.spi.mapper.JacksonMappingProvider;
import com.jayway.jsonpath.spi.mapper.MappingProvider;

import java.util.EnumSet;
import java.util.Set;

import static com.fasterxml.jackson.annotation.JsonSubTypes.*;
import static com.fasterxml.jackson.annotation.JsonTypeInfo.*;

@JsonTypeInfo(use = Id.NAME, include = As.PROPERTY, property = "op")

@JsonSubTypes({
@Type(name = "add", value = AddOperation.class),
@Type(name = "copy", value = CopyOperation.class),
@Type(name = "move", value = MoveOperation.class),
@Type(name = "remove", value = RemoveOperation.class),
@Type(name = "replace", value = ReplaceOperation.class),
@Type(name = "test", value = TestOperation.class)
@Type(name = "add", value = AddOperation.class),
@Type(name = "copy", value = CopyOperation.class),
@Type(name = "move", value = MoveOperation.class),
@Type(name = "remove", value = RemoveOperation.class),
@Type(name = "replace", value = ReplaceOperation.class),
@Type(name = "test", value = TestOperation.class)
})

/**
Expand All @@ -56,11 +65,27 @@
* </ul>
*/
@JsonIgnoreProperties(ignoreUnknown = true)
public abstract class JsonPatchOperation
implements JsonSerializable
{
protected static final MessageBundle BUNDLE
= MessageBundles.getBundle(JsonPatchMessages.class);
public abstract class JsonPatchOperation implements JsonSerializable {
protected static final MessageBundle BUNDLE = MessageBundles.getBundle(JsonPatchMessages.class);

static {
Configuration.setDefaults(new Configuration.Defaults() {
@Override
public JsonProvider jsonProvider() {
return new JacksonJsonNodeJsonProvider();
}

@Override
public Set<Option> options() {
return EnumSet.of(Option.SUPPRESS_EXCEPTIONS);
}

@Override
public MappingProvider mappingProvider() {
return new JacksonMappingProvider();
}
});
}

protected final String op;

Expand All @@ -70,16 +95,15 @@ public abstract class JsonPatchOperation
*
* However, we need to serialize using .toString().
*/
protected final JsonPointer path;
protected final String path;

/**
* Constructor
*
* @param op the operation name
* @param op the operation name
* @param path the JSON Pointer for this operation
*/
protected JsonPatchOperation(final String op, final JsonPointer path)
{
protected JsonPatchOperation(final String op, final String path) {
this.op = op;
this.path = path;
}
Expand All @@ -91,14 +115,13 @@ protected JsonPatchOperation(final String op, final JsonPointer path)
* @return the patched value
* @throws JsonPatchException operation failed to apply to this value
*/
public abstract JsonNode apply(final JsonNode node)
throws JsonPatchException;
public abstract JsonNode apply(final JsonNode node) throws JsonPatchException;

public final String getOp() {
return op;
}

public final JsonPointer getPath() {
public final String getPath() {
return path;
}

Expand Down
17 changes: 17 additions & 0 deletions src/main/java/com/github/fge/jsonpatch/JsonPathParser.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.github.fge.jsonpatch;

public class JsonPathParser {

private static final String ARRAY_ELEMENT_REGEX = "\\.(\\d+)\\.";
private static final String ARRAY_ELEMENT_LAST_REGEX = "\\.(\\d+)$";

public static String tmfStringToJsonPath(String path) {
if ("/".equals(path)) {
return "$";
}
final String jsonPath = "$" + path.replace('/', '.')
.replaceAll(ARRAY_ELEMENT_REGEX, ".[$1].")
.replaceAll(ARRAY_ELEMENT_LAST_REGEX, ".[$1]");
return jsonPath;
}
}
Loading