From 926d140a47019305bf9c21d2c30f52a23c08e928 Mon Sep 17 00:00:00 2001
From: njhanley <njhanley@users.noreply.github.com>
Date: Sun, 30 Aug 2020 18:13:39 -0400
Subject: [PATCH] Respect order of subconfigs in znc.conf

---
 include/znc/Config.h | 25 +++++++++++++++----------
 src/Config.cpp       | 11 +++++++----
 test/ConfigTest.cpp  |  8 ++++++--
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/include/znc/Config.h b/include/znc/Config.h
index dc18ad1b..17783bc7 100644
--- a/include/znc/Config.h
+++ b/include/znc/Config.h
@@ -35,10 +35,11 @@ struct CConfigEntry {
 
 class CConfig {
   public:
-    CConfig() : m_ConfigEntries(), m_SubConfigs() {}
+    CConfig() : m_ConfigEntries(), m_SubConfigs(), m_SubConfigNameSets() {}
 
     typedef std::map<CString, VCString> EntryMap;
-    typedef std::map<CString, CConfigEntry> SubConfig;
+    typedef std::pair<CString, CConfigEntry> SubConfigEntry;
+    typedef std::vector<SubConfigEntry> SubConfig;
     typedef std::map<CString, SubConfig> SubConfigMap;
 
     typedef EntryMap::const_iterator EntryMapIterator;
@@ -62,14 +63,13 @@ class CConfig {
 
     bool AddSubConfig(const CString& sTag, const CString& sName,
                       CConfig Config) {
-        SubConfig& conf = m_SubConfigs[sTag];
-        SubConfig::const_iterator it = conf.find(sName);
+        auto& nameset = m_SubConfigNameSets[sTag];
 
-        if (it != conf.end()) {
-            return false;
-        }
+        if (nameset.find(sName) != nameset.end()) return false;
+
+        nameset.insert(sName);
+        m_SubConfigs[sTag].emplace_back(sName, Config);
 
-        conf[sName] = Config;
         return true;
     }
 
@@ -142,9 +142,9 @@ class CConfig {
         return false;
     }
 
-    bool FindSubConfig(const CString& sName, SubConfig& Config,
+    bool FindSubConfig(const CString& sTag, SubConfig& Config,
                        bool bErase = true) {
-        SubConfigMap::iterator it = m_SubConfigs.find(sName);
+        auto it = m_SubConfigs.find(sTag);
         if (it == m_SubConfigs.end()) {
             Config.clear();
             return false;
@@ -153,6 +153,7 @@ class CConfig {
 
         if (bErase) {
             m_SubConfigs.erase(it);
+            m_SubConfigNameSets.erase(sTag);
         }
 
         return true;
@@ -166,8 +167,12 @@ class CConfig {
     void Write(CFile& file, unsigned int iIndentation = 0);
 
   private:
+    typedef SCString SubConfigNameSet;
+    typedef std::map<CString, SubConfigNameSet> SubConfigNameSetMap;
+
     EntryMap m_ConfigEntries;
     SubConfigMap m_SubConfigs;
+    SubConfigNameSetMap m_SubConfigNameSets;
 };
 
 #endif  // !ZNC_CONFIG_H
diff --git a/src/Config.cpp b/src/Config.cpp
index 520e4139..a3df463a 100644
--- a/src/Config.cpp
+++ b/src/Config.cpp
@@ -68,6 +68,7 @@ bool CConfig::Parse(CFile& file, CString& sErrorMsg) {
         std::stringstream stream;                              \
         stream << "Error on line " << uLineNum << ": " << arg; \
         sErrorMsg = stream.str();                              \
+        m_SubConfigNameSets.clear();                           \
         m_SubConfigs.clear();                                  \
         m_ConfigEntries.clear();                               \
         return false;                                          \
@@ -122,14 +123,16 @@ bool CConfig::Parse(CFile& file, CString& sErrorMsg) {
                 else
                     pActiveConfig = &ConfigStack.top().Config;
 
-                SubConfig& conf = pActiveConfig->m_SubConfigs[sTag.AsLower()];
-                SubConfig::const_iterator it = conf.find(sName);
+                const auto sTagLower = sTag.AsLower();
+                auto& nameset = pActiveConfig->m_SubConfigNameSets[sTagLower];
 
-                if (it != conf.end())
+                if (nameset.find(sName) != nameset.end())
                     ERROR("Duplicate entry for tag \"" << sTag << "\" name \""
                                                        << sName << "\".");
 
-                conf[sName] = CConfigEntry(myConfig);
+                nameset.insert(sName);
+                pActiveConfig->m_SubConfigs[sTagLower].emplace_back(sName,
+                                                                    myConfig);
             } else {
                 if (sValue.empty())
                     ERROR("Empty block name at begin of block.");
diff --git a/test/ConfigTest.cpp b/test/ConfigTest.cpp
index 3795ea10..30533536 100644
--- a/test/ConfigTest.cpp
+++ b/test/ConfigTest.cpp
@@ -87,8 +87,7 @@ class CConfigSuccessTest : public CConfigTest {
 
         CConfig::SubConfigMapIterator it2 = conf.BeginSubConfigs();
         while (it2 != conf.EndSubConfigs()) {
-            std::map<CString, CConfigEntry>::const_iterator it3 =
-                it2->second.begin();
+            auto it3 = it2->second.begin();
 
             while (it3 != it2->second.end()) {
                 sRes += "->" + it2->first + "/" + it3->first + "\n";
@@ -146,6 +145,11 @@ TEST_F(CConfigSuccessTest, SubConf8) {
     TEST_SUCCESS(" \t <A B>\nfoo = bar\n\tFooO = bar\n</a>",
                  "->a/B\nfoo=bar\nfooo=bar\n<-\n");
 }
+// ensure order is preserved i.e. subconfigs should not be sorted by name
+TEST_F(CConfigSuccessTest, SubConf9) {
+    TEST_SUCCESS("<foo b>\n</foo>\n<foo a>\n</foo>",
+                 "->foo/b\n<-\n->foo/a\n<-\n");
+}
 
 /* comments */
 TEST_F(CConfigSuccessTest, Comment1) {