summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCarson Labrado <clabrado@google.com>2022-11-23 23:55:18 +0300
committerEd Tanous <ed@tanous.net>2023-01-12 02:50:43 +0300
commit11987af6e58a2d341c97f752e90c929e0a1189e5 (patch)
tree94edea150ce45146ab5867c05911e73b21a16af9
parent7c8e064d33a1b9136b10f224d9ae9e9b066be012 (diff)
downloadbmcweb-11987af6e58a2d341c97f752e90c929e0a1189e5.tar.xz
Aggregation: Improve prefix fixup matching
Utilize the new array of top level collection URIs to determine if a given URI in the response needs to have the aggregation prefix added. This removes the need to check for specific collections like /redfish/v1/UpdateService/FirmwareInventory which do not fit the generic format of /redfish/v1/<collection>. Future patches will use this same approach to improve the logic for initially determining if and how a request should be aggregated. This patch also adds a series of unit tests for the function responsible for adding a prefix to a given URI. Cases covered include valid URIs that involve a selection of aggregated resources, top level collection URIs, other invalid URIs, and URIs with a trailing "/". Tested: Unit tests pass. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: I676983d3c77ae3126c04e9f57ad8698c51df2675
-rw-r--r--redfish-core/include/aggregation_utils.hpp90
-rw-r--r--redfish-core/include/redfish_aggregator.hpp85
-rwxr-xr-xscripts/generate_schema_collections.py6
-rw-r--r--test/redfish-core/include/redfish_aggregator_test.cpp98
4 files changed, 192 insertions, 87 deletions
diff --git a/redfish-core/include/aggregation_utils.hpp b/redfish-core/include/aggregation_utils.hpp
index bb1782f1b0..673b56873c 100644
--- a/redfish-core/include/aggregation_utils.hpp
+++ b/redfish-core/include/aggregation_utils.hpp
@@ -15,52 +15,52 @@
namespace redfish
{
-// Note that each URI actually begins with "/redfish/v1/"
+// Note that each URI actually begins with "/redfish/v1"
// They've been omitted to save space and reduce search time
constexpr std::array<std::string_view, 44> topCollections{
- "AggregationService/Aggregates",
- "AggregationService/AggregationSources",
- "AggregationService/ConnectionMethods",
- "Cables",
- "Chassis",
- "ComponentIntegrity",
- "CompositionService/ActivePool",
- "CompositionService/CompositionReservations",
- "CompositionService/FreePool",
- "CompositionService/ResourceBlocks",
- "CompositionService/ResourceZones",
- "EventService/Subscriptions",
- "Fabrics",
- "Facilities",
- "JobService/Jobs",
- "JobService/Log/Entries",
- "KeyService/NVMeoFKeyPolicies",
- "KeyService/NVMeoFSecrets",
- "LicenseService/Licenses",
- "Managers",
- "NVMeDomains",
- "PowerEquipment/ElectricalBuses",
- "PowerEquipment/FloorPDUs",
- "PowerEquipment/PowerShelves",
- "PowerEquipment/RackPDUs",
- "PowerEquipment/Switchgear",
- "PowerEquipment/TransferSwitches",
- "RegisteredClients",
- "Registries",
- "ResourceBlocks",
- "Storage",
- "StorageServices",
- "StorageSystems",
- "Systems",
- "TaskService/Tasks",
- "TelemetryService/LogService/Entries",
- "TelemetryService/MetricDefinitions",
- "TelemetryService/MetricReportDefinitions",
- "TelemetryService/MetricReports",
- "TelemetryService/Triggers",
- "UpdateService/ClientCertificates",
- "UpdateService/FirmwareInventory",
- "UpdateService/RemoteServerCertificates",
- "UpdateService/SoftwareInventory",
+ "/AggregationService/Aggregates",
+ "/AggregationService/AggregationSources",
+ "/AggregationService/ConnectionMethods",
+ "/Cables",
+ "/Chassis",
+ "/ComponentIntegrity",
+ "/CompositionService/ActivePool",
+ "/CompositionService/CompositionReservations",
+ "/CompositionService/FreePool",
+ "/CompositionService/ResourceBlocks",
+ "/CompositionService/ResourceZones",
+ "/EventService/Subscriptions",
+ "/Fabrics",
+ "/Facilities",
+ "/JobService/Jobs",
+ "/JobService/Log/Entries",
+ "/KeyService/NVMeoFKeyPolicies",
+ "/KeyService/NVMeoFSecrets",
+ "/LicenseService/Licenses",
+ "/Managers",
+ "/NVMeDomains",
+ "/PowerEquipment/ElectricalBuses",
+ "/PowerEquipment/FloorPDUs",
+ "/PowerEquipment/PowerShelves",
+ "/PowerEquipment/RackPDUs",
+ "/PowerEquipment/Switchgear",
+ "/PowerEquipment/TransferSwitches",
+ "/RegisteredClients",
+ "/Registries",
+ "/ResourceBlocks",
+ "/Storage",
+ "/StorageServices",
+ "/StorageSystems",
+ "/Systems",
+ "/TaskService/Tasks",
+ "/TelemetryService/LogService/Entries",
+ "/TelemetryService/MetricDefinitions",
+ "/TelemetryService/MetricReportDefinitions",
+ "/TelemetryService/MetricReports",
+ "/TelemetryService/Triggers",
+ "/UpdateService/ClientCertificates",
+ "/UpdateService/FirmwareInventory",
+ "/UpdateService/RemoteServerCertificates",
+ "/UpdateService/SoftwareInventory",
};
} // namespace redfish
diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp
index 293dad2491..bd56d680fd 100644
--- a/redfish-core/include/redfish_aggregator.hpp
+++ b/redfish-core/include/redfish_aggregator.hpp
@@ -72,53 +72,66 @@ static void addPrefixToItem(nlohmann::json& item, std::string_view prefix)
// version mismatches between aggregator and satellite BMCs. For now
// assume that the aggregator has all the schemas and versions that the
// aggregated server has.
- std::string collectionItem;
if (crow::utility::readUrlSegments(thisUrl, "redfish", "v1", "JsonSchemas",
- std::ref(collectionItem),
crow::utility::OrMorePaths()))
{
BMCWEB_LOG_DEBUG << "Skipping JsonSchemas URI prefix fixing";
return;
}
- // We don't need to add prefixes to these URIs since
- // /redfish/v1/UpdateService/ itself is not a collection:
- // /redfish/v1/UpdateService/FirmwareInventory
- // /redfish/v1/UpdateService/SoftwareInventory
- if (crow::utility::readUrlSegments(thisUrl, "redfish", "v1",
- "UpdateService", "FirmwareInventory") ||
- crow::utility::readUrlSegments(thisUrl, "redfish", "v1",
- "UpdateService", "SoftwareInventory"))
+ // The first two segments should be "/redfish/v1". We need to check that
+ // before we can search topCollections
+ if (!crow::utility::readUrlSegments(thisUrl, "redfish", "v1",
+ crow::utility::OrMorePaths()))
{
- BMCWEB_LOG_DEBUG << "Skipping UpdateService URI prefix fixing";
return;
}
- // We need to add a prefix to FirmwareInventory and SoftwareInventory
- // resources:
- // /redfish/v1/UpdateService/FirmwareInventory/<id>
- // /redfish/v1/UpdateService/SoftwareInventory/<id>
- std::string collectionName;
- if (crow::utility::readUrlSegments(
- thisUrl, "redfish", "v1", "UpdateService", std::ref(collectionName),
- std::ref(collectionItem), crow::utility::OrMorePaths()))
+ // Check array adding a segment each time until collection is identified
+ // Add prefix to segment after the collection
+ const boost::urls::segments_view urlSegments = thisUrl.segments();
+ bool addedPrefix = false;
+ boost::urls::url url("/");
+ boost::urls::segments_view::iterator it = urlSegments.begin();
+ const boost::urls::segments_view::const_iterator end = urlSegments.end();
+
+ // Skip past the leading "/redfish/v1"
+ it++;
+ it++;
+ for (; it != end; it++)
{
- collectionItem.insert(0, "_");
- collectionItem.insert(0, prefix);
- item = crow::utility::replaceUrlSegment(thisUrl, 4, collectionItem);
- return;
+ // Trailing "/" will result in an empty segment. In that case we need
+ // to return so we don't apply a prefix to top level collections such
+ // as "/redfish/v1/Chassis/"
+ if ((*it).empty())
+ {
+ return;
+ }
+
+ if (std::binary_search(topCollections.begin(), topCollections.end(),
+ url.buffer()))
+ {
+ std::string collectionItem(prefix);
+ collectionItem += "_" + (*it);
+ url.segments().push_back(collectionItem);
+ it++;
+ addedPrefix = true;
+ break;
+ }
+
+ url.segments().push_back(*it);
+ }
+
+ // Finish constructing the URL here (if needed) to avoid additional checks
+ for (; it != end; it++)
+ {
+ url.segments().push_back(*it);
}
- // If we reach here then we need to add a prefix to resource IDs that take
- // the general form of "/redfish/v1/<collection>/<id> such as:
- // /redfish/v1/Chassis/foo
- if (crow::utility::readUrlSegments(
- thisUrl, "redfish", "v1", std::ref(collectionName),
- std::ref(collectionItem), crow::utility::OrMorePaths()))
+ if (addedPrefix)
{
- collectionItem.insert(0, "_");
- collectionItem.insert(0, prefix);
- item = crow::utility::replaceUrlSegment(thisUrl, 3, collectionItem);
+ url.segments().insert(url.segments().begin(), {"redfish", "v1"});
+ item = url;
}
}
@@ -164,12 +177,6 @@ class RedfishAggregator
RedfishAggregator()
{
- // TODO: Remove in future patches, this is here to prevent compiler
- // warnings and get an accurate idea of the increase in the size of the
- // binary.
- BMCWEB_LOG_DEBUG << "There are " << topCollections.size()
- << " top level collections:";
-
getSatelliteConfigs(constructorCallback);
// Setup the retry policy to be used by Redfish Aggregation
@@ -776,7 +783,7 @@ class RedfishAggregator
{
using crow::utility::OrMorePaths;
using crow::utility::readUrlSegments;
- const boost::urls::url_view& url = thisReq.urlView;
+ const boost::urls::url_view url = thisReq.urlView;
// UpdateService is the only top level resource that is not a Collection
if (readUrlSegments(url, "redfish", "v1", "UpdateService"))
{
diff --git a/scripts/generate_schema_collections.py b/scripts/generate_schema_collections.py
index c9f8b5956d..d6efa0efe2 100755
--- a/scripts/generate_schema_collections.py
+++ b/scripts/generate_schema_collections.py
@@ -266,17 +266,17 @@ def generate_top_collections():
"\n"
"namespace redfish\n"
"{{\n"
- '// Note that each URI actually begins with "/redfish/v1/"\n'
+ '// Note that each URI actually begins with "/redfish/v1"\n'
"// They've been omitted to save space and reduce search time\n"
"constexpr std::array<std::string_view, {TOTAL}> "
"topCollections{{\n".format(WARNING=WARNING, TOTAL=TOTAL)
)
for collection in sorted(top_collections):
- # All URIs start with "/redfish/v1/". We can omit that portion to
+ # All URIs start with "/redfish/v1". We can omit that portion to
# save memory and reduce lookup time
hpp_file.write(
- ' "{}",\n'.format(collection.split("/redfish/v1/")[1])
+ ' "{}",\n'.format(collection.split("/redfish/v1")[1])
)
hpp_file.write("};\n} // namespace redfish\n")
diff --git a/test/redfish-core/include/redfish_aggregator_test.cpp b/test/redfish-core/include/redfish_aggregator_test.cpp
index e5c718148b..1b3f9244be 100644
--- a/test/redfish-core/include/redfish_aggregator_test.cpp
+++ b/test/redfish-core/include/redfish_aggregator_test.cpp
@@ -38,4 +38,102 @@ TEST(IsPropertyUri, UnsupportedPropertyReturnsFalse)
EXPECT_FALSE(isPropertyUri("Id"));
}
+TEST(addPrefixToItem, ValidURIs)
+{
+ nlohmann::json jsonRequest;
+ constexpr std::array validRoots{"Cables",
+ "Chassis",
+ "Fabrics",
+ "PowerEquipment/FloorPDUs",
+ "Systems",
+ "TaskService/Tasks",
+ "TelemetryService/LogService/Entries",
+ "UpdateService/SoftwareInventory"};
+
+ // We're only testing prefix fixing so it's alright that some of the
+ // resulting URIs will not actually be possible as defined by the schema
+ constexpr std::array validIDs{"1",
+ "1/",
+ "Test",
+ "Test/",
+ "Extra_Test",
+ "Extra_Test/",
+ "Extra_Test/Sensors",
+ "Extra_Test/Sensors/",
+ "Extra_Test/Sensors/power_sensor",
+ "Extra_Test/Sensors/power_sensor/"};
+
+ // Construct URIs which should have prefix fixing applied
+ for (const auto& root : validRoots)
+ {
+ for (const auto& id : validIDs)
+ {
+ std::string initial("/redfish/v1/" + std::string(root) + "/");
+ std::string correct(initial + "asdfjkl_" + std::string(id));
+ initial += id;
+ jsonRequest["@odata.id"] = initial;
+ addPrefixToItem(jsonRequest["@odata.id"], "asdfjkl");
+ EXPECT_EQ(jsonRequest["@odata.id"], correct);
+ }
+ }
+}
+
+TEST(addPrefixToItem, UnsupportedURIs)
+{
+ nlohmann::json jsonRequest;
+ constexpr std::array invalidRoots{
+ "FakeCollection", "JsonSchemas",
+ "PowerEquipment", "TaskService",
+ "TelemetryService/Entries", "UpdateService"};
+
+ constexpr std::array validIDs{"1",
+ "1/",
+ "Test",
+ "Test/",
+ "Extra_Test",
+ "Extra_Test/",
+ "Extra_Test/Sensors",
+ "Extra_Test/Sensors/",
+ "Extra_Test/Sensors/power_sensor",
+ "Extra_Test/Sensors/power_sensor/"};
+
+ // Construct URIs which should NOT have prefix fixing applied
+ for (const auto& root : invalidRoots)
+ {
+ for (const auto& id : validIDs)
+ {
+ std::string initial("/redfish/v1/" + std::string(root) + "/");
+ std::string correct(initial + "asdfjkl_" + std::string(id));
+ initial += id;
+ jsonRequest["@odata.id"] = initial;
+ addPrefixToItem(jsonRequest["@odata.id"], "asdfjkl");
+ EXPECT_EQ(jsonRequest["@odata.id"], initial);
+ }
+ }
+}
+
+TEST(addPrefixToItem, TopLevelCollections)
+{
+ nlohmann::json jsonRequest;
+ constexpr std::array validRoots{"Cables",
+ "Chassis/",
+ "Fabrics",
+ "JsonSchemas",
+ "PowerEquipment/FloorPDUs",
+ "Systems",
+ "TaskService/Tasks",
+ "TelemetryService/LogService/Entries",
+ "TelemetryService/LogService/Entries/",
+ "UpdateService/SoftwareInventory/"};
+
+ // Construct URIs for top level collections. Prefixes should NOT be
+ // applied to any of the URIs
+ for (const auto& root : validRoots)
+ {
+ std::string initial("/redfish/v1/" + std::string(root));
+ jsonRequest["@odata.id"] = initial;
+ addPrefixToItem(jsonRequest["@odata.id"], "perfix");
+ EXPECT_EQ(jsonRequest["@odata.id"], initial);
+ }
+}
} // namespace redfish