mirror of
https://github.com/nlohmann/json.git
synced 2026-09-23 08:20:30 +00:00
Make diff() account for member order in ordered_json objects (#5465)
* Make diff() account for member order in ordered_json objects diff() compared source/target objects purely by key set, ignoring relative member order. For ordered_json (insertion-ordered, vector- backed object_t), two objects that differ only in member order are unequal via operator==, but diff() never emitted any patch operation to fix the order, so source.patch(diff(source, target)) == target could fail to hold. Fix by detecting when common keys appear in a different relative order in source vs. target (or when a new key would need to land somewhere other than the end), and in that case removing and re-adding the affected keys in target's order, which relies on patch()'s "add" op appending new keys at the end of an ordered_map. For plain json (std::map-backed, always key-sorted iteration) this is a no-op and the original minimal per-key diff path is unchanged. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Avoid redundant lookups in diff()'s object-order tracking The previous fix for ordered_json member order re-derived common-key order and suffix information with extra target.find()/source.find() calls layered on top of the pre-existing removed/added-key passes, instead of reusing those same passes. This roughly tripled the number of map lookups per diff() call for every object, including plain `json`, where the reordering path is never taken. Piggyback the order tracking (and the "add" op construction for new keys) onto the two passes the algorithm already needs to detect removed/added keys, and walk the fast path's recursion in lockstep with the precomputed common-key list instead of re-querying `target`. This restores diff() to its pre-existing lookup count; benchmarked at n=1000 keys, ordered_json::diff() was roughly 2x slower than baseline before this change and is back within noise of baseline after it. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Preserve diff()'s original op ordering and fix a slow-path deletion gap Splitting removed-key detection and common-key recursion into separate passes (for the earlier lookup-count fix) changed the emitted patch's op order: all "remove" ops now came before all recursive per-key diffs, instead of interleaved in source's iteration order as the original implementation did. This broke docs/mkdocs/docs/examples/diff.output's exact-match CI check (ci_test_examples) even though the patch was still semantically correct. Defer "remove" emission into the same walk that does the recursive diffs, so common keys and deleted keys are interleaved in source order again, matching historical output. While restructuring that walk, the reordering ("slow path") branch was only emitting "remove" for keys common to both objects, never for keys present in source but genuinely absent from target -- a key deleted alongside an actual reorder would silently survive the patch. Fixed by removing every source key in the slow path (both deleted and common keys need removing there; common keys are then re-added in target's order). Verified with a targeted reorder+deletion case and a fresh 20,000-case round-trip fuzz run (0 failures). Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
@@ -81,3 +81,84 @@ TEST_CASE("regression test for issue #3732 - iteration_proxy_value<iter_impl<ord
|
||||
};
|
||||
static_cast<void>(fn);
|
||||
}
|
||||
|
||||
TEST_CASE("regression test - diff() must account for ordered_json member order")
|
||||
{
|
||||
SECTION("pure reorder, no value changes")
|
||||
{
|
||||
ordered_json a = {{"a", 1}, {"b", 2}};
|
||||
ordered_json b = {{"b", 2}, {"a", 1}};
|
||||
CHECK(a != b); // order-sensitive equality
|
||||
CHECK(a.patch(ordered_json::diff(a, b)) == b);
|
||||
}
|
||||
|
||||
SECTION("new key must land at the front")
|
||||
{
|
||||
ordered_json c = {{"b", 2}};
|
||||
ordered_json e = {{"a", 1}, {"b", 2}};
|
||||
CHECK(c.patch(ordered_json::diff(c, e)) == e);
|
||||
}
|
||||
|
||||
SECTION("reorder plus a value change on one of the reordered keys")
|
||||
{
|
||||
ordered_json a = {{"a", 1}, {"b", 2}};
|
||||
ordered_json b = {{"b", 20}, {"a", 1}};
|
||||
CHECK(a != b);
|
||||
CHECK(a.patch(ordered_json::diff(a, b)) == b);
|
||||
}
|
||||
|
||||
SECTION("reorder plus a deleted key")
|
||||
{
|
||||
ordered_json a = {{"a", 1}, {"b", 2}, {"c", 3}};
|
||||
ordered_json b = {{"b", 2}, {"a", 1}};
|
||||
CHECK(a != b);
|
||||
CHECK(a.patch(ordered_json::diff(a, b)) == b);
|
||||
}
|
||||
|
||||
SECTION("reorder plus a nested value that itself needs a recursive diff")
|
||||
{
|
||||
ordered_json a = {{"a", {{"x", 1}, {"y", 2}}}, {"b", 2}};
|
||||
ordered_json b = {{"b", 2}, {"a", {{"x", 1}, {"y", 99}}}};
|
||||
CHECK(a != b);
|
||||
CHECK(a.patch(ordered_json::diff(a, b)) == b);
|
||||
}
|
||||
|
||||
SECTION("three or more keys shuffled into a different order")
|
||||
{
|
||||
ordered_json a = {{"a", 1}, {"b", 2}, {"c", 3}, {"d", 4}};
|
||||
ordered_json b = {{"d", 4}, {"b", 2}, {"a", 1}, {"c", 3}};
|
||||
CHECK(a != b);
|
||||
CHECK(a.patch(ordered_json::diff(a, b)) == b);
|
||||
}
|
||||
|
||||
SECTION("matching order still produces a minimal patch (fast path unaffected)")
|
||||
{
|
||||
ordered_json a = {{"a", 1}, {"b", 2}, {"c", 3}};
|
||||
ordered_json b = {{"a", 1}, {"b", 20}, {"c", 3}};
|
||||
auto p = ordered_json::diff(a, b);
|
||||
// only the changed value should be touched, not a wholesale remove+add
|
||||
CHECK(p.size() == 1);
|
||||
CHECK(p[0]["op"] == "replace");
|
||||
CHECK(p[0]["path"] == "/b");
|
||||
CHECK(a.patch(p) == b);
|
||||
}
|
||||
|
||||
SECTION("plain json (std::map-backed) is unaffected by same-key-different-insertion-order")
|
||||
{
|
||||
json a;
|
||||
a["b"] = 2;
|
||||
a["a"] = 1;
|
||||
|
||||
json b;
|
||||
b["a"] = 1;
|
||||
b["b"] = 2;
|
||||
|
||||
// std::map iteration is always sorted by key, so a == b regardless of
|
||||
// insertion order, and diff() must still produce the same minimal
|
||||
// (empty) result as before this fix
|
||||
CHECK(a == b);
|
||||
auto p = json::diff(a, b);
|
||||
CHECK(p.empty());
|
||||
CHECK(a.patch(p) == b);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user