From fb54e212b666ecc37272740278384a8bc9fc2f5f Mon Sep 17 00:00:00 2001 From: Niels Date: Sun, 24 Apr 2016 19:03:33 +0200 Subject: [PATCH] clean up and added tests --- src/json.hpp | 120 ++++++++++++++--------------- src/json.hpp.re2c | 120 ++++++++++++++--------------- test/unit.cpp | 192 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 310 insertions(+), 122 deletions(-) diff --git a/src/json.hpp b/src/json.hpp index 8d5291ce..b7a6f64a 100644 --- a/src/json.hpp +++ b/src/json.hpp @@ -9518,42 +9518,44 @@ basic_json_parser_63: */ basic_json apply_patch(const basic_json& patch) const { + // make a working copy to apply the patch to basic_json result = *this; - if (not patch.is_array()) - { - // a JSON patch must be an array of objects - throw std::domain_error("JSON patch must be an array of objects"); - } - - const auto operation_add = [&result](json_pointer & ptr, - basic_json & value) + // wrapper for "add" operation; add value at ptr + const auto operation_add = [&result](json_pointer & ptr, basic_json value) { + // get reference to parent of JSON pointer ptr const auto last_path = ptr.pop_back(); basic_json& parent = result.at(ptr); if (parent.is_object()) { + // use operator[] to add value parent[last_path] = value; } else if (parent.is_array()) { if (last_path == "-") { + // special case: append to back parent.push_back(value); } else { - parent.insert(parent.begin() + std::stoi(last_path), - value); + // default case: insert add offset + parent.insert(parent.begin() + std::stoi(last_path), value); } } }; + // wrapper for "remove" operation; remove value at ptr const auto operation_remove = [&result](json_pointer & ptr) { + // get reference to parent of JSON pointer ptr const auto last_path = ptr.pop_back(); basic_json& parent = result.at(ptr); + + // remove child if (parent.is_object()) { parent.erase(parent.find(last_path)); @@ -9564,41 +9566,57 @@ basic_json_parser_63: } }; + // type check + if (not patch.is_array()) + { + // a JSON patch must be an array of objects + throw std::domain_error("JSON patch must be an array of objects"); + } + + // iterate and apply th eoperations for (const auto& val : patch) { + // wrapper to get a value for an operation + const auto get_value = [&val](const std::string & op, + const std::string & member, + bool string_type = false) -> basic_json& + { + // find value + auto it = val.m_value.object->find(member); + + // context-sensitive error message + const auto error_msg = (op == "op") ? "operation" : "operation '" + op + "'"; + + // check if desired value is present + if (it == val.m_value.object->end()) + { + throw std::domain_error(error_msg + " must have member '" + member + "'"); + } + + // check if result is of type string + if (string_type and not it->second.is_string()) + { + throw std::domain_error(error_msg + " must have string member '" + member + "'"); + } + + // no error: return value + return it->second; + }; + + // type check if (not val.is_object()) { throw std::domain_error("JSON patch must be an array of objects"); } - // collect members - const auto it_op = val.m_value.object->find("op"); - const auto it_path = val.m_value.object->find("path"); - const auto it_value = val.m_value.object->find("value"); - const auto it_from = val.m_value.object->find("from"); - - if (it_op == val.m_value.object->end() or not it_op->second.is_string()) - { - throw std::domain_error("operation must have a string 'op' member"); - } - - if (it_path == val.m_value.object->end() or not it_op->second.is_string()) - { - throw std::domain_error("operation must have a string 'path' member"); - } - - const std::string op = it_op->second; - const std::string path = it_path->second; - json_pointer ptr(path); + // collect mandatory members + const std::string op = get_value("op", "op", true); + const std::string path = get_value(op, "path", true); + json_pointer ptr(get_value(op, "path", true)); if (op == "add") { - if (it_value == val.m_value.object->end()) - { - throw std::domain_error("'add' operation must have member 'value'"); - } - - operation_add(ptr, it_value->second); + operation_add(ptr, get_value("add", "value")); } else if (op == "remove") { @@ -9606,21 +9624,11 @@ basic_json_parser_63: } else if (op == "replace") { - if (it_value == val.m_value.object->end()) - { - throw std::domain_error("'replace' operation must have member 'value'"); - } - - result.at(ptr) = it_value->second; + result.at(ptr) = get_value("replace", "value"); } else if (op == "move") { - if (it_from == val.m_value.object->end()) - { - throw std::domain_error("'move' operation must have member 'from'"); - } - - const std::string from_path = it_from->second; + const std::string from_path = get_value("move", "from", true); json_pointer from_ptr(from_path); basic_json v = result[from_ptr]; @@ -9629,32 +9637,22 @@ basic_json_parser_63: } else if (op == "copy") { - if (it_from == val.m_value.object->end()) - { - throw std::domain_error("'copy' operation must have member 'from'"); - } - - const std::string from_path = it_from->second; + const std::string from_path = get_value("copy", "from", true);; const json_pointer from_ptr(from_path); result[ptr] = result.at(from_ptr); } else if (op == "test") { - if (it_value == val.m_value.object->end()) - { - throw std::domain_error("'test' operation must have member 'value'"); - } - - if (result.at(ptr) != it_value->second) + if (result.at(ptr) != get_value("test", "value")) { throw std::domain_error("unsuccessful: " + val.dump()); } } else { - // op must be "add", "remove", "replace", "move", - // "copy", or "test" + // op must be "add", "remove", "replace", "move", "copy", or + // "test" throw std::domain_error("operation value '" + op + "' is invalid"); } } diff --git a/src/json.hpp.re2c b/src/json.hpp.re2c index 37feeec6..c4c87f5d 100644 --- a/src/json.hpp.re2c +++ b/src/json.hpp.re2c @@ -8828,42 +8828,44 @@ class basic_json */ basic_json apply_patch(const basic_json& patch) const { + // make a working copy to apply the patch to basic_json result = *this; - if (not patch.is_array()) - { - // a JSON patch must be an array of objects - throw std::domain_error("JSON patch must be an array of objects"); - } - - const auto operation_add = [&result](json_pointer & ptr, - basic_json & value) + // wrapper for "add" operation; add value at ptr + const auto operation_add = [&result](json_pointer & ptr, basic_json value) { + // get reference to parent of JSON pointer ptr const auto last_path = ptr.pop_back(); basic_json& parent = result.at(ptr); if (parent.is_object()) { + // use operator[] to add value parent[last_path] = value; } else if (parent.is_array()) { if (last_path == "-") { + // special case: append to back parent.push_back(value); } else { - parent.insert(parent.begin() + std::stoi(last_path), - value); + // default case: insert add offset + parent.insert(parent.begin() + std::stoi(last_path), value); } } }; + // wrapper for "remove" operation; remove value at ptr const auto operation_remove = [&result](json_pointer & ptr) { + // get reference to parent of JSON pointer ptr const auto last_path = ptr.pop_back(); basic_json& parent = result.at(ptr); + + // remove child if (parent.is_object()) { parent.erase(parent.find(last_path)); @@ -8874,41 +8876,57 @@ class basic_json } }; + // type check + if (not patch.is_array()) + { + // a JSON patch must be an array of objects + throw std::domain_error("JSON patch must be an array of objects"); + } + + // iterate and apply th eoperations for (const auto& val : patch) { + // wrapper to get a value for an operation + const auto get_value = [&val](const std::string & op, + const std::string & member, + bool string_type = false) -> basic_json& + { + // find value + auto it = val.m_value.object->find(member); + + // context-sensitive error message + const auto error_msg = (op == "op") ? "operation" : "operation '" + op + "'"; + + // check if desired value is present + if (it == val.m_value.object->end()) + { + throw std::domain_error(error_msg + " must have member '" + member + "'"); + } + + // check if result is of type string + if (string_type and not it->second.is_string()) + { + throw std::domain_error(error_msg + " must have string member '" + member + "'"); + } + + // no error: return value + return it->second; + }; + + // type check if (not val.is_object()) { throw std::domain_error("JSON patch must be an array of objects"); } - // collect members - const auto it_op = val.m_value.object->find("op"); - const auto it_path = val.m_value.object->find("path"); - const auto it_value = val.m_value.object->find("value"); - const auto it_from = val.m_value.object->find("from"); - - if (it_op == val.m_value.object->end() or not it_op->second.is_string()) - { - throw std::domain_error("operation must have a string 'op' member"); - } - - if (it_path == val.m_value.object->end() or not it_op->second.is_string()) - { - throw std::domain_error("operation must have a string 'path' member"); - } - - const std::string op = it_op->second; - const std::string path = it_path->second; - json_pointer ptr(path); + // collect mandatory members + const std::string op = get_value("op", "op", true); + const std::string path = get_value(op, "path", true); + json_pointer ptr(get_value(op, "path", true)); if (op == "add") { - if (it_value == val.m_value.object->end()) - { - throw std::domain_error("'add' operation must have member 'value'"); - } - - operation_add(ptr, it_value->second); + operation_add(ptr, get_value("add", "value")); } else if (op == "remove") { @@ -8916,21 +8934,11 @@ class basic_json } else if (op == "replace") { - if (it_value == val.m_value.object->end()) - { - throw std::domain_error("'replace' operation must have member 'value'"); - } - - result.at(ptr) = it_value->second; + result.at(ptr) = get_value("replace", "value"); } else if (op == "move") { - if (it_from == val.m_value.object->end()) - { - throw std::domain_error("'move' operation must have member 'from'"); - } - - const std::string from_path = it_from->second; + const std::string from_path = get_value("move", "from", true); json_pointer from_ptr(from_path); basic_json v = result[from_ptr]; @@ -8939,32 +8947,22 @@ class basic_json } else if (op == "copy") { - if (it_from == val.m_value.object->end()) - { - throw std::domain_error("'copy' operation must have member 'from'"); - } - - const std::string from_path = it_from->second; + const std::string from_path = get_value("copy", "from", true);; const json_pointer from_ptr(from_path); result[ptr] = result.at(from_ptr); } else if (op == "test") { - if (it_value == val.m_value.object->end()) - { - throw std::domain_error("'test' operation must have member 'value'"); - } - - if (result.at(ptr) != it_value->second) + if (result.at(ptr) != get_value("test", "value")) { throw std::domain_error("unsuccessful: " + val.dump()); } } else { - // op must be "add", "remove", "replace", "move", - // "copy", or "test" + // op must be "add", "remove", "replace", "move", "copy", or + // "test" throw std::domain_error("operation value '" + op + "' is invalid"); } } diff --git a/test/unit.cpp b/test/unit.cpp index 907e68bf..664648cb 100644 --- a/test/unit.cpp +++ b/test/unit.cpp @@ -12822,6 +12822,198 @@ TEST_CASE("JSON patch") CHECK(doc.apply_patch(patch) == expected); } } + + SECTION("errors") + { + SECTION("unknown operation") + { + SECTION("missing 'op'") + { + json j; + json patch = {{{"foo", "bar"}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation must have member 'op'"); + } + + SECTION("non-string 'op'") + { + json j; + json patch = {{{"op", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation must have string member 'op'"); + } + } + + SECTION("add") + { + SECTION("missing 'path'") + { + json j; + json patch = {{{"op", "add"}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'add' must have member 'path'"); + } + + SECTION("non-string 'path'") + { + json j; + json patch = {{{"op", "add"}, {"path", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'add' must have string member 'path'"); + } + + SECTION("missing 'value'") + { + json j; + json patch = {{{"op", "add"}, {"path", ""}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'add' must have member 'value'"); + } + } + + SECTION("remove") + { + SECTION("missing 'path'") + { + json j; + json patch = {{{"op", "remove"}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'remove' must have member 'path'"); + } + + SECTION("non-string 'path'") + { + json j; + json patch = {{{"op", "remove"}, {"path", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'remove' must have string member 'path'"); + } + } + + SECTION("replace") + { + SECTION("missing 'path'") + { + json j; + json patch = {{{"op", "replace"}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'replace' must have member 'path'"); + } + + SECTION("non-string 'path'") + { + json j; + json patch = {{{"op", "replace"}, {"path", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'replace' must have string member 'path'"); + } + + SECTION("missing 'value'") + { + json j; + json patch = {{{"op", "replace"}, {"path", ""}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'replace' must have member 'value'"); + } + } + + SECTION("move") + { + SECTION("missing 'path'") + { + json j; + json patch = {{{"op", "move"}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'move' must have member 'path'"); + } + + SECTION("non-string 'path'") + { + json j; + json patch = {{{"op", "move"}, {"path", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'move' must have string member 'path'"); + } + + SECTION("missing 'from'") + { + json j; + json patch = {{{"op", "move"}, {"path", ""}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'move' must have member 'from'"); + } + + SECTION("non-string 'from'") + { + json j; + json patch = {{{"op", "move"}, {"path", ""}, {"from", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'move' must have string member 'from'"); + } + } + + SECTION("copy") + { + SECTION("missing 'path'") + { + json j; + json patch = {{{"op", "copy"}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'copy' must have member 'path'"); + } + + SECTION("non-string 'path'") + { + json j; + json patch = {{{"op", "copy"}, {"path", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'copy' must have string member 'path'"); + } + + SECTION("missing 'from'") + { + json j; + json patch = {{{"op", "copy"}, {"path", ""}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'copy' must have member 'from'"); + } + + SECTION("non-string 'from'") + { + json j; + json patch = {{{"op", "copy"}, {"path", ""}, {"from", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'copy' must have string member 'from'"); + } + } + + SECTION("test") + { + SECTION("missing 'path'") + { + json j; + json patch = {{{"op", "test"}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'test' must have member 'path'"); + } + + SECTION("non-string 'path'") + { + json j; + json patch = {{{"op", "test"}, {"path", 1}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'test' must have string member 'path'"); + } + + SECTION("missing 'value'") + { + json j; + json patch = {{{"op", "test"}, {"path", ""}}}; + CHECK_THROWS_AS(j.apply_patch(patch), std::domain_error); + CHECK_THROWS_WITH(j.apply_patch(patch), "operation 'test' must have member 'value'"); + } + } + } } TEST_CASE("regression tests")