Redfish(Network): Support for deletion of IP address through PATCH operation

Before this commit we were reading the IPV4address through readJSON
function in vector of json, readJSON doesn't like the null list
parameter and throws the error as below

"The value null for the property IPv4Addresses is of a different type
than the property can accept"

This commit fixes that problem and some restructuring the code to make
delete work.

TestedBy:
PATCH {"IPv4Addresses": [{},null,{},{}]}
   deletes second object
PATCH {"IPv4Addresses": [null,{}]}
   deletes first object

Suppose if the IPv4Address has two addresses then the
Following PATCH request would give the folowing error.

PATCH {"IPv4Addresses": [{},{},null]
{
  "IPv4Addresses/0@Message.ExtendedInfo": [
    {
      "@odata.type": "/redfish/v1/$metadata#Message.v1_0_0.Message",
      "Message": "The value [{},{},null] for the property IPv4Addresses/2 is of a
                  different format than the property can accept.",
      "MessageArgs": [
        "[{},{},null]",
        "IPv4Addresses/2"
      ],
}

Change-Id: Iaefd59de469cb345e86b19349b27567a4fcec3fa
Signed-off-by: Ratan Gupta <ratagupt@linux.vnet.ibm.com>

1 files changed, 80 insertions, 76 deletions

diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index c98c8f4b12..2a0b618d8b 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -965,10 +965,17 @@ class EthernetInterface : public Node
     }
 
     void handleIPv4Patch(
-        const std::string &ifaceId, std::vector<nlohmann::json> &input,
+        const std::string &ifaceId, nlohmann::json &input,
         const boost::container::flat_set<IPv4AddressData> &ipv4Data,
         const std::shared_ptr<AsyncResp> asyncResp)
     {
+        if (!input.is_array())
+        {
+            messages::propertyValueTypeError(asyncResp->res, input.dump(),
+                                             "IPv4Addresses");
+            return;
+        }
+
         int entryIdx = 0;
         boost::container::flat_set<IPv4AddressData>::const_iterator thisData =
             ipv4Data.begin();
@@ -977,6 +984,28 @@ class EthernetInterface : public Node
             std::string pathString =
                 "IPv4Addresses/" + std::to_string(entryIdx);
 
+            if (thisJson.is_null())
+            {
+                if (thisData != ipv4Data.end())
+                {
+                    deleteIPv4(ifaceId, thisData->id, entryIdx, asyncResp);
+                    thisData++;
+                }
+                else
+                {
+                    messages::propertyValueFormatError(
+                        asyncResp->res, input.dump(), pathString);
+                    return;
+                    // TODO(ratagupt) Not sure about the property where value is
+                    // list and if unable to update one of the
+                    // list value then should we proceed further or
+                    // break there, would ask in the redfish forum
+                    // till then we stop processing the next list item.
+                }
+                entryIdx++;
+                continue; // not an error as per the redfish spec.
+            }
+
             if (thisJson.empty())
             {
                 if (thisData != ipv4Data.end())
@@ -988,7 +1017,7 @@ class EthernetInterface : public Node
                     messages::propertyMissing(asyncResp->res,
                                               pathString + "/Address");
                     return;
-                    // TODO Not sure about the property where value is
+                    // TODO(ratagupt) Not sure about the property where value is
                     // list and if unable to update one of the
                     // list value then should we proceed further or
                     // break there, would ask in the redfish forum
@@ -1057,96 +1086,72 @@ class EthernetInterface : public Node
                 }
             }
 
-            // if a vlan already exists, modify the existing
+            // if IP address exist then  modify it.
             if (thisData != ipv4Data.end())
             {
-                // Existing object that should be modified/deleted/remain
-                // unchanged
-                if (thisJson.is_null())
+                // Apply changes
+                if (address)
                 {
-                    auto callback = [entryIdx{std::to_string(entryIdx)},
-                                     asyncResp](
+                    auto callback = [asyncResp, entryIdx,
+                                     address{std::string(*address)}](
                                         const boost::system::error_code ec) {
                         if (ec)
                         {
                             messages::internalError(asyncResp->res);
                             return;
                         }
-                        asyncResp->res.jsonValue["IPv4Addresses"][entryIdx] =
-                            nullptr;
+                        asyncResp->res
+                            .jsonValue["IPv4Addresses"][entryIdx]["Address"] =
+                            std::move(address);
                     };
+
                     crow::connections::systemBus->async_method_call(
                         std::move(callback), "xyz.openbmc_project.Network",
                         "/xyz/openbmc_project/network/" + ifaceId + "/ipv4/" +
                             thisData->id,
-                        "xyz.openbmc_project.Object.Delete", "Delete");
+                        "org.freedesktop.DBus.Properties", "Set",
+                        "xyz.openbmc_project.Network.IP", "Address",
+                        std::variant<std::string>(*address));
                 }
-                else if (thisJson.is_object())
+
+                if (subnetMask)
                 {
-                    // Apply changes
-                    if (address)
-                    {
-                        auto callback =
-                            [asyncResp, entryIdx, address = *address](
-                                const boost::system::error_code ec) {
-                                if (ec)
-                                {
-                                    messages::internalError(asyncResp->res);
-                                    return;
-                                }
-                                asyncResp->res.jsonValue["IPv4Addresses"]
-                                                        [entryIdx]["Address"] =
-                                    address;
-                            };
-
-                        crow::connections::systemBus->async_method_call(
-                            std::move(callback), "xyz.openbmc_project.Network",
-                            "/xyz/openbmc_project/network/" + ifaceId +
-                                "/ipv4/" + thisData->id,
-                            "org.freedesktop.DBus.Properties", "Set",
-                            "xyz.openbmc_project.Network.IP", "Address",
-                            std::variant<std::string>(*address));
-                    }
+                    changeIPv4SubnetMaskProperty(ifaceId, entryIdx,
+                                                 thisData->id, *subnetMask,
+                                                 prefixLength, asyncResp);
+                }
 
-                    if (subnetMask)
-                    {
-                        changeIPv4SubnetMaskProperty(ifaceId, entryIdx,
-                                                     thisData->id, *subnetMask,
-                                                     prefixLength, asyncResp);
-                    }
+                if (addressOrigin)
+                {
+                    changeIPv4Origin(ifaceId, entryIdx, thisData->id,
+                                     *addressOrigin, addressOriginInDBusFormat,
+                                     asyncResp);
+                }
 
-                    if (addressOrigin)
-                    {
-                        changeIPv4Origin(ifaceId, entryIdx, thisData->id,
-                                         *addressOrigin,
-                                         addressOriginInDBusFormat, asyncResp);
-                    }
+                if (gateway)
+                {
+                    auto callback = [asyncResp, entryIdx,
+                                     gateway{std::string(*gateway)}](
+                                        const boost::system::error_code ec) {
+                        if (ec)
+                        {
+                            messages::internalError(asyncResp->res);
+                            return;
+                        }
+                        asyncResp->res
+                            .jsonValue["IPv4Addresses"][entryIdx]["Gateway"] =
+                            std::move(gateway);
+                    };
 
-                    if (gateway)
-                    {
-                        auto callback =
-                            [asyncResp, entryIdx,
-                             gateway{std::string(*gateway)}](
-                                const boost::system::error_code ec) {
-                                if (ec)
-                                {
-                                    messages::internalError(asyncResp->res);
-                                    return;
-                                }
-                                asyncResp->res.jsonValue["IPv4Addresses"]
-                                                        [entryIdx]["Gateway"] =
-                                    std::move(gateway);
-                            };
-
-                        crow::connections::systemBus->async_method_call(
-                            std::move(callback), "xyz.openbmc_project.Network",
-                            "/xyz/openbmc_project/network/" + ifaceId +
-                                "/ipv4/" + thisData->id,
-                            "org.freedesktop.DBus.Properties", "Set",
-                            "xyz.openbmc_project.Network.IP", "Gateway",
-                            std::variant<std::string>(*gateway));
-                    }
+                    crow::connections::systemBus->async_method_call(
+                        std::move(callback), "xyz.openbmc_project.Network",
+                        "/xyz/openbmc_project/network/" + ifaceId + "/ipv4/" +
+                            thisData->id,
+                        "org.freedesktop.DBus.Properties", "Set",
+                        "xyz.openbmc_project.Network.IP", "Gateway",
+                        std::variant<std::string>(*gateway));
                 }
+
                 thisData++;
             }
             else
@@ -1297,8 +1302,8 @@ class EthernetInterface : public Node
 
         std::optional<nlohmann::json> vlan;
         std::optional<std::string> hostname;
-        std::optional<std::vector<nlohmann::json>> ipv4Addresses;
-        std::optional<std::vector<nlohmann::json>> ipv6Addresses;
+        std::optional<nlohmann::json> ipv4Addresses;
+        std::optional<nlohmann::json> ipv6Addresses;
 
         if (!json_util::readJson(req, res, "VLAN", vlan, "HostName", hostname,
                                  "IPv4Addresses", ipv4Addresses,
@@ -1375,8 +1380,7 @@ class EthernetInterface : public Node
                     // 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
-                    std::vector<nlohmann::json> ipv4 =
-                        std::move(*ipv4Addresses);
+                    nlohmann::json ipv4 = std::move(*ipv4Addresses);
                     handleIPv4Patch(iface_id, ipv4, ipv4Data, asyncResp);
                 }