SERVER-8510 Add function to extract only explicitly set values from Environment

6 files changed, 126 insertions, 47 deletions

diff --git a/src/mongo/util/options_parser/environment.cpp b/src/mongo/util/options_parser/environment.cpp
index 7a2e33bfd06..41e94d90d40 100644
--- a/src/mongo/util/options_parser/environment.cpp
+++ b/src/mongo/util/options_parser/environment.cpp
@@ -41,14 +41,15 @@ namespace optionenvironment {
         typedef std::map<Key, Value>::const_iterator it_type;
         it_type value = values.find(get_key);
         if (value == values.end()) {
-            StringBuilder sb;
-            sb << "Value not found for key: " << get_key;
-            return Status(ErrorCodes::NoSuchKey, sb.str());
-        }
-        else {
-            *get_value = value->second;
-            return Status::OK();
+            value = default_values.find(get_key);
+            if (value == default_values.end()) {
+                StringBuilder sb;
+                sb << "Value not found for key: " << get_key;
+                return Status(ErrorCodes::NoSuchKey, sb.str());
+            }
         }
+        *get_value = value->second;
+        return Status::OK();
     }
 
     /** Set the Value in our Environment.  Always disallow empty values */
@@ -78,6 +79,26 @@ namespace optionenvironment {
         return Status::OK();
     }
 
+    /** Set the default Value for the given Key in our Environment.  Always disallow empty values */
+    Status Environment::setDefault(const Key& add_key, const Value& add_value) {
+
+        // 1. Make sure value is not empty
+        if (add_value.isEmpty()) {
+            return Status(ErrorCodes::InternalError, "Attempted to set an empty default value");
+        }
+
+        // 2. Disallow modifying defaults after calling validate on this Environment
+        if (valid) {
+            return Status(ErrorCodes::InternalError,
+                          "Attempted to set a default value after calling validate");
+        }
+
+        // 3. Add this value to our defaults
+        default_values[add_key] = add_value;
+
+        return Status::OK();
+    }
+
     /** Set all the Values from the source Environment in our Environment.  Does not check for empty
      *  values as the source Environment should not have been allowed to have any */
     Status Environment::setAll(const Environment& add_environment) {
diff --git a/src/mongo/util/options_parser/environment.h b/src/mongo/util/options_parser/environment.h
index 305a74fa284..e2163ca5ecf 100644
--- a/src/mongo/util/options_parser/environment.h
+++ b/src/mongo/util/options_parser/environment.h
@@ -117,6 +117,12 @@ namespace optionenvironment {
              */
             Status set(const Key& key, const Value& value);
 
+            /** Add a default Value to this Environment with the given Key.  Fails if validate has
+             *  already been called on our environment.  The get functions will return the default
+             *  if one exists and the value has not been explicitly set.
+             */
+            Status setDefault(const Key& key, const Value& value);
+
             /** Populate the given Value with the Value stored for the given Key.  Return a success
              *  status if the value was found, or an error status if the value was not found.
              *  Leaves the Value unchanged on error.
@@ -154,6 +160,15 @@ namespace optionenvironment {
              */
             Value operator[](const Key& key) const;
 
+            /**
+             * Get all values that we have set explicitly as a map in case we need to iterate or
+             * move to another structure, as is currently the use case for the parsed command line
+             * options structure that we present to the user.
+             */
+            const std::map<Key, Value>& getExplicitlySet() const {
+                return values;
+            }
+
             /* Debugging */
             void dump();
 
@@ -161,19 +176,24 @@ namespace optionenvironment {
             std::vector<boost::shared_ptr<Constraint> > constraints;
             std::vector<boost::shared_ptr<KeyConstraint> > keyConstraints;
             std::map <Key, Value> values;
+            std::map <Key, Value> default_values;
             bool valid;
     };
 
     template <typename T>
     Status Environment::get(const Key& get_key, T* get_value) const {
-        typedef std::map<Key, Value>::const_iterator it_type;
-        it_type value = values.find(get_key);
-        if (value == values.end()) {
-            return Status(ErrorCodes::NoSuchKey, "Value not found!");
+        Value value;
+        Status ret = get(get_key, &value);
+        if (!ret.isOK()) {
+            return ret;
         }
-        else {
-            return value->second.get(get_value);
+        ret = value.get(get_value);
+        if (!ret.isOK()) {
+            StringBuilder sb;
+            sb << "Error getting value for key: \"" << get_key << "\": " << ret.toString();
+            return Status(ErrorCodes::NoSuchKey, sb.str());
         }
+        return Status::OK();
     }
 
 } // namespace optionenvironment
diff --git a/src/mongo/util/options_parser/environment_test.cpp b/src/mongo/util/options_parser/environment_test.cpp
index d92a9f682ff..8893ea9db9f 100644
--- a/src/mongo/util/options_parser/environment_test.cpp
+++ b/src/mongo/util/options_parser/environment_test.cpp
@@ -78,4 +78,29 @@ namespace {
         ASSERT_EQUALS(number, 5);
     }
 
+    TEST(Environment, DefaultValueIterateExplicit) {
+        moe::Environment environment;
+        ASSERT_OK(environment.setDefault(moe::Key("val1"), moe::Value(5)));
+        ASSERT_OK(environment.setDefault(moe::Key("val2"), moe::Value(5)));
+        ASSERT_OK(environment.set(moe::Key("val1"), moe::Value(6)));
+        int val1;
+        ASSERT_OK(environment.get(moe::Key("val1"), &val1));
+        ASSERT_EQUALS(val1, 6);
+        int val2;
+        ASSERT_OK(environment.get(moe::Key("val2"), &val2));
+        ASSERT_EQUALS(val2, 5);
+
+        const std::map<moe::Key, moe::Value> values = environment.getExplicitlySet();
+        ASSERT_EQUALS((static_cast<std::map<moe::Key, moe::Value>::size_type>(1)), values.size());
+
+        typedef std::map<moe::Key, moe::Value>::const_iterator it_type;
+        for(it_type iterator = values.begin();
+            iterator != values.end(); iterator++) {
+            ASSERT_EQUALS(moe::Key("val1"), iterator->first);
+            int val1;
+            ASSERT_OK(iterator->second.get(&val1));
+            ASSERT_EQUALS(6, val1);
+        }
+    }
+
 } // unnamed namespace
diff --git a/src/mongo/util/options_parser/options_parser.cpp b/src/mongo/util/options_parser/options_parser.cpp
index c420cdcb277..e40e0799da4 100644
--- a/src/mongo/util/options_parser/options_parser.cpp
+++ b/src/mongo/util/options_parser/options_parser.cpp
@@ -518,22 +518,13 @@ namespace optionenvironment {
     }
 
     /**
-     * Extract default values from the given options and add to environment
+     * Add default values from the given OptionSection to the given Environment
      */
-    Status OptionsParser::getDefaultValues(const OptionSection& options,
+    Status OptionsParser::addDefaultValues(const OptionSection& options,
                                            Environment* environment) {
-        Environment defaultEnvironment;
-
-        // This should have been caught at the time we registered our options, but we check that the
-        // default types match our declared types here just to be sure.
-        Status ret = addTypeConstraints(options, &defaultEnvironment);
-        if (!ret.isOK()) {
-            return ret;
-        }
-
         std::map <Key, Value> defaultOptions;
 
-        ret = options.getDefaults(&defaultOptions);
+        Status ret = options.getDefaults(&defaultOptions);
         if (!ret.isOK()) {
             return ret;
         }
@@ -541,19 +532,12 @@ namespace optionenvironment {
         typedef std::map<Key, Value>::iterator it_type;
         for(it_type iterator = defaultOptions.begin();
             iterator != defaultOptions.end(); iterator++) {
-            ret = defaultEnvironment.set(iterator->first, iterator->second);
+            ret = environment->setDefault(iterator->first, iterator->second);
             if (!ret.isOK()) {
                 return ret;
             }
         }
 
-        ret = defaultEnvironment.validate();
-        if (!ret.isOK()) {
-            return ret;
-        }
-
-        *environment = defaultEnvironment;
-
         return Status::OK();
     }
 
@@ -657,17 +641,11 @@ namespace optionenvironment {
             const std::map<std::string, std::string>& env, // XXX: Currently unused
             Environment* environment) {
 
-        Environment defaultEnvironment;
         Environment commandLineEnvironment;
         Environment configEnvironment;
         Environment composedEnvironment;
 
-        Status ret = getDefaultValues(options, &defaultEnvironment);
-        if (!ret.isOK()) {
-            return ret;
-        }
-
-        ret = parseCommandLine(options, argv, &commandLineEnvironment);
+        Status ret = parseCommandLine(options, argv, &commandLineEnvironment);
         if (!ret.isOK()) {
             return ret;
         }
@@ -729,17 +707,14 @@ namespace optionenvironment {
             return ret;
         }
 
-        ret = addCompositions(options, defaultEnvironment, &composedEnvironment);
+        // Add the default values to our resulting environment
+        ret = addDefaultValues(options, environment);
         if (!ret.isOK()) {
             return ret;
         }
 
         // Add the values to our result in the order of override
         // NOTE: This should not fail validation as we haven't called environment->validate() yet
-        ret = environment->setAll(defaultEnvironment);
-        if (!ret.isOK()) {
-            return ret;
-        }
         ret = environment->setAll(configEnvironment);
         if (!ret.isOK()) {
             return ret;
diff --git a/src/mongo/util/options_parser/options_parser.h b/src/mongo/util/options_parser/options_parser.h
index ceef90bb864..b1a65bc5cc0 100644
--- a/src/mongo/util/options_parser/options_parser.h
+++ b/src/mongo/util/options_parser/options_parser.h
@@ -103,8 +103,8 @@ namespace optionenvironment {
         /** Handles parsing of a JSON config string and adds the results to the given Environment */
         Status parseJSONConfigFile(const OptionSection&, const std::string& config, Environment*);
 
-        /** Gets defaults from the OptionSection and adds the results to the given Environment */
-        Status getDefaultValues(const OptionSection&, Environment*);
+        /** Gets defaults from the OptionSection and adds them to the given Environment */
+        Status addDefaultValues(const OptionSection&, Environment*);
 
         /** Detects whether the given string represents a JSON config file or an INI config file */
         bool isJSONConfig(const std::string& config);
diff --git a/src/mongo/util/options_parser/options_parser_test.cpp b/src/mongo/util/options_parser/options_parser_test.cpp
index 69ef9081e66..49c56a361a2 100644
--- a/src/mongo/util/options_parser/options_parser_test.cpp
+++ b/src/mongo/util/options_parser/options_parser_test.cpp
@@ -461,6 +461,44 @@ namespace {
         ASSERT_EQUALS(port, 6);
     }
 
+    TEST(Parsing, DefaultValueIterateExplicit) {
+        moe::OptionsParser parser;
+        moe::Environment environment;
+
+        moe::OptionSection testOpts;
+        testOpts.addOption(moe::OptionDescription("help", "help", moe::Switch, "Display help"));
+        testOpts.addOption(moe::OptionDescription("val1", "val1", moe::Int, "Val1", true,
+                                                                                    moe::Value(5)));
+        testOpts.addOption(moe::OptionDescription("val2", "val2", moe::Int, "Val2", true,
+                                                                                    moe::Value(5)));
+
+        std::vector<std::string> argv;
+        argv.push_back("binaryname");
+        argv.push_back("--val1");
+        argv.push_back("6");
+        std::map<std::string, std::string> env_map;
+
+        ASSERT_OK(parser.run(testOpts, argv, env_map, &environment));
+
+        const std::map<moe::Key, moe::Value> values = environment.getExplicitlySet();
+        ASSERT_EQUALS((static_cast<std::map<moe::Key, moe::Value>::size_type>(1)), values.size());
+
+        typedef std::map<moe::Key, moe::Value>::const_iterator it_type;
+        for(it_type iterator = values.begin();
+            iterator != values.end(); iterator++) {
+            ASSERT_EQUALS(moe::Key("val1"), iterator->first);
+            int val1;
+            ASSERT_OK(iterator->second.get(&val1));
+            ASSERT_EQUALS(6, val1);
+        }
+
+        moe::Value value;
+        ASSERT_OK(environment.get(moe::Key("val2"), &value));
+        int val2;
+        ASSERT_OK(value.get(&val2));
+        ASSERT_EQUALS(val2, 5);
+    }
+
     TEST(Parsing, ImplicitValue) {
         moe::OptionsParser parser;
         moe::Environment environment;