[1607] (cleanup) reorganized datasrc loadable moule oraganizations and tests.

extracted the conflicting symbols into <datasrc>_link.cc so that the main
part of the data source implementations can be shared in the single test.

src/lib/datasrc/Makefile.am

@@ -34,6 +34,7 @@ nodist_libdatasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 pkglib_LTLIBRARIES =  sqlite3_ds.la memory_ds.la
 
 sqlite3_ds_la_SOURCES = sqlite3_accessor.h sqlite3_accessor.cc
+sqlite3_ds_la_SOURCES += sqlite3_accessor_link.cc
 sqlite3_ds_la_LDFLAGS = -module
 sqlite3_ds_la_LDFLAGS += -no-undefined -version-info 1:0:0
 sqlite3_ds_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
@@ -41,6 +42,7 @@ sqlite3_ds_la_LIBADD += libdatasrc.la
 sqlite3_ds_la_LIBADD += $(SQLITE_LIBS)
 
 memory_ds_la_SOURCES = memory_datasrc.h memory_datasrc.cc
+memory_ds_la_SOURCES += memory_datasrc_link.cc
 memory_ds_la_LDFLAGS = -module
 memory_ds_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
 memory_ds_la_LIBADD += libdatasrc.la

src/lib/datasrc/memory_datasrc.cc

@@ -21,7 +21,6 @@
 #include <boost/shared_ptr.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <boost/bind.hpp>
-#include <boost/foreach.hpp>
 
 #include <exceptions/exceptions.h>
 
@@ -39,12 +38,9 @@
 #include <datasrc/data_source.h>
 #include <datasrc/factory.h>
 
-#include <cc/data.h>
-
 using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
-using namespace isc::data;
 using boost::scoped_ptr;
 
 namespace isc {
@@ -1220,146 +1216,5 @@ InMemoryClient::getJournalReader(const isc::dns::Name&, uint32_t,
               "in memory data source");
 }
 
-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
-checkConfigElementString(ConstElementPtr config, const std::string& name,
-                         ElementPtr errors)
-{
-    if (!config->contains(name)) {
-        addError(errors,
-                 "Config for memory backend does not contain a '"
-                 +name+
-                 "' 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;
-}
-
-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);
-}
-
-} // end anonymous namespace
-
-DataSourceClient *
-createInstance(isc::data::ConstElementPtr config, std::string& error) {
-    ElementPtr errors(Element::createList());
-    if (!checkConfig(config, errors)) {
-        error = "Configuration error: " + errors->str();
-        return (NULL);
-    }
-    try {
-        return (new InMemoryClient());
-    } catch (const std::exception& exc) {
-        error = std::string("Error creating memory datasource: ") + exc.what();
-        return (NULL);
-    } catch (...) {
-        error = std::string("Error creating memory datasource, "
-                            "unknown exception");
-        return (NULL);
-    }
-}
-
-void destroyInstance(DataSourceClient* instance) {
-    delete instance;
-}
-
-
 } // end of namespace datasrc
 } // end of namespace isc

src/lib/datasrc/sqlite3_accessor.cc

@@ -31,8 +31,6 @@ using namespace isc::data;
 
 #define SQLITE_SCHEMA_VERSION 1
 
-#define CONFIG_ITEM_DATABASE_FILE "database_file"
-
 namespace isc {
 namespace datasrc {
 
@@ -1096,75 +1094,5 @@ SQLite3Accessor::findPreviousName(int zone_id, const std::string& rname)
     return (result);
 }
 
-namespace {
-void
-addError(ElementPtr errors, const std::string& error) {
-    if (errors != ElementPtr() && errors->getType() == Element::list) {
-        errors->add(Element::create(error));
-    }
-}
-
-bool
-checkConfig(ConstElementPtr config, ElementPtr errors) {
-    /* Specific configuration is under discussion, right now this accepts
-     * the 'old' configuration, see header file
-     */
-    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(CONFIG_ITEM_DATABASE_FILE)) {
-            addError(errors,
-                     "Config for SQlite3 backend does not contain a '"
-                     CONFIG_ITEM_DATABASE_FILE
-                     "' value");
-            result = false;
-        } 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(CONFIG_ITEM_DATABASE_FILE)->stringValue() ==
-                   "") {
-            addError(errors, "value of " CONFIG_ITEM_DATABASE_FILE
-                     " in SQLite3 backend is empty");
-            result = false;
-        }
-    }
-
-    return (result);
-}
-
-} // end anonymous namespace
-
-DataSourceClient *
-createInstance(isc::data::ConstElementPtr config, std::string& error) {
-    ElementPtr errors(Element::createList());
-    if (!checkConfig(config, errors)) {
-        error = "Configuration error: " + errors->str();
-        return (NULL);
-    }
-    std::string dbfile = config->get(CONFIG_ITEM_DATABASE_FILE)->stringValue();
-    try {
-        boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
-            new SQLite3Accessor(dbfile, "IN")); // XXX: avoid hardcode RR class
-        return (new DatabaseClient(isc::dns::RRClass::IN(), sqlite3_accessor));
-    } catch (const std::exception& exc) {
-        error = std::string("Error creating sqlite3 datasource: ") + exc.what();
-        return (NULL);
-    } catch (...) {
-        error = std::string("Error creating sqlite3 datasource, "
-                            "unknown exception");
-        return (NULL);
-    }
-}
-
-void destroyInstance(DataSourceClient* instance) {
-    delete instance;
-}
-
 } // end of namespace datasrc
 } // end of namespace isc

src/lib/datasrc/tests/Makefile.am

@@ -20,17 +20,13 @@ CLEANFILES = *.gcno *.gcda
 TESTS =
 noinst_PROGRAMS =
 if HAVE_GTEST
-TESTS += run_unittests run_unittests_sqlite3 run_unittests_memory
+TESTS += run_unittests
 
-#
-# For each specific datasource, there is a separate binary that includes
-# the code itself (we can't unittest through the public API). These need
-# to be separate because the included code, by design, contains conflicting
-# symbols.
-# We also have a 'general' run_unittests with non-datasource-specific tests
-#
+# We have two sets of tests: the general tests and factory tests (see below
+# for the latter).  They are separate binary files sharing some program files
+# and libraries.
 
-# First define the parts shared by all
+# First define the parts shared by both
 common_sources = run_unittests.cc
 common_sources += $(top_srcdir)/src/lib/dns/tests/unittest_util.h
 common_sources += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
@@ -46,7 +42,6 @@ common_ldadd += $(top_builddir)/src/lib/cc/libcc.la
 common_ldadd += $(top_builddir)/src/lib/testutils/libtestutils.la
 common_ldadd += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 
-
 # The general tests
 run_unittests_SOURCES = $(common_sources)
 run_unittests_SOURCES += datasrc_unittest.cc
@@ -57,35 +52,21 @@ run_unittests_SOURCES += test_datasrc.h test_datasrc.cc
 run_unittests_SOURCES += rbtree_unittest.cc
 run_unittests_SOURCES += logger_unittest.cc
 run_unittests_SOURCES += client_unittest.cc
+run_unittests_SOURCES += database_unittest.cc
+run_unittests_SOURCES += sqlite3_unittest.cc
+run_unittests_SOURCES += sqlite3_accessor_unittest.cc
+run_unittests_SOURCES += memory_datasrc_unittest.cc
+
+# We need the actually module implementation in the tests (they are not part
+# of libdatasrc)
+run_unittests_SOURCES += $(top_srcdir)/src/lib/datasrc/sqlite3_accessor.cc
+run_unittests_SOURCES += $(top_srcdir)/src/lib/datasrc/memory_datasrc.cc
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
 
 run_unittests_LDADD = $(common_ldadd)
 
-
-# SQlite3 datasource tests
-run_unittests_sqlite3_SOURCES = $(common_sources)
-run_unittests_sqlite3_SOURCES += database_unittest.cc
-run_unittests_sqlite3_SOURCES += sqlite3_unittest.cc
-run_unittests_sqlite3_SOURCES += sqlite3_accessor_unittest.cc
-run_unittests_sqlite3_SOURCES += $(top_srcdir)/src/lib/datasrc/sqlite3_accessor.cc
-
-run_unittests_sqlite3_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
-run_unittests_sqlite3_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
-
-run_unittests_sqlite3_LDADD = $(common_ldadd)
-
-# In-memory datasource tests
-run_unittests_memory_SOURCES = $(common_sources)
-run_unittests_memory_SOURCES += memory_datasrc_unittest.cc
-run_unittests_memory_SOURCES += $(top_srcdir)/src/lib/datasrc/memory_datasrc.cc
-
-run_unittests_memory_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
-run_unittests_memory_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
-
-run_unittests_memory_LDADD = $(common_ldadd)
-
 noinst_PROGRAMS+= $(TESTS)
 
 # For the factory unit tests, we need to specify that we want