-
Notifications
You must be signed in to change notification settings - Fork 158
Add smart equals for arrays #214
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce support for custom equality functions in JSON array diffing, allowing users to define how elements are considered equal (e.g., by ID). This enables more concise and semantically meaningful patches, such as using move and replace operations instead of remove and add. Documentation and comprehensive tests are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant JsonDiff
participant JsonNodeEqualsFunction
participant PatchResult
User->>JsonDiff: asJson(source, target, flags, equalsFunction)
JsonDiff->>JsonNodeEqualsFunction: equals(nodeA, nodeB) (for array elements)
JsonNodeEqualsFunction-->>JsonDiff: true/false
JsonDiff->>JsonDiff: Generate patch (move/replace/add) using custom equality
JsonDiff-->>User: PatchResult (JSON Patch)
Possibly related issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
README.md (2)
58-123
: Well-documented feature with clear examples!The documentation effectively explains the smart equals functionality with before/after comparisons. Consider showing a lambda syntax alternative in addition to the anonymous inner class example for brevity.
104-104
: Remove trailing whitespace.- /// With a "smart equals": + /// With a "smart equals":src/test/java/com/flipkart/zjsonpatch/JsonSmartArrayDiffTest.java (1)
54-66
: Extract equality function and improve null handling.The equality function should be extracted as a constant to avoid recreation in each iteration. Also, consider more robust null handling.
+ private static final JsonNodeEqualsFunction ID_BASED_EQUALS = new JsonNodeEqualsFunction() { + @Override + public boolean equals(JsonNode jsonNode1, JsonNode jsonNode2) { + if (jsonNode1 == jsonNode2) { + return true; + } + if (jsonNode1 == null || jsonNode2 == null) { + return false; + } + if (jsonNode1.has("id") && jsonNode2.has("id")) { + return jsonNode1.get("id").asInt() == jsonNode2.get("id").asInt(); + } + return jsonNode1.equals(jsonNode2); + } + }; + @Test public void testSampleJsonDiff() { for (int i = START_INDEX; i < jsonNode.size(); i++) { JsonNode first = jsonNode.get(i).get("first"); JsonNode second = jsonNode.get(i).get("second"); JsonNode patch = jsonNode.get(i).get("patch"); - JsonNodeEqualsFunction jsonNodeEqualFunction = new JsonNodeEqualsFunction() { - @Override - public boolean equals(JsonNode jsonNode1, JsonNode jsonNode2) { - if (jsonNode1 == null || jsonNode2 == null) { - return false; - } - if (jsonNode1.has("id") && jsonNode2.has("id")) { - return jsonNode1.get("id").asInt() == jsonNode2.get("id").asInt(); - } - return jsonNode1.equals(jsonNode2); - } - - }; - JsonNode actualPatch = JsonDiff.asJson(first, second, DiffFlags.defaults(), jsonNodeEqualFunction); + JsonNode actualPatch = JsonDiff.asJson(first, second, DiffFlags.defaults(), ID_BASED_EQUALS);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md
(1 hunks)src/main/java/com/flipkart/zjsonpatch/JsonDiff.java
(6 hunks)src/main/java/com/flipkart/zjsonpatch/JsonNodeEqualsFunction.java
(1 hunks)src/test/java/com/flipkart/zjsonpatch/JsonSmartArrayDiffTest.java
(1 hunks)src/test/resources/testdata/smart-array-move.json
(1 hunks)
🔇 Additional comments (2)
src/test/resources/testdata/smart-array-move.json (1)
1-84
: Comprehensive test data coverage!The test data effectively covers various scenarios including simple moves, cross moves with modifications, moves with additions, and object-based operations. Well-structured and thorough.
src/main/java/com/flipkart/zjsonpatch/JsonDiff.java (1)
267-267
: Avoid reference equality comparison for function check.Using
!=
to compare withDEFAULT_EQUALS_REF_FUNCTION
is fragile and could break if the default is recreated. Consider using a more robust approach.- if (equalsRefFunction != DEFAULT_EQUALS_REF_FUNCTION) { // if a custom equals function is used, a further object comparison is wanted to reflect changes + // Check if custom equality is used by comparing behavior, not reference + boolean isCustomEquals = !equalsRefFunction.equals(removedNode.getValue(), addedNode.getValue()) || + !removedNode.getValue().equals(addedNode.getValue()); + if (isCustomEquals) { // if a custom equals function is used, a further object comparison is wanted to reflect changesLikely an incorrect or invalid review comment.
Implements #213
Summary by CodeRabbit
New Features
Documentation
Tests