Fix tagged JSON parsing

include/simfil/model/json.h

@@ -9,13 +9,25 @@
 #include "model.h"
 #include "simfil/error.h"
 
+#include <nlohmann/json_fwd.hpp>
 #include <tl/expected.hpp>
 
 namespace simfil::json
 {
 
+/**
+ * Build a ModelNode subtree from JSON using simfil's tagged JSON conventions
+ * such as `_bytes` and `_multimap`.
+ */
+auto buildModelNode(const nlohmann::json& input, ModelPool& model) -> tl::expected<ModelNode::Ptr, Error>;
+
+/** Parse JSON from a stream and append the resulting root node to `model`. */
 auto parse(std::istream& input, ModelPoolPtr const& model) -> tl::expected<void, Error>;
+
+/** Parse a JSON string and append the resulting root node to `model`. */
 auto parse(const std::string& input, ModelPoolPtr const& model) -> tl::expected<void, Error>;
+
+/** Parse a JSON string into a freshly created ModelPool containing one root. */
 auto parse(const std::string& input) -> tl::expected<ModelPoolPtr, Error>;
 
 }

src/model/json.cpp

@@ -3,6 +3,10 @@
 #include "simfil/model/json.h"
 #include "simfil/model/model.h"
 
+#include <cstdint>
+#include <limits>
+#include <string>
+
 #include <nlohmann/json.hpp>
 
 #include "../expected.h"
@@ -11,85 +15,167 @@
 {
 using json = nlohmann::json;
 
-static auto build(const json& j, ModelPool & model) -> tl::expected<ModelNode::Ptr, Error>
+namespace
+{
+/** Resolve the canonical simfil null node instance. */
+auto nullNode(ModelPool& model) -> ModelNode::Ptr
+{
+    return model.resolve<ModelNode>(
+        ModelNodeAddress{Model::Null, 1},
+        ScalarValueType{});
+}
+
+/** Decode one object field through the shared recursive JSON builder. */
+auto buildObjectField(
+    model_ptr<Object>& object,
+    std::string const& key,
+    const json& value,
+    ModelPool& model) -> tl::expected<void, Error>
+{
+    auto child = buildModelNode(value, model);
+    TRY_EXPECTED(child);
+    auto result = object->addField(key, *child);
+    TRY_EXPECTED(result);
+    return {};
+}
+
+/** Expand `_multimap`-tagged objects into repeated simfil object fields. */
+auto buildMultimapObject(
+    const json& input,
+    model_ptr<Object>& object,
+    ModelPool& model) -> tl::expected<void, Error>
 {
-    switch (j.type()) {
+    for (auto&& [key, value] : input.items()) {
+        if (key == "_multimap") {
+            continue;
+        }
+
+        if (!value.is_array()) {
+            return tl::unexpected<Error>(
+                Error::ParserError,
+                "Invalid multimap object: expected array values for every field");
+        }
+
+        for (const auto& item : value) {
+            auto result = buildObjectField(object, key, item, model);
+            TRY_EXPECTED(result);
+        }
+    }
+
+    return {};
+}
+}
+
+/** Build a simfil node tree from JSON while honoring simfil's tagged encodings. */
+auto buildModelNode(const json& input, ModelPool& model) -> tl::expected<ModelNode::Ptr, Error>
+{
+    switch (input.type()) {
     case json::value_t::null:
-        return {};
+        return nullNode(model);
     case json::value_t::boolean:
-        return model.newSmallValue(j.get<bool>());
+        return model.newSmallValue(input.get<bool>());
     case json::value_t::number_float:
-        return model.newValue(j.get<double>());
+        return model.newValue(input.get<double>());
     case json::value_t::number_unsigned:
-        return model.newValue((int64_t)j.get<uint64_t>());
+        if (input.get<uint64_t>() > static_cast<uint64_t>(std::numeric_limits<int64_t>::max())) {
+            // simfil stores JSON integers as signed int64_t, so reject lossy unsigned values.
+            return tl::unexpected<Error>(
+                Error::ParserError,
+                "Unsigned integer does not fit into simfil's signed JSON number storage");
+        }
+        return model.newValue(static_cast<int64_t>(input.get<uint64_t>()));
     case json::value_t::number_integer:
-        return model.newValue(j.get<int64_t>());
+        return model.newValue(input.get<int64_t>());
     case json::value_t::string:
-        if (auto stringId = model.strings()->emplace(j.get<std::string>()); stringId)
-            return model.newValue((StringId)*stringId);
-        else
-            return tl::unexpected<Error>(stringId.error());
+        {
+            // JSON strings are expected to participate in simfil's pooled-string
+            // facilities such as completion of uppercase constants.
+            auto stringId = model.strings()->emplace(input.get<std::string>());
+            if (!stringId) {
+                return tl::unexpected<Error>(stringId.error());
+            }
+            return model.newValue(static_cast<StringId>(*stringId));
+        }
     default:
         break;
     }
 
-    if (j.is_object()) {
-        if (auto it = j.find("_bytes"); it != j.end() && it->is_boolean() && it->get<bool>()) {
-            auto hex = j.find("hex");
-            if (hex == j.end() || !hex->is_string())
-                return tl::unexpected<Error>(Error::ParserError, "Invalid tagged bytes object: expected string field 'hex'");
+    if (input.is_object()) {
+        if (auto it = input.find("_bytes"); it != input.end() && it->is_boolean() && it->get<bool>()) {
+            // `_bytes` is the tagged JSON form emitted by ModelNode::toJson() for byte arrays.
+            auto hex = input.find("hex");
+            if (hex == input.end() || !hex->is_string()) {
+                return tl::unexpected<Error>(
+                    Error::ParserError,
+                    "Invalid tagged bytes object: expected string field 'hex'");
+            }
 
             auto decoded = ByteArray::fromHex(hex->get<std::string>());
-            if (!decoded)
-                return tl::unexpected<Error>(Error::ParserError, "Invalid tagged bytes object: hex decode failed");
+            if (!decoded) {
+                return tl::unexpected<Error>(
+                    Error::ParserError,
+                    "Invalid tagged bytes object: hex decode failed");
+            }
 
             return model.newValue(std::move(*decoded));
         }
 
-        auto object = model.newObject(j.size(), true);
-        for (auto&& [key, value] : j.items()) {
-            auto child = build(value, model);
-            TRY_EXPECTED(child);
-            object->addField(key, *child);
+        auto object = model.newObject(input.size(), true);
+        auto multimap = input.find("_multimap");
+        if (multimap != input.end() && multimap->is_boolean() && multimap->get<bool>()) {
+            // Repeated object keys are reconstructed only for explicitly tagged multimaps.
+            auto result = buildMultimapObject(input, object, model);
+            TRY_EXPECTED(result);
+            return object;
+        }
+
+        for (auto&& [key, value] : input.items()) {
+            auto result = buildObjectField(object, key, value, model);
+            TRY_EXPECTED(result);
         }
         return object;
     }
 
-    if (j.is_array()) {
-        auto array = model.newArray(j.size(), true);
-        for (const auto& value : j) {
-            auto child = build(value, model);
+    if (input.is_array()) {
+        auto array = model.newArray(input.size(), true);
+        for (const auto& value : input) {
+            auto child = buildModelNode(value, model);
             TRY_EXPECTED(child);
             array->append(*child);
         }
         return array;
     }
 
-    return {};
+    return tl::unexpected<Error>(
+        Error::ParserError,
+        "Unsupported JSON value type");
 }
 
+/** Parse JSON from a stream into an existing model pool. */
 auto parse(std::istream& input, ModelPoolPtr const& model) -> tl::expected<void, Error>
 {
-    auto root = build(json::parse(input), *model);
+    auto root = buildModelNode(json::parse(input), *model);
     if (!root)
         return tl::unexpected<Error>(root.error());
     model->addRoot(*root);
     return model->validate();
 }
 
+/** Parse a JSON string into an existing model pool. */
 auto parse(const std::string& input, ModelPoolPtr const& model) -> tl::expected<void, Error>
 {
-    auto root = build(json::parse(input), *model);
+    auto root = buildModelNode(json::parse(input), *model);
     if (!root)
         return tl::unexpected<Error>(root.error());
     model->addRoot(*root);
     return model->validate();
 }
 
+/** Parse a JSON string into a new model pool with one validated root. */
 auto parse(const std::string& input) -> tl::expected<ModelPoolPtr, Error>
 {
     auto model = std::make_shared<simfil::ModelPool>();
-    auto root = build(json::parse(input), *model);
+    auto root = buildModelNode(json::parse(input), *model);
     if (!root)
         return tl::unexpected<Error>(root.error());
     model->addRoot(*root);

test/complex.cpp

@@ -98,8 +98,27 @@ TEST_CASE("Multimap JSON", "[multimap.serialization]") {
     root->addField("c", array);
     root->addField("c", static_cast<uint16_t>(2));
 
+    auto const expected = nlohmann::json::parse(R"([{"a":[1],"b":[1,2,3],"c":[[1],2],"_multimap":true}])");
     INFO(model->toJson().dump(2));
-    REQUIRE(model->toJson() == nlohmann::json::parse(R"([{"a":[1],"b":[1,2,3],"c":[[1],2],"_multimap":true}])"));
+    REQUIRE(model->toJson() == expected);
+
+    auto roundTrip = json::parse(expected.dump());
+    REQUIRE(roundTrip);
+    auto roundTripRoot = roundTrip.value()->root(0);
+    REQUIRE(roundTripRoot);
+    auto roundTripObject = roundTripRoot.value()->at(0);
+    REQUIRE(roundTripObject);
+    REQUIRE(roundTripObject->size() == 6);
+    REQUIRE(roundTripObject->toJson() == expected[0]);
+}
+
+TEST_CASE("Null JSON values build explicit null nodes", "[json.serialization][null]") {
+    auto model = json::parse(R"({"a":null,"b":[null,{"c":null}]})");
+    REQUIRE(model);
+
+    auto root = model.value()->root(0);
+    REQUIRE(root);
+    REQUIRE(root.value()->toJson() == nlohmann::json::parse(R"({"a":null,"b":[null,{"c":null}]})"));
 }
 
 TEST_CASE("Tagged bytes JSON", "[bytes.serialization]") {