diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 3bc72805c..7f0f1d7c2 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -6156,12 +6156,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // the same time, determine whether every added key comes // after every common key in target's order (a precondition // for the fast path below, which only ever appends new keys - // at the very end): for an object_t whose iteration order is - // a pure function of the key set (e.g. the default std::map, - // which always iterates in sorted key order), the order - // check further below is always true and this whole - // mechanism is effectively a no-op; it only matters for a - // reorderable object_t such as the one backing `ordered_json`. + // at the very end). Both are only needed for an object_t that + // keeps its members in insertion order, such as the one + // backing `ordered_json`; for any other object_t, the fast + // path is always taken and they are not computed. // patch ops for keys that were added (i.e., in target but not // in source); built here so the fast path below can reuse // them without a second source.find() per target key. Only @@ -6185,15 +6183,32 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } else { - common_keys_target_order.push_back(it.key()); - if (seen_new_key) +#ifdef JSON_HEDLEY_MSVC_VERSION +#pragma warning(push ) +#pragma warning(disable : 4127) // ignore warning to replace if with if constexpr +#endif + if (detail::is_ordered_map::value) { - new_keys_form_suffix = false; + common_keys_target_order.push_back(it.key()); + if (seen_new_key) + { + new_keys_form_suffix = false; + } } +#ifdef JSON_HEDLEY_MSVC_VERSION +#pragma warning( pop ) +#endif } } - if (common_keys_source_order == common_keys_target_order && new_keys_form_suffix) + // Only an object type that keeps its members in insertion + // order, such as nlohmann::ordered_map, can need reordering: + // patch() appends a new member at the end of such an object. + // Any other object type places its members itself - std::map + // in key order, a hash map in an order its operator== ignores - + // so a member-by-member diff always reproduces target there. + if (!detail::is_ordered_map::value + || (common_keys_source_order == common_keys_target_order && new_keys_form_suffix)) { // fast path: order of common keys already matches (or the // object_t's iteration order does not depend on diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 8b442e3db..96a3e67ad 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -33040,12 +33040,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec // the same time, determine whether every added key comes // after every common key in target's order (a precondition // for the fast path below, which only ever appends new keys - // at the very end): for an object_t whose iteration order is - // a pure function of the key set (e.g. the default std::map, - // which always iterates in sorted key order), the order - // check further below is always true and this whole - // mechanism is effectively a no-op; it only matters for a - // reorderable object_t such as the one backing `ordered_json`. + // at the very end). Both are only needed for an object_t that + // keeps its members in insertion order, such as the one + // backing `ordered_json`; for any other object_t, the fast + // path is always taken and they are not computed. // patch ops for keys that were added (i.e., in target but not // in source); built here so the fast path below can reuse // them without a second source.find() per target key. Only @@ -33069,15 +33067,32 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec } else { - common_keys_target_order.push_back(it.key()); - if (seen_new_key) +#ifdef JSON_HEDLEY_MSVC_VERSION +#pragma warning(push ) +#pragma warning(disable : 4127) // ignore warning to replace if with if constexpr +#endif + if (detail::is_ordered_map::value) { - new_keys_form_suffix = false; + common_keys_target_order.push_back(it.key()); + if (seen_new_key) + { + new_keys_form_suffix = false; + } } +#ifdef JSON_HEDLEY_MSVC_VERSION +#pragma warning( pop ) +#endif } } - if (common_keys_source_order == common_keys_target_order && new_keys_form_suffix) + // Only an object type that keeps its members in insertion + // order, such as nlohmann::ordered_map, can need reordering: + // patch() appends a new member at the end of such an object. + // Any other object type places its members itself - std::map + // in key order, a hash map in an order its operator== ignores - + // so a member-by-member diff always reproduces target there. + if (!detail::is_ordered_map::value + || (common_keys_source_order == common_keys_target_order && new_keys_form_suffix)) { // fast path: order of common keys already matches (or the // object_t's iteration order does not depend on diff --git a/tests/src/unit-json_patch.cpp b/tests/src/unit-json_patch.cpp index 216d00c41..ef239e9f4 100644 --- a/tests/src/unit-json_patch.cpp +++ b/tests/src/unit-json_patch.cpp @@ -1752,6 +1752,58 @@ TEST_CASE("JSON patch - diff emits array removals in descending index order") } } +TEST_CASE("JSON patch - diff() takes the fast path for non-reorderable object types (regression #5639)") +{ + // #5465 added an order check to diff()'s object handling so a + // member-by-member diff is only used when it would also reproduce + // target's member *order* -- needed for ordered_json, whose object_t + // keeps insertion order and whose patch() "add" op appends a new + // member at the end. For json's default object_t (std::map, which + // orders members by key regardless of insertion history), that check + // could still fail: a new key that sorts before an existing common key + // makes target's iteration interleave the new key between common keys, + // even though nothing else about the object changed. That sent the + // whole object through the slow (remove-every-member, + // re-add-every-member) path instead of the minimal one. + SECTION("json: added key sorts before an existing common key") + { + const json source = {{"a", 1}, {"c", {{"x", 1}, {"y", 2}}}}; + const json target = {{"a", 1}, {"b", 0}, {"c", {{"x", 1}, {"y", 2}}}}; + + const json patch = json::diff(source, target); + + // only the new key is added; "a" and "c" are left alone instead of + // being removed and re-added + const json expected = R"([{"op": "add", "path": "/b", "value": 0}])"_json; + CHECK(patch == expected); + CHECK(source.patch(patch) == target); + } + + SECTION("ordered_json: reordering behavior from #5465 is unchanged") + { + using nlohmann::ordered_json; + + // same key/value shape as the json case above, but for ordered_json + // the *target*'s member order must be reproduced, so the slow path + // is still required here. + ordered_json source; + source["a"] = 1; + source["c"] = ordered_json{{"x", 1}, {"y", 2}}; + + ordered_json target; + target["a"] = 1; + target["b"] = 0; + target["c"] = ordered_json{{"x", 1}, {"y", 2}}; + + const ordered_json patch = ordered_json::diff(source, target); + + // unlike the json case: every member is still removed and re-added + // so the result ends up in target's order (2 removes + 3 adds) + CHECK(patch.size() == 5); + CHECK(source.patch(patch) == target); + } +} + TEST_CASE("JSON patch - every operation on ordered_json") { using nlohmann::ordered_json;