Only parse to the depth requested
There are cases in aggregation where an expand parameter might get
forwarded to a client. Because our previous expand algorithm assumed
that any endpoint within bmcweb would only produce "depth=1" responses,
it was reasonable to assume that the pre-response could not contain
expanded content. Aggregated resources can't make that assumption.
This commit attempts to pass through depth through the request, to
ensure that we only expand the level that the user requested, and not
any level returned by the request. This is done by using the existence
of the resource identifer "@odata.id" to indicate each level in an
expanded response. This should be fine since the Redfish spec requires
that property to exist.
Added unit tests to cover aggregation scenarios. Modified existing
$expand tests to comply with the resource identifier dependency.
Tested:
New unit tests pass
Queried '/redfish/v1/Systems?$expand=.($levels=2)' on an aggregated
system whose satellite BMC supported $expand. The overall response was
correctly expanded for both resources on the aggregating BMC as well as
on the satellite BMC. Expanding the satellite resources did not require
sending multiple queries to the satellite.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I20ba60ee39bac11ffb3fe1768cec6299cf9ee13e
Signed-off-by: Carson Labrado <clabrado@google.com>
diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 9c38248..96885eb 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -536,7 +536,7 @@
// with the keys from the jsonResponse object
inline void findNavigationReferencesRecursive(
ExpandType eType, nlohmann::json& jsonResponse,
- const nlohmann::json::json_pointer& p, bool inLinks,
+ const nlohmann::json::json_pointer& p, int depth, bool inLinks,
std::vector<ExpandNode>& out)
{
// If no expand is needed, return early
@@ -544,6 +544,7 @@
{
return;
}
+
nlohmann::json::array_t* array =
jsonResponse.get_ptr<nlohmann::json::array_t*>();
if (array != nullptr)
@@ -554,8 +555,8 @@
{
nlohmann::json::json_pointer newPtr = p / index;
BMCWEB_LOG_DEBUG << "Traversing response at " << newPtr.to_string();
- findNavigationReferencesRecursive(eType, element, newPtr, inLinks,
- out);
+ findNavigationReferencesRecursive(eType, element, newPtr, depth,
+ inLinks, out);
index++;
}
}
@@ -574,11 +575,29 @@
obj->begin()->second.get_ptr<const std::string*>();
if (uri != nullptr)
{
- BMCWEB_LOG_DEBUG << "Found element at " << p.to_string();
+ BMCWEB_LOG_DEBUG << "Found " << *uri << " at " << p.to_string();
out.push_back({p, *uri});
+ return;
}
}
}
+
+ int newDepth = depth;
+ auto odataId = obj->find("@odata.id");
+ if (odataId != obj->end())
+ {
+ // The Redfish spec requires all resources to include the resource
+ // identifier. If the object has multiple elements and one of them is
+ // "@odata.id" then that means we have entered a new level / expanded
+ // resource. We need to stop traversing if we're already at the desired
+ // depth
+ if ((obj->size() > 1) && (depth == 0))
+ {
+ return;
+ }
+ newDepth--;
+ }
+
// Loop the object and look for links
for (auto& element : *obj)
{
@@ -602,16 +621,24 @@
BMCWEB_LOG_DEBUG << "Traversing response at " << newPtr;
findNavigationReferencesRecursive(eType, element.second, newPtr,
- localInLinks, out);
+ newDepth, localInLinks, out);
}
}
+// TODO: When aggregation is enabled and we receive a partially expanded
+// response we may need need additional handling when the original URI was
+// up tree from a top level collection.
+// Isn't a concern until https://gerrit.openbmc.org/c/openbmc/bmcweb/+/60556
+// lands. May want to avoid forwarding query params when request is uptree from
+// a top level collection.
inline std::vector<ExpandNode>
- findNavigationReferences(ExpandType eType, nlohmann::json& jsonResponse)
+ findNavigationReferences(ExpandType eType, int depth,
+ nlohmann::json& jsonResponse)
{
std::vector<ExpandNode> ret;
const nlohmann::json::json_pointer root = nlohmann::json::json_pointer("");
- findNavigationReferencesRecursive(eType, jsonResponse, root, false, ret);
+ findNavigationReferencesRecursive(eType, jsonResponse, root, depth, false,
+ ret);
return ret;
}
@@ -772,8 +799,8 @@
// for deeper levels.
void startQuery(const Query& query)
{
- std::vector<ExpandNode> nodes =
- findNavigationReferences(query.expandType, finalRes->res.jsonValue);
+ std::vector<ExpandNode> nodes = findNavigationReferences(
+ query.expandType, query.expandLevel, finalRes->res.jsonValue);
BMCWEB_LOG_DEBUG << nodes.size() << " nodes to traverse";
const std::optional<std::string> queryStr = formatQueryForExpand(query);
if (!queryStr)
diff --git a/test/redfish-core/include/utils/query_param_test.cpp b/test/redfish-core/include/utils/query_param_test.cpp
index 51f0b71..d049314 100644
--- a/test/redfish-core/include/utils/query_param_test.cpp
+++ b/test/redfish-core/include/utils/query_param_test.cpp
@@ -652,57 +652,203 @@
{
using nlohmann::json;
- json singleTreeNode = R"({"Foo" : {"@odata.id": "/foobar"}})"_json;
+ // Responses must include their "@odata.id" property for $expand to work
+ // correctly
+ json singleTreeNode =
+ R"({"@odata.id": "/redfish/v1",
+ "Foo" : {"@odata.id": "/foobar"}})"_json;
// Parsing as the root should net one entry
- EXPECT_THAT(findNavigationReferences(ExpandType::Both, singleTreeNode),
+ EXPECT_THAT(findNavigationReferences(ExpandType::Both, 1, singleTreeNode),
UnorderedElementsAre(
ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
// Parsing in Non-hyperlinks mode should net one entry
- EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, singleTreeNode),
- UnorderedElementsAre(
- ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
+ EXPECT_THAT(
+ findNavigationReferences(ExpandType::NotLinks, 1, singleTreeNode),
+ UnorderedElementsAre(
+ ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
// Searching for not types should return empty set
EXPECT_TRUE(
- findNavigationReferences(ExpandType::None, singleTreeNode).empty());
+ findNavigationReferences(ExpandType::None, 1, singleTreeNode).empty());
// Searching for hyperlinks only should return empty set
EXPECT_TRUE(
- findNavigationReferences(ExpandType::Links, singleTreeNode).empty());
+ findNavigationReferences(ExpandType::Links, 1, singleTreeNode).empty());
+ // Responses must include their "@odata.id" property for $expand to work
+ // correctly
json multiTreeNodes =
- R"({"Links": {"@odata.id": "/links"}, "Foo" : {"@odata.id": "/foobar"}})"_json;
+ R"({"@odata.id": "/redfish/v1",
+ "Links": {"@odata.id": "/links"},
+ "Foo" : {"@odata.id": "/foobar"}})"_json;
+
// Should still find Foo
- EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, multiTreeNodes),
- UnorderedElementsAre(
- ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
+ EXPECT_THAT(
+ findNavigationReferences(ExpandType::NotLinks, 1, multiTreeNodes),
+ UnorderedElementsAre(
+ ExpandNode{json::json_pointer("/Foo"), "/foobar"}));
}
TEST(QueryParams, FindNavigationReferencesLink)
{
using nlohmann::json;
+ // Responses must include their "@odata.id" property for $expand to work
+ // correctly
json singleLinkNode =
- R"({"Links" : {"Sessions": {"@odata.id": "/foobar"}}})"_json;
+ R"({"@odata.id": "/redfish/v1",
+ "Links" : {"Sessions": {"@odata.id": "/foobar"}}})"_json;
// Parsing as the root should net one entry
- EXPECT_THAT(findNavigationReferences(ExpandType::Both, singleLinkNode),
+ EXPECT_THAT(findNavigationReferences(ExpandType::Both, 1, singleLinkNode),
UnorderedElementsAre(ExpandNode{
json::json_pointer("/Links/Sessions"), "/foobar"}));
// Parsing in hyperlinks mode should net one entry
- EXPECT_THAT(findNavigationReferences(ExpandType::Links, singleLinkNode),
+ EXPECT_THAT(findNavigationReferences(ExpandType::Links, 1, singleLinkNode),
UnorderedElementsAre(ExpandNode{
json::json_pointer("/Links/Sessions"), "/foobar"}));
// Searching for not types should return empty set
EXPECT_TRUE(
- findNavigationReferences(ExpandType::None, singleLinkNode).empty());
+ findNavigationReferences(ExpandType::None, 1, singleLinkNode).empty());
// Searching for non-hyperlinks only should return empty set
EXPECT_TRUE(
- findNavigationReferences(ExpandType::NotLinks, singleLinkNode).empty());
+ findNavigationReferences(ExpandType::NotLinks, 1, singleLinkNode)
+ .empty());
+}
+
+TEST(QueryParams, PreviouslyExpanded)
+{
+ using nlohmann::json;
+
+ // Responses must include their "@odata.id" property for $expand to work
+ // correctly
+ json expNode = json::parse(R"(
+{
+ "@odata.id": "/redfish/v1/Chassis",
+ "@odata.type": "#ChassisCollection.ChassisCollection",
+ "Members": [
+ {
+ "@odata.id": "/redfish/v1/Chassis/5B247A_Sat1",
+ "@odata.type": "#Chassis.v1_17_0.Chassis",
+ "Sensors": {
+ "@odata.id": "/redfish/v1/Chassis/5B247A_Sat1/Sensors"
+ }
+ },
+ {
+ "@odata.id": "/redfish/v1/Chassis/5B247A_Sat2",
+ "@odata.type": "#Chassis.v1_17_0.Chassis",
+ "Sensors": {
+ "@odata.id": "/redfish/v1/Chassis/5B247A_Sat2/Sensors"
+ }
+ }
+ ],
+ "Members@odata.count": 2,
+ "Name": "Chassis Collection"
+}
+)",
+ nullptr, false);
+
+ // Expand has already occurred so we should not do anything
+ EXPECT_TRUE(
+ findNavigationReferences(ExpandType::NotLinks, 1, expNode).empty());
+
+ // Previous expand was only a single level so we should further expand
+ EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 2, expNode),
+ UnorderedElementsAre(
+ ExpandNode{json::json_pointer("/Members/0/Sensors"),
+ "/redfish/v1/Chassis/5B247A_Sat1/Sensors"},
+ ExpandNode{json::json_pointer("/Members/1/Sensors"),
+ "/redfish/v1/Chassis/5B247A_Sat2/Sensors"}));
+
+ // Make sure we can handle when an array was expanded further down the tree
+ json expNode2 = R"({"@odata.id" : "/redfish/v1"})"_json;
+ expNode2["Chassis"] = std::move(expNode);
+ EXPECT_TRUE(
+ findNavigationReferences(ExpandType::NotLinks, 1, expNode2).empty());
+ EXPECT_TRUE(
+ findNavigationReferences(ExpandType::NotLinks, 2, expNode2).empty());
+
+ // Previous expand was two levels so we should further expand
+ EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 3, expNode2),
+ UnorderedElementsAre(
+ ExpandNode{json::json_pointer("/Chassis/Members/0/Sensors"),
+ "/redfish/v1/Chassis/5B247A_Sat1/Sensors"},
+ ExpandNode{json::json_pointer("/Chassis/Members/1/Sensors"),
+ "/redfish/v1/Chassis/5B247A_Sat2/Sensors"}));
+}
+
+TEST(QueryParams, PartiallyPreviouslyExpanded)
+{
+ using nlohmann::json;
+
+ // Responses must include their "@odata.id" property for $expand to work
+ // correctly
+ json expNode = json::parse(R"(
+{
+ "@odata.id": "/redfish/v1/Chassis",
+ "@odata.type": "#ChassisCollection.ChassisCollection",
+ "Members": [
+ {
+ "@odata.id": "/redfish/v1/Chassis/Local"
+ },
+ {
+ "@odata.id": "/redfish/v1/Chassis/5B247A_Sat1",
+ "@odata.type": "#Chassis.v1_17_0.Chassis",
+ "Sensors": {
+ "@odata.id": "/redfish/v1/Chassis/5B247A_Sat1/Sensors"
+ }
+ }
+ ],
+ "Members@odata.count": 2,
+ "Name": "Chassis Collection"
+}
+)",
+ nullptr, false);
+
+ // The 5B247A_Sat1 Chassis was already expanded a single level so we should
+ // only want to expand the Local Chassis
+ EXPECT_THAT(
+ findNavigationReferences(ExpandType::NotLinks, 1, expNode),
+ UnorderedElementsAre(ExpandNode{json::json_pointer("/Members/0"),
+ "/redfish/v1/Chassis/Local"}));
+
+ // The 5B247A_Sat1 Chassis was already expanded a single level so we should
+ // further expand it as well as the Local Chassis
+ EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 2, expNode),
+ UnorderedElementsAre(
+ ExpandNode{json::json_pointer("/Members/0"),
+ "/redfish/v1/Chassis/Local"},
+ ExpandNode{json::json_pointer("/Members/1/Sensors"),
+ "/redfish/v1/Chassis/5B247A_Sat1/Sensors"}));
+
+ // Now the response has paths that have been expanded 0, 1, and 2 times
+ json expNode2 = R"({"@odata.id" : "/redfish/v1",
+ "Systems": {"@odata.id": "/redfish/v1/Systems"}})"_json;
+ expNode2["Chassis"] = std::move(expNode);
+
+ EXPECT_THAT(findNavigationReferences(ExpandType::NotLinks, 1, expNode2),
+ UnorderedElementsAre(ExpandNode{json::json_pointer("/Systems"),
+ "/redfish/v1/Systems"}));
+
+ EXPECT_THAT(
+ findNavigationReferences(ExpandType::NotLinks, 2, expNode2),
+ UnorderedElementsAre(
+ ExpandNode{json::json_pointer("/Systems"), "/redfish/v1/Systems"},
+ ExpandNode{json::json_pointer("/Chassis/Members/0"),
+ "/redfish/v1/Chassis/Local"}));
+
+ EXPECT_THAT(
+ findNavigationReferences(ExpandType::NotLinks, 3, expNode2),
+ UnorderedElementsAre(
+ ExpandNode{json::json_pointer("/Systems"), "/redfish/v1/Systems"},
+ ExpandNode{json::json_pointer("/Chassis/Members/0"),
+ "/redfish/v1/Chassis/Local"},
+ ExpandNode{json::json_pointer("/Chassis/Members/1/Sensors"),
+ "/redfish/v1/Chassis/5B247A_Sat1/Sensors"}));
}
} // namespace