config_parser: Split up sections

We can't always combine sections together in network files as sections
like

[Address]
Address=::1/128
Peer=fe80::1
[Address]
Address=::2/128
Peer=fe80::2

Require that they are grouped accordingly. Rewrite the storage logic of
the config parser to support this logical organization.

Change-Id: I34ae1523202f8770fe3dcac010fb6226dd28b9ec
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/src/config_parser.cpp b/src/config_parser.cpp
index 27a7b15..80f71e7 100644
--- a/src/config_parser.cpp
+++ b/src/config_parser.cpp
@@ -8,6 +8,7 @@
 #include <stdplus/exception.hpp>
 #include <stdplus/fd/create.hpp>
 #include <stdplus/fd/line.hpp>
+#include <string>
 #include <utility>
 
 namespace phosphor
@@ -50,30 +51,39 @@
     return dir / fmt::format(FMT_COMPILE("{}.netdev"), intf);
 }
 
+const std::string*
+    SectionMap::getLastValueString(std::string_view section,
+                                   std::string_view key) const noexcept
+{
+    auto sit = find(section);
+    if (sit == end())
+    {
+        return nullptr;
+    }
+    for (auto it = sit->second.rbegin(); it != sit->second.rend(); ++it)
+    {
+        auto kit = it->find(key);
+        if (kit == it->end() || kit->second.empty())
+        {
+            continue;
+        }
+        return &kit->second.back();
+    }
+    return nullptr;
+}
+
+std::vector<std::string> SectionMap::getValueStrings(std::string_view section,
+                                                     std::string_view key) const
+{
+    return getValues(section, key,
+                     [](const Value& v) { return std::string(v); });
+}
+
 Parser::Parser(const fs::path& filename)
 {
     setFile(filename);
 }
 
-const ValueList& Parser::getValues(std::string_view section,
-                                   std::string_view key) const noexcept
-{
-    static const ValueList empty;
-    auto sit = sections.find(section);
-    if (sit == sections.end())
-    {
-        return empty;
-    }
-
-    auto kit = sit->second.find(key);
-    if (kit == sit->second.end())
-    {
-        return empty;
-    }
-
-    return kit->second;
-}
-
 inline bool isspace(char c) noexcept
 {
     return c == ' ' || c == '\t';
@@ -136,9 +146,9 @@
         if (it == sections.end())
         {
             std::tie(it, std::ignore) =
-                sections.emplace(Section(s), KeyValuesMap{});
+                sections.emplace(Section(s), KeyValuesMapList{});
         }
-        section = &it->second;
+        section = &it->second.emplace_back();
     }
 
     void pumpKV(std::string_view line)
diff --git a/src/config_parser.hpp b/src/config_parser.hpp
index 7ff20ba..9b203c2 100644
--- a/src/config_parser.hpp
+++ b/src/config_parser.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <filesystem>
+#include <functional>
 #include <optional>
 #include <string>
 #include <string_view>
@@ -35,8 +36,41 @@
 using ValueList = std::vector<Value>;
 using KeyValuesMap =
     std::unordered_map<Key, ValueList, string_hash, std::equal_to<>>;
-using SectionMap =
-    std::unordered_map<Section, KeyValuesMap, string_hash, std::equal_to<>>;
+using KeyValuesMapList = std::vector<KeyValuesMap>;
+using SectionMapInt =
+    std::unordered_map<Section, KeyValuesMapList, string_hash, std::equal_to<>>;
+
+class SectionMap : public SectionMapInt
+{
+  public:
+    const std::string* getLastValueString(std::string_view section,
+                                          std::string_view key) const noexcept;
+    inline auto getValues(std::string_view section, std::string_view key,
+                          auto&& conv) const
+    {
+        std::vector<std::invoke_result_t<decltype(conv), const Value&>> values;
+        auto sit = find(section);
+        if (sit == end())
+        {
+            return values;
+        }
+        for (const auto& secv : sit->second)
+        {
+            auto kit = secv.find(key);
+            if (kit == secv.end())
+            {
+                continue;
+            }
+            for (auto v : kit->second)
+            {
+                values.push_back(conv(v));
+            }
+        }
+        return values;
+    }
+    std::vector<std::string> getValueStrings(std::string_view section,
+                                             std::string_view key) const;
+};
 
 class Parser
 {
@@ -48,13 +82,11 @@
      */
     Parser(const fs::path& filename);
 
-    /** @brief Get the values of the given key and section.
-     *  @param[in] section - section name.
-     *  @param[in] key - key to look for.
-     *  @returns   The ValueList or nullptr if no key + section exists.
-     */
-    const ValueList& getValues(std::string_view section,
-                               std::string_view key) const noexcept;
+    /** @brief Retrieve the map of all values in the file */
+    inline const SectionMap& getMap() const noexcept
+    {
+        return sections;
+    }
 
     /** @brief Determine if there were warnings parsing the file
      *  @return The number of parsing issues in the file
diff --git a/src/dhcp_configuration.cpp b/src/dhcp_configuration.cpp
index 4308f96..ef0a6a6 100644
--- a/src/dhcp_configuration.cpp
+++ b/src/dhcp_configuration.cpp
@@ -85,18 +85,18 @@
     config::Parser parser(config::pathForIntfConf(manager.getConfDir(),
                                                   *interfaceStrList.begin()));
 
-    const auto& values = parser.getValues("DHCP", prop);
-    if (values.empty())
+    auto value = parser.getMap().getLastValueString("DHCP", prop);
+    if (value == nullptr)
     {
         auto msg = fmt::format("Missing config section DHCP[{}]", prop);
         log<level::NOTICE>(msg.c_str(), entry("PROP=%s", prop.c_str()));
         return true;
     }
-    auto ret = config::parseBool(values.back());
+    auto ret = config::parseBool(*value);
     if (!ret.has_value())
     {
-        auto msg = fmt::format("Failed to parse section DHCP[{}]: `{}`", prop,
-                               values.back());
+        auto msg =
+            fmt::format("Failed to parse section DHCP[{}]: `{}`", prop, *value);
         log<level::NOTICE>(msg.c_str(), entry("PROP=%s", prop.c_str()));
     }
     return ret.value_or(true);
diff --git a/src/ethernet_interface.cpp b/src/ethernet_interface.cpp
index db87e8a..99ec98a 100644
--- a/src/ethernet_interface.cpp
+++ b/src/ethernet_interface.cpp
@@ -110,7 +110,8 @@
     {
         MacAddressIntf::macAddress(getMACAddress(intfName));
     }
-    EthernetInterfaceIntf::ntpServers(config.getValues("Network", "NTP"));
+    EthernetInterfaceIntf::ntpServers(
+        config.getMap().getValueStrings("Network", "NTP"));
 
     EthernetInterfaceIntf::linkUp(linkUp());
     EthernetInterfaceIntf::mtu(mtu());
@@ -785,7 +786,7 @@
 {
     EthernetInterfaceIntf::nameservers(getNameServerFromResolvd());
     EthernetInterfaceIntf::staticNameServers(
-        config.getValues("Network", "DNS"));
+        config.getMap().getValueStrings("Network", "DNS"));
 }
 
 ServerList EthernetInterface::getNameServerFromResolvd()
diff --git a/src/util.cpp b/src/util.cpp
index 26d9b47..724eae6 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -366,8 +366,8 @@
 
 bool getIPv6AcceptRA(const config::Parser& config)
 {
-    const auto& values = config.getValues("Network", "IPv6AcceptRA");
-    if (values.empty())
+    auto value = config.getMap().getLastValueString("Network", "IPv6AcceptRA");
+    if (value == nullptr)
     {
         auto msg = fmt::format(
             "Unable to get the value for Network[IPv6AcceptRA] from {}",
@@ -376,23 +376,23 @@
                            entry("FILE=%s", config.getFilename().c_str()));
         return false;
     }
-    auto ret = config::parseBool(values.back());
+    auto ret = config::parseBool(*value);
     if (!ret.has_value())
     {
         auto msg = fmt::format(
             "Failed to parse section Network[IPv6AcceptRA] from {}: `{}`",
-            config.getFilename().native(), values.back());
+            config.getFilename().native(), *value);
         log<level::NOTICE>(msg.c_str(),
                            entry("FILE=%s", config.getFilename().c_str()),
-                           entry("VALUE=%s", values.back().c_str()));
+                           entry("VALUE=%s", value->c_str()));
     }
     return ret.value_or(false);
 }
 
 EthernetInterfaceIntf::DHCPConf getDHCPValue(const config::Parser& config)
 {
-    const auto& values = config.getValues("Network", "DHCP");
-    if (values.empty())
+    const auto value = config.getMap().getLastValueString("Network", "DHCP");
+    if (value == nullptr)
     {
         auto msg =
             fmt::format("Unable to get the value for Network[DHCP] from {}",
@@ -401,22 +401,22 @@
                            entry("FILE=%s", config.getFilename().c_str()));
         return EthernetInterfaceIntf::DHCPConf::none;
     }
-    if (config::icaseeq(values.back(), "ipv4"))
+    if (config::icaseeq(*value, "ipv4"))
     {
         return EthernetInterfaceIntf::DHCPConf::v4;
     }
-    if (config::icaseeq(values.back(), "ipv6"))
+    if (config::icaseeq(*value, "ipv6"))
     {
         return EthernetInterfaceIntf::DHCPConf::v6;
     }
-    auto ret = config::parseBool(values.back());
+    auto ret = config::parseBool(*value);
     if (!ret.has_value())
     {
         auto str = fmt::format("Unable to parse Network[DHCP] from {}: `{}`",
-                               config.getFilename().native(), values.back());
+                               config.getFilename().native(), *value);
         log<level::NOTICE>(str.c_str(),
                            entry("FILE=%s", config.getFilename().c_str()),
-                           entry("VALUE=%s", values.back().c_str()));
+                           entry("VALUE=%s", value->c_str()));
     }
     return ret.value_or(false) ? EthernetInterfaceIntf::DHCPConf::both
                                : EthernetInterfaceIntf::DHCPConf::none;
diff --git a/test/test_config_parser.cpp b/test/test_config_parser.cpp
index 896d61e..909ad7f 100644
--- a/test/test_config_parser.cpp
+++ b/test/test_config_parser.cpp
@@ -65,12 +65,31 @@
                    << "Key=val\nAddress=::/0\n[]\n=\nKey";
         filestream.close();
     }
+
+    void ValidateSectionMap()
+    {
+        EXPECT_THAT(
+            parser.getMap(),
+            testing::ContainerEq(SectionMap(SectionMapInt{
+                {"Match", {{{"Name", {"eth0"}}}}},
+                {"Network",
+                 {
+                     {{"DHCP", {"true"}}},
+                     {{"DHCP", {"false #hi", "yes"}}},
+                     {{"Key", {"val"}}, {"Address", {"::/0"}}},
+                 }},
+                {"DHCP", {{{"ClientIdentifier", {"mac"}}}}},
+                {" SEC ", {{{"'DHCP#'", {"\"#hi\""}}, {"DHCP#", {"ho"}}}}},
+                {"", {{{"", {""}}}}},
+            })));
+    }
 };
 
 TEST_F(TestConfigParser, EmptyObject)
 {
     EXPECT_TRUE(parser.getFilename().empty());
     EXPECT_EQ(0, parser.getWarnings().size());
+    EXPECT_EQ(SectionMap(), parser.getMap());
 }
 
 TEST_F(TestConfigParser, ReadDirectory)
@@ -78,6 +97,7 @@
     parser.setFile("/");
     EXPECT_EQ("/", parser.getFilename());
     EXPECT_EQ(1, parser.getWarnings().size());
+    EXPECT_EQ(SectionMap(), parser.getMap());
 }
 
 TEST_F(TestConfigParser, ReadConfigDataMissingFile)
@@ -85,6 +105,7 @@
     parser.setFile("/no-such-path");
     EXPECT_EQ("/no-such-path", parser.getFilename());
     EXPECT_EQ(1, parser.getWarnings().size());
+    EXPECT_EQ(SectionMap(), parser.getMap());
 }
 
 TEST_F(TestConfigParser, ReadConfigDataFromFile)
@@ -93,15 +114,25 @@
     parser.setFile(filename);
     EXPECT_EQ(filename, parser.getFilename());
     EXPECT_EQ(4, parser.getWarnings().size());
+    ValidateSectionMap();
 
-    EXPECT_THAT(parser.getValues("Match", "Name"), ElementsAre("eth0"));
-    EXPECT_THAT(parser.getValues("DHCP", "ClientIdentifier"),
+    const auto& map = parser.getMap();
+
+    EXPECT_EQ("eth0", *map.getLastValueString("Match", "Name"));
+    EXPECT_EQ("yes", *map.getLastValueString("Network", "DHCP"));
+    EXPECT_EQ(nullptr, map.getLastValueString("Match", "BadKey"));
+    EXPECT_EQ(nullptr, map.getLastValueString("BadSec", "Name"));
+    EXPECT_EQ(nullptr, map.getLastValueString("BadSec", "Name"));
+
+    EXPECT_THAT(map.getValueStrings("Match", "Name"), ElementsAre("eth0"));
+    EXPECT_THAT(map.getValueStrings("DHCP", "ClientIdentifier"),
                 ElementsAre("mac"));
-    EXPECT_THAT(parser.getValues("Network", "DHCP"),
+    EXPECT_THAT(map.getValueStrings("Network", "DHCP"),
                 ElementsAre("true", "false #hi", "yes"));
-    EXPECT_THAT(parser.getValues(" SEC ", "'DHCP#'"), ElementsAre("\"#hi\""));
-    EXPECT_THAT(parser.getValues("Blah", "nil"), ElementsAre());
-    EXPECT_THAT(parser.getValues("Network", "nil"), ElementsAre());
+    EXPECT_THAT(map.getValueStrings(" SEC ", "'DHCP#'"),
+                ElementsAre("\"#hi\""));
+    EXPECT_THAT(map.getValueStrings("Blah", "nil"), ElementsAre());
+    EXPECT_THAT(map.getValueStrings("Network", "nil"), ElementsAre());
 }
 
 TEST_F(TestConfigParser, Perf)
diff --git a/test/test_ethernet_interface.cpp b/test/test_ethernet_interface.cpp
index e9ed3fd..e340d7b 100644
--- a/test/test_ethernet_interface.cpp
+++ b/test/test_ethernet_interface.cpp
@@ -165,7 +165,7 @@
     fs::path filePath = confDir;
     filePath /= "00-bmc-test0.network";
     config::Parser parser(filePath.string());
-    EXPECT_EQ(servers, parser.getValues("Network", "DNS"));
+    EXPECT_EQ(servers, parser.getMap().getValueStrings("Network", "DNS"));
 }
 
 TEST_F(TestEthernetInterface, getDynamicNameServers)
@@ -184,7 +184,7 @@
     fs::path filePath = confDir;
     filePath /= "00-bmc-test0.network";
     config::Parser parser(filePath.string());
-    EXPECT_EQ(servers, parser.getValues("Network", "NTP"));
+    EXPECT_EQ(servers, parser.getMap().getValueStrings("Network", "NTP"));
 }
 
 TEST_F(TestEthernetInterface, addGateway)
diff --git a/test/test_vlan_interface.cpp b/test/test_vlan_interface.cpp
index 5ee1cf9..728199a 100644
--- a/test/test_vlan_interface.cpp
+++ b/test/test_vlan_interface.cpp
@@ -102,27 +102,25 @@
     fs::path filePath = confDir;
     filePath /= "test0.50.netdev";
 
-    config::Parser parser(filePath.string());
-
-    EXPECT_EQ(parser.getValues("NetDev", "Name"),
-              (config::ValueList{"test0.50"}));
-    EXPECT_EQ(parser.getValues("NetDev", "Kind"), (config::ValueList{"vlan"}));
-    EXPECT_EQ(parser.getValues("VLAN", "Id"), (config::ValueList{"50"}));
+    config::Parser parser(filePath);
+    EXPECT_EQ(parser.getMap(),
+              config::SectionMap(config::SectionMapInt{
+                  {"NetDev",
+                   {
+                       {{"Name", {"test0.50"}}, {"Kind", {"vlan"}}},
+                   }},
+                  {"VLAN", {{{"Id", {"50"}}}}},
+              }));
 }
 
 TEST_F(TestVlanInterface, deleteVLAN)
 {
     createVlan(50);
     deleteVlan("test0.50");
-    bool fileFound = false;
 
     fs::path filePath = confDir;
     filePath /= "test0.50.netdev";
-    if (fs::is_regular_file(filePath.string()))
-    {
-        fileFound = true;
-    }
-    EXPECT_EQ(fileFound, false);
+    EXPECT_FALSE(fs::is_regular_file(filePath));
 }
 
 TEST_F(TestVlanInterface, createMultipleVLAN)
@@ -132,19 +130,27 @@
 
     fs::path filePath = confDir;
     filePath /= "test0.50.netdev";
-    config::Parser parser(filePath.string());
-    EXPECT_EQ(parser.getValues("NetDev", "Name"),
-              (config::ValueList{"test0.50"}));
-    EXPECT_EQ(parser.getValues("NetDev", "Kind"), (config::ValueList{"vlan"}));
-    EXPECT_EQ(parser.getValues("VLAN", "Id"), (config::ValueList{"50"}));
+    config::Parser parser(filePath);
+    EXPECT_EQ(parser.getMap(),
+              config::SectionMap(config::SectionMapInt{
+                  {"NetDev",
+                   {
+                       {{"Name", {"test0.50"}}, {"Kind", {"vlan"}}},
+                   }},
+                  {"VLAN", {{{"Id", {"50"}}}}},
+              }));
 
     filePath = confDir;
     filePath /= "test0.60.netdev";
-    parser.setFile(filePath.string());
-    EXPECT_EQ(parser.getValues("NetDev", "Name"),
-              (config::ValueList{"test0.60"}));
-    EXPECT_EQ(parser.getValues("NetDev", "Kind"), (config::ValueList{"vlan"}));
-    EXPECT_EQ(parser.getValues("VLAN", "Id"), (config::ValueList{"60"}));
+    parser.setFile(filePath);
+    EXPECT_EQ(parser.getMap(),
+              config::SectionMap(config::SectionMapInt{
+                  {"NetDev",
+                   {
+                       {{"Name", {"test0.60"}}, {"Kind", {"vlan"}}},
+                   }},
+                  {"VLAN", {{{"Id", {"60"}}}}},
+              }));
 
     deleteVlan("test0.50");
     deleteVlan("test0.60");