[1179] remove some unneceesary comments, and share find() code

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

@@ -57,8 +57,6 @@ public:
 typedef CPPPyObjectContainer<s_DataSourceClient, DataSourceClient>
     DataSourceClientContainer;
 
-// These are the functions we export
-//
 PyObject*
 DataSourceClient_findZone(PyObject* po_self, PyObject* args) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
@@ -145,8 +143,6 @@ DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
     }
 }
 
-// These are the functions we export
-
 // This list contains the actual set of functions we have in
 // python. Each entry has
 // 1. Python method name
@@ -164,9 +160,6 @@ PyMethodDef DataSourceClient_methods[] = {
     { NULL, NULL, 0, NULL }
 };
 
-// This is a template of typical code logic of python class initialization
-// with C++ backend.  You'll need to adjust it according to details of the
-// actual C++ class.
 int
 DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
     // TODO: we should use the factory function which hasn't been written
@@ -201,8 +194,6 @@ DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
     return (-1);
 }
 
-// This is a template of typical code logic of python object destructor.
-// In many cases you can use it without modification, but check that carefully.
 void
 DataSourceClient_destroy(s_DataSourceClient* const self) {
     delete self->cppobj;

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

@@ -43,11 +43,11 @@ PyObject*
 getDataSourceException(const char* ex_name) {
     PyObject* ex_obj = NULL;
 
-    PyObject* acl_module = PyImport_AddModule("isc.datasrc");
-    if (acl_module != NULL) {
-        PyObject* acl_dict = PyModule_GetDict(acl_module);
-        if (acl_dict != NULL) {
-            ex_obj = PyDict_GetItemString(acl_dict, ex_name);
+    PyObject* datasrc_module = PyImport_AddModule("isc.datasrc");
+    if (datasrc_module != NULL) {
+        PyObject* datasrc_dict = PyModule_GetDict(datasrc_module);
+        if (datasrc_dict != NULL) {
+            ex_obj = PyDict_GetItemString(datasrc_dict, ex_name);
         }
     }
 

src/lib/python/isc/datasrc/datasrc.h

@@ -29,7 +29,7 @@ namespace python {
 // C/C++ symbols defined in that module.  So we get access to these object
 // using the Python interpretor through this wrapper function.
 //
-// The __init__.py file should ensure isc.acl.acl has been loaded by the time
+// The __init__.py file should ensure isc.datasrc has been loaded by the time
 // whenever this function is called, and there shouldn't be any operation
 // within this function that can fail (such as dynamic memory allocation),
 // so this function should always succeed.  Yet there may be an overlooked

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

@@ -45,6 +45,59 @@ using namespace isc::dns::python;
 using namespace isc::datasrc;
 using namespace isc::datasrc::python;
 
+namespace isc_datasrc_internal {
+// This is the shared code for the find() call in the finder and the updater
+// Is is intentionally not available through any header, nor at our standard
+// namespace.
+PyObject* ZoneFinder_helper(ZoneFinder* finder, PyObject* args) {
+    if (finder == NULL) {
+        PyErr_SetString(getDataSourceException("Error"),
+                        "Internal error in find() wrapper; finder object NULL");
+        return (NULL);
+    }
+    PyObject *name;
+    PyObject *rrtype;
+    PyObject *target;
+    int options_int;
+    if (PyArg_ParseTuple(args, "O!O!OI", &name_type, &name,
+                                         &rrtype_type, &rrtype,
+                                         &target, &options_int)) {
+        try {
+            ZoneFinder::FindOptions options =
+                static_cast<ZoneFinder::FindOptions>(options_int);
+            ZoneFinder::FindResult find_result(
+                finder->find(PyName_ToName(name),
+                                   PyRRType_ToRRType(rrtype),
+                                   NULL,
+                                   options
+                                   ));
+            ZoneFinder::Result r = find_result.code;
+            isc::dns::ConstRRsetPtr rrsp = find_result.rrset;
+            if (rrsp) {
+                // Use N instead of O so the refcount isn't increased twice
+                return (Py_BuildValue("IN", r, createRRsetObject(*rrsp)));
+            } else {
+                return (Py_BuildValue("IO", r, Py_None));
+            }
+        } catch (const DataSourceError& dse) {
+            PyErr_SetString(getDataSourceException("Error"), dse.what());
+            return (NULL);
+        } catch (const std::exception& exc) {
+            PyErr_SetString(getDataSourceException("Error"), exc.what());
+            return (NULL);
+        } catch (...) {
+            PyErr_SetString(getDataSourceException("Error"),
+                            "Unexpected exception");
+            return (NULL);
+        }
+    } else {
+        return (NULL);
+    }
+    return Py_BuildValue("I", 1);
+}
+
+} // end namespace internal
+
 namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneFinder : public PyObject {
@@ -74,8 +127,6 @@ ZoneFinder_destroy(s_ZoneFinder* const self) {
     Py_TYPE(self)->tp_free(self);
 }
 
-// These are the functions we export
-//
 PyObject*
 ZoneFinder_getClass(PyObject* po_self, PyObject*) {
     s_ZoneFinder* self = static_cast<s_ZoneFinder*>(po_self);
@@ -105,45 +156,7 @@ ZoneFinder_getOrigin(PyObject* po_self, PyObject*) {
 PyObject*
 ZoneFinder_find(PyObject* po_self, PyObject* args) {
     s_ZoneFinder* const self = static_cast<s_ZoneFinder*>(po_self);
-    PyObject *name;
-    PyObject *rrtype;
-    PyObject *target;
-    int options_int;
-    if (PyArg_ParseTuple(args, "O!O!OI", &name_type, &name,
-                                         &rrtype_type, &rrtype,
-                                         &target, &options_int)) {
-        try {
-            ZoneFinder::FindOptions options =
-                static_cast<ZoneFinder::FindOptions>(options_int);
-            ZoneFinder::FindResult find_result(
-                self->cppobj->find(PyName_ToName(name),
-                                   PyRRType_ToRRType(rrtype),
-                                   NULL,
-                                   options
-                                   ));
-            ZoneFinder::Result r = find_result.code;
-            isc::dns::ConstRRsetPtr rrsp = find_result.rrset;
-            if (rrsp) {
-                // Use N instead of O so the refcount isn't increased twice
-                return (Py_BuildValue("IN", r, createRRsetObject(*rrsp)));
-            } else {
-                return (Py_BuildValue("IO", r, Py_None));
-            }
-        } catch (const DataSourceError& dse) {
-            PyErr_SetString(getDataSourceException("Error"), dse.what());
-            return (NULL);
-        } catch (const std::exception& exc) {
-            PyErr_SetString(getDataSourceException("Error"), exc.what());
-            return (NULL);
-        } catch (...) {
-            PyErr_SetString(getDataSourceException("Error"),
-                            "Unexpected exception");
-            return (NULL);
-        }
-    } else {
-        return (NULL);
-    }
-    return Py_BuildValue("I", 1);
+    return (isc_datasrc_internal::ZoneFinder_helper(self->cppobj.get(), args));
 }
 
 // This list contains the actual set of functions we have in
@@ -167,6 +180,7 @@ PyMethodDef ZoneFinder_methods[] = {
 namespace isc {
 namespace datasrc {
 namespace python {
+
 PyTypeObject zonefinder_type = {
     PyVarObject_HEAD_INIT(NULL, 0)
     "datasrc.ZoneFinder",

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

@@ -63,8 +63,6 @@ ZoneIterator_init(s_ZoneIterator* self, PyObject* args) {
     return (-1);
 }
 
-// This is a template of typical code logic of python object destructor.
-// In many cases you can use it without modification, but check that carefully.
 void
 ZoneIterator_destroy(s_ZoneIterator* const self) {
     // cppobj is a shared ptr, but to make sure things are not destroyed in

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

@@ -45,6 +45,11 @@ using namespace isc::dns::python;
 using namespace isc::datasrc;
 using namespace isc::datasrc::python;
 
+namespace isc_datasrc_internal {
+// See finder_python.cc
+PyObject* ZoneFinder_helper(ZoneFinder* finder, PyObject* args);
+}
+
 namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneUpdater : public PyObject {
@@ -71,8 +76,6 @@ ZoneUpdater_init(s_ZoneUpdater* self, PyObject* args) {
     return (-1);
 }
 
-// This is a template of typical code logic of python object destructor.
-// In many cases you can use it without modification, but check that carefully.
 void
 ZoneUpdater_destroy(s_ZoneUpdater* const self) {
     // cppobj is a shared ptr, but to make sure things are not destroyed in
@@ -81,8 +84,6 @@ ZoneUpdater_destroy(s_ZoneUpdater* const self) {
     Py_TYPE(self)->tp_free(self);
 }
 
-// These are the functions we export
-//
 PyObject*
 ZoneUpdater_addRRset(PyObject* po_self, PyObject* args) {
     s_ZoneUpdater* const self = static_cast<s_ZoneUpdater*>(po_self);
@@ -138,8 +139,6 @@ ZoneUpdater_commit(PyObject* po_self, PyObject*) {
     }
 }
 
-// These are the functions we export
-//
 PyObject*
 ZoneUpdater_getClass(PyObject* po_self, PyObject*) {
     s_ZoneUpdater* self = static_cast<s_ZoneUpdater*>(po_self);
@@ -173,6 +172,13 @@ ZoneUpdater_getOrigin(PyObject* po_self, PyObject*) {
 PyObject*
 ZoneUpdater_find(PyObject* po_self, PyObject* args) {
     s_ZoneUpdater* const self = static_cast<s_ZoneUpdater*>(po_self);
+    return (isc_datasrc_internal::ZoneFinder_helper(&self->cppobj->getFinder(),
+                                                    args));
+}
+
+PyObject*
+AZoneUpdater_find(PyObject* po_self, PyObject* args) {
+    s_ZoneUpdater* const self = static_cast<s_ZoneUpdater*>(po_self);
     PyObject *name;
     PyObject *rrtype;
     PyObject *target;