From 3c875683a559e7adae462a6c7c3306c39ae62f65 Mon Sep 17 00:00:00 2001 From: Avionic Harshit <78672319+avionicharshit-byte@users.noreply.github.com> Date: Sat, 5 Sep 2026 13:22:53 +0530 Subject: [PATCH] Make diff() linear when an array shrinks (#5461) Signed-off-by: avionicharshit-byte --- include/nlohmann/json.hpp | 14 +++--- single_include/nlohmann/json.hpp | 14 +++--- tests/src/unit-json_patch.cpp | 81 ++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 16 deletions(-) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 969942183..be4ccb90f 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -5128,19 +5128,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // We now reached the end of at least one array // in a second pass, traverse the remaining elements - // remove my remaining elements - const auto end_index = static_cast(result.size()); - while (i < source.size()) + // remove my remaining elements, highest index first; appending + // in that order avoids the quadratic reinsertion done before + for (std::size_t j = source.size(); j > i; --j) { - // add operations in reverse order to avoid invalid - // indices - result.insert(result.begin() + end_index, object( + result.push_back(object( { {"op", "remove"}, - {"path", detail::concat(path, '/', detail::to_string(i))} + {"path", detail::concat(path, '/', detail::to_string(j - 1))} })); - ++i; } + i = source.size(); // add other remaining elements while (i < target.size()) diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index d5a808d0b..9c6ed1335 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -26547,19 +26547,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // We now reached the end of at least one array // in a second pass, traverse the remaining elements - // remove my remaining elements - const auto end_index = static_cast(result.size()); - while (i < source.size()) + // remove my remaining elements, highest index first; appending + // in that order avoids the quadratic reinsertion done before + for (std::size_t j = source.size(); j > i; --j) { - // add operations in reverse order to avoid invalid - // indices - result.insert(result.begin() + end_index, object( + result.push_back(object( { {"op", "remove"}, - {"path", detail::concat(path, '/', detail::to_string(i))} + {"path", detail::concat(path, '/', detail::to_string(j - 1))} })); - ++i; } + i = source.size(); // add other remaining elements while (i < target.size()) diff --git a/tests/src/unit-json_patch.cpp b/tests/src/unit-json_patch.cpp index 404dee43c..17d30f189 100644 --- a/tests/src/unit-json_patch.cpp +++ b/tests/src/unit-json_patch.cpp @@ -1388,3 +1388,84 @@ TEST_CASE("JSON patch - add to a primitive parent (regression #4292)") CHECK_THROWS_AS(doc.patch(patch), json::out_of_range&); } } + +TEST_CASE("JSON patch - diff emits array removals in descending index order") +{ + SECTION("array shrunk to empty") + { + json const source = {0, 1, 2, 3, 4}; + json const target = json::array(); + + json const patch = json::diff(source, target); + + json const expected = R"( + [ + {"op": "remove", "path": "/4"}, + {"op": "remove", "path": "/3"}, + {"op": "remove", "path": "/2"}, + {"op": "remove", "path": "/1"}, + {"op": "remove", "path": "/0"} + ] + )"_json; + + CHECK(patch == expected); + CHECK(source.patch(patch) == target); + } + + SECTION("array partially shrunk, after a replacement at a common index") + { + json const source = {0, 1, 2, 3, 4}; + json const target = {0, 9}; + + json const patch = json::diff(source, target); + + // the replacement comes first, then the removals, highest index first + json const expected = R"( + [ + {"op": "replace", "path": "/1", "value": 9}, + {"op": "remove", "path": "/4"}, + {"op": "remove", "path": "/3"}, + {"op": "remove", "path": "/2"} + ] + )"_json; + + CHECK(patch == expected); + CHECK(source.patch(patch) == target); + } + + SECTION("nested array shrunk") + { + json const source = {{"a", {0, 1, 2}}}; + json const target = {{"a", json::array()}}; + + json const patch = json::diff(source, target); + + json const expected = R"( + [ + {"op": "remove", "path": "/a/2"}, + {"op": "remove", "path": "/a/1"}, + {"op": "remove", "path": "/a/0"} + ] + )"_json; + + CHECK(patch == expected); + CHECK(source.patch(patch) == target); + } + + SECTION("many removals still round-trip") + { + json source = json::array(); + for (int i = 0; i < 1000; ++i) + { + source.push_back(i); + } + json const target = json::array(); + + json const patch = json::diff(source, target); + + CHECK(patch.size() == 1000); + CHECK(patch.front().at("path") == "/999"); + CHECK(patch.back().at("path") == "/0"); + CHECK(source.patch(patch) == target); + } +}