From ed4de7a8a5601b58b2e29824b10bf7fcc699a1b8 Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Tue, 26 Mar 2024 17:30:42 -0700 Subject: Add type safety for NTP server objects NTPServers is our last usage of nlohmann::json in a readJson unpack. The capability and unit tests are left in place for that type in case we need them in the future, but for now, document them as deprecated. Tested: Redfish service validator passes. Redfish protocol validator passes most tests (1 known failure in SSE is unrelated to this change). Change-Id: If4b2ea061a941cc23d47189af7ff453094dc7dca Signed-off-by: Ed Tanous --- redfish-core/include/utils/json_utils.hpp | 33 ++++++++++++++++++++++----- redfish-core/lib/network_protocol.hpp | 37 ++++++++++++++++++------------- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp index d7d87a60cf..a013847872 100644 --- a/redfish-core/include/utils/json_utils.hpp +++ b/redfish-core/include/utils/json_utils.hpp @@ -151,7 +151,7 @@ UnpackErrorCode unpackValueVariant(nlohmann::json& j, std::string_view key, { if constexpr (Index < std::variant_size_v>) { - std::variant_alternative_t> type; + std::variant_alternative_t> type{}; UnpackErrorCode unpack = unpackValueWithErrorCode(j, key, type); if (unpack == UnpackErrorCode::success) { @@ -235,6 +235,23 @@ UnpackErrorCode unpackValueWithErrorCode(nlohmann::json& jsonValue, return UnpackErrorCode::invalidType; } } + else if constexpr (IsVector::value) + { + nlohmann::json::object_t* obj = + jsonValue.get_ptr(); + if (obj == nullptr) + { + return UnpackErrorCode::invalidType; + } + + for (const auto& val : *obj) + { + value.emplace_back(); + ret = unpackValueWithErrorCode( + val, key, value.back()) && + ret; + } + } else { using JsonType = std::add_const_t>; @@ -403,7 +420,6 @@ using UnpackVariant = std::variant< bool*, double*, std::string*, - nlohmann::json*, nlohmann::json::object_t*, std::variant*, std::variant*, @@ -425,7 +441,6 @@ using UnpackVariant = std::variant< //std::vector*, std::vector*, std::vector*, - std::vector*, std::vector*, std::optional*, std::optional*, @@ -437,7 +452,6 @@ using UnpackVariant = std::variant< std::optional*, std::optional*, std::optional*, - std::optional*, std::optional*, std::optional>*, std::optional>*, @@ -449,7 +463,6 @@ using UnpackVariant = std::variant< //std::optional>*, std::optional>*, std::optional>*, - std::optional>*, std::optional>*, std::optional>*, std::optional>*, @@ -462,7 +475,15 @@ using UnpackVariant = std::variant< std::optional>*, std::optional>*, std::optional>>*, - std::optional>* + std::optional>>*, + + // Note, these types are kept for historical completeness, but should not be used, + // As they do not provide object type safety. Instead, rely on nlohmann::json::object_t + // Will be removed Q2 2025 + nlohmann::json*, + std::optional>*, + std::vector*, + std::optional* >; // clang-format on diff --git a/redfish-core/lib/network_protocol.hpp b/redfish-core/lib/network_protocol.hpp index 00b5d9e309..747abbf97d 100644 --- a/redfish-core/lib/network_protocol.hpp +++ b/redfish-core/lib/network_protocol.hpp @@ -259,17 +259,24 @@ inline void handleNTPProtocolEnabled( "TimeSyncMethod", "NTP/ProtocolEnabled", timeSyncMethod); } +// Redfish states that ip addresses can be +// string, to set a value +// null, to delete the value +// object_t, empty json object, to ignore the value +using IpAddress = + std::variant; + inline void handleNTPServersPatch(const std::shared_ptr& asyncResp, - const std::vector& ntpServerObjects, + const std::vector& ntpServerObjects, std::vector currentNtpServers) { std::vector::iterator currentNtpServer = currentNtpServers.begin(); for (size_t index = 0; index < ntpServerObjects.size(); index++) { - const nlohmann::json& ntpServer = ntpServerObjects[index]; - if (ntpServer.is_null()) + const IpAddress& ntpServer = ntpServerObjects[index]; + if (std::holds_alternative(ntpServer)) { // Can't delete an item that doesn't exist if (currentNtpServer == currentNtpServers.end()) @@ -284,22 +291,22 @@ inline void continue; } const nlohmann::json::object_t* ntpServerObject = - ntpServer.get_ptr(); + std::get_if(&ntpServer); if (ntpServerObject != nullptr) { if (!ntpServerObject->empty()) { - messages::propertyValueNotInList(asyncResp->res, ntpServer, - "NTP/NTPServers/" + - std::to_string(index)); + messages::propertyValueNotInList( + asyncResp->res, *ntpServerObject, + "NTP/NTPServers/" + std::to_string(index)); return; } // Can't retain an item that doesn't exist if (currentNtpServer == currentNtpServers.end()) { - messages::propertyValueOutOfRange(asyncResp->res, ntpServer, - "NTP/NTPServers/" + - std::to_string(index)); + messages::propertyValueOutOfRange( + asyncResp->res, *ntpServerObject, + "NTP/NTPServers/" + std::to_string(index)); return; } @@ -308,13 +315,10 @@ inline void continue; } - const std::string* ntpServerStr = - ntpServer.get_ptr(); + const std::string* ntpServerStr = std::get_if(&ntpServer); if (ntpServerStr == nullptr) { - messages::propertyValueTypeError(asyncResp->res, ntpServer, - "NTP/NTPServers/" + - std::to_string(index)); + messages::internalError(asyncResp->res); return; } if (currentNtpServer == currentNtpServers.end()) @@ -470,7 +474,8 @@ inline void handleManagersNetworkProtocolPatch( return; } std::optional newHostName; - std::optional> ntpServerObjects; + + std::optional> ntpServerObjects; std::optional ntpEnabled; std::optional ipmiEnabled; std::optional sshEnabled; -- cgit v1.2.3