Skip to content

Commit

Permalink
Fixed basic_json::diff and basic_json::patch not compiling with custo…
Browse files Browse the repository at this point in the history 
…m string type (nlohmann#4134)
  • Loading branch information
@tomalakgeretkal
tomalakgeretkal committed Sep 25, 2023
1 parent fac07e2 commit 77ec6e4
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 29 deletions.
39 changes: 26 additions & 13 deletions include/nlohmann/json.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4702,7 +4702,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
// the valid JSON Patch operations
enum class patch_operations {add, remove, replace, move, copy, test, invalid};

const auto get_op = [](const std::string & op)
const auto get_op = [](const StringType & op)
{
if (op == "add")
{
Expand Down Expand Up @@ -4839,8 +4839,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
for (const auto& val : json_patch)
{
// wrapper to get a value for an operation
const auto get_value = [&val](const std::string & op,
const std::string & member,
const auto get_value = [&val](const StringType & op,
const StringType & member,
bool string_type) -> basic_json &
{
// find value
Expand Down Expand Up @@ -4874,8 +4874,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
}

// collect mandatory members
const auto op = get_value("op", "op", true).template get<std::string>();
const auto path = get_value(op, "path", true).template get<std::string>();
const auto op = get_value("op", "op", true).template get<StringType>();
const auto path = get_value(op, "path", true).template get<StringType>();
json_pointer ptr(path);

switch (get_op(op))
Expand All @@ -4901,7 +4901,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec

case patch_operations::move:
{
const auto from_path = get_value("move", "from", true).template get<std::string>();
const auto from_path = get_value("move", "from", true).template get<StringType>();
json_pointer from_ptr(from_path);

// the "from" location must exist - use at()
Expand All @@ -4918,7 +4918,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec

case patch_operations::copy:
{
const auto from_path = get_value("copy", "from", true).template get<std::string>();
const auto from_path = get_value("copy", "from", true).template get<StringType>();
const json_pointer from_ptr(from_path);

// the "from" location must exist - use at()
Expand Down Expand Up @@ -4978,7 +4978,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
/// @sa https://json.nlohmann.me/api/basic_json/diff/
JSON_HEDLEY_WARN_UNUSED_RESULT
static basic_json diff(const basic_json& source, const basic_json& target,
const std::string& path = "")
const StringType& path = "")
{
// the patch
basic_json result(value_t::array);
Expand Down Expand Up @@ -5007,8 +5007,14 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
std::size_t i = 0;
while (i < source.size() && i < target.size())
{
StringType array_index_str;
{
using namespace detail;
int_to_string(array_index_str, i);
}

// recursive call to compare array values at index i
auto temp_diff = diff(source[i], target[i], detail::concat(path, '/', std::to_string(i)));
auto temp_diff = diff(source[i], target[i], detail::concat<StringType>(path, '/', array_index_str));
result.insert(result.end(), temp_diff.begin(), temp_diff.end());
++i;
}
Expand All @@ -5022,10 +5028,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{
// add operations in reverse order to avoid invalid
// indices

StringType array_index_str;
{
using namespace detail;
int_to_string(array_index_str, i);
}

result.insert(result.begin() + end_index, object(
{
{"op", "remove"},
{"path", detail::concat(path, '/', std::to_string(i))}
{"path", detail::concat<StringType>(path, '/', array_index_str)}
}));
++i;
}
Expand All @@ -5036,7 +5049,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
result.push_back(
{
{"op", "add"},
{"path", detail::concat(path, "/-")},
{"path", detail::concat<StringType>(path, "/-")},
{"value", target[i]}
});
++i;
Expand All @@ -5051,7 +5064,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
for (auto it = source.cbegin(); it != source.cend(); ++it)
{
// escape the key name to be used in a JSON patch
const auto path_key = detail::concat(path, '/', detail::escape(it.key()));
const auto path_key = detail::concat<StringType>(path, '/', detail::escape(it.key()));

if (target.find(it.key()) != target.end())
{
Expand All @@ -5075,7 +5088,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
if (source.find(it.key()) == source.end())
{
// found a key that is not in this -> add it
const auto path_key = detail::concat(path, '/', detail::escape(it.key()));
const auto path_key = detail::concat<StringType>(path, '/', detail::escape(it.key()));
result.push_back(
{
{"op", "add"}, {"path", path_key},
Expand Down
39 changes: 26 additions & 13 deletions single_include/nlohmann/json.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23916,7 +23916,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
// the valid JSON Patch operations
enum class patch_operations {add, remove, replace, move, copy, test, invalid};

const auto get_op = [](const std::string & op)
const auto get_op = [](const StringType & op)
{
if (op == "add")
{
Expand Down Expand Up @@ -24053,8 +24053,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
for (const auto& val : json_patch)
{
// wrapper to get a value for an operation
const auto get_value = [&val](const std::string & op,
const std::string & member,
const auto get_value = [&val](const StringType & op,
const StringType & member,
bool string_type) -> basic_json &
{
// find value
Expand Down Expand Up @@ -24088,8 +24088,8 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
}

// collect mandatory members
const auto op = get_value("op", "op", true).template get<std::string>();
const auto path = get_value(op, "path", true).template get<std::string>();
const auto op = get_value("op", "op", true).template get<StringType>();
const auto path = get_value(op, "path", true).template get<StringType>();
json_pointer ptr(path);

switch (get_op(op))
Expand All @@ -24115,7 +24115,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec

case patch_operations::move:
{
const auto from_path = get_value("move", "from", true).template get<std::string>();
const auto from_path = get_value("move", "from", true).template get<StringType>();
json_pointer from_ptr(from_path);

// the "from" location must exist - use at()
Expand All @@ -24132,7 +24132,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec

case patch_operations::copy:
{
const auto from_path = get_value("copy", "from", true).template get<std::string>();
const auto from_path = get_value("copy", "from", true).template get<StringType>();
const json_pointer from_ptr(from_path);

// the "from" location must exist - use at()
Expand Down Expand Up @@ -24192,7 +24192,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
/// @sa https://json.nlohmann.me/api/basic_json/diff/
JSON_HEDLEY_WARN_UNUSED_RESULT
static basic_json diff(const basic_json& source, const basic_json& target,
const std::string& path = "")
const StringType& path = "")
{
// the patch
basic_json result(value_t::array);
Expand Down Expand Up @@ -24221,8 +24221,14 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
std::size_t i = 0;
while (i < source.size() && i < target.size())
{
StringType array_index_str;
{
using namespace detail;
int_to_string(array_index_str, i);
}

// recursive call to compare array values at index i
auto temp_diff = diff(source[i], target[i], detail::concat(path, '/', std::to_string(i)));
auto temp_diff = diff(source[i], target[i], detail::concat<StringType>(path, '/', array_index_str));
result.insert(result.end(), temp_diff.begin(), temp_diff.end());
++i;
}
Expand All @@ -24236,10 +24242,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
{
// add operations in reverse order to avoid invalid
// indices

StringType array_index_str;
{
using namespace detail;
int_to_string(array_index_str, i);
}

result.insert(result.begin() + end_index, object(
{
{"op", "remove"},
{"path", detail::concat(path, '/', std::to_string(i))}
{"path", detail::concat<StringType>(path, '/', array_index_str)}
}));
++i;
}
Expand All @@ -24250,7 +24263,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
result.push_back(
{
{"op", "add"},
{"path", detail::concat(path, "/-")},
{"path", detail::concat<StringType>(path, "/-")},
{"value", target[i]}
});
++i;
Expand All @@ -24265,7 +24278,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
for (auto it = source.cbegin(); it != source.cend(); ++it)
{
// escape the key name to be used in a JSON patch
const auto path_key = detail::concat(path, '/', detail::escape(it.key()));
const auto path_key = detail::concat<StringType>(path, '/', detail::escape(it.key()));

if (target.find(it.key()) != target.end())
{
Expand All @@ -24289,7 +24302,7 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
if (source.find(it.key()) == source.end())
{
// found a key that is not in this -> add it
const auto path_key = detail::concat(path, '/', detail::escape(it.key()));
const auto path_key = detail::concat<StringType>(path, '/', detail::escape(it.key()));
result.push_back(
{
{"op", "add"}, {"path", path_key},
Expand Down
44 changes: 41 additions & 3 deletions tests/src/unit-alt-string.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,33 @@ class alt_string
alt_string(size_t count, char chr): str_impl(count, chr) {}
alt_string() = default;

template <typename...TParams>
alt_string& append(TParams&& ...params)
alt_string& append(std::size_t count, char ch)
{
str_impl.append(std::forward<TParams>(params)...);
str_impl.append(count, ch);
return *this;
}

alt_string& append(const alt_string& str)
{
str_impl.append(str.str_impl);
return *this;
}

alt_string& append(const char* s, std::size_t count)
{
str_impl.append(s, count);
return *this;
}

alt_string& append(const char* s)
{
str_impl.append(s);
return *this;
}

alt_string& operator+=(char ch)
{
str_impl += ch;
return *this;
}

Expand Down Expand Up @@ -74,6 +97,11 @@ class alt_string
return str_impl.size();
}

void reserve (std::size_t n)
{
str_impl.reserve(n);
}

void resize (std::size_t n)
{
str_impl.resize(n);
Expand Down Expand Up @@ -319,4 +347,14 @@ TEST_CASE("alternative string type")
CHECK(j.at(alt_json::json_pointer("/foo/0")) == j["foo"][0]);
CHECK(j.at(alt_json::json_pointer("/foo/1")) == j["foo"][1]);
}

SECTION("JSON patch")
{
// for now, just ensures successful compilation (see #4134)
alt_json base, target;
alt_json patch = alt_json::array();

base.patch(patch);
CHECK_NOTHROW(alt_json::diff(target, base));
}
}

0 comments on commit 77ec6e4

Please sign in to comment.