BIND 10 trac2908, updated. 003aa4bedf13cd9d7b2e73983aa593deb6281435 [2908] numerous fixes from review
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Jun 6 02:30:43 UTC 2013
The branch, trac2908 has been updated
via 003aa4bedf13cd9d7b2e73983aa593deb6281435 (commit)
from 95af51c86a9d480562526421863e47df7260e6a3 (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 003aa4bedf13cd9d7b2e73983aa593deb6281435
Author: Paul Selkirk <pselkirk at isc.org>
Date: Wed Jun 5 22:30:36 2013 -0400
[2908] numerous fixes from review
- remove ZoneTableIterator.next, is_last
- rename get_zone_table -> get_zone_table_accessor
- rename get_zones -> get_iterator
- allow get_zone_table_accessor(None) instead of "" for "any data source"
- add s_ZoneTableAccessor.base_obj to ensure in-order destruction
- get_current returns Name instead of string
- additional test cases for iterator
- general code cleanup and doc cleanup
-----------------------------------------------------------------------
Summary of changes:
.../isc/datasrc/configurableclientlist_python.cc | 20 +++---
.../python/isc/datasrc/tests/clientlist_test.py | 68 +++++++++++---------
.../isc/datasrc/zonetable_accessor_python.cc | 61 +++++++++++-------
.../python/isc/datasrc/zonetable_accessor_python.h | 2 +-
.../isc/datasrc/zonetable_iterator_python.cc | 52 +++------------
5 files changed, 95 insertions(+), 108 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/python/isc/datasrc/configurableclientlist_python.cc b/src/lib/python/isc/datasrc/configurableclientlist_python.cc
index 87cffba..a1d3ab4 100644
--- a/src/lib/python/isc/datasrc/configurableclientlist_python.cc
+++ b/src/lib/python/isc/datasrc/configurableclientlist_python.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -178,16 +178,17 @@ ConfigurableClientList_getZoneTableAccessor(PyObject* po_self, PyObject* args) {
try {
const char* datasrc_name;
int use_cache;
- if (PyArg_ParseTuple(args, "si", &datasrc_name, &use_cache)) {
+ if (PyArg_ParseTuple(args, "zi", &datasrc_name, &use_cache)) {
+ // python 'None' will be read as NULL, which we convert to an
+ // empty string, meaning "any data source"
+ const std::string name(datasrc_name ? datasrc_name : "");
const ConstZoneTableAccessorPtr
- z(self->cppobj->getZoneTableAccessor(datasrc_name, use_cache));
- PyObjectContainer accessor;
+ z(self->cppobj->getZoneTableAccessor(name, use_cache));
if (z == NULL) {
- accessor.reset(Py_BuildValue(""));
+ Py_RETURN_NONE;
} else {
- accessor.reset(createZoneTableAccessorObject(z));
+ return (createZoneTableAccessorObject(z, po_self));
}
- return (Py_BuildValue("O", accessor.get()));
} else {
return (NULL);
}
@@ -243,9 +244,10 @@ you don't need it, but if you do need it, it is better to set it to True\
instead of getting it from the datasrc_client later.\n\
\n\
If no answer is found, the datasrc_client and zone_finder are None." },
- { "get_zone_table", ConfigurableClientList_getZoneTableAccessor,
+ { "get_zone_table_accessor", ConfigurableClientList_getZoneTableAccessor,
METH_VARARGS,
-"get_zone_table(datasrc_name, use_cache) -> isc.datasrc.ZoneTableAccessor\n\
+"get_zone_table_accessor(datasrc_name, use_cache) -> \
+isc.datasrc.ZoneTableAccessor\n\
\n\
Create a ZoneTableAccessor object for the specified data source.\n\
\n\
diff --git a/src/lib/python/isc/datasrc/tests/clientlist_test.py b/src/lib/python/isc/datasrc/tests/clientlist_test.py
index 68d923d..6ade4e4 100644
--- a/src/lib/python/isc/datasrc/tests/clientlist_test.py
+++ b/src/lib/python/isc/datasrc/tests/clientlist_test.py
@@ -151,16 +151,16 @@ class ClientListTest(unittest.TestCase):
self.assertRaises(TypeError, self.clist.find, "example.org")
self.assertRaises(TypeError, self.clist.find)
- def test_get_zone_table(self):
+ def test_get_zone_table_accessor(self):
"""
- Test that we can get the zone table accessor and, thereby,
+ Test that we can get the zone table accessor and, thereby,
the zone table iterator.
"""
self.clist = isc.datasrc.ConfigurableClientList(isc.dns.RRClass.IN)
# null configuration
self.clist.configure("[]", True)
- self.assertIsNone(self.clist.get_zone_table("", True))
+ self.assertIsNone(self.clist.get_zone_table_accessor(None, True))
# empty configuration
self.clist.configure('''[{
@@ -169,15 +169,13 @@ class ClientListTest(unittest.TestCase):
"cache-enable": true
}]''', True)
# bogus datasrc
- self.assertIsNone(self.clist.get_zone_table("bogus", True))
+ self.assertIsNone(self.clist.get_zone_table_accessor("bogus", True))
# first datasrc - empty zone table
- table = self.clist.get_zone_table("", True)
+ table = self.clist.get_zone_table_accessor(None, True)
self.assertIsNotNone(table)
- zones = table.get_zones()
- self.assertIsNotNone(zones)
- self.assertTrue(zones.is_last())
- self.assertRaises(isc.datasrc.Error, zones.get_current)
- self.assertRaises(isc.datasrc.Error, zones.next)
+ iterator = table.get_iterator()
+ self.assertIsNotNone(iterator)
+ self.assertRaises(isc.datasrc.Error, iterator.get_current)
# normal configuration
self.clist.configure('''[{
@@ -187,31 +185,27 @@ class ClientListTest(unittest.TestCase):
},
"cache-enable": true
}]''', True)
- # use_cache = false
+ # !use_cache => NotImplemented
self.assertRaises(isc.datasrc.Error,
- self.clist.get_zone_table, "", False)
+ self.clist.get_zone_table_accessor, None, False)
# bogus datasrc
- self.assertIsNone(self.clist.get_zone_table("bogus", True))
+ self.assertIsNone(self.clist.get_zone_table_accessor("bogus", True))
# first datasrc
- table = self.clist.get_zone_table("", True)
+ table = self.clist.get_zone_table_accessor(None, True)
self.assertIsNotNone(table)
- zones = table.get_zones()
- self.assertIsNotNone(zones)
- self.assertFalse(zones.is_last())
- index, origin = zones.get_current()
- self.assertEqual(origin, "example.org.")
- zones.next()
- self.assertTrue(zones.is_last())
- # reset iterator and count zones
- zones = table.get_zones()
- self.assertEqual(1, len(list(zones)))
+ iterator = table.get_iterator()
+ self.assertIsNotNone(iterator)
+ index, origin = iterator.get_current()
+ self.assertEqual(origin.to_text(), "example.org.")
+ self.assertEqual(1, len(list(iterator)))
# named datasrc
- zones = self.clist.get_zone_table("MasterFiles", True).get_zones()
- index, origin = zones.get_current()
- self.assertEqual(origin, "example.org.")
- self.assertEqual(1, len(list(zones)))
+ table = self.clist.get_zone_table_accessor("MasterFiles", True)
+ iterator = table.get_iterator()
+ index, origin = iterator.get_current()
+ self.assertEqual(origin.to_text(), "example.org.")
+ self.assertEqual(1, len(list(iterator)))
# longer zone list for non-trivial iteration
self.clist.configure('''[{
@@ -225,9 +219,23 @@ class ClientListTest(unittest.TestCase):
},
"cache-enable": true
}]''', True)
- zonelist = list(self.clist.get_zone_table("", True).get_zones())
+ zonelist = list(self.clist.get_zone_table_accessor(None, True).
+ get_iterator())
self.assertEqual(5, len(zonelist))
- self.assertTrue((0, "example.net.") in zonelist)
+ self.assertTrue((0, isc.dns.Name("example.net.")) in zonelist)
+
+ # ensure the iterator returns exactly and only the zones we expect
+ zonelist = [
+ isc.dns.Name("example.org"),
+ isc.dns.Name("example.com"),
+ isc.dns.Name("example.net"),
+ isc.dns.Name("example.biz"),
+ isc.dns.Name("example.edu")]
+ table = self.clist.get_zone_table_accessor("MasterFiles", True)
+ for index, zone in table.get_iterator():
+ self.assertTrue(zone in zonelist)
+ zonelist.remove(zone)
+ self.assertEqual(0, len(zonelist))
if __name__ == "__main__":
isc.log.init("bind10")
diff --git a/src/lib/python/isc/datasrc/zonetable_accessor_python.cc b/src/lib/python/isc/datasrc/zonetable_accessor_python.cc
index de58842..2919a0f 100644
--- a/src/lib/python/isc/datasrc/zonetable_accessor_python.cc
+++ b/src/lib/python/isc/datasrc/zonetable_accessor_python.cc
@@ -20,8 +20,6 @@
// http://docs.python.org/py3k/extending/extending.html#a-simple-example
#include <Python.h>
-//#include <util/python/pycppwrapper_util.h>
-//#include <datasrc/exceptions.h>
#include <datasrc/zone_table_accessor.h>
#include "datasrc.h"
@@ -38,8 +36,36 @@ class s_ZoneTableAccessor : public PyObject {
public:
s_ZoneTableAccessor() : cppobj(ConstZoneTableAccessorPtr()) {};
ConstZoneTableAccessorPtr 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;
};
+int
+ZoneTableAccessor_init(PyObject*, PyObject*, PyObject*) {
+ // can't be called directly
+ PyErr_SetString(PyExc_TypeError,
+ "ZoneTableAccessor cannot be constructed directly");
+
+ return (-1);
+}
+
+void
+ZoneTableAccessor_destroy(PyObject* po_self) {
+ s_ZoneTableAccessor* const self =
+ static_cast<s_ZoneTableAccessor*>(po_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);
+}
+
PyObject*
ZoneTableAccessor_getIterator(PyObject* po_self, PyObject* args) {
s_ZoneTableAccessor* const self =
@@ -64,9 +90,9 @@ ZoneTableAccessor_getIterator(PyObject* po_self, PyObject* args) {
// 3. Argument type
// 4. Documentation
PyMethodDef ZoneTableAccessor_methods[] = {
- { "get_zones",
+ { "get_iterator",
ZoneTableAccessor_getIterator, METH_NOARGS,
-"get_zones() -> isc.datasrc.ZoneTableIterator\n\
+"get_iterator() -> isc.datasrc.ZoneTableIterator\n\
\n\
Return a zone table iterator.\n\
\n" },
@@ -79,25 +105,6 @@ An accessor to a zone table for a data source.\n\
This class object is intended to be used by applications that load zones\
into memory, so that the application can get a list of zones to be loaded.";
-int
-ZoneTableAccessor_init(PyObject*, PyObject*, PyObject*) {
- // can't be called directly
- PyErr_SetString(PyExc_TypeError,
- "ZoneTableAccessor cannot be constructed directly");
-
- return (-1);
-}
-
-void
-ZoneTableAccessor_destroy(PyObject* po_self) {
- s_ZoneTableAccessor* const self =
- static_cast<s_ZoneTableAccessor*>(po_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();
- Py_TYPE(self)->tp_free(self);
-}
-
} // end anonymous namespace
namespace isc {
@@ -157,11 +164,17 @@ PyTypeObject zonetableaccessor_type = {
};
PyObject*
-createZoneTableAccessorObject(isc::datasrc::ConstZoneTableAccessorPtr source) {
+createZoneTableAccessorObject(isc::datasrc::ConstZoneTableAccessorPtr source,
+ PyObject* base_obj)
+{
s_ZoneTableAccessor* py_zt = static_cast<s_ZoneTableAccessor*>(
zonetableaccessor_type.tp_alloc(&zonetableaccessor_type, 0));
if (py_zt != NULL) {
py_zt->cppobj = source;
+ py_zt->base_obj = base_obj;
+ if (base_obj != NULL) {
+ Py_INCREF(base_obj);
+ }
}
return (py_zt);
}
diff --git a/src/lib/python/isc/datasrc/zonetable_accessor_python.h b/src/lib/python/isc/datasrc/zonetable_accessor_python.h
index 00d5bb1..6ebcd92 100644
--- a/src/lib/python/isc/datasrc/zonetable_accessor_python.h
+++ b/src/lib/python/isc/datasrc/zonetable_accessor_python.h
@@ -31,7 +31,7 @@ extern PyTypeObject zonetableaccessor_type;
/// this zone iterator is destroyed, making sure that the
/// base object is never destroyed before this zonefinder.
PyObject* createZoneTableAccessorObject(
- isc::datasrc::ConstZoneTableAccessorPtr source);
+ isc::datasrc::ConstZoneTableAccessorPtr source, PyObject* base_obj);
} // namespace python
} // namespace datasrc
diff --git a/src/lib/python/isc/datasrc/zonetable_iterator_python.cc b/src/lib/python/isc/datasrc/zonetable_iterator_python.cc
index 46e4578..a67e5cf 100644
--- a/src/lib/python/isc/datasrc/zonetable_iterator_python.cc
+++ b/src/lib/python/isc/datasrc/zonetable_iterator_python.cc
@@ -20,8 +20,8 @@
// http://docs.python.org/py3k/extending/extending.html#a-simple-example
#include <Python.h>
-//#include <util/python/pycppwrapper_util.h>
#include <datasrc/zone_table_accessor.h>
+#include <dns/python/name_python.h>
#include "datasrc.h"
#include "zonetable_iterator_python.h"
@@ -73,38 +73,12 @@ ZoneTableIterator_destroy(s_ZoneTableIterator* const self) {
// the type definition of the object, since both can use the other
//
PyObject*
-ZoneTableIterator_isLast(PyObject* po_self, PyObject*) {
- s_ZoneTableIterator* self = static_cast<s_ZoneTableIterator*>(po_self);
- try {
- return (Py_BuildValue("i", self->cppobj->isLast()));
- } catch (...) {
- return (NULL);
- }
-}
-
-PyObject*
-ZoneTableIterator_next(PyObject* po_self, PyObject*) {
- s_ZoneTableIterator* self = static_cast<s_ZoneTableIterator*>(po_self);
- try {
- self->cppobj->next();
- return (Py_BuildValue(""));
- } catch (const std::exception& ex) {
- // isc::InvalidOperation is thrown when we call next()
- // when we are already done iterating ('iterating past end')
- // We could also simply return None again
- PyErr_SetString(getDataSourceException("Error"), ex.what());
- return (NULL);
- } catch (...) {
- return (NULL);
- }
-}
-
-PyObject*
ZoneTableIterator_getCurrent(PyObject* po_self, PyObject*) {
s_ZoneTableIterator* self = static_cast<s_ZoneTableIterator*>(po_self);
try {
const isc::datasrc::ZoneSpec& zs = self->cppobj->getCurrent();
- return (Py_BuildValue("is", zs.index, zs.origin.toText().c_str()));
+ return (Py_BuildValue("iO", zs.index,
+ isc::dns::python::createNameObject(zs.origin)));
} catch (const std::exception& ex) {
// isc::InvalidOperation is thrown when we call getCurrent()
// when we are already done iterating ('iterating past end')
@@ -119,13 +93,13 @@ ZoneTableIterator_getCurrent(PyObject* po_self, PyObject*) {
}
PyObject*
-ZoneTableIterator_piter(PyObject *self) {
+ZoneTableIterator_iter(PyObject *self) {
Py_INCREF(self);
return (self);
}
PyObject*
-ZoneTableIterator_pnext(PyObject* po_self) {
+ZoneTableIterator_next(PyObject* po_self) {
s_ZoneTableIterator* self = static_cast<s_ZoneTableIterator*>(po_self);
if (!self->cppobj || self->cppobj->isLast()) {
return (NULL);
@@ -145,14 +119,6 @@ ZoneTableIterator_pnext(PyObject* po_self) {
}
PyMethodDef ZoneTableIterator_methods[] = {
- { "is_last", ZoneTableIterator_isLast, METH_NOARGS,
-"is_last() -> bool\n\
-\n\
-Return whether the iterator is at the end of the zone table.\n" },
- { "next", ZoneTableIterator_next, METH_NOARGS,
-"next()\n\
-\n\
-Move the iterator to the next zone of the table.\n" },
{ "get_current", ZoneTableIterator_getCurrent, METH_NOARGS,
"get_current() -> isc.datasrc.ZoneSpec\n\
\n\
@@ -168,9 +134,7 @@ const char* const ZoneTableIterator_doc = "\
Read-only iterator to a zone table.\n\
\n\
You can get an instance of the ZoneTableIterator from the\
-ZoneTableAccessor.get_iterator() method. The actual concrete\
-C++ implementation will be different depending on the actual data source\
-used. This is the abstract interface.\n\
+ZoneTableAccessor.get_iterator() method.\n\
\n\
There's no way to start iterating from the beginning again or return.\n\
\n\
@@ -209,8 +173,8 @@ PyTypeObject zonetableiterator_type = {
NULL, // tp_clear
NULL, // tp_richcompare
0, // tp_weaklistoffset
- ZoneTableIterator_piter, // tp_iter
- ZoneTableIterator_pnext, // tp_iternext
+ ZoneTableIterator_iter, // tp_iter
+ ZoneTableIterator_next, // tp_iternext
ZoneTableIterator_methods, // tp_methods
NULL, // tp_members
NULL, // tp_getset
More information about the bind10-changes
mailing list