[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)

src/lib/datasrc/Makefile.am

@@ -28,6 +28,9 @@ 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 += $(top_builddir)/src/lib/datasrc/libdatasrc.la
+sqlite3_ds_la_LIBADD += $(SQLITE_LIBS)
 
 memory_ds_la_SOURCES = memory_datasrc.h memory_datasrc.cc
 memory_ds_la_LDFLAGS = -module

src/lib/python/isc/datasrc/Makefile.am

@@ -21,7 +21,7 @@ datasrc_la_SOURCES += updater_python.cc updater_python.h
 # 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_SOURCES += ${top_srcdir}/src/lib/datasrc/sqlite3_accessor.cc
 
 datasrc_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 datasrc_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)

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,8 +119,8 @@ DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
         bool replace = (replace_obj != Py_False);
         try {
             return (createZoneUpdaterObject(
-                        self->cppobj->getUpdater(PyName_ToName(name_obj),
-                                                 replace)));
+                        self->cppobj->getInstance().getUpdater(PyName_ToName(name_obj),
+                                                 replace), po_self));
         } catch (const isc::NotImplemented& ne) {
             PyErr_SetString(getDataSourceException("NotImplemented"),
                             ne.what());
@@ -167,12 +165,14 @@ DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
     // string for the database file. (similar to how the 'old direct'
     // sqlite3_ds code works)
     try {
-        char* db_file_name;
-        if (PyArg_ParseTuple(args, "s", &db_file_name)) {
-            boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
-                new SQLite3Accessor(db_file_name, isc::dns::RRClass::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
+        char *ds_type_str;
+        char* ds_config_str;
+
+        // 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);

src/lib/python/isc/datasrc/finder_python.cc

@@ -105,6 +105,12 @@ class s_ZoneFinder : public PyObject {
 public:
     s_ZoneFinder() : cppobj(ZoneFinderPtr()) {};
     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);
 }

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

src/lib/python/isc/datasrc/iterator_python.cc

@@ -47,6 +47,12 @@ class s_ZoneIterator : public PyObject {
 public:
     s_ZoneIterator() : cppobj(ZoneIteratorPtr()) {};
     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);
 }

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

src/lib/python/isc/datasrc/tests/datasrc_test.py

@@ -65,7 +65,7 @@ class DataSrcClient(unittest.TestCase):
 
 
     def test_iterate(self):
-        dsc = isc.datasrc.DataSourceClient(READ_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", "{ \"database_file\": \"" + READ_ZONE_DB_FILE + "\" }")
 
         # for RRSIGS, the TTL's are currently modified. This test should
         # start failing when we fix that.
@@ -176,7 +176,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", "{ \"database_file\": \"" + READ_ZONE_DB_FILE + "\"}")
 
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))
         self.assertEqual(finder.SUCCESS, result)
@@ -260,7 +260,7 @@ class DataSrcUpdater(unittest.TestCase):
 
     def test_update_delete_commit(self):
 
-        dsc = isc.datasrc.DataSourceClient(WRITE_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", "{ \"database_file\": \"" + WRITE_ZONE_DB_FILE + "\"}")
 
         # first make sure, through a separate finder, that some record exists
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))
@@ -334,7 +334,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", "{ \"database_file\": \"" + WRITE_ZONE_DB_FILE + "\"}")
 
         # first make sure, through a separate finder, that some record exists
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))

src/lib/python/isc/datasrc/updater_python.cc

@@ -56,6 +56,12 @@ class s_ZoneUpdater : public PyObject {
 public:
     s_ZoneUpdater() : cppobj(ZoneUpdaterPtr()) {};
     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);
 }
 

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