diff --git a/doc/manual/rl-next/store-object-info.md b/doc/manual/rl-next/store-object-info.md new file mode 100644 index 000000000000..aae97ad78d32 --- /dev/null +++ b/doc/manual/rl-next/store-object-info.md @@ -0,0 +1,11 @@ +--- +synopsis: Store object info JSON format now uses `null` rather than omitting fields. +prs: 9995 +--- + +The [store object info JSON format](@docroot@/protocols/json/store-object-info.md) no longer omits fields to indicate absent information, but instead includes the fields with a `null` value. +For example, `"ca": null` is used to to indcate a store object that isn't content-addressed rather than omitting the `ca` field entirely. +This makes records of this sort more self-describing, and easier to consume programmatically. + +We would like to follow this design principle going forward; +the [JSON guidlines](@docroot@/contributing/json-guideline.md) in the contributing section have been updated accordingly. diff --git a/src/libstore/path-info.cc b/src/libstore/path-info.cc index 6523cb4258aa..16274cd23455 100644 --- a/src/libstore/path-info.cc +++ b/src/libstore/path-info.cc @@ -161,27 +161,24 @@ nlohmann::json UnkeyedValidPathInfo::toJSON( jsonObject["narSize"] = narSize; { - auto& jsonRefs = (jsonObject["references"] = json::array()); + auto & jsonRefs = jsonObject["references"] = json::array(); for (auto & ref : references) jsonRefs.emplace_back(store.printStorePath(ref)); } - if (ca) - jsonObject["ca"] = renderContentAddress(ca); + jsonObject["ca"] = ca ? (std::optional { renderContentAddress(*ca) }) : std::nullopt; if (includeImpureInfo) { - if (deriver) - jsonObject["deriver"] = store.printStorePath(*deriver); + jsonObject["deriver"] = deriver ? (std::optional { store.printStorePath(*deriver) }) : std::nullopt; - if (registrationTime) - jsonObject["registrationTime"] = registrationTime; + jsonObject["registrationTime"] = registrationTime ? (std::optional { registrationTime }) : std::nullopt; - if (ultimate) - jsonObject["ultimate"] = ultimate; + jsonObject["ultimate"] = ultimate; + auto & sigsObj = jsonObject["signatures"] = json::array(); if (!sigs.empty()) { for (auto & sig : sigs) - jsonObject["signatures"].push_back(sig); + sigsObj.push_back(sig); } } @@ -210,20 +207,25 @@ UnkeyedValidPathInfo UnkeyedValidPathInfo::fromJSON( throw; } + // New format as this as nullable but mandatory field; handling + // missing is for back-compat. if (json.contains("ca")) - res.ca = ContentAddress::parse(getString(valueAt(json, "ca"))); + if (auto * rawCa = getNullable(valueAt(json, "ca"))) + res.ca = ContentAddress::parse(getString(*rawCa)); if (json.contains("deriver")) - res.deriver = store.parseStorePath(getString(valueAt(json, "deriver"))); + if (auto * rawDeriver = getNullable(valueAt(json, "deriver"))) + res.deriver = store.parseStorePath(getString(*rawDeriver)); if (json.contains("registrationTime")) - res.registrationTime = getInteger(valueAt(json, "registrationTime")); + if (auto * rawRegistrationTime = getNullable(valueAt(json, "registrationTime"))) + res.registrationTime = getInteger(*rawRegistrationTime); if (json.contains("ultimate")) res.ultimate = getBoolean(valueAt(json, "ultimate")); if (json.contains("signatures")) - res.sigs = valueAt(json, "signatures"); + res.sigs = getStringSet(valueAt(json, "signatures")); return res; } diff --git a/src/libutil/json-utils.cc b/src/libutil/json-utils.cc index 1b911bf75eb9..dff068e07c5f 100644 --- a/src/libutil/json-utils.cc +++ b/src/libutil/json-utils.cc @@ -39,12 +39,9 @@ std::optional optionalValueAt(const nlohmann::json::object_t & m } -std::optional getNullable(const nlohmann::json & value) +const nlohmann::json * getNullable(const nlohmann::json & value) { - if (value.is_null()) - return std::nullopt; - - return value.get(); + return value.is_null() ? nullptr : &value; } /** diff --git a/src/libutil/json-utils.hh b/src/libutil/json-utils.hh index 08c98cc8c0d4..fe7a406cf914 100644 --- a/src/libutil/json-utils.hh +++ b/src/libutil/json-utils.hh @@ -29,7 +29,7 @@ std::optional optionalValueAt(const nlohmann::json::object_t & v * Downcast the json object, failing with a nice error if the conversion fails. * See https://json.nlohmann.me/features/types/ */ -std::optional getNullable(const nlohmann::json & value); +const nlohmann::json * getNullable(const nlohmann::json & value); const nlohmann::json::object_t & getObject(const nlohmann::json & value); const nlohmann::json::array_t & getArray(const nlohmann::json & value); const nlohmann::json::string_t & getString(const nlohmann::json & value); diff --git a/tests/functional/signing.sh b/tests/functional/signing.sh index bcbd3b675d23..cf84ab37708d 100755 --- a/tests/functional/signing.sh +++ b/tests/functional/signing.sh @@ -15,9 +15,9 @@ outPath=$(nix-build dependencies.nix --no-out-link --secret-key-files "$TEST_ROO # Verify that the path got signed. info=$(nix path-info --json $outPath) -[[ $info =~ '"ultimate":true' ]] -[[ $info =~ 'cache1.example.org' ]] -[[ $info =~ 'cache2.example.org' ]] +echo $info | jq -e '.[] | .ultimate == true' +echo $info | jq -e '.[] | .signatures.[] | select(startswith("cache1.example.org"))' +echo $info | jq -e '.[] | .signatures.[] | select(startswith("cache2.example.org"))' # Test "nix store verify". nix store verify -r $outPath @@ -39,8 +39,8 @@ nix store verify -r $outPath # Verify that the path did not get signed but does have the ultimate bit. info=$(nix path-info --json $outPath2) -[[ $info =~ '"ultimate":true' ]] -(! [[ $info =~ 'signatures' ]]) +echo $info | jq -e '.[] | .ultimate == true' +echo $info | jq -e '.[] | .signatures == []' # Test "nix store verify". nix store verify -r $outPath2 @@ -57,7 +57,7 @@ nix store verify -r $outPath2 --sigs-needed 1 --trusted-public-keys $pk1 # Build something content-addressed. outPathCA=$(IMPURE_VAR1=foo IMPURE_VAR2=bar nix-build ./fixed.nix -A good.0 --no-out-link) -[[ $(nix path-info --json $outPathCA) =~ '"ca":"fixed:md5:' ]] +nix path-info --json $outPathCA | jq -e '.[] | .ca | startswith("fixed:md5:")' # Content-addressed paths don't need signatures, so they verify # regardless of --sigs-needed. @@ -73,15 +73,15 @@ nix copy --to file://$cacheDir $outPath2 # Verify that signatures got copied. info=$(nix path-info --store file://$cacheDir --json $outPath2) -(! [[ $info =~ '"ultimate":true' ]]) -[[ $info =~ 'cache1.example.org' ]] -(! [[ $info =~ 'cache2.example.org' ]]) +echo $info | jq -e '.[] | .ultimate == false' +echo $info | jq -e '.[] | .signatures.[] | select(startswith("cache1.example.org"))' +echo $info | expect 4 jq -e '.[] | .signatures.[] | select(startswith("cache2.example.org"))' # Verify that adding a signature to a path in a binary cache works. nix store sign --store file://$cacheDir --key-file $TEST_ROOT/sk2 $outPath2 info=$(nix path-info --store file://$cacheDir --json $outPath2) -[[ $info =~ 'cache1.example.org' ]] -[[ $info =~ 'cache2.example.org' ]] +echo $info | jq -e '.[] | .signatures.[] | select(startswith("cache1.example.org"))' +echo $info | jq -e '.[] | .signatures.[] | select(startswith("cache2.example.org"))' # Copying to a diverted store should fail due to a lack of signatures by trusted keys. chmod -R u+w $TEST_ROOT/store0 || true diff --git a/tests/unit/libstore/data/path-info/empty_impure.json b/tests/unit/libstore/data/path-info/empty_impure.json new file mode 100644 index 000000000000..be982dcef853 --- /dev/null +++ b/tests/unit/libstore/data/path-info/empty_impure.json @@ -0,0 +1,10 @@ +{ + "ca": null, + "deriver": null, + "narHash": "sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc=", + "narSize": 0, + "references": [], + "registrationTime": null, + "signatures": [], + "ultimate": false +} diff --git a/tests/unit/libstore/data/path-info/empty_pure.json b/tests/unit/libstore/data/path-info/empty_pure.json new file mode 100644 index 000000000000..10d9f508a360 --- /dev/null +++ b/tests/unit/libstore/data/path-info/empty_pure.json @@ -0,0 +1,6 @@ +{ + "ca": null, + "narHash": "sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc=", + "narSize": 0, + "references": [] +} diff --git a/tests/unit/libstore/path-info.cc b/tests/unit/libstore/path-info.cc index 80d6fcfed20e..06c662b74bcc 100644 --- a/tests/unit/libstore/path-info.cc +++ b/tests/unit/libstore/path-info.cc @@ -19,7 +19,15 @@ class PathInfoTest : public CharacterizationTest, public LibStoreTest } }; -static UnkeyedValidPathInfo makePathInfo(const Store & store, bool includeImpureInfo) { +static UnkeyedValidPathInfo makeEmpty() +{ + return { + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }; +} + +static UnkeyedValidPathInfo makeFull(const Store & store, bool includeImpureInfo) +{ UnkeyedValidPathInfo info = ValidPathInfo { store, "foo", @@ -50,22 +58,21 @@ static UnkeyedValidPathInfo makePathInfo(const Store & store, bool includeImpure return info; } -#define JSON_TEST(STEM, PURE) \ +#define JSON_TEST(STEM, OBJ, PURE) \ TEST_F(PathInfoTest, PathInfo_ ## STEM ## _from_json) { \ readTest(#STEM, [&](const auto & encoded_) { \ auto encoded = json::parse(encoded_); \ UnkeyedValidPathInfo got = UnkeyedValidPathInfo::fromJSON( \ *store, \ encoded); \ - auto expected = makePathInfo(*store, PURE); \ + auto expected = OBJ; \ ASSERT_EQ(got, expected); \ }); \ } \ \ TEST_F(PathInfoTest, PathInfo_ ## STEM ## _to_json) { \ writeTest(#STEM, [&]() -> json { \ - return makePathInfo(*store, PURE) \ - .toJSON(*store, PURE, HashFormat::SRI); \ + return OBJ.toJSON(*store, PURE, HashFormat::SRI); \ }, [](const auto & file) { \ return json::parse(readFile(file)); \ }, [](const auto & file, const auto & got) { \ @@ -73,7 +80,10 @@ static UnkeyedValidPathInfo makePathInfo(const Store & store, bool includeImpure }); \ } -JSON_TEST(pure, false) -JSON_TEST(impure, true) +JSON_TEST(empty_pure, makeEmpty(), false) +JSON_TEST(empty_impure, makeEmpty(), true) + +JSON_TEST(pure, makeFull(*store, false), false) +JSON_TEST(impure, makeFull(*store, true), true) } diff --git a/tests/unit/libutil/json-utils.cc b/tests/unit/libutil/json-utils.cc index c9370a74bfda..704a4acb05df 100644 --- a/tests/unit/libutil/json-utils.cc +++ b/tests/unit/libutil/json-utils.cc @@ -175,13 +175,16 @@ TEST(optionalValueAt, empty) { TEST(getNullable, null) { auto json = R"(null)"_json; - ASSERT_EQ(getNullable(json), std::nullopt); + ASSERT_EQ(getNullable(json), nullptr); } TEST(getNullable, empty) { auto json = R"({})"_json; - ASSERT_EQ(getNullable(json), std::optional { R"({})"_json }); + auto * p = getNullable(json); + + ASSERT_NE(p, nullptr); + ASSERT_EQ(*p, R"({})"_json); } } /* namespace nix */