Add ConfigDefinition unit tests, fixes

llarp/config/definition.cpp

@@ -29,11 +29,11 @@ Configuration::addDefinition(ConfigDefinition_ptr def)
   return *this;
 }
 
-configuration&
-Configuration::addconfigvalue(string_view section, string_view name, string_view value)
+Configuration&
+Configuration::addConfigValue(string_view section, string_view name, string_view value)
 {
-  configdefinition_ptr& definition = lookupdefinitionorthrow(section, name);
-  definition->parsevalue(std::string(value));
+  ConfigDefinition_ptr& definition = lookupDefinitionOrThrow(section, name);
+  definition->parseValue(std::string(value));
 
   return *this;
 }
@@ -60,6 +60,29 @@ Configuration::lookupDefinitionOrThrow(string_view section, string_view name)
       const_cast<const Configuration*>(this)->lookupDefinitionOrThrow(section, name));
 }
 
+void
+Configuration::validate()
+{
+  for (const auto& pair : m_definitions)
+  {
+    const std::string& section = pair.first;
+
+    const auto& sectionDefinitions = pair.second;
+    for (const auto& defPair : sectionDefinitions)
+    {
+      const auto& def = defPair.second;
+      if (def->required and def->numFound < 1)
+      {
+        throw std::invalid_argument(stringify(
+              "[", section, "]:", def->name, " is required but missing"));
+      }
+
+      // should be handled earlier in ConfigDefinition::parseValue()
+      assert(def->numFound == 1 or def->multiValued);
+    }
+  }
+}
+
 std::string
 Configuration::generateDefaultConfig()
 {

llarp/config/definition.hpp

@@ -78,7 +78,7 @@ namespace llarp
     void
     parseValue(const std::string& input)
     {
-      if (not multiValued and parsedValue)
+      if (not multiValued and parsedValue.has_value())
       {
         throw std::invalid_argument(stringify("duplicate value for ", name,
               ", previous value: ", parsedValue.value()));
@@ -88,9 +88,15 @@ namespace llarp
       T t;
       iss >> t;
       if (iss.fail())
+      {
         throw std::invalid_argument(stringify(input, " is not a valid" , typeid(T).name()));
+      }
       else
+      {
         parsedValue = t;
+        numFound++;
+      }
+
     }
 
     std::string
@@ -121,8 +127,8 @@ namespace llarp
     Configuration&
     addDefinition(ConfigDefinition_ptr def);
 
-    configuration&
-    addconfigvalue(string_view section,
+    Configuration&
+    addConfigValue(string_view section,
                                   string_view name,
                                   string_view value);
 

test/config/test_llarp_config_definition.cpp

@@ -0,0 +1,85 @@
+#include <config/definition.hpp>
+
+#include <catch2/catch.hpp>
+
+TEST_CASE("ConfigDefinition parse test", "[config]")
+{
+  llarp::ConfigDefinition<int> def("foo", "bar", false, false, 42);
+
+  CHECK(def.getValue() == 42);
+  CHECK(def.numFound == 0);
+
+  CHECK(def.defaultValueAsString() == "42");
+
+  CHECK_NOTHROW(def.parseValue("43"));
+  CHECK(def.getValue() == 43);
+  CHECK(def.numFound == 1);
+
+  CHECK(def.defaultValueAsString() == "42");
+}
+
+TEST_CASE("ConfigDefinition multiple parses test", "[config]")
+{
+  {
+    llarp::ConfigDefinition<int> def("foo", "bar", false, true, 8);
+
+    CHECK_NOTHROW(def.parseValue("9"));
+    CHECK(def.getValue() == 9);
+    CHECK(def.numFound == 1);
+
+    // should allow since it is multi-value
+    CHECK_NOTHROW(def.parseValue("12"));
+    CHECK(def.getValue() == 12);
+    CHECK(def.numFound == 2);
+  }
+
+  {
+    llarp::ConfigDefinition<int> def("foo", "baz", false, false, 4);
+
+    CHECK_NOTHROW(def.parseValue("3"));
+    CHECK(def.getValue() == 3);
+    CHECK(def.numFound == 1);
+
+    // shouldn't allow since not multi-value
+    CHECK_THROWS(def.parseValue("2"));
+    CHECK(def.numFound == 1);
+  }
+
+}
+
+TEST_CASE("Configuration basic add/get test", "[config]")
+{
+  llarp::Configuration config;
+  config.addDefinition(std::make_unique<llarp::ConfigDefinition<int>>(
+            "router",
+            "threads",
+            false,
+            false,
+            4));
+
+  CHECK(config.getConfigValue<int>("router", "threads") == 4);
+
+  CHECK_NOTHROW(config.addConfigValue(
+        "router",
+        "threads",
+        "5"));
+
+  CHECK(config.getConfigValue<int>("router", "threads") == 5);
+}
+
+TEST_CASE("Configuration required test", "[config]")
+{
+  llarp::Configuration config;
+  config.addDefinition(std::make_unique<llarp::ConfigDefinition<int>>(
+            "router",
+            "threads",
+            true,
+            false,
+            1));
+
+  CHECK_THROWS(config.validate());
+
+  config.addConfigValue("router", "threads", "12");
+
+  CHECK_NOTHROW(config.validate());
+}