Merge pull request #14351 from obsidiansystems/json-project-reference

Clean up JSON utils in a few ways

src/libfetchers/fetchers.cc

@@ -519,10 +519,11 @@ using namespace nix;
 fetchers::PublicKey adl_serializer<fetchers::PublicKey>::from_json(const json & json)
 {
     fetchers::PublicKey res = {};
-    if (auto type = optionalValueAt(json, "type"))
+    auto & obj = getObject(json);
+    if (auto * type = optionalValueAt(obj, "type"))
         res.type = getString(*type);
 
-    res.key = getString(valueAt(json, "key"));
+    res.key = getString(valueAt(obj, "key"));
 
     return res;
 }

src/libstore/derivation-options.cc

@@ -116,27 +116,29 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt
     DerivationOptions defaults = {};
 
     if (shouldWarn && parsed) {
-        if (get(parsed->structuredAttrs, "allowedReferences")) {
+        auto & structuredAttrs = getObject(parsed->structuredAttrs);
+
+        if (get(structuredAttrs, "allowedReferences")) {
             warn(
                 "'structuredAttrs' disables the effect of the top-level attribute 'allowedReferences'; use 'outputChecks' instead");
         }
-        if (get(parsed->structuredAttrs, "allowedRequisites")) {
+        if (get(structuredAttrs, "allowedRequisites")) {
             warn(
                 "'structuredAttrs' disables the effect of the top-level attribute 'allowedRequisites'; use 'outputChecks' instead");
         }
-        if (get(parsed->structuredAttrs, "disallowedRequisites")) {
+        if (get(structuredAttrs, "disallowedRequisites")) {
             warn(
                 "'structuredAttrs' disables the effect of the top-level attribute 'disallowedRequisites'; use 'outputChecks' instead");
         }
-        if (get(parsed->structuredAttrs, "disallowedReferences")) {
+        if (get(structuredAttrs, "disallowedReferences")) {
             warn(
                 "'structuredAttrs' disables the effect of the top-level attribute 'disallowedReferences'; use 'outputChecks' instead");
         }
-        if (get(parsed->structuredAttrs, "maxSize")) {
+        if (get(structuredAttrs, "maxSize")) {
             warn(
                 "'structuredAttrs' disables the effect of the top-level attribute 'maxSize'; use 'outputChecks' instead");
         }
-        if (get(parsed->structuredAttrs, "maxClosureSize")) {
+        if (get(structuredAttrs, "maxClosureSize")) {
             warn(
                 "'structuredAttrs' disables the effect of the top-level attribute 'maxClosureSize'; use 'outputChecks' instead");
         }
@@ -145,11 +147,15 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt
     return {
         .outputChecks = [&]() -> OutputChecksVariant {
             if (parsed) {
+                auto & structuredAttrs = getObject(parsed->structuredAttrs);
+
                 std::map<std::string, OutputChecks> res;
-                if (auto outputChecks = get(parsed->structuredAttrs, "outputChecks")) {
+                if (auto * outputChecks = get(structuredAttrs, "outputChecks")) {
-                    for (auto & [outputName, output] : getObject(*outputChecks)) {
+                    for (auto & [outputName, output_] : getObject(*outputChecks)) {
                         OutputChecks checks;
 
+                        auto & output = getObject(output_);
+
                         if (auto maxSize = get(output, "maxSize"))
                             checks.maxSize = maxSize->get<uint64_t>();
 
@@ -195,7 +201,9 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt
                 std::map<std::string, bool> res;
 
                 if (parsed) {
-                    if (auto udr = get(parsed->structuredAttrs, "unsafeDiscardReferences")) {
+                    auto & structuredAttrs = getObject(parsed->structuredAttrs);
+
+                    if (auto * udr = get(structuredAttrs, "unsafeDiscardReferences")) {
                         for (auto & [outputName, output] : getObject(*udr)) {
                             if (!output.is_boolean())
                                 throw Error("attribute 'unsafeDiscardReferences.\"%s\"' must be a Boolean", outputName);
@@ -226,7 +234,7 @@ DerivationOptions::fromStructuredAttrs(const StringMap & env, const StructuredAt
                 std::map<std::string, StringSet> ret;
 
                 if (parsed) {
-                    auto e = optionalValueAt(parsed->structuredAttrs, "exportReferencesGraph");
+                    auto e = optionalValueAt(getObject(parsed->structuredAttrs), "exportReferencesGraph");
                     if (!e || !e->is_object())
                         return ret;
                     for (auto & [key, value] : getObject(*e)) {
@@ -333,8 +341,10 @@ namespace nlohmann {
 
 using namespace nix;
 
-DerivationOptions adl_serializer<DerivationOptions>::from_json(const json & json)
+DerivationOptions adl_serializer<DerivationOptions>::from_json(const json & json_)
 {
+    auto & json = getObject(json_);
+
     return {
         .outputChecks = [&]() -> OutputChecksVariant {
             auto outputChecks = getObject(valueAt(json, "outputChecks"));
@@ -397,13 +407,24 @@ void adl_serializer<DerivationOptions>::to_json(json & json, const DerivationOpt
     json["allowSubstitutes"] = o.allowSubstitutes;
 }
 
-DerivationOptions::OutputChecks adl_serializer<DerivationOptions::OutputChecks>::from_json(const json & json)
+template<typename T>
+static inline std::optional<T> ptrToOwned(const json * ptr)
 {
+    if (ptr)
+        return std::optional{*ptr};
+    else
+        return std::nullopt;
+}
+
+DerivationOptions::OutputChecks adl_serializer<DerivationOptions::OutputChecks>::from_json(const json & json_)
+{
+    auto & json = getObject(json_);
+
     return {
         .ignoreSelfRefs = getBoolean(valueAt(json, "ignoreSelfRefs")),
-        .allowedReferences = nullableValueAt(json, "allowedReferences"),
+        .allowedReferences = ptrToOwned<StringSet>(getNullable(valueAt(json, "allowedReferences"))),
         .disallowedReferences = getStringSet(valueAt(json, "disallowedReferences")),
-        .allowedRequisites = nullableValueAt(json, "allowedRequisites"),
+        .allowedRequisites = ptrToOwned<StringSet>(getNullable(valueAt(json, "allowedRequisites"))),
         .disallowedRequisites = getStringSet(valueAt(json, "disallowedRequisites")),
     };
 }

src/libstore/nar-info.cc

@@ -159,17 +159,19 @@ NarInfo NarInfo::fromJSON(const StoreDirConfig & store, const StorePath & path,
         UnkeyedValidPathInfo::fromJSON(store, json),
     }};
 
+    auto & obj = getObject(json);
+
     if (json.contains("url"))
-        res.url = getString(valueAt(json, "url"));
+        res.url = getString(valueAt(obj, "url"));
 
     if (json.contains("compression"))
-        res.compression = getString(valueAt(json, "compression"));
+        res.compression = getString(valueAt(obj, "compression"));
 
     if (json.contains("downloadHash"))
-        res.fileHash = Hash::parseAny(getString(valueAt(json, "downloadHash")), std::nullopt);
+        res.fileHash = Hash::parseAny(getString(valueAt(obj, "downloadHash")), std::nullopt);
 
     if (json.contains("downloadSize"))
-        res.fileSize = getUnsigned(valueAt(json, "downloadSize"));
+        res.fileSize = getUnsigned(valueAt(obj, "downloadSize"));
 
     return res;
 }

src/libutil-tests/json-utils.cc

@@ -70,7 +70,7 @@ TEST(valueAt, simpleObject)
 
     auto nested = R"({ "hello": { "world": "" } })"_json;
 
-    ASSERT_EQ(valueAt(valueAt(getObject(nested), "hello"), "world"), "");
+    ASSERT_EQ(valueAt(getObject(valueAt(getObject(nested), "hello")), "world"), "");
 }
 
 TEST(valueAt, missingKey)
@@ -119,10 +119,12 @@ TEST(getArray, wrongAssertions)
 {
     auto json = R"({ "object": {}, "array": [], "string": "", "int": 0, "boolean": false })"_json;
 
-    ASSERT_THROW(getArray(valueAt(json, "object")), Error);
+    auto & obj = getObject(json);
-    ASSERT_THROW(getArray(valueAt(json, "string")), Error);
+
-    ASSERT_THROW(getArray(valueAt(json, "int")), Error);
+    ASSERT_THROW(getArray(valueAt(obj, "object")), Error);
-    ASSERT_THROW(getArray(valueAt(json, "boolean")), Error);
+    ASSERT_THROW(getArray(valueAt(obj, "string")), Error);
+    ASSERT_THROW(getArray(valueAt(obj, "int")), Error);
+    ASSERT_THROW(getArray(valueAt(obj, "boolean")), Error);
 }
 
 TEST(getString, rightAssertions)
@@ -136,10 +138,12 @@ TEST(getString, wrongAssertions)
 {
     auto json = R"({ "object": {}, "array": [], "string": "", "int": 0, "boolean": false })"_json;
 
-    ASSERT_THROW(getString(valueAt(json, "object")), Error);
+    auto & obj = getObject(json);
-    ASSERT_THROW(getString(valueAt(json, "array")), Error);
+
-    ASSERT_THROW(getString(valueAt(json, "int")), Error);
+    ASSERT_THROW(getString(valueAt(obj, "object")), Error);
-    ASSERT_THROW(getString(valueAt(json, "boolean")), Error);
+    ASSERT_THROW(getString(valueAt(obj, "array")), Error);
+    ASSERT_THROW(getString(valueAt(obj, "int")), Error);
+    ASSERT_THROW(getString(valueAt(obj, "boolean")), Error);
 }
 
 TEST(getIntegralNumber, rightAssertions)
@@ -156,18 +160,20 @@ TEST(getIntegralNumber, wrongAssertions)
     auto json =
         R"({ "object": {}, "array": [], "string": "", "int": 0, "signed": -256, "large": 128, "boolean": false })"_json;
 
-    ASSERT_THROW(getUnsigned(valueAt(json, "object")), Error);
+    auto & obj = getObject(json);
-    ASSERT_THROW(getUnsigned(valueAt(json, "array")), Error);
-    ASSERT_THROW(getUnsigned(valueAt(json, "string")), Error);
-    ASSERT_THROW(getUnsigned(valueAt(json, "boolean")), Error);
-    ASSERT_THROW(getUnsigned(valueAt(json, "signed")), Error);
 
-    ASSERT_THROW(getInteger<int8_t>(valueAt(json, "object")), Error);
+    ASSERT_THROW(getUnsigned(valueAt(obj, "object")), Error);
-    ASSERT_THROW(getInteger<int8_t>(valueAt(json, "array")), Error);
+    ASSERT_THROW(getUnsigned(valueAt(obj, "array")), Error);
-    ASSERT_THROW(getInteger<int8_t>(valueAt(json, "string")), Error);
+    ASSERT_THROW(getUnsigned(valueAt(obj, "string")), Error);
-    ASSERT_THROW(getInteger<int8_t>(valueAt(json, "boolean")), Error);
+    ASSERT_THROW(getUnsigned(valueAt(obj, "boolean")), Error);
-    ASSERT_THROW(getInteger<int8_t>(valueAt(json, "large")), Error);
+    ASSERT_THROW(getUnsigned(valueAt(obj, "signed")), Error);
-    ASSERT_THROW(getInteger<int8_t>(valueAt(json, "signed")), Error);
+
+    ASSERT_THROW(getInteger<int8_t>(valueAt(obj, "object")), Error);
+    ASSERT_THROW(getInteger<int8_t>(valueAt(obj, "array")), Error);
+    ASSERT_THROW(getInteger<int8_t>(valueAt(obj, "string")), Error);
+    ASSERT_THROW(getInteger<int8_t>(valueAt(obj, "boolean")), Error);
+    ASSERT_THROW(getInteger<int8_t>(valueAt(obj, "large")), Error);
+    ASSERT_THROW(getInteger<int8_t>(valueAt(obj, "signed")), Error);
 }
 
 TEST(getBoolean, rightAssertions)
@@ -181,24 +187,28 @@ TEST(getBoolean, wrongAssertions)
 {
     auto json = R"({ "object": {}, "array": [], "string": "", "int": 0, "boolean": false })"_json;
 
-    ASSERT_THROW(getBoolean(valueAt(json, "object")), Error);
+    auto & obj = getObject(json);
-    ASSERT_THROW(getBoolean(valueAt(json, "array")), Error);
+
-    ASSERT_THROW(getBoolean(valueAt(json, "string")), Error);
+    ASSERT_THROW(getBoolean(valueAt(obj, "object")), Error);
-    ASSERT_THROW(getBoolean(valueAt(json, "int")), Error);
+    ASSERT_THROW(getBoolean(valueAt(obj, "array")), Error);
+    ASSERT_THROW(getBoolean(valueAt(obj, "string")), Error);
+    ASSERT_THROW(getBoolean(valueAt(obj, "int")), Error);
 }
 
 TEST(optionalValueAt, existing)
 {
     auto json = R"({ "string": "ssh-rsa" })"_json;
 
-    ASSERT_EQ(optionalValueAt(json, "string"), std::optional{"ssh-rsa"});
+    auto * ptr = optionalValueAt(getObject(json), "string");
+    ASSERT_TRUE(ptr);
+    ASSERT_EQ(*ptr, R"("ssh-rsa")"_json);
 }
 
 TEST(optionalValueAt, empty)
 {
     auto json = R"({})"_json;
 
-    ASSERT_EQ(optionalValueAt(json, "string"), std::nullopt);
+    ASSERT_EQ(optionalValueAt(getObject(json), "string"), nullptr);
 }
 
 TEST(getNullable, null)

src/libutil/include/nix/util/json-utils.hh

@@ -2,7 +2,6 @@
 ///@file
 
 #include <nlohmann/json.hpp>
-#include <list>
 
 #include "nix/util/error.hh"
 #include "nix/util/types.hh"
@@ -12,20 +11,25 @@ namespace nix {
 
 enum struct ExperimentalFeature;
 
-const nlohmann::json * get(const nlohmann::json & map, const std::string & key);
-
-nlohmann::json * get(nlohmann::json & map, const std::string & key);
-
 /**
  * Get the value of a json object at a key safely, failing with a nice
  * error if the key does not exist.
  *
  * Use instead of nlohmann::json::at() to avoid ugly exceptions.
  */
-const nlohmann::json & valueAt(const nlohmann::json::object_t & map, const std::string & key);
+const nlohmann::json & valueAt(const nlohmann::json::object_t & map, std::string_view key);
 
-std::optional<nlohmann::json> optionalValueAt(const nlohmann::json::object_t & value, const std::string & key);
+/**
-std::optional<nlohmann::json> nullableValueAt(const nlohmann::json::object_t & value, const std::string & key);
+ * @return A pointer to the value assiocated with `key` if `value`
+ * contains `key`, otherwise return  `nullptr` (not JSON `null`!).
+ */
+const nlohmann::json * optionalValueAt(const nlohmann::json::object_t & value, std::string_view key);
+
+/**
+ * Prevents bugs; see `get` for the same trick.
+ */
+const nlohmann::json & valueAt(nlohmann::json::object_t && map, std::string_view key) = delete;
+const nlohmann::json * optionalValueAt(nlohmann::json::object_t && value, std::string_view key) = delete;
 
 /**
  * Downcast the json object, failing with a nice error if the conversion fails.

src/libutil/json-utils.cc

@@ -1,52 +1,21 @@
 #include "nix/util/json-utils.hh"
 #include "nix/util/error.hh"
 #include "nix/util/types.hh"
-#include <nlohmann/json_fwd.hpp>
+#include "nix/util/util.hh"
-#include <iostream>
-#include <optional>
 
 namespace nix {
 
-const nlohmann::json * get(const nlohmann::json & map, const std::string & key)
+const nlohmann::json & valueAt(const nlohmann::json::object_t & map, std::string_view key)
 {
-    auto i = map.find(key);
+    if (auto * p = optionalValueAt(map, key))
-    if (i == map.end())
+        return *p;
-        return nullptr;
+    else
-    return &*i;
-}
-
-nlohmann::json * get(nlohmann::json & map, const std::string & key)
-{
-    auto i = map.find(key);
-    if (i == map.end())
-        return nullptr;
-    return &*i;
-}
-
-const nlohmann::json & valueAt(const nlohmann::json::object_t & map, const std::string & key)
-{
-    if (!map.contains(key))
         throw Error("Expected JSON object to contain key '%s' but it doesn't: %s", key, nlohmann::json(map).dump());
-
-    return map.at(key);
 }
 
-std::optional<nlohmann::json> optionalValueAt(const nlohmann::json::object_t & map, const std::string & key)
+const nlohmann::json * optionalValueAt(const nlohmann::json::object_t & map, std::string_view key)
 {
-    if (!map.contains(key))
+    return get(map, key);
-        return std::nullopt;
-
-    return std::optional{map.at(key)};
-}
-
-std::optional<nlohmann::json> nullableValueAt(const nlohmann::json::object_t & map, const std::string & key)
-{
-    auto value = valueAt(map, key);
-
-    if (value.is_null())
-        return std::nullopt;
-
-    return std::optional{std::move(value)};
 }
 
 const nlohmann::json * getNullable(const nlohmann::json & value)