Skip to content

add flag to not generate Move or Copy operations. #81

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/main/java/com/github/fge/jsonpatch/diff/DiffOptions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.github.fge.jsonpatch.diff;

/**
* Created by AMA on 24/09/2020.
*/
public class DiffOptions {

public static final boolean DIFF_DOESNT_REQUIRE_SOURCE = false;

final boolean diffDoesntRequireSource;

public static final DiffOptions DEFAULT_OPTIONS = new DiffOptions(DIFF_DOESNT_REQUIRE_SOURCE);

private DiffOptions(boolean diffDoesntRequireSource) {
this.diffDoesntRequireSource = diffDoesntRequireSource;
}

public boolean isDiffDoesntRequireSource() {
return diffDoesntRequireSource;
}

public static class Builder {
private boolean diffDoesntRequireSource = DIFF_DOESNT_REQUIRE_SOURCE;

public Builder diffDoesntRequireSource() {
diffDoesntRequireSource = true;
return this;
}

public Builder diffRequireSource() {
diffDoesntRequireSource = false;
return this;
}

public DiffOptions build(){
return new DiffOptions(diffDoesntRequireSource);
}

}
}
10 changes: 5 additions & 5 deletions src/main/java/com/github/fge/jsonpatch/diff/DiffProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,18 @@ final class DiffProcessor

private final Map<JsonPointer, JsonNode> unchanged;

private final boolean diffDoesntRequireSource;
private final DiffOptions options;
public static final boolean DIFF_DOESNT_REQUIRE_SOURCE = false;

private final List<DiffOperation> diffs = new ArrayList<DiffOperation>();

DiffProcessor(final Map<JsonPointer, JsonNode> unchanged) {
this(unchanged, DIFF_DOESNT_REQUIRE_SOURCE);
this(unchanged, DiffOptions.DEFAULT_OPTIONS);
}

DiffProcessor(final Map<JsonPointer, JsonNode> unchanged, boolean diffDoesntRequireSource)
DiffProcessor(final Map<JsonPointer, JsonNode> unchanged, DiffOptions options)
{
this.diffDoesntRequireSource = diffDoesntRequireSource;
this.options = options;
this.unchanged = Collections.unmodifiableMap(new HashMap<JsonPointer, JsonNode>(unchanged));
}

Expand All @@ -64,7 +64,7 @@ void valueRemoved(final JsonPointer pointer, final JsonNode value)

void valueAdded(final JsonPointer pointer, final JsonNode value)
{
if (diffDoesntRequireSource) {
if (options.isDiffDoesntRequireSource()) {
diffs.add(DiffOperation.add(pointer, value));
return;
}
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/com/github/fge/jsonpatch/diff/JsonDiff.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,16 @@ private JsonDiff()
public static JsonPatch asJsonPatch(final JsonNode source,
final JsonNode target)
{
return asJsonPatch(source, target, DIFF_DOESNT_REQUIRE_SOURCE);
return asJsonPatch(source, target, DiffOptions.DEFAULT_OPTIONS);
}
public static JsonPatch asJsonPatch(final JsonNode source,
final JsonNode target, final boolean diffDoesntRequireSource)
final JsonNode target, DiffOptions options)
{
BUNDLE.checkNotNull(source, "common.nullArgument");
BUNDLE.checkNotNull(target, "common.nullArgument");
final Map<JsonPointer, JsonNode> unchanged
= getUnchangedValues(source, target);
final DiffProcessor processor = new DiffProcessor(unchanged, diffDoesntRequireSource);
final DiffProcessor processor = new DiffProcessor(unchanged, options);

generateDiffs(processor, JsonPointer.empty(), source, target);
return processor.getPatch();
Expand All @@ -117,15 +117,15 @@ public static JsonPatch asJsonPatch(final JsonNode source,
*/
public static JsonNode asJson(final JsonNode source, final JsonNode target)
{
return asJson(source, target, DIFF_DOESNT_REQUIRE_SOURCE);
return asJson(source, target, DiffOptions.DEFAULT_OPTIONS);
}


public static JsonNode asJson(final JsonNode source, final JsonNode target, final boolean withMoveOrCopyOperation)
public static JsonNode asJson(final JsonNode source, final JsonNode target, DiffOptions options)

Choose a reason for hiding this comment

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

options param should be final

Copy link
Author

Choose a reason for hiding this comment

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

done

{
final String s;
try {
s = MAPPER.writeValueAsString(asJsonPatch(source, target, withMoveOrCopyOperation));
s = MAPPER.writeValueAsString(asJsonPatch(source, target, options));
return MAPPER.readTree(s);
} catch (IOException e) {
throw new RuntimeException("cannot generate JSON diff", e);
Expand Down
26 changes: 19 additions & 7 deletions src/test/java/com/github/fge/jsonpatch/diff/JsonDiffTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import java.util.List;

import static com.github.fge.jsonpatch.diff.DiffProcessor.DIFF_DOESNT_REQUIRE_SOURCE;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.*;

public final class JsonDiffTest
{
Expand All @@ -56,18 +56,18 @@ public Iterator<Object[]> getPatchesOnly()
for (final JsonNode node: testData)
list.add(new Object[] {
node.get("first"), node.get("second"),
node.has("diffDoesntRequireSource") ? node.get("diffDoesntRequireSource").booleanValue() : DIFF_DOESNT_REQUIRE_SOURCE
node.has("options") ? getLiteralOptions(node.get("options")) : DiffOptions.DEFAULT_OPTIONS
});

return list.iterator();
}

@Test(dataProvider = "getPatchesOnly")
public void generatedPatchAppliesCleanly(final JsonNode first,
final JsonNode second, final boolean withMoveOrCopy)
final JsonNode second, final DiffOptions options)
throws JsonPatchException
{
final JsonPatch patch = JsonDiff.asJsonPatch(first, second, withMoveOrCopy);
final JsonPatch patch = JsonDiff.asJsonPatch(first, second, options);
final JsonNode actual = patch.apply(first);

assertThat(EQUIVALENCE.equivalent(second, actual)).overridingErrorMessage(
Expand All @@ -87,21 +87,33 @@ public Iterator<Object[]> getLiteralPatches()
list.add(new Object[] {
node.get("message").textValue(), node.get("first"),
node.get("second"), node.get("patch"),
node.has("diffDoesntRequireSource") ? node.get("diffDoesntRequireSource").booleanValue() : DIFF_DOESNT_REQUIRE_SOURCE
node.has("options") ? getLiteralOptions(node.get("options")) : DiffOptions.DEFAULT_OPTIONS
});
}

return list.iterator();
}

public DiffOptions getLiteralOptions(JsonNode jsonNode) {
DiffOptions.Builder builder = new DiffOptions.Builder();
if (jsonNode.has("diffDoesntRequireSource")) {
if (jsonNode.get("diffDoesntRequireSource").booleanValue()){
builder.diffDoesntRequireSource();
} else {
builder.diffRequireSource();
}
}
return builder.build();
}

@Test(
dataProvider = "getLiteralPatches",
dependsOnMethods = "generatedPatchAppliesCleanly"
)
public void generatedPatchesAreWhatIsExpected(final String message,
final JsonNode first, final JsonNode second, final JsonNode expected, final boolean diffDoesntRequireSource)
final JsonNode first, final JsonNode second, final JsonNode expected, final DiffOptions options)
{
final JsonNode actual = JsonDiff.asJson(first, second, diffDoesntRequireSource);
final JsonNode actual = JsonDiff.asJson(first, second, options);

assertThat(EQUIVALENCE.equivalent(expected, actual)).overridingErrorMessage(
"patch is not what was expected\nscenario: %s\n"
Expand Down
24 changes: 20 additions & 4 deletions src/test/resources/jsonpatch/diff/diff.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
{ "op": "move", "path": "/c", "from": "/a" }
]
},{
"message": "similar element is copied instead of added",
"message": "when diffDoesntRequireSource similar element is added",
"first": {
"a": "c"
},
Expand All @@ -146,16 +146,32 @@
"patch": [
{ "op": "add", "path": "/d", "value": "c" }
],
"diffDoesntRequireSource": true
"options": {
"diffDoesntRequireSource": true
}
},
{
"message": "similar element removed then added is moved instead",
"message": "when diffDoesntRequireSource similar element is removed then added",
"first": { "a": "b" },
"second": { "c": "b" },
"patch": [
{ "op": "remove", "path": "/a" },
{ "op": "add", "path": "/c", "value": "b" }
],
"diffDoesntRequireSource": true
"options": {
"diffDoesntRequireSource": true
}

},
{
"message": "with default options, similar element removed then added is moved instead",
"first": { "a": "b" },
"second": { "c": "b" },
"patch": [
{ "op": "move", "path": "/c", "from": "/a" }
],
"options": {
}

}
]