SERVER-37088 Update OptionSection to handle merging sections

3 files changed, 75 insertions, 13 deletions

diff --git a/src/mongo/util/options_parser/option_section.cpp b/src/mongo/util/options_parser/option_section.cpp
index 8eebd290d74..2e4634ca609 100644
--- a/src/mongo/util/options_parser/option_section.cpp
+++ b/src/mongo/util/options_parser/option_section.cpp
@@ -46,17 +46,31 @@ using std::shared_ptr;
 // Registration interface
 
 // TODO: Make sure the section we are adding does not have duplicate options
-Status OptionSection::addSection(const OptionSection& subSection) {
-    std::list<OptionDescription>::const_iterator oditerator;
-    for (oditerator = subSection._options.begin(); oditerator != subSection._options.end();
-         oditerator++) {
-        if (oditerator->_positionalStart != -1) {
-            StringBuilder sb;
-            sb << "Attempted to add subsection with positional option: " << oditerator->_dottedName;
-            return Status(ErrorCodes::InternalError, sb.str());
+Status OptionSection::addSection(const OptionSection& newSection) {
+    if (newSection._subSections.size()) {
+        return {ErrorCodes::InternalError, "Option subsections may not contain nested subsections"};
+    }
+
+    for (const auto& opt : newSection._options) {
+        if (opt._positionalStart != -1) {
+            return {ErrorCodes::InternalError,
+                    str::stream() << "Attempted to add subsection with positional option: "
+                                  << opt._dottedName};
         }
     }
-    _subSections.push_back(subSection);
+
+    for (auto& oldSection : _subSections) {
+        if (newSection._name == oldSection._name) {
+            // Matches existing section name, merge options.
+            std::copy(newSection._options.cbegin(),
+                      newSection._options.cend(),
+                      std::back_inserter(oldSection._options));
+            return Status::OK();
+        }
+    }
+
+    // New section name, just adopt it.
+    _subSections.push_back(newSection);
     return Status::OK();
 }
 
@@ -585,6 +599,10 @@ Status OptionSection::countOptions(int* numOptions, bool visibleOnly, OptionSour
     return Status::OK();
 }
 
+size_t OptionSection::countSubSections() const {
+    return _subSections.size();
+}
+
 Status OptionSection::getConstraints(std::vector<std::shared_ptr<Constraint>>* constraints) const {
     std::list<OptionDescription>::const_iterator oditerator;
     for (oditerator = _options.begin(); oditerator != _options.end(); oditerator++) {
diff --git a/src/mongo/util/options_parser/option_section.h b/src/mongo/util/options_parser/option_section.h
index 2efaeb03348..685e3d869f2 100644
--- a/src/mongo/util/options_parser/option_section.h
+++ b/src/mongo/util/options_parser/option_section.h
@@ -86,9 +86,14 @@ public:
 
     /**
      * Add a sub section to this section.  Used mainly to keep track of section headers for when
-     * we need generate the help std::string for the command line
+     * we need generate the help std::string for the command line.
+     *
+     * Note that while the structure of this class allows for a nested hierarchy of sections,
+     * our actual use-case enforces a maximum depth of 2.
+     * The base node plus one level of subsections.
+     * This means that subsections being added must not contain subsections of their own.
      */
-    Status addSection(const OptionSection& subSection);
+    Status addSection(const OptionSection& newSection);
 
     /**
      * Add an option to this section, and returns a reference to an OptionDescription to allow
@@ -139,6 +144,11 @@ public:
     Status countOptions(int* numOptions, bool visibleOnly, OptionSources sources) const;
 
     /**
+     * Returns the number of subsections which have been added to this OptionSection.
+     */
+    size_t countSubSections() const;
+
+    /**
      * Populates the given map with all the default values for any options in this option
      * section and all sub sections.
      */
diff --git a/src/mongo/util/options_parser/options_parser_test.cpp b/src/mongo/util/options_parser/options_parser_test.cpp
index e4098ba4e09..b798cecbc73 100644
--- a/src/mongo/util/options_parser/options_parser_test.cpp
+++ b/src/mongo/util/options_parser/options_parser_test.cpp
@@ -277,6 +277,40 @@ TEST(Registration, StringFormatConstraint) {
     }
 }
 
+TEST(Registration, NestedSubSections) {
+    moe::OptionSection root;
+    moe::OptionSection childSection;
+    moe::OptionSection grandchildSection;
+
+    ASSERT_OK(childSection.addSection(grandchildSection));
+    ASSERT_NOT_OK(root.addSection(childSection));
+}
+
+TEST(Registration, MergeSubSections) {
+    moe::OptionSection root;
+    ASSERT_EQ(root.countSubSections(), 0UL);
+
+    {
+        moe::OptionSection opts("Options");
+        opts.addOptionChaining("option1", "option1", moe::String, "A string option");
+        ASSERT_OK(root.addSection(opts));
+        ASSERT_EQ(root.countSubSections(), 1UL);
+    }
+
+    {
+        moe::OptionSection moreOpts("Options");
+        moreOpts.addOptionChaining("option2", "option2", moe::Int, "An integer option");
+        ASSERT_OK(root.addSection(moreOpts));
+        ASSERT_EQ(root.countSubSections(), 1UL);
+    }
+
+    std::vector<moe::OptionDescription> options;
+    ASSERT_OK(root.getAllOptions(&options));
+    ASSERT_EQ(options.size(), 2UL);
+    ASSERT_EQ(options[0]._dottedName, "option1");
+    ASSERT_EQ(options[1]._dottedName, "option2");
+}
+
 TEST(Parsing, Good) {
     moe::OptionsParser parser;
     moe::Environment environment;
@@ -309,7 +343,7 @@ TEST(Parsing, SubSection) {
     moe::OptionSection subSection("Section Name");
 
     subSection.addOptionChaining("port", "port", moe::Int, "Port");
-    testOpts.addSection(subSection).transitional_ignore();
+    ASSERT_OK(testOpts.addSection(subSection));
 
     std::vector<std::string> argv;
     argv.push_back("binaryname");
@@ -4167,7 +4201,7 @@ TEST(OptionCount, Basic) {
     moe::OptionSection subSection("Section Name");
     subSection.addOptionChaining("port", "port", moe::Int, "Port")
         .setSources(moe::SourceYAMLConfig);
-    testOpts.addSection(subSection).transitional_ignore();
+    ASSERT_OK(testOpts.addSection(subSection));
 
     int numOptions;
     ASSERT_OK(testOpts.countOptions(&numOptions, true /*visibleOnly*/, moe::SourceCommandLine));