Clean up Ethernet to use readJson
Today, patching ethernet ip address arrays can use several
styles.
IpAddresses: [{}, {value: value}, null]
All 3 of those elements are legal. Today, we unpack values like that
with nlohmann::json, then iterate and unpack further. This leads to
problems where:
IpAddresses: [{}, {value: value}, 1.0]
would have the same behavior as the prior, given that we check for
"is_object()" to determine the null state. This is messy at best, and
not typesafe at worst.
Changing this code to use the new class NullOr<> allows the readJson
parser to fail the second example.
Change-Id: Id91f48bb64271dd568041a7c0b1ad285b59d5674
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index dd3473d..ee84c14 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -32,11 +32,13 @@
#include <boost/url/format.hpp>
#include <array>
+#include <cstddef>
#include <memory>
#include <optional>
#include <ranges>
#include <regex>
#include <string_view>
+#include <variant>
#include <vector>
namespace redfish
@@ -999,7 +1001,8 @@
*/
inline void handleIPv6DefaultGateway(
- const std::string& ifaceId, const nlohmann::json::array_t& input,
+ const std::string& ifaceId,
+ std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>& input,
const std::vector<StaticGatewayData>& staticGatewayData,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
@@ -1007,7 +1010,8 @@
std::vector<StaticGatewayData>::const_iterator staticGatewayEntry =
staticGatewayData.begin();
- for (const nlohmann::json& thisJson : input)
+ for (std::variant<nlohmann::json::object_t, std::nullptr_t>& thisJson :
+ input)
{
// find the next gateway entry
while (staticGatewayEntry != staticGatewayData.end())
@@ -1021,7 +1025,9 @@
}
std::string pathString = "IPv6StaticDefaultGateways/" +
std::to_string(entryIdx);
- if (thisJson.is_null())
+ nlohmann::json::object_t* obj =
+ std::get_if<nlohmann::json::object_t>(&thisJson);
+ if (obj == nullptr)
{
if (staticGatewayEntry == staticGatewayData.end())
{
@@ -1031,12 +1037,12 @@
deleteIPv6Gateway(staticGatewayEntry->id, asyncResp);
return;
}
- if (thisJson.is_object() && thisJson.empty())
+ if (obj->empty())
{
// Do nothing, but make sure the entry exists.
if (staticGatewayEntry == staticGatewayData.end())
{
- messages::propertyValueFormatError(asyncResp->res, thisJson,
+ messages::propertyValueFormatError(asyncResp->res, *obj,
pathString);
return;
}
@@ -1044,9 +1050,8 @@
std::optional<std::string> address;
std::optional<size_t> prefixLength;
- nlohmann::json thisJsonCopy = thisJson;
- if (!json_util::readJson(thisJsonCopy, asyncResp->res, "Address",
- address, "PrefixLength", prefixLength))
+ if (!json_util::readJsonObject(*obj, asyncResp->res, "Address", address,
+ "PrefixLength", prefixLength))
{
return;
}
@@ -1111,7 +1116,7 @@
[ethifaceId{std::string{ethifaceId}},
callback{std::forward<CallbackFunc>(callback)}](
const boost::system::error_code& ec,
- const dbus::utility::ManagedObjectType& resp) {
+ const dbus::utility::ManagedObjectType& resp) mutable {
EthernetInterfaceData ethData{};
std::vector<IPv4AddressData> ipv4Data;
std::vector<IPv6AddressData> ipv6Data;
@@ -1484,19 +1489,12 @@
});
}
-inline void
- handleIPv4StaticPatch(const std::string& ifaceId,
- nlohmann::json::array_t& input,
- const std::vector<IPv4AddressData>& ipv4Data,
- const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
+inline void handleIPv4StaticPatch(
+ const std::string& ifaceId,
+ std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>& input,
+ const std::vector<IPv4AddressData>& ipv4Data,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
- if (input.empty())
- {
- messages::propertyValueTypeError(asyncResp->res, input,
- "IPv4StaticAddresses");
- return;
- }
-
unsigned entryIdx = 1;
// Find the first static IP address currently active on the NIC and
// match it to the first JSON element in the IPv4StaticAddresses array.
@@ -1505,22 +1503,24 @@
std::vector<IPv4AddressData>::const_iterator nicIpEntry =
getNextStaticIpEntry(ipv4Data.cbegin(), ipv4Data.cend());
- for (nlohmann::json& thisJson : input)
+ for (std::variant<nlohmann::json::object_t, std::nullptr_t>& thisJson :
+ input)
{
std::string pathString = "IPv4StaticAddresses/" +
std::to_string(entryIdx);
-
- if (!thisJson.is_null() && !thisJson.empty())
+ nlohmann::json::object_t* obj =
+ std::get_if<nlohmann::json::object_t>(&thisJson);
+ if (obj != nullptr && !obj->empty())
{
std::optional<std::string> address;
std::optional<std::string> subnetMask;
std::optional<std::string> gateway;
- if (!json_util::readJson(thisJson, asyncResp->res, "Address",
- address, "SubnetMask", subnetMask,
- "Gateway", gateway))
+ if (!json_util::readJsonObject(*obj, asyncResp->res, "Address",
+ address, "SubnetMask", subnetMask,
+ "Gateway", gateway))
{
- messages::propertyValueFormatError(asyncResp->res, thisJson,
+ messages::propertyValueFormatError(asyncResp->res, *obj,
pathString);
return;
}
@@ -1621,17 +1621,17 @@
// Requesting a DELETE/DO NOT MODIFY action for an item
// that isn't present on the eth(n) interface. Input JSON is
// in error, so bail out.
- if (thisJson.is_null())
+ if (obj == nullptr)
{
messages::resourceCannotBeDeleted(asyncResp->res);
return;
}
- messages::propertyValueFormatError(asyncResp->res, thisJson,
+ messages::propertyValueFormatError(asyncResp->res, *obj,
pathString);
return;
}
- if (thisJson.is_null())
+ if (obj == nullptr)
{
deleteIPAddress(ifaceId, nicIpEntry->id, asyncResp);
}
@@ -1659,33 +1659,31 @@
}
inline void handleIPv6StaticAddressesPatch(
- const std::string& ifaceId, const nlohmann::json::array_t& input,
+ const std::string& ifaceId,
+ std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>& input,
const std::vector<IPv6AddressData>& ipv6Data,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
- if (input.empty())
- {
- messages::propertyValueTypeError(asyncResp->res, input,
- "IPv6StaticAddresses");
- return;
- }
size_t entryIdx = 1;
std::vector<IPv6AddressData>::const_iterator nicIpEntry =
getNextStaticIpEntry(ipv6Data.cbegin(), ipv6Data.cend());
- for (const nlohmann::json& thisJson : input)
+ for (std::variant<nlohmann::json::object_t, std::nullptr_t>& thisJson :
+ input)
{
std::string pathString = "IPv6StaticAddresses/" +
std::to_string(entryIdx);
-
- if (!thisJson.is_null() && !thisJson.empty())
+ nlohmann::json::object_t* obj =
+ std::get_if<nlohmann::json::object_t>(&thisJson);
+ if (obj != nullptr && !obj->empty())
{
std::optional<std::string> address;
std::optional<uint8_t> prefixLength;
- nlohmann::json thisJsonCopy = thisJson;
- if (!json_util::readJson(thisJsonCopy, asyncResp->res, "Address",
- address, "PrefixLength", prefixLength))
+ nlohmann::json::object_t thisJsonCopy = *obj;
+ if (!json_util::readJsonObject(thisJsonCopy, asyncResp->res,
+ "Address", address, "PrefixLength",
+ prefixLength))
{
- messages::propertyValueFormatError(asyncResp->res, thisJson,
+ messages::propertyValueFormatError(asyncResp->res, thisJsonCopy,
pathString);
return;
}
@@ -1737,17 +1735,17 @@
// Requesting a DELETE/DO NOT MODIFY action for an item
// that isn't present on the eth(n) interface. Input JSON is
// in error, so bail out.
- if (thisJson.is_null())
+ if (obj == nullptr)
{
messages::resourceCannotBeDeleted(asyncResp->res);
return;
}
- messages::propertyValueFormatError(asyncResp->res, thisJson,
+ messages::propertyValueFormatError(asyncResp->res, *obj,
pathString);
return;
}
- if (thisJson.is_null())
+ if (obj == nullptr)
{
deleteIPAddress(ifaceId, nicIpEntry->id, asyncResp);
}
@@ -2074,7 +2072,7 @@
bool vlanEnable = false;
uint32_t vlanId = 0;
- nlohmann::json::array_t relatedInterfaces;
+ std::vector<nlohmann::json::object_t> relatedInterfaces;
if (!json_util::readJsonPatch(req, asyncResp->res, "VLAN/VLANEnable",
vlanEnable, "VLAN/VLANId", vlanId,
@@ -2093,8 +2091,8 @@
}
std::string parentInterfaceUri;
- if (!json_util::readJson(relatedInterfaces[0], asyncResp->res,
- "@odata.id", parentInterfaceUri))
+ if (!json_util::readJsonObject(relatedInterfaces[0], asyncResp->res,
+ "@odata.id", parentInterfaceUri))
{
messages::propertyMissing(asyncResp->res,
"Links/RelatedInterfaces/0/@odata.id");
@@ -2197,22 +2195,31 @@
std::optional<std::string> fqdn;
std::optional<std::string> macAddress;
std::optional<std::string> ipv6DefaultGateway;
- std::optional<nlohmann::json::array_t> ipv4StaticAddresses;
- std::optional<nlohmann::json::array_t> ipv6StaticAddresses;
- std::optional<nlohmann::json::array_t> ipv6StaticDefaultGateways;
+ std::optional<
+ std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>>
+ ipv4StaticAddresses;
+ std::optional<
+ std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>>
+ ipv6StaticAddresses;
+ std::optional<
+ std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>>
+ ipv6StaticDefaultGateways;
std::optional<std::vector<std::string>> staticNameServers;
- std::optional<nlohmann::json> dhcpv4;
- std::optional<nlohmann::json> dhcpv6;
std::optional<bool> ipv6AutoConfigEnabled;
std::optional<bool> interfaceEnabled;
std::optional<size_t> mtuSize;
DHCPParameters v4dhcpParms;
DHCPParameters v6dhcpParms;
// clang-format off
- if (!json_util::readJsonPatch(
- req, asyncResp->res,
- "DHCPv4", dhcpv4,
- "DHCPv6", dhcpv6,
+ if (!json_util::readJsonPatch(req, asyncResp->res,
+ "DHCPv4/DHCPEnabled", v4dhcpParms.dhcpv4Enabled,
+ "DHCPv4/UseDNSServers", v4dhcpParms.useDnsServers,
+ "DHCPv4/UseDomainName", v4dhcpParms.useDomainName,
+ "DHCPv4/UseNTPServers", v4dhcpParms.useNtpServers,
+ "DHCPv6/OperatingMode", v6dhcpParms.dhcpv6OperatingMode,
+ "DHCPv6/UseDNSServers", v6dhcpParms.useDnsServers,
+ "DHCPv6/UseDomainName", v6dhcpParms.useDomainName,
+ "DHCPv6/UseNTPServers", v6dhcpParms.useNtpServers,
"FQDN", fqdn,
"HostName", hostname,
"IPv4StaticAddresses", ipv4StaticAddresses,
@@ -2230,30 +2237,6 @@
return;
}
// clang-format on
- if (dhcpv4)
- {
- if (!json_util::readJson(*dhcpv4, asyncResp->res, "DHCPEnabled",
- v4dhcpParms.dhcpv4Enabled, "UseDNSServers",
- v4dhcpParms.useDnsServers, "UseNTPServers",
- v4dhcpParms.useNtpServers, "UseDomainName",
- v4dhcpParms.useDomainName))
- {
- return;
- }
- }
-
- if (dhcpv6)
- {
- if (!json_util::readJson(*dhcpv6, asyncResp->res, "OperatingMode",
- v6dhcpParms.dhcpv6OperatingMode,
- "UseDNSServers", v6dhcpParms.useDnsServers,
- "UseNTPServers", v6dhcpParms.useNtpServers,
- "UseDomainName",
- v6dhcpParms.useDomainName))
- {
- return;
- }
- }
// Get single eth interface data, and call the below callback
// for JSON preparation
@@ -2265,14 +2248,13 @@
ipv6DefaultGateway = std::move(ipv6DefaultGateway),
ipv6StaticAddresses = std::move(ipv6StaticAddresses),
ipv6StaticDefaultGateway = std::move(ipv6StaticDefaultGateways),
- staticNameServers = std::move(staticNameServers),
- dhcpv4 = std::move(dhcpv4), dhcpv6 = std::move(dhcpv6), mtuSize,
+ staticNameServers = std::move(staticNameServers), mtuSize,
ipv6AutoConfigEnabled, v4dhcpParms = std::move(v4dhcpParms),
v6dhcpParms = std::move(v6dhcpParms), interfaceEnabled](
- const bool& success, const EthernetInterfaceData& ethData,
+ const bool success, const EthernetInterfaceData& ethData,
const std::vector<IPv4AddressData>& ipv4Data,
const std::vector<IPv6AddressData>& ipv6Data,
- const std::vector<StaticGatewayData>& ipv6GatewayData) {
+ const std::vector<StaticGatewayData>& ipv6GatewayData) mutable {
if (!success)
{
// ... otherwise return error
@@ -2283,11 +2265,8 @@
return;
}
- if (dhcpv4 || dhcpv6)
- {
- handleDHCPPatch(ifaceId, ethData, v4dhcpParms, v6dhcpParms,
- asyncResp);
- }
+ handleDHCPPatch(ifaceId, ethData, v4dhcpParms, v6dhcpParms,
+ asyncResp);
if (hostname)
{
@@ -2312,15 +2291,8 @@
if (ipv4StaticAddresses)
{
- // TODO(ed) for some reason the capture of
- // ipv4Addresses above is returning a const value,
- // not a non-const value. This doesn't really work
- // for us, as we need to be able to efficiently move
- // out the intermedia nlohmann::json objects. This
- // makes a copy of the structure, and operates on
- // that, but could be done more efficiently
- nlohmann::json::array_t ipv4Static = *ipv4StaticAddresses;
- handleIPv4StaticPatch(ifaceId, ipv4Static, ipv4Data, asyncResp);
+ handleIPv4StaticPatch(ifaceId, *ipv4StaticAddresses, ipv4Data,
+ asyncResp);
}
if (staticNameServers)