[1206] check for config as specified in current auth

src/lib/datasrc/memory_datasrc.cc

@@ -16,6 +16,7 @@
 #include <cassert>
 #include <boost/shared_ptr.hpp>
 #include <boost/bind.hpp>
+#include <boost/foreach.hpp>
 
 #include <exceptions/exceptions.h>
 
@@ -804,12 +805,122 @@ InMemoryClient::getUpdater(const isc::dns::Name&, bool) const {
     isc_throw(isc::NotImplemented, "Update attempt on in memory data source");
 }
 
-// due to a c++ symbol lookup oddity, we need to explicitely put this into
-// an anonymous namespace. Once we remove memory.cc from the general datasource
-// lib, we can export this
+
+namespace {
+// convencience function to add an error message to a list of those
+// (TODO: move functions like these to some util lib?)
+void
+addError(ElementPtr errors, const std::string& error) {
+    if (errors != ElementPtr() && errors->getType() == Element::list) {
+        errors->add(Element::create(error));
+    }
+}
+
+/// Check if the given element exists in the map, and if it is a string
 bool
-checkConfig(ConstElementPtr, ElementPtr) {
-    // current inmem has no options (yet)
+checkConfigElementString(ConstElementPtr config, const std::string& name,
+                         ElementPtr errors)
+{
+    if (!config->contains(name)) {
+        addError(errors,
+                 "Config for memory backend does not contain a '"
+                 "type"
+                 "' value");
+        return false;
+    } else if (!config->get(name) ||
+               config->get(name)->getType() != Element::string) {
+        addError(errors, "value of " + name +
+                 " in memory backend config is not a string");
+        return false;
+    } else {
+        return true;
+    }
+}
+
+bool
+checkZoneConfig(ConstElementPtr config, ElementPtr errors) {
+    bool result = true;
+    if (!config || config->getType() != Element::map) {
+        addError(errors, "Elements in memory backend's zone list must be maps");
+        result = false;
+    } else {
+        if (!checkConfigElementString(config, "origin", errors)) {
+            result = false;
+        }
+        if (!checkConfigElementString(config, "file", errors)) {
+            result = false;
+        }
+        // we could add some existence/readabilty/parsability checks here
+        // if we want
+    }
+    return result;
+}
+
+} // end anonymous namespace
+
+bool
+checkConfig(ConstElementPtr config, ElementPtr errors) {
+    /* Specific configuration is under discussion, right now this accepts
+     * the 'old' configuration, see [TODO]
+     * So for memory datasource, we get a structure like this:
+     * { "type": string ("memory"),
+     *   "class": string ("IN"/"CH"/etc),
+     *   "zones": list
+     * }
+     * Zones list is a list of maps:
+     * { "origin": string,
+     *     "file": string
+     * }
+     *
+     * At this moment we cannot be completely sure of the contents of the
+     * structure, so we have to do some more extensive tests than should
+     * strictly be necessary (e.g. existence and type of elements)
+     */
+    bool result = true;
+
+    if (!config || config->getType() != Element::map) {
+        addError(errors, "Base config for memory backend must be a map");
+        result = false;
+    } else {
+        if (!checkConfigElementString(config, "type", errors)) {
+            result = false;
+        } else {
+            if (config->get("type")->stringValue() != "memory") {
+                addError(errors,
+                         "Config for memory backend is not of type \"memory\"");
+                result = false;
+            }
+        }
+        if (!checkConfigElementString(config, "class", errors)) {
+            result = false;
+        } else {
+            try {
+                RRClass rrc(config->get("class")->stringValue());
+            } catch (const isc::Exception& rrce) {
+                addError(errors,
+                         "Error parsing class config for memory backend: " +
+                         std::string(rrce.what()));
+                result = false;
+            }
+        }
+        if (!config->contains("zones")) {
+            addError(errors, "No 'zones' element in memory backend config");
+            result = false;
+        } else if (!config->get("zones") ||
+                   config->get("zones")->getType() != Element::list) {
+            addError(errors, "'zones' element in memory backend config is not a list");
+            result = false;
+        } else {
+            BOOST_FOREACH(ConstElementPtr zone_config,
+                          config->get("zones")->listValue()) {
+                if (!checkZoneConfig(zone_config, errors)) {
+                    result = false;
+                }
+            }
+        }
+    }
+
+    return (result);
     return true;
 }
 

src/lib/datasrc/sqlite3_accessor.cc

@@ -30,6 +30,8 @@ using namespace isc::data;
 
 #define SQLITE_SCHEMA_VERSION 1
 
+#define CONFIG_ITEM_DATABASE_FILE "database_file"
+
 namespace isc {
 namespace datasrc {
 
@@ -671,42 +673,33 @@ addError(ElementPtr errors, const std::string& error) {
 
 bool
 checkConfig(ConstElementPtr config, ElementPtr errors) {
+    /* Specific configuration is under discussion, right now this accepts
+     * the 'old' configuration, see [TODO]
+     */
     bool result = true;
+
     if (!config || config->getType() != Element::map) {
         addError(errors, "Base config for SQlite3 backend must be a map");
         result = false;
     } else {
-        if (!config->contains("file")) {
+        if (!config->contains(CONFIG_ITEM_DATABASE_FILE)) {
             addError(errors,
-                     "Config for SQlite3 backend does not contain a 'file' value");
+                     "Config for SQlite3 backend does not contain a '"
+                     CONFIG_ITEM_DATABASE_FILE
+                     "' value");
             result = false;
-        } else if (!config->get("file") ||
-                   config->get("file")->getType() != Element::string) {
-            addError(errors, "file value in SQLite3 backend is not a string");
+        } else if (!config->get(CONFIG_ITEM_DATABASE_FILE) ||
+                   config->get(CONFIG_ITEM_DATABASE_FILE)->getType() !=
+                   Element::string) {
+            addError(errors, "value of " CONFIG_ITEM_DATABASE_FILE
+                     " in SQLite3 backend is not a string");
             result = false;
-        } else if (config->get("file")->stringValue() == "") {
-            addError(errors, "file value in SQLite3 backend is empty");
+        } else if (config->get(CONFIG_ITEM_DATABASE_FILE)->stringValue() ==
+                   "") {
+            addError(errors, "value of " CONFIG_ITEM_DATABASE_FILE
+                     " in SQLite3 backend is empty");
             result = false;
         }
-
-        if (!config->contains("class")) {
-            addError(errors, "Config for SQlite3 backend does not contain a 'class' value");
-            result = false;
-        } else if (!config->get("class") ||
-                   config->get("class")->getType() != Element::string) {
-            addError(errors, "class value in SQLite3 backend is not a string");
-            result = false;
-        } else {
-            try {
-                isc::dns::RRClass rrclass(config->get("class")->stringValue());
-            } catch (const isc::dns::InvalidRRClass& ivrc) {
-                addError(errors, ivrc.what());
-                result = false;
-            } catch (const isc::dns::IncompleteRRClass& icrc) {
-                addError(errors, icrc.what());
-                result = false;
-            }
-        }
     }
 
     return (result);
@@ -718,11 +711,10 @@ createInstance(isc::data::ConstElementPtr config) {
     if (!checkConfig(config, errors)) {
         isc_throw(DataSourceConfigError, errors->str());
     }
-    isc::dns::RRClass rrclass(config->get("class")->stringValue());
-    std::string dbfile = config->get("file")->stringValue();
+    std::string dbfile = config->get(CONFIG_ITEM_DATABASE_FILE)->stringValue();
     boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
-        new SQLite3Accessor(dbfile, rrclass));
-    return (new DatabaseClient(rrclass, sqlite3_accessor));
+        new SQLite3Accessor(dbfile, isc::dns::RRClass::IN()));
+    return (new DatabaseClient(isc::dns::RRClass::IN(), sqlite3_accessor));
 }
 
 void destroyInstance(DataSourceClient* instance) {

src/lib/datasrc/tests/factory_unittest.cc

@@ -30,7 +30,60 @@ namespace {
 
 // The default implementation is NotImplemented
 TEST(FactoryTest, memoryClient) {
-    DataSourceClientContainer client("memory", ElementPtr());
+    ElementPtr config;
+    ASSERT_THROW(DataSourceClientContainer client("memory", config),
+                 DataSourceConfigError);
+
+    config = Element::create("asdf");
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config = Element::createMap();
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("type", ElementPtr());
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("type", Element::create(1));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("type", Element::create("FOO"));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("type", Element::create("memory"));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("class", ElementPtr());
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("class", Element::create(1));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("class", Element::create("FOO"));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("class", Element::create("IN"));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("zones", ElementPtr());
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("zones", Element::create(1));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
+                 DataSourceConfigError);
+
+    config->set("zones", Element::createList());
+    DataSourceClientContainer("memory", config);
 }
 
 TEST(FactoryTest, badType) {
@@ -66,15 +119,15 @@ TEST(FactoryTest, sqlite3ClientBadConfig) {
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("file", ElementPtr());
+    config->set("database_file", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("file", Element::create(1));
+    config->set("database_file", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("file", Element::create("/foo/bar/doesnotexist"));
+    config->set("database_file", Element::create("/foo/bar/doesnotexist"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  SQLite3Error);
 }
@@ -93,35 +146,20 @@ TEST(FactoryTest, sqlite3ClientBadConfig3) {
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("class", ElementPtr());
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
-
-    config->set("class", Element::create(1));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
-
-    config->set("class", Element::create("FOO"));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
-
-    config->set("class", Element::create("IN"));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
-
-    config->set("file", ElementPtr());
+    config->set("database_file", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("file", Element::create(1));
+    config->set("database_file", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("file", Element::create("/foo/bar/doesnotexist"));
+    config->set("database_file", Element::create("/foo/bar/doesnotexist"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  SQLite3Error);
 
-    config->set("file", Element::create("/tmp/some_file.sqlite3"));
+    // TODO remove this one (now config isn't bad anymore) or find better filename
+    config->set("database_file", Element::create("/tmp/some_file.sqlite3"));
     DataSourceClientContainer dsc("sqlite3", config);
 }
 } // end anonymous namespace