BIND 10 master, updated. df1298668ac3e758576b8b2bd6475c70cff7a57f [trac1253] one final addition to makefile

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Oct 10 09:56:15 UTC 2011


The branch, master has been updated
       via  df1298668ac3e758576b8b2bd6475c70cff7a57f (commit)
       via  f3f87eb305123de57135aaa96c12190f3bf1951b (commit)
       via  f5bb60e5636d908de8534d35b5f06142ae2a8c3a (commit)
       via  8f3c0649785d7fb0df37a9ba9e0e20c978044bb7 (commit)
       via  2a2aa4ccfb548b2a18b10e97acd80df324c5d4a8 (commit)
       via  02acd96cff43650110f4af6d2fb2a8143887ac00 (commit)
       via  a6790c80bfcefde81e032db9d3a45c7a9e48faad (commit)
       via  6b206d435a3dd92ef4a18f1c4558da147016fe4f (commit)
       via  e114429f15c0ff8b5eb77728985281afcfc0d37a (commit)
       via  6dbe35be17827ccf8bfc904be707aea01fb4ef94 (commit)
       via  a8a8ceb589f9f3bf4da29717eec446cb2766032c (commit)
       via  1f6c32ac6941c3c2ec456017e73ea74ca5944e1c (commit)
       via  cadfcca91ef5bdb2c72c9db4e918ff6ac7b10e65 (commit)
      from  0a149e0c7faf8fc0db56d4804acfb3df99dcebb4 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit df1298668ac3e758576b8b2bd6475c70cff7a57f
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Oct 10 09:50:58 2011 +0000

    [trac1253] one final addition to makefile

commit f3f87eb305123de57135aaa96c12190f3bf1951b
Merge: f5bb60e5636d908de8534d35b5f06142ae2a8c3a 0a149e0c7faf8fc0db56d4804acfb3df99dcebb4
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Oct 10 10:37:58 2011 +0200

    [1253] Merge branch 'master' into trac1253
    
    Conflicts:
    	src/lib/datasrc/sqlite3_accessor.cc
    	src/lib/python/isc/datasrc/client_python.cc

commit f5bb60e5636d908de8534d35b5f06142ae2a8c3a
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Oct 7 11:47:08 2011 +0200

    [1253] catch (...) in createInstance()s

commit 8f3c0649785d7fb0df37a9ba9e0e20c978044bb7
Author: Jelte Jansen <jelte at isc.org>
Date:   Thu Oct 6 14:33:19 2011 +0200

    [1253] catch ... as well

commit 2a2aa4ccfb548b2a18b10e97acd80df324c5d4a8
Author: Jelte Jansen <jelte at isc.org>
Date:   Thu Oct 6 13:14:43 2011 +0200

    [1253] update docs and add safeguard catch

commit 02acd96cff43650110f4af6d2fb2a8143887ac00
Author: Jelte Jansen <jelte at isc.org>
Date:   Thu Oct 6 03:46:18 2011 -0700

    [1253] slightly different way to set ld

commit a6790c80bfcefde81e032db9d3a45c7a9e48faad
Author: Jelte Jansen <jelte at isc.org>
Date:   Thu Oct 6 11:48:41 2011 +0200

    [1253] make createInstance set an error instead of raising an exception

commit 6b206d435a3dd92ef4a18f1c4558da147016fe4f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Oct 5 15:41:47 2011 +0200

    [1253] Fix dependencies in Makefile
    
    Autotools don't track dependencies on libraries with absolute path. As
    the library lives in the same directory, the directory order does not
    help, so we need the dep to be tracked explicitly, which means we need
    to use relative path.
    
    The old version breaks with make -j<a lot>.

commit e114429f15c0ff8b5eb77728985281afcfc0d37a
Author: Jelte Jansen <jelte at isc.org>
Date:   Tue Oct 4 18:32:09 2011 +0200

    [1253] makefile update and one more test

commit 6dbe35be17827ccf8bfc904be707aea01fb4ef94
Author: Jelte Jansen <jelte at isc.org>
Date:   Tue Oct 4 16:41:36 2011 +0200

    [1253] makefile cleanup

commit a8a8ceb589f9f3bf4da29717eec446cb2766032c
Author: Jelte Jansen <jelte at isc.org>
Date:   Tue Oct 4 16:28:27 2011 +0200

    [1253] better exception handling, tests and docs

commit 1f6c32ac6941c3c2ec456017e73ea74ca5944e1c
Author: Jelte Jansen <jelte at isc.org>
Date:   Tue Oct 4 15:14:44 2011 +0200

    [1253] initialize new member in constructors

commit cadfcca91ef5bdb2c72c9db4e918ff6ac7b10e65
Author: Jelte Jansen <jelte at isc.org>
Date:   Tue Oct 4 12:34:04 2011 +0200

    [1253] use client container instead of client directly
    
    in client_python.cc. This hides the container implementation details from python API, and make datasourceclient usable 'directly'.
    
    Added a bit of optional niftyness to some of the createXXX functions; if you pass it any python object, it'll INCREF it and DECREF it again upon its own destruction. With this, we can make sure some objects don't live past the lifetime of other objects they depend on (in this case iterator, updater and finder, regarding the DataSourceClient they came from)

-----------------------------------------------------------------------

Summary of changes:
 src/bin/xfrin/tests/Makefile.am                  |    3 +
 src/bin/xfrin/tests/xfrin_test.py                |    5 ++-
 src/lib/datasrc/Makefile.am                      |    6 ++-
 src/lib/datasrc/factory.cc                       |   13 +++++-
 src/lib/datasrc/factory.h                        |   22 ++-------
 src/lib/datasrc/memory_datasrc.cc                |   16 +++++-
 src/lib/datasrc/memory_datasrc.h                 |    9 +++-
 src/lib/datasrc/sqlite3_accessor.cc              |   20 ++++++--
 src/lib/datasrc/sqlite3_accessor.h               |    9 +++-
 src/lib/datasrc/tests/factory_unittest.cc        |   46 +++++++++---------
 src/lib/python/isc/datasrc/Makefile.am           |    7 ---
 src/lib/python/isc/datasrc/client_inc.cc         |   15 ++++++-
 src/lib/python/isc/datasrc/client_python.cc      |   53 +++++++++++++---------
 src/lib/python/isc/datasrc/finder_python.cc      |   17 ++++++-
 src/lib/python/isc/datasrc/finder_python.h       |   10 ++++-
 src/lib/python/isc/datasrc/iterator_python.cc    |   19 +++++++-
 src/lib/python/isc/datasrc/iterator_python.h     |   10 ++++-
 src/lib/python/isc/datasrc/tests/Makefile.am     |    7 +++-
 src/lib/python/isc/datasrc/tests/datasrc_test.py |   31 +++++++++---
 src/lib/python/isc/datasrc/updater_python.cc     |   18 +++++++-
 src/lib/python/isc/datasrc/updater_python.h      |   10 ++++-
 21 files changed, 245 insertions(+), 101 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/xfrin/tests/Makefile.am b/src/bin/xfrin/tests/Makefile.am
index ae6620c..8f4fa91 100644
--- a/src/bin/xfrin/tests/Makefile.am
+++ b/src/bin/xfrin/tests/Makefile.am
@@ -9,6 +9,9 @@ EXTRA_DIST = $(PYTESTS)
 LIBRARY_PATH_PLACEHOLDER =
 if SET_ENV_LIBRARY_PATH
 LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/util/io/.libs:$(abs_top_builddir)/src/lib/datasrc/.libs:$$$(ENV_LIBRARY_PATH)
+else
+# sunstudio needs the ds path even if not all paths are necessary
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/datasrc/.libs
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS
diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py
index 5f544db..a91f7f5 100644
--- a/src/bin/xfrin/tests/xfrin_test.py
+++ b/src/bin/xfrin/tests/xfrin_test.py
@@ -1483,11 +1483,14 @@ class TestXFRSessionWithSQLite3(TestXfrinConnection):
     def setUp(self):
         self.sqlite3db_src = TESTDATA_SRCDIR + '/example.com.sqlite3'
         self.sqlite3db_obj = TESTDATA_OBJDIR + '/example.com.sqlite3.copy'
+        self.sqlite3db_cfg = "{ \"database_file\": \"" +\
+                             self.sqlite3db_obj + "\"}"
         super().setUp()
         if os.path.exists(self.sqlite3db_obj):
             os.unlink(self.sqlite3db_obj)
         shutil.copyfile(self.sqlite3db_src, self.sqlite3db_obj)
-        self.conn._datasrc_client = DataSourceClient(self.sqlite3db_obj)
+        self.conn._datasrc_client = DataSourceClient("sqlite3",
+                                                     self.sqlite3db_cfg)
 
     def tearDown(self):
         if os.path.exists(self.sqlite3db_obj):
diff --git a/src/lib/datasrc/Makefile.am b/src/lib/datasrc/Makefile.am
index 5e193d2..bf1171e 100644
--- a/src/lib/datasrc/Makefile.am
+++ b/src/lib/datasrc/Makefile.am
@@ -22,15 +22,19 @@ libdatasrc_la_SOURCES += result.h
 libdatasrc_la_SOURCES += logger.h logger.cc
 libdatasrc_la_SOURCES += client.h iterator.h
 libdatasrc_la_SOURCES += database.h database.cc
-#libdatasrc_la_SOURCES += sqlite3_accessor.h sqlite3_accessor.cc
 libdatasrc_la_SOURCES += factory.h factory.cc
 nodist_libdatasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 
 sqlite3_ds_la_SOURCES = sqlite3_accessor.h sqlite3_accessor.cc
 sqlite3_ds_la_LDFLAGS = -module
+sqlite3_ds_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+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_LDFLAGS = -module
+memory_ds_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+memory_ds_la_LIBADD += libdatasrc.la
 
 libdatasrc_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
 libdatasrc_la_LIBADD += $(top_builddir)/src/lib/dns/libdns++.la
diff --git a/src/lib/datasrc/factory.cc b/src/lib/datasrc/factory.cc
index eddd4f4..70a73e8 100644
--- a/src/lib/datasrc/factory.cc
+++ b/src/lib/datasrc/factory.cc
@@ -70,7 +70,18 @@ DataSourceClientContainer::DataSourceClientContainer(const std::string& type,
     ds_creator* ds_create = (ds_creator*)ds_lib_.getSym("createInstance");
     destructor_ = (ds_destructor*)ds_lib_.getSym("destroyInstance");
 
-    instance_ = ds_create(config);
+    std::string error;
+    try {
+        instance_ = ds_create(config, error);
+        if (instance_ == NULL) {
+            isc_throw(DataSourceError, error);
+        }
+    } catch (const std::exception& exc) {
+        isc_throw(DataSourceError, "Unknown uncaught exception from " + type +
+                                   " createInstance: " + exc.what());
+    } catch (...) {
+        isc_throw(DataSourceError, "Unknown uncaught exception from " + type);
+    }
 }
 
 DataSourceClientContainer::~DataSourceClientContainer() {
diff --git a/src/lib/datasrc/factory.h b/src/lib/datasrc/factory.h
index 8db9ec9..0284067 100644
--- a/src/lib/datasrc/factory.h
+++ b/src/lib/datasrc/factory.h
@@ -44,21 +44,8 @@ public:
         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) {}
-    // This exception is created in the dynamic modules. Apparently
-    // sunstudio can't handle it if we then automatically derive the
-    // destructor, so we provide it explicitely
-    ~DataSourceConfigError() throw() {}
-};
-
-typedef DataSourceClient* ds_creator(isc::data::ConstElementPtr config);
+typedef DataSourceClient* ds_creator(isc::data::ConstElementPtr config,
+                                     std::string& error);
 typedef void ds_destructor(DataSourceClient* instance);
 
 /// \brief Container class for dynamically loaded libraries
@@ -146,8 +133,9 @@ public:
     ///            backend library
     /// \exception DataSourceLibrarySymbolError if the library does not have
     ///            the needed symbols, or if there is an error reading them
-    /// \exception DataSourceConfigError if the given config is not correct
-    ///            for the given type
+    /// \exception DataError if the given config is not correct
+    ///            for the given type, or if there was a problem during
+    ///            initialization
     ///
     /// \param type The type of the datasource client. Based on the value of
     ///             type, a specific backend library is used, by appending the
diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc
index 4c9e53f..3f84437 100644
--- a/src/lib/datasrc/memory_datasrc.cc
+++ b/src/lib/datasrc/memory_datasrc.cc
@@ -931,12 +931,22 @@ checkConfig(ConstElementPtr config, ElementPtr errors) {
 } // end anonymous namespace
 
 DataSourceClient *
-createInstance(isc::data::ConstElementPtr config) {
+createInstance(isc::data::ConstElementPtr config, std::string& error) {
     ElementPtr errors(Element::createList());
     if (!checkConfig(config, errors)) {
-        isc_throw(DataSourceConfigError, errors->str());
+        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);
     }
-    return (new InMemoryClient());
 }
 
 void destroyInstance(DataSourceClient* instance) {
diff --git a/src/lib/datasrc/memory_datasrc.h b/src/lib/datasrc/memory_datasrc.h
index cf467a2..610deff 100644
--- a/src/lib/datasrc/memory_datasrc.h
+++ b/src/lib/datasrc/memory_datasrc.h
@@ -310,7 +310,14 @@ private:
 ///
 /// This configuration setup is currently under discussion and will change in
 /// the near future.
-extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config);
+///
+/// \param config The configuration for the datasource instance
+/// \param error This string will be set to an error message if an error occurs
+///              during initialization
+/// \return An instance of the memory datasource client, or NULL if there was
+///         an error
+extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config,
+                                            std::string& error);
 
 /// \brief Destroy the instance created by createInstance()
 extern "C" void destroyInstance(DataSourceClient* instance);
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index 8fb95af..6d6dbba 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -751,15 +751,25 @@ checkConfig(ConstElementPtr config, ElementPtr errors) {
 } // end anonymous namespace
 
 DataSourceClient *
-createInstance(isc::data::ConstElementPtr config) {
+createInstance(isc::data::ConstElementPtr config, std::string& error) {
     ElementPtr errors(Element::createList());
     if (!checkConfig(config, errors)) {
-        isc_throw(DataSourceConfigError, errors->str());
+        error = "Configuration error: " + errors->str();
+        return (NULL);
     }
     std::string dbfile = config->get(CONFIG_ITEM_DATABASE_FILE)->stringValue();
-    boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
-        new SQLite3Accessor(dbfile, "IN")); // XXX: avoid hardcode RR class
-    return (new DatabaseClient(isc::dns::RRClass::IN(), sqlite3_accessor));
+    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) {
diff --git a/src/lib/datasrc/sqlite3_accessor.h b/src/lib/datasrc/sqlite3_accessor.h
index 86b9e6d..8b74309 100644
--- a/src/lib/datasrc/sqlite3_accessor.h
+++ b/src/lib/datasrc/sqlite3_accessor.h
@@ -190,7 +190,14 @@ private:
 ///
 /// This configuration setup is currently under discussion and will change in
 /// the near future.
-extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config);
+///
+/// \param config The configuration for the datasource instance
+/// \param error This string will be set to an error message if an error occurs
+///              during initialization
+/// \return An instance of the sqlite3 datasource client, or NULL if there was
+///         an error
+extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config,
+                                            std::string& error);
 
 /// \brief Destroy the instance created by createInstance()
 extern "C" void destroyInstance(DataSourceClient* instance);
diff --git a/src/lib/datasrc/tests/factory_unittest.cc b/src/lib/datasrc/tests/factory_unittest.cc
index 94d1118..0133508 100644
--- a/src/lib/datasrc/tests/factory_unittest.cc
+++ b/src/lib/datasrc/tests/factory_unittest.cc
@@ -37,43 +37,43 @@ TEST(FactoryTest, sqlite3ClientBadConfig) {
     // tests are left to the implementation-specific backends)
     ElementPtr config;
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config = Element::create("asdf");
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config = Element::createMap();
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create("FOO"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create("IN"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("database_file", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("database_file", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("database_file", Element::create("/foo/bar/doesnotexist"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 SQLite3Error);
+                 DataSourceError);
 
     config->set("database_file", Element::create(SQLITE_DBFILE_EXAMPLE_ORG));
     DataSourceClientContainer dsc("sqlite3", config);
@@ -100,55 +100,55 @@ TEST(FactoryTest, memoryClient) {
     // tests are left to the implementation-specific backends)
     ElementPtr config;
     ASSERT_THROW(DataSourceClientContainer client("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config = Element::create("asdf");
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config = Element::createMap();
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("type", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("type", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("type", Element::create("FOO"));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("type", Element::create("memory"));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create("FOO"));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create("IN"));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("zones", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("zones", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("zones", Element::createList());
     DataSourceClientContainer dsc("memory", config);
diff --git a/src/lib/python/isc/datasrc/Makefile.am b/src/lib/python/isc/datasrc/Makefile.am
index 07fb417..60282d9 100644
--- a/src/lib/python/isc/datasrc/Makefile.am
+++ b/src/lib/python/isc/datasrc/Makefile.am
@@ -16,12 +16,6 @@ datasrc_la_SOURCES += client_python.cc client_python.h
 datasrc_la_SOURCES += iterator_python.cc iterator_python.h
 datasrc_la_SOURCES += finder_python.cc finder_python.h
 datasrc_la_SOURCES += updater_python.cc updater_python.h
-# This is a temporary workaround for #1206, where the InMemoryClient has been
-# moved to an ldopened library. We could add that library to LDADD, but that
-# is nonportable. When #1207 is done this becomes moot anyway, and the
-# specific workaround is not needed anymore, so we can then remove this
-# line again.
-datasrc_la_SOURCES += ${top_srcdir}/src/lib/datasrc/sqlite3_accessor.cc
 
 datasrc_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 datasrc_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
@@ -30,7 +24,6 @@ datasrc_la_LDFLAGS += -module
 datasrc_la_LIBADD = $(top_builddir)/src/lib/datasrc/libdatasrc.la
 datasrc_la_LIBADD += $(top_builddir)/src/lib/dns/python/libpydnspp.la
 datasrc_la_LIBADD += $(PYTHON_LIB)
-#datasrc_la_LIBADD += $(SQLITE_LIBS)
 
 EXTRA_DIST = client_inc.cc
 EXTRA_DIST += finder_inc.cc
diff --git a/src/lib/python/isc/datasrc/client_inc.cc b/src/lib/python/isc/datasrc/client_inc.cc
index 9bfedac..b81f48d 100644
--- a/src/lib/python/isc/datasrc/client_inc.cc
+++ b/src/lib/python/isc/datasrc/client_inc.cc
@@ -7,7 +7,20 @@ This is the python wrapper for the abstract base class that defines\n\
 the common interface for various types of data source clients. A data\n\
 source client is a top level access point to a data source, allowing \n\
 various operations on the data source such as lookups, traversing or \n\
-updates. The client class itself has limited focus and delegates \n\
+updates.\n\
+This class serves as both the factory and the main interface to those \n\
+classes.\n\
+\n\
+The constructor takes two arguments; a type (string), and\n\
+configuration data for a datasource client of that type. The configuration\n\
+data is currently passed as a JSON in string form, and its contents depend\n\
+on the type of datasource from the first argument. For instance, a\n\
+datasource of type \"sqlite3\" takes the config \n\
+{ \"database_file\": \"/var/example.org\" }\n\
+We may in the future add support for passing configuration data,\n\
+but right now we limit it to a JSON-formatted string\n\
+\n\
+The client class itself has limited focus and delegates \n\
 the responsibility for these specific operations to other (c++) classes;\n\
 in general methods of this class act as factories of these other classes.\n\
 \n\
diff --git a/src/lib/python/isc/datasrc/client_python.cc b/src/lib/python/isc/datasrc/client_python.cc
index 8a3d408..caebd25 100644
--- a/src/lib/python/isc/datasrc/client_python.cc
+++ b/src/lib/python/isc/datasrc/client_python.cc
@@ -23,6 +23,7 @@
 #include <util/python/pycppwrapper_util.h>
 
 #include <datasrc/client.h>
+#include <datasrc/factory.h>
 #include <datasrc/database.h>
 #include <datasrc/data_source.h>
 #include <datasrc/sqlite3_accessor.h>
@@ -50,13 +51,9 @@ namespace {
 class s_DataSourceClient : public PyObject {
 public:
     s_DataSourceClient() : cppobj(NULL) {};
-    DataSourceClient* cppobj;
+    DataSourceClientContainer* cppobj;
 };
 
-// Shortcut type which would be convenient for adding class variables safely.
-typedef CPPPyObjectContainer<s_DataSourceClient, DataSourceClient>
-    DataSourceClientContainer;
-
 PyObject*
 DataSourceClient_findZone(PyObject* po_self, PyObject* args) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
@@ -64,12 +61,12 @@ DataSourceClient_findZone(PyObject* po_self, PyObject* args) {
     if (PyArg_ParseTuple(args, "O!", &name_type, &name)) {
         try {
             DataSourceClient::FindResult find_result(
-                self->cppobj->findZone(PyName_ToName(name)));
+                self->cppobj->getInstance().findZone(PyName_ToName(name)));
 
             result::Result r = find_result.code;
             ZoneFinderPtr zfp = find_result.zone_finder;
             // Use N instead of O so refcount isn't increased twice
-            return (Py_BuildValue("IN", r, createZoneFinderObject(zfp)));
+            return (Py_BuildValue("IN", r, createZoneFinderObject(zfp, po_self)));
         } catch (const std::exception& exc) {
             PyErr_SetString(getDataSourceException("Error"), exc.what());
             return (NULL);
@@ -90,7 +87,8 @@ DataSourceClient_getIterator(PyObject* po_self, PyObject* args) {
     if (PyArg_ParseTuple(args, "O!", &name_type, &name_obj)) {
         try {
             return (createZoneIteratorObject(
-                        self->cppobj->getIterator(PyName_ToName(name_obj))));
+                self->cppobj->getInstance().getIterator(PyName_ToName(name_obj)),
+                po_self));
         } catch (const isc::NotImplemented& ne) {
             PyErr_SetString(getDataSourceException("NotImplemented"),
                             ne.what());
@@ -121,11 +119,12 @@ DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
         bool replace = (replace_obj != Py_False);
         try {
             ZoneUpdaterPtr updater =
-                self->cppobj->getUpdater(PyName_ToName(name_obj), replace);
+                self->cppobj->getInstance().getUpdater(PyName_ToName(name_obj),
+                                                       replace);
             if (!updater) {
                 return (Py_None);
             }
-            return (createZoneUpdaterObject(updater));
+            return (createZoneUpdaterObject(updater, po_self));
         } catch (const isc::NotImplemented& ne) {
             PyErr_SetString(getDataSourceException("NotImplemented"),
                             ne.what());
@@ -165,23 +164,33 @@ PyMethodDef DataSourceClient_methods[] = {
 
 int
 DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
-    // TODO: we should use the factory function which hasn't been written
-    // yet. For now we hardcode the sqlite3 initialization, and pass it one
-    // string for the database file. (similar to how the 'old direct'
-    // sqlite3_ds code works).  Of course, we shouldn't hardcode the RR class
-    // at that point.
+    char* ds_type_str;
+    char* ds_config_str;
     try {
-        char* db_file_name;
-        if (PyArg_ParseTuple(args, "s", &db_file_name)) {
-            boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
-                new SQLite3Accessor(db_file_name, "IN"));
-            self->cppobj = new DatabaseClient(isc::dns::RRClass::IN(),
-                                              sqlite3_accessor);
+        // Turn the given argument into config Element; then simply call
+        // factory class to do its magic
+
+        // for now, ds_config must be JSON string
+        if (PyArg_ParseTuple(args, "ss", &ds_type_str, &ds_config_str)) {
+            isc::data::ConstElementPtr ds_config =
+                isc::data::Element::fromJSON(ds_config_str);
+            self->cppobj = new DataSourceClientContainer(ds_type_str,
+                                                         ds_config);
             return (0);
         } else {
             return (-1);
         }
-
+    } catch (const isc::data::JSONError& je) {
+        const string ex_what = "JSON parse error in data source configuration "
+                               "data for type " +
+                               string(ds_type_str) + ":" + je.what();
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+        return (-1);
+    } catch (const DataSourceError& dse) {
+        const string ex_what = "Failed to create DataSourceClient of type " +
+                               string(ds_type_str) + ":" + dse.what();
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+        return (-1);
     } catch (const exception& ex) {
         const string ex_what = "Failed to construct DataSourceClient object: " +
             string(ex.what());
diff --git a/src/lib/python/isc/datasrc/finder_python.cc b/src/lib/python/isc/datasrc/finder_python.cc
index 598d300..3a574ba 100644
--- a/src/lib/python/isc/datasrc/finder_python.cc
+++ b/src/lib/python/isc/datasrc/finder_python.cc
@@ -103,8 +103,14 @@ namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneFinder : public PyObject {
 public:
-    s_ZoneFinder() : cppobj(ZoneFinderPtr()) {};
+    s_ZoneFinder() : cppobj(ZoneFinderPtr()), base_obj(NULL) {};
     ZoneFinderPtr cppobj;
+    // This is a reference to a base object; if the object of this class
+    // depends on another object to be in scope during its lifetime,
+    // we use INCREF the base object upon creation, and DECREF it at
+    // the end of the destructor
+    // This is an optional argument to createXXX(). If NULL, it is ignored.
+    PyObject* base_obj;
 };
 
 // Shortcut type which would be convenient for adding class variables safely.
@@ -125,6 +131,9 @@ ZoneFinder_destroy(s_ZoneFinder* const self) {
     // cppobj is a shared ptr, but to make sure things are not destroyed in
     // the wrong order, we reset it here.
     self->cppobj.reset();
+    if (self->base_obj != NULL) {
+        Py_DECREF(self->base_obj);
+    }
     Py_TYPE(self)->tp_free(self);
 }
 
@@ -233,11 +242,15 @@ PyTypeObject zonefinder_type = {
 };
 
 PyObject*
-createZoneFinderObject(isc::datasrc::ZoneFinderPtr source) {
+createZoneFinderObject(isc::datasrc::ZoneFinderPtr source, PyObject* base_obj) {
     s_ZoneFinder* py_zi = static_cast<s_ZoneFinder*>(
         zonefinder_type.tp_alloc(&zonefinder_type, 0));
     if (py_zi != NULL) {
         py_zi->cppobj = source;
+        py_zi->base_obj = base_obj;
+    }
+    if (base_obj != NULL) {
+        Py_INCREF(base_obj);
     }
     return (py_zi);
 }
diff --git a/src/lib/python/isc/datasrc/finder_python.h b/src/lib/python/isc/datasrc/finder_python.h
index 5f2404e..23bc457 100644
--- a/src/lib/python/isc/datasrc/finder_python.h
+++ b/src/lib/python/isc/datasrc/finder_python.h
@@ -24,7 +24,15 @@ namespace python {
 
 extern PyTypeObject zonefinder_type;
 
-PyObject* createZoneFinderObject(isc::datasrc::ZoneFinderPtr source);
+/// \brief Create a ZoneFinder python object
+///
+/// \param source The zone iterator pointer to wrap
+/// \param base_obj An optional PyObject that this ZoneFinder depends on
+///                 Its refcount is increased, and will be decreased when
+///                 this zone iterator is destroyed, making sure that the
+///                 base object is never destroyed before this zonefinder.
+PyObject* createZoneFinderObject(isc::datasrc::ZoneFinderPtr source,
+                                 PyObject* base_obj = NULL);
 
 } // namespace python
 } // namespace datasrc
diff --git a/src/lib/python/isc/datasrc/iterator_python.cc b/src/lib/python/isc/datasrc/iterator_python.cc
index b482ea6..c52ab4a 100644
--- a/src/lib/python/isc/datasrc/iterator_python.cc
+++ b/src/lib/python/isc/datasrc/iterator_python.cc
@@ -45,8 +45,14 @@ namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneIterator : public PyObject {
 public:
-    s_ZoneIterator() : cppobj(ZoneIteratorPtr()) {};
+    s_ZoneIterator() : cppobj(ZoneIteratorPtr()), base_obj(NULL) {};
     ZoneIteratorPtr cppobj;
+    // This is a reference to a base object; if the object of this class
+    // depends on another object to be in scope during its lifetime,
+    // we use INCREF the base object upon creation, and DECREF it at
+    // the end of the destructor
+    // This is an optional argument to createXXX(). If NULL, it is ignored.
+    PyObject* base_obj;
 };
 
 // Shortcut type which would be convenient for adding class variables safely.
@@ -68,6 +74,9 @@ ZoneIterator_destroy(s_ZoneIterator* const self) {
     // cppobj is a shared ptr, but to make sure things are not destroyed in
     // the wrong order, we reset it here.
     self->cppobj.reset();
+    if (self->base_obj != NULL) {
+        Py_DECREF(self->base_obj);
+    }
     Py_TYPE(self)->tp_free(self);
 }
 
@@ -187,11 +196,17 @@ PyTypeObject zoneiterator_type = {
 };
 
 PyObject*
-createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source) {
+createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source,
+                         PyObject* base_obj)
+{
     s_ZoneIterator* py_zi = static_cast<s_ZoneIterator*>(
         zoneiterator_type.tp_alloc(&zoneiterator_type, 0));
     if (py_zi != NULL) {
         py_zi->cppobj = source;
+        py_zi->base_obj = base_obj;
+    }
+    if (base_obj != NULL) {
+        Py_INCREF(base_obj);
     }
     return (py_zi);
 }
diff --git a/src/lib/python/isc/datasrc/iterator_python.h b/src/lib/python/isc/datasrc/iterator_python.h
index b457740..7c1b0eb 100644
--- a/src/lib/python/isc/datasrc/iterator_python.h
+++ b/src/lib/python/isc/datasrc/iterator_python.h
@@ -25,7 +25,15 @@ namespace python {
 
 extern PyTypeObject zoneiterator_type;
 
-PyObject* createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source);
+/// \brief Create a ZoneIterator python object
+///
+/// \param source The zone iterator pointer to wrap
+/// \param base_obj An optional PyObject that this ZoneIterator depends on
+///                 Its refcount is increased, and will be decreased when
+///                 this zone iterator is destroyed, making sure that the
+///                 base object is never destroyed before this zone iterator.
+PyObject* createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source,
+                                   PyObject* base_obj = NULL);
 
 
 } // namespace python
diff --git a/src/lib/python/isc/datasrc/tests/Makefile.am b/src/lib/python/isc/datasrc/tests/Makefile.am
index be30dfa..411b5cc 100644
--- a/src/lib/python/isc/datasrc/tests/Makefile.am
+++ b/src/lib/python/isc/datasrc/tests/Makefile.am
@@ -10,9 +10,14 @@ CLEANFILES = $(abs_builddir)/rwtest.sqlite3.copied
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # required by loadable python modules.
-LIBRARY_PATH_PLACEHOLDER =
+# We always add one, the location of the data source modules
+# We may want to add an API method for this to the ds factory, but that is out
+# of scope for this ticket
+LIBRARY_PATH_PLACEHOLDER = $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/datasrc/.libs:
 if SET_ENV_LIBRARY_PATH
 LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/datasrc/.libs:$$$(ENV_LIBRARY_PATH)
+else
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/datasrc/.libs:$$$(ENV_LIBRARY_PATH)
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS
diff --git a/src/lib/python/isc/datasrc/tests/datasrc_test.py b/src/lib/python/isc/datasrc/tests/datasrc_test.py
index 4a7f4ea..0e7e284 100644
--- a/src/lib/python/isc/datasrc/tests/datasrc_test.py
+++ b/src/lib/python/isc/datasrc/tests/datasrc_test.py
@@ -24,9 +24,10 @@ TESTDATA_PATH = os.environ['TESTDATA_PATH'] + os.sep
 TESTDATA_WRITE_PATH = os.environ['TESTDATA_WRITE_PATH'] + os.sep
 
 READ_ZONE_DB_FILE = TESTDATA_PATH + "example.com.sqlite3"
-BROKEN_DB_FILE = TESTDATA_PATH + "brokendb.sqlite3"
 WRITE_ZONE_DB_FILE = TESTDATA_WRITE_PATH + "rwtest.sqlite3.copied"
-NEW_DB_FILE = TESTDATA_WRITE_PATH + "new_db.sqlite3"
+
+READ_ZONE_DB_CONFIG = "{ \"database_file\": \"" + READ_ZONE_DB_FILE + "\" }"
+WRITE_ZONE_DB_CONFIG = "{ \"database_file\": \"" + WRITE_ZONE_DB_FILE + "\"}"
 
 def add_rrset(rrset_list, name, rrclass, rrtype, ttl, rdatas):
     rrset_to_add = isc.dns.RRset(name, rrclass, rrtype, ttl)
@@ -59,13 +60,27 @@ def check_for_rrset(expected_rrsets, rrset):
 
 class DataSrcClient(unittest.TestCase):
 
-    def test_construct(self):
+    def test_constructors(self):
         # can't construct directly
         self.assertRaises(TypeError, isc.datasrc.ZoneIterator)
 
+        self.assertRaises(TypeError, isc.datasrc.DataSourceClient, 1, "{}")
+        self.assertRaises(TypeError, isc.datasrc.DataSourceClient, "sqlite3", 1)
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "foo", "{}")
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "sqlite3", "")
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "sqlite3", "{}")
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "sqlite3",
+                          "{ \"foo\": 1 }")
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "memory",
+                          "{ \"foo\": 1 }")
 
     def test_iterate(self):
-        dsc = isc.datasrc.DataSourceClient(READ_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
 
         # for RRSIGS, the TTL's are currently modified. This test should
         # start failing when we fix that.
@@ -176,7 +191,7 @@ class DataSrcClient(unittest.TestCase):
         self.assertRaises(TypeError, isc.datasrc.ZoneFinder)
 
     def test_find(self):
-        dsc = isc.datasrc.DataSourceClient(READ_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
 
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))
         self.assertEqual(finder.SUCCESS, result)
@@ -260,7 +275,7 @@ class DataSrcUpdater(unittest.TestCase):
 
     def test_update_delete_commit(self):
 
-        dsc = isc.datasrc.DataSourceClient(WRITE_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
 
         # first make sure, through a separate finder, that some record exists
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))
@@ -334,7 +349,7 @@ class DataSrcUpdater(unittest.TestCase):
                          rrset.to_text())
 
     def test_update_delete_abort(self):
-        dsc = isc.datasrc.DataSourceClient(WRITE_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
 
         # first make sure, through a separate finder, that some record exists
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))
@@ -384,7 +399,7 @@ class DataSrcUpdater(unittest.TestCase):
                          rrset.to_text())
 
     def test_update_for_no_zone(self):
-        dsc = isc.datasrc.DataSourceClient(WRITE_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
         self.assertEqual(None,
                          dsc.get_updater(isc.dns.Name("notexistent.example"),
                                          True))
diff --git a/src/lib/python/isc/datasrc/updater_python.cc b/src/lib/python/isc/datasrc/updater_python.cc
index a9dc581..8060e8f 100644
--- a/src/lib/python/isc/datasrc/updater_python.cc
+++ b/src/lib/python/isc/datasrc/updater_python.cc
@@ -54,8 +54,14 @@ namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneUpdater : public PyObject {
 public:
-    s_ZoneUpdater() : cppobj(ZoneUpdaterPtr()) {};
+    s_ZoneUpdater() : cppobj(ZoneUpdaterPtr()), base_obj(NULL) {};
     ZoneUpdaterPtr cppobj;
+    // This is a reference to a base object; if the object of this class
+    // depends on another object to be in scope during its lifetime,
+    // we use INCREF the base object upon creation, and DECREF it at
+    // the end of the destructor
+    // This is an optional argument to createXXX(). If NULL, it is ignored.
+    PyObject* base_obj;
 };
 
 // Shortcut type which would be convenient for adding class variables safely.
@@ -81,6 +87,9 @@ ZoneUpdater_destroy(s_ZoneUpdater* const self) {
     // cppobj is a shared ptr, but to make sure things are not destroyed in
     // the wrong order, we reset it here.
     self->cppobj.reset();
+    if (self->base_obj != NULL) {
+        Py_DECREF(self->base_obj);
+    }
     Py_TYPE(self)->tp_free(self);
 }
 
@@ -303,12 +312,17 @@ PyTypeObject zoneupdater_type = {
 };
 
 PyObject*
-createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source) {
+createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source,
+                        PyObject* base_obj)
+{
     s_ZoneUpdater* py_zi = static_cast<s_ZoneUpdater*>(
         zoneupdater_type.tp_alloc(&zoneupdater_type, 0));
     if (py_zi != NULL) {
         py_zi->cppobj = source;
     }
+    if (base_obj != NULL) {
+        Py_INCREF(base_obj);
+    }
     return (py_zi);
 }
 
diff --git a/src/lib/python/isc/datasrc/updater_python.h b/src/lib/python/isc/datasrc/updater_python.h
index 3886aa3..8228578 100644
--- a/src/lib/python/isc/datasrc/updater_python.h
+++ b/src/lib/python/isc/datasrc/updater_python.h
@@ -26,7 +26,15 @@ namespace python {
 
 extern PyTypeObject zoneupdater_type;
 
-PyObject* createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source);
+/// \brief Create a ZoneUpdater python object
+///
+/// \param source The zone iterator pointer to wrap
+/// \param base_obj An optional PyObject that this ZoneUpdater depends on
+///                 It's refcount is increased, and will be decreased when
+///                 this zone iterator is destroyed, making sure that the
+///                 base object is never destroyed before this zone updater.
+PyObject* createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source,
+                                  PyObject* base_obj = NULL);
 
 
 } // namespace python




More information about the bind10-changes mailing list