[1206] update to docs and tests

src/lib/datasrc/factory.cc

@@ -27,12 +27,10 @@ using namespace isc::datasrc;
 namespace isc {
 namespace datasrc {
 
-
-DLHolder::DLHolder(const std::string& name) : ds_name_(name) {
-    ds_lib_ = dlopen(ds_name_.c_str(), RTLD_NOW | RTLD_LOCAL);
+DLHolder::DLHolder(const std::string& name) {
+    ds_lib_ = dlopen(name.c_str(), RTLD_NOW | RTLD_LOCAL);
     if (ds_lib_ == NULL) {
-        isc_throw(DataSourceError, "Unable to load " << ds_name_ <<
-                  ": " << dlerror());
+        isc_throw(DataSourceLibraryError, dlerror());
     }
 }
 
@@ -51,9 +49,7 @@ DLHolder::getSym(const char* name) {
 
     const char* dlsym_error = dlerror();
     if (dlsym_error != NULL) {
-        dlclose(ds_lib_);
-        isc_throw(DataSourceError, "Error in library " << ds_name_ <<
-                  ": " << dlsym_error);
+        isc_throw(DataSourceLibraryError, dlsym_error);
     }
 
     return (sym);

src/lib/datasrc/factory.h

@@ -20,6 +20,7 @@
 
 //#include <exceptions/exceptions.h>
 
+#include <datasrc/data_source.h>
 #include <datasrc/client.h>
 #include <exceptions/exceptions.h>
 
@@ -28,24 +29,117 @@
 namespace isc {
 namespace datasrc {
 
+
+/// \brief Raised if there is an error loading the datasource implementation
+///        library, or if that library misses a needed symbol
+class DataSourceLibraryError : public DataSourceError {
+public:
+    DataSourceLibraryError(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what) {}
+};
+
+/// \brief Raised if the given config contains bad data
+///
+/// Depending on the datasource type, the configuration may differ (for
+/// instance, the sqlite3 datasource needs a database file).
+class DataSourceConfigError : public DataSourceError {
+public:
+    DataSourceConfigError(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what) {}
+};
+
 typedef DataSourceClient* ds_creator(isc::data::ConstElementPtr config);
 typedef void ds_destructor(DataSourceClient* instance);
 
+/// \brief Container class for dynamically loaded libraries
+///
+/// This class is used to dlopen() a library, provides access to dlsym(),
+/// and cleans up the dlopened library when the instance of this class is
+/// destroyed.
+///
+/// Its main function is to provide RAII-style access to dlopen'ed libraries.
+///
+/// \note Currently it is Datasource-backend specific. If we have need for this
+///       in other places than for dynamically loading datasources, then, apart
+///       from moving it to another location, we also need to make the
+///       exceptions raised more general.
 class DLHolder {
 public:
+    /// \brief Constructor
+    ///
+    /// \param name The name of the library (.so) file. This file must be in
+    ///             the library path.
+    ///
+    /// \exception DataSourceLibraryError If the library cannot be found or
+    ///            cannot be loaded.
     DLHolder(const std::string& name);
+
+    /// \brief Destructor
+    ///
+    /// Cleans up the library by calling dlclose()
     ~DLHolder();
+
+    /// \brief Retrieve a symbol
+    ///
+    /// This retrieves a symbol from the loaded library.
+    ///
+    /// \exception DataSourceLibraryError if the symbol cannot be found, or if
+    ///            another error (as reported by dlerror() occurs.
+    ///
+    /// \param name The name of the symbol to retrieve
+    /// \return A pointer to the symbol
     void* getSym(const char* name);
 private:
-    const std::string ds_name_;
+    /// Pointer to the dynamically loaded library structure
     void *ds_lib_;
 };
 
+
+/// \brief Container for a specific instance of a dynamically loaded
+///        DataSourceClient implementation
+///
+/// Given a datasource type and a type-specific set of configuration data,
+/// the corresponding dynamic library is loaded (if it hadn't been already),
+/// and an instance is created. This instance is stored within this structure,
+/// and can be accessed through getInstance().
+///
+/// The 'type' is actually the name of the library, minus the '_ds.so' postfix
+/// Datasource implementation libraries therefore have a fixed name, both for
+/// easy recognition and to reduce potential mistakes.
+/// For example, the sqlite3 implementation has the type 'sqlite3', and the
+/// derived filename 'sqlite3_ds.so'
+///
+/// There are of course some demands to an implementation, not all of which
+/// can be verified compile-time. It must provide a creator and destructor
+/// functions. The creator function must return an instance of a subclass of
+/// DataSourceClient.
+///
 class DataSourceClientContainer {
 public:
+    /// \brief Constructor
+    ///
+    /// \exception DataSourceLibraryError if there is an error loading the
+    ///            backend library
+    /// \exception DataSourceConfigError if the given config is not correct
+    ///            for the given type
+    ///
+    /// \param type The type of the datasource client. Based on the value of
+    ///             type, a specific backend library is used, by appending the
+    ///             string '_ds.so' to the given type, and loading that as the
+    ///             implementation library
+    /// \param config Type-specific configuration data, see the documentation
+    ///               of the datasource backend type for information on what
+    ///               configuration data to pass.
     DataSourceClientContainer(const std::string& type,
                               isc::data::ConstElementPtr config);
+
+    /// \brief Destructor
     ~DataSourceClientContainer();
+
+    /// \brief Accessor to the instance
+    ///
+    /// \return Reference to the DataSourceClient instance contained in this
+    ///         container.
     DataSourceClient& getInstance() { return *instance_; }
 
 private:
@@ -54,17 +148,6 @@ private:
     DLHolder ds_lib_;
 };
 
-
-/// \brief Raised if the given config contains bad data
-///
-/// Depending on the datasource type, the configuration may differ (for
-/// instance, the sqlite3 datasource needs a database file).
-class DataSourceConfigError : public isc::Exception {
-public:
-    DataSourceConfigError(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) {}
-};
-
 /// \brief Create a datasource instance
 ///
 /// This function is a fixed generator for datasource instances of all types.

src/lib/datasrc/tests/factory_unittest.cc

@@ -26,141 +26,150 @@
 using namespace isc::datasrc;
 using namespace isc::data;
 
+std::string SQLITE_DBFILE_EXAMPLE_ORG = TEST_DATA_DIR "/example.org.sqlite3";
+
 namespace {
 
-// The default implementation is NotImplemented
-TEST(FactoryTest, memoryClient) {
+TEST(FactoryTest, sqlite3ClientBadConfig) {
+    // We start out by building the configuration data bit by bit,
+    // testing each form of 'bad config', until we have a good one.
+    // Then we do some very basic operation on the client (detailed
+    // tests are left to the implementation-specific backends)
     ElementPtr config;
-    ASSERT_THROW(DataSourceClientContainer client("memory", config),
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
     config = Element::create("asdf");
-    ASSERT_THROW(DataSourceClientContainer("memory", config),
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", 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),
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
     config->set("class", ElementPtr());
-    ASSERT_THROW(DataSourceClientContainer("memory", config),
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
     config->set("class", Element::create(1));
-    ASSERT_THROW(DataSourceClientContainer("memory", config),
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
     config->set("class", Element::create("FOO"));
-    ASSERT_THROW(DataSourceClientContainer("memory", config),
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
     config->set("class", Element::create("IN"));
-    ASSERT_THROW(DataSourceClientContainer("memory", config),
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("zones", ElementPtr());
-    ASSERT_THROW(DataSourceClientContainer("memory", config),
+    config->set("database_file", ElementPtr());
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("zones", Element::create(1));
-    ASSERT_THROW(DataSourceClientContainer("memory", config),
+    config->set("database_file", Element::create(1));
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
                  DataSourceConfigError);
 
-    config->set("zones", Element::createList());
-    DataSourceClientContainer("memory", config);
-}
+    config->set("database_file", Element::create("/foo/bar/doesnotexist"));
+    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+                 SQLite3Error);
 
-TEST(FactoryTest, badType) {
-    ASSERT_THROW(DataSourceClientContainer("foo", ElementPtr()), DataSourceError);
+    config->set("database_file", Element::create(SQLITE_DBFILE_EXAMPLE_ORG));
+    DataSourceClientContainer dsc("sqlite3", config);
+
+    DataSourceClient::FindResult result1(
+        dsc.getInstance().findZone(isc::dns::Name("example.org.")));
+    ASSERT_EQ(result::SUCCESS, result1.code);
+
+    DataSourceClient::FindResult result2(
+        dsc.getInstance().findZone(isc::dns::Name("no.such.zone.")));
+    ASSERT_EQ(result::NOTFOUND, result2.code);
+
+    ZoneIteratorPtr iterator(dsc.getInstance().getIterator(
+        isc::dns::Name("example.org.")));
+
+    ZoneUpdaterPtr updater(dsc.getInstance().getUpdater(
+        isc::dns::Name("example.org."), false));
 }
 
-TEST(FactoryTest, sqlite3ClientBadConfig) {
+TEST(FactoryTest, memoryClient) {
+    // We start out by building the configuration data bit by bit,
+    // testing each form of 'bad config', until we have a good one.
+    // Then we do some very basic operation on the client (detailed
+    // tests are left to the implementation-specific backends)
     ElementPtr config;
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    ASSERT_THROW(DataSourceClientContainer client("memory", config),
                  DataSourceConfigError);
 
     config = Element::create("asdf");
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
     config = Element::createMap();
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("class", ElementPtr());
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("type", ElementPtr());
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("class", Element::create(1));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("type", Element::create(1));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("class", Element::create("FOO"));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("type", Element::create("FOO"));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("class", Element::create("IN"));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("type", Element::create("memory"));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("database_file", ElementPtr());
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("class", ElementPtr());
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("database_file", Element::create(1));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("class", Element::create(1));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("database_file", Element::create("/foo/bar/doesnotexist"));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 SQLite3Error);
-}
-
-
-TEST(FactoryTest, sqlite3ClientBadConfig3) {
-    ElementPtr config;
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("class", Element::create("FOO"));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config = Element::create("asdf");
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("class", Element::create("IN"));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config = Element::createMap();
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("zones", ElementPtr());
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("database_file", ElementPtr());
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
+    config->set("zones", Element::create(1));
+    ASSERT_THROW(DataSourceClientContainer("memory", config),
                  DataSourceConfigError);
 
-    config->set("database_file", Element::create(1));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+    config->set("zones", Element::createList());
+    DataSourceClientContainer dsc("memory", config);
 
-    config->set("database_file", Element::create("/foo/bar/doesnotexist"));
-    ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 SQLite3Error);
+    // Once it is able to load some zones, we should add a few tests
+    // here to see that it does.
+    DataSourceClient::FindResult result(
+        dsc.getInstance().findZone(isc::dns::Name("no.such.zone.")));
+    ASSERT_EQ(result::NOTFOUND, result.code);
 
-    // 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);
+    ASSERT_THROW(dsc.getInstance().getIterator(isc::dns::Name("example.org.")),
+                 DataSourceError);
+
+    ASSERT_THROW(dsc.getInstance().getUpdater(isc::dns::Name("no.such.zone."),
+                                              false), isc::NotImplemented);
 }
+
+TEST(FactoryTest, badType) {
+    ASSERT_THROW(DataSourceClientContainer("foo", ElementPtr()),
+                                           DataSourceError);
+}
+
 } // end anonymous namespace