Merge #2051

src/lib/datasrc/client_list.cc

@@ -167,6 +167,39 @@ ConfigurableClientList::configure(const Element& config, bool allow_cache) {
     }
 }
 
+namespace {
+
+class CacheKeeper : public ClientList::FindResult::LifeKeeper {
+public:
+    CacheKeeper(const boost::shared_ptr<InMemoryClient>& cache) :
+        cache_(cache)
+    {}
+private:
+    const boost::shared_ptr<InMemoryClient> cache_;
+};
+
+class ContainerKeeper : public ClientList::FindResult::LifeKeeper {
+public:
+    ContainerKeeper(const DataSourceClientContainerPtr& container) :
+        container_(container)
+    {}
+private:
+    const DataSourceClientContainerPtr container_;
+};
+
+boost::shared_ptr<ClientList::FindResult::LifeKeeper>
+genKeeper(const ConfigurableClientList::DataSourceInfo& info) {
+    if (info.cache_) {
+        return (boost::shared_ptr<ClientList::FindResult::LifeKeeper>(
+            new CacheKeeper(info.cache_)));
+    } else {
+        return (boost::shared_ptr<ClientList::FindResult::LifeKeeper>(
+            new ContainerKeeper(info.container_)));
+    }
+}
+
+}
+
 ClientList::FindResult
 ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                             bool) const
@@ -185,10 +218,11 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
         ZoneFinderPtr finder;
         uint8_t matched_labels;
         bool matched;
+        boost::shared_ptr<FindResult::LifeKeeper> keeper;
         operator FindResult() const {
             // Conversion to the right result. If we return this, there was
             // a partial match at best.
-            return (FindResult(datasrc_client, finder, false));
+            return (FindResult(datasrc_client, finder, false, keeper));
         }
     } candidate;
 
@@ -206,7 +240,7 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                 // TODO: In case we have only the datasource and not the finder
                 // and the need_updater parameter is true, get the zone there.
                 return (FindResult(client, result.zone_finder,
-                                   true));
+                                   true, genKeeper(info)));
             case result::PARTIALMATCH:
                 if (!want_exact_match) {
                     // In case we have a partial match, check if it is better
@@ -230,6 +264,7 @@ ConfigurableClientList::find(const dns::Name& name, bool want_exact_match,
                         candidate.finder = result.zone_finder;
                         candidate.matched_labels = labels;
                         candidate.matched = true;
+                        candidate.keeper = genKeeper(info);
                     }
                 }
                 break;

src/lib/datasrc/client_list.h

@@ -65,15 +65,27 @@ public:
     /// Instead, all the member variables are defined as const and can be
     /// accessed directly.
     struct FindResult {
+        /// \brief Internal class for holding a reference.
+        ///
+        /// This is used to make sure the data source client isn't released
+        /// too soon.
+        ///
+        /// \see life_keeper_;
+        class LifeKeeper {
+        public:
+            virtual ~LifeKeeper() {};
+        };
         /// \brief Constructor.
         ///
         /// It simply fills in the member variables according to the
         /// parameters. See the member descriptions for their meaning.
         FindResult(DataSourceClient* dsrc_client, const ZoneFinderPtr& finder,
-                   bool exact_match) :
+                   bool exact_match,
+                   const boost::shared_ptr<LifeKeeper>& life_keeper) :
             dsrc_client_(dsrc_client),
             finder_(finder),
-            exact_match_(exact_match)
+            exact_match_(exact_match),
+            life_keeper_(life_keeper)
         {}
 
         /// \brief Negative answer constructor.
@@ -101,8 +113,9 @@ public:
         /// If no such data source exists, this is NULL pointer.
         ///
         /// Note that the pointer is valid only as long the ClientList which
-        /// returned the pointer is alive and was not reconfigured. The
-        /// ownership is preserved within the ClientList.
+        /// returned the pointer is alive and was not reconfigured or you hold
+        /// a reference to life_keeper_. The ownership is preserved within the
+        /// ClientList.
         DataSourceClient* const dsrc_client_;
 
         /// \brief The finder for the requested zone.
@@ -116,6 +129,12 @@ public:
 
         /// \brief If the result is an exact match.
         const bool exact_match_;
+
+        /// \brief Something that holds the dsrc_client_ valid.
+        ///
+        /// As long as you hold the life_keeper_, the dsrc_client_ is
+        /// guaranteed to be valid.
+        const boost::shared_ptr<LifeKeeper> life_keeper_;
     };
 
     /// \brief Search for a zone through the data sources.

src/lib/datasrc/tests/client_list_unittest.cc

@@ -245,6 +245,12 @@ public:
         ASSERT_NE(ZoneFinderPtr(), result.finder_);
         EXPECT_EQ(name, result.finder_->getOrigin());
         EXPECT_EQ(exact, result.exact_match_);
+        // If it is a positive result, there's something to keep
+        // alive, even when we don't know what it is.
+        // Any better idea how to test it actually keeps the thing
+        // alive?
+        EXPECT_NE(shared_ptr<ClientList::FindResult::LifeKeeper>(),
+                  result.life_keeper_);
         if (from_cache) {
             EXPECT_NE(shared_ptr<InMemoryZoneFinder>(),
                       dynamic_pointer_cast<InMemoryZoneFinder>(
@@ -315,6 +321,9 @@ TEST_F(ListTest, selfTest) {
     EXPECT_EQ(result::NOTFOUND, ds_[1]->findZone(Name("example.org")).code);
     EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("aaa")).code);
     EXPECT_EQ(result::NOTFOUND, ds_[0]->findZone(Name("zzz")).code);
+    // Nothing to keep alive here.
+    EXPECT_EQ(shared_ptr<ClientList::FindResult::LifeKeeper>(),
+                  negativeResult_.life_keeper_);
 }
 
 // Test the list we create with empty configuration is, in fact, empty

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

@@ -18,6 +18,8 @@ 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
 datasrc_la_SOURCES += journal_reader_python.cc journal_reader_python.h
+datasrc_la_SOURCES += configurableclientlist_python.cc
+datasrc_la_SOURCES += configurableclientlist_python.h
 
 datasrc_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 datasrc_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)

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

@@ -28,6 +28,7 @@
 #include <datasrc/data_source.h>
 #include <datasrc/sqlite3_accessor.h>
 #include <datasrc/iterator.h>
+#include <datasrc/client_list.h>
 
 #include <dns/python/name_python.h>
 #include <dns/python/rrset_python.h>
@@ -51,8 +52,17 @@ namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_DataSourceClient : public PyObject {
 public:
-    s_DataSourceClient() : cppobj(NULL) {};
+    s_DataSourceClient() :
+        cppobj(NULL),
+        client(NULL),
+        keeper(NULL)
+    {};
     DataSourceClientContainer* cppobj;
+    DataSourceClient* client;
+    // We can't rely on the constructor or destructor being
+    // called, so this is a pointer to shared pointer, so we
+    // can call the new and delete explicitly.
+    boost::shared_ptr<ClientList::FindResult::LifeKeeper>* keeper;
 };
 
 PyObject*
@@ -62,7 +72,7 @@ DataSourceClient_findZone(PyObject* po_self, PyObject* args) {
     if (PyArg_ParseTuple(args, "O!", &name_type, &name)) {
         try {
             DataSourceClient::FindResult find_result(
-                self->cppobj->getInstance().findZone(PyName_ToName(name)));
+                self->client->findZone(PyName_ToName(name)));
 
             result::Result r = find_result.code;
             ZoneFinderPtr zfp = find_result.zone_finder;
@@ -103,7 +113,7 @@ DataSourceClient_getIterator(PyObject* po_self, PyObject* args) {
                 }
             }
             return (createZoneIteratorObject(
-                self->cppobj->getInstance().getIterator(PyName_ToName(name_obj),
+                self->client->getIterator(PyName_ToName(name_obj),
                                                         separate_rrs),
                 po_self));
         } catch (const isc::NotImplemented& ne) {
@@ -139,7 +149,7 @@ DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
         const bool journaling = (journaling_obj == Py_True);
         try {
             ZoneUpdaterPtr updater =
-                self->cppobj->getInstance().getUpdater(PyName_ToName(name_obj),
+                self->client->getUpdater(PyName_ToName(name_obj),
                                                        replace, journaling);
             if (!updater) {
                 return (Py_None);
@@ -184,7 +194,7 @@ DataSourceClient_getJournalReader(PyObject* po_self, PyObject* args) {
                          &begin_obj, &end_obj)) {
         try {
             pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> result =
-                self->cppobj->getInstance().getJournalReader(
+                self->client->getJournalReader(
                     PyName_ToName(name_obj), static_cast<uint32_t>(begin_obj),
                     static_cast<uint32_t>(end_obj));
             PyObject* po_reader;
@@ -245,6 +255,8 @@ DataSourceClient_init(PyObject* po_self, PyObject* args, PyObject*) {
                 isc::data::Element::fromJSON(ds_config_str);
             self->cppobj = new DataSourceClientContainer(ds_type_str,
                                                          ds_config);
+            self->client = &self->cppobj->getInstance();
+            self->keeper = NULL;
             return (0);
         } else {
             return (-1);
@@ -280,7 +292,10 @@ void
 DataSourceClient_destroy(PyObject* po_self) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
     delete self->cppobj;
+    delete self->keeper;
     self->cppobj = NULL;
+    self->client = NULL;
+    self->keeper = NULL;
     Py_TYPE(self)->tp_free(self);
 }
 
@@ -342,6 +357,23 @@ PyTypeObject datasourceclient_type = {
     0                                   // tp_version_tag
 };
 
+PyObject*
+wrapDataSourceClient(DataSourceClient* client,
+                     const boost::shared_ptr<ClientList::FindResult::
+                     LifeKeeper>& life_keeper)
+{
+    s_DataSourceClient *result =
+        static_cast<s_DataSourceClient*>(PyObject_New(s_DataSourceClient,
+                                                      &datasourceclient_type));
+    CPPPyObjectContainer<s_DataSourceClient, DataSourceClientContainer>
+        container(result);
+    result->cppobj = NULL;
+    result->keeper =
+        new boost::shared_ptr<ClientList::FindResult::LifeKeeper>(life_keeper);
+    result->client = client;
+    return (container.release());
+}
+
 } // namespace python
 } // namespace datasrc
 } // namespace isc

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

@@ -15,6 +15,8 @@
 #ifndef __PYTHON_DATASRC_CLIENT_H
 #define __PYTHON_DATASRC_CLIENT_H 1
 
+#include <datasrc/client_list.h>
+
 #include <Python.h>
 
 namespace isc {
@@ -25,6 +27,23 @@ namespace python {
 
 extern PyTypeObject datasourceclient_type;
 
+/// \brief Create a DataSourceClient python object
+///
+/// Unlike many similar functions, this one does not create a copied instance
+/// of the passed object. It wraps the given one. This is why the name is
+/// different than the usual.
+///
+/// \param client The client to wrap.
+/// \param life_keeper An optional object which keeps the client pointer valid.
+///     The object will be kept inside the wrapper too, making sure that the
+///     client is not destroyed sooner than the python object. The keeper thing
+///     is designed to acommodate the interface of the ClientList.
+PyObject*
+wrapDataSourceClient(DataSourceClient* client,
+                     const boost::shared_ptr<ClientList::FindResult::
+                     LifeKeeper>& life_keeper = boost::shared_ptr<ClientList::
+                     FindResult::LifeKeeper>());
+
 } // namespace python
 } // namespace datasrc
 } // namespace isc

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

+// Copyright (C) 2012  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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// Enable this if you use s# variants with PyArg_ParseTuple(), see
+// http://docs.python.org/py3k/c-api/arg.html#strings-and-buffers
+//#define PY_SSIZE_T_CLEAN
+
+// Python.h needs to be placed at the head of the program file, see:
+// http://docs.python.org/py3k/extending/extending.html#a-simple-example
+#include <Python.h>
+
+#include <string>
+#include <stdexcept>
+
+#include <util/python/pycppwrapper_util.h>
+
+#include <dns/python/rrclass_python.h>
+#include <dns/python/name_python.h>
+
+#include <datasrc/client_list.h>
+
+#include "configurableclientlist_python.h"
+#include "datasrc.h"
+#include "finder_python.h"
+#include "client_python.h"
+
+using namespace std;
+using namespace isc::util::python;
+using namespace isc::datasrc;
+using namespace isc::datasrc::python;
+
+//
+// ConfigurableClientList
+//
+
+// Trivial constructor.
+s_ConfigurableClientList::s_ConfigurableClientList() : cppobj(NULL) {
+}
+
+namespace {
+
+int
+ConfigurableClientList_init(PyObject* po_self, PyObject* args, PyObject*) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    try {
+        const PyObject* rrclass;
+        if (PyArg_ParseTuple(args, "O!", &isc::dns::python::rrclass_type,
+                             &rrclass)) {
+            self->cppobj =
+                new ConfigurableClientList(isc::dns::python::
+                                           PyRRClass_ToRRClass(rrclass));
+            return (0);
+        }
+    } catch (const exception& ex) {
+        const string ex_what = "Failed to construct ConfigurableClientList object: " +
+            string(ex.what());
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+        return (-1);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError, "Unexpected C++ exception");
+        return (-1);
+    }
+
+    return (-1);
+}
+
+void
+ConfigurableClientList_destroy(PyObject* po_self) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    delete self->cppobj;
+    self->cppobj = NULL;
+    Py_TYPE(self)->tp_free(self);
+}
+
+PyObject*
+ConfigurableClientList_configure(PyObject* po_self, PyObject* args) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    try {
+        const char* configuration;
+        int allow_cache;
+        if (PyArg_ParseTuple(args, "si", &configuration, &allow_cache)) {
+            const isc::data::ConstElementPtr
+                element(isc::data::Element::fromJSON(string(configuration)));
+            self->cppobj->configure(*element, allow_cache);
+            Py_RETURN_NONE;
+        } else {
+            return (NULL);
+        }
+    } catch (const isc::data::JSONError& jse) {
+        const string ex_what(std::string("JSON parse error in data source"
+                               " configuration: ") + jse.what());
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+        return (NULL);
+    } catch (const std::exception& exc) {
+        PyErr_SetString(getDataSourceException("Error"), exc.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(getDataSourceException("Error"),
+                        "Unknown C++ exception");
+        return (NULL);
+    }
+}
+
+PyObject*
+ConfigurableClientList_find(PyObject* po_self, PyObject* args) {
+    s_ConfigurableClientList* self =
+        static_cast<s_ConfigurableClientList*>(po_self);
+    try {
+        PyObject* name_obj;
+        int want_exact_match = 0;
+        int want_finder = 1;
+        if (PyArg_ParseTuple(args, "O!|ii", &isc::dns::python::name_type,
+                             &name_obj, &want_exact_match, &want_finder)) {
+            const isc::dns::Name
+                name(isc::dns::python::PyName_ToName(name_obj));
+            const ClientList::FindResult
+                result(self->cppobj->find(name, want_exact_match,
+                                          want_finder));
+            PyObjectContainer dsrc;
+            if (result.dsrc_client_ == NULL) {
+                // Use the Py_BuildValue, as it takes care of the
+                // reference counts correctly.
+                dsrc.reset(Py_BuildValue(""));
+            } else {
+                // Make sure we have a keeper there too, so it doesn't
+                // die when the underlying client list dies or is
+                // reconfigured.
+                //
+                // However, as it is inside the C++ part, is there a
+                // reasonable way to test it?
+                dsrc.reset(wrapDataSourceClient(result.dsrc_client_,
+                                                result.life_keeper_));
+            }
+            PyObjectContainer finder;
+            if (result.finder_ == NULL) {
+                finder.reset(Py_BuildValue(""));
+            } else {
+                // Make sure it keeps the data source client alive.
+                finder.reset(createZoneFinderObject(result.finder_,
+                                                    dsrc.get()));
+            }
+            PyObjectContainer exact(PyBool_FromLong(result.exact_match_));
+
+            return (Py_BuildValue("OOO", dsrc.get(), finder.get(),
+                                  exact.get()));
+        } else {
+            return (NULL);
+        }
+    } catch (const std::exception& exc) {
+        PyErr_SetString(getDataSourceException("Error"), exc.what());
+        return (NULL);
+    } catch (...) {
+        PyErr_SetString(getDataSourceException("Error"),
+                        "Unknown C++ exception");
+        return (NULL);
+    }
+}
+
+// This list contains the actual set of functions we have in
+// python. Each entry has
+// 1. Python method name
+// 2. Our static function here
+// 3. Argument type
+// 4. Documentation
+PyMethodDef ConfigurableClientList_methods[] = {
+    { "configure", ConfigurableClientList_configure, METH_VARARGS,
+        "configure(configuration, allow_cache) -> None\n\
+\n\
+Wrapper around C++ ConfigurableClientList::configure\n\
+\n\
+This sets the active configuration. It fills the ConfigurableClientList with\
+corresponding data source clients.\n\
+\n\
+If any error is detected, an exception is raised and the previous\
+configuration preserved.\n\
+\n\
+Parameters:\n\
+  configuration     The configuration, as a JSON encoded string.\
+  allow_cache       If caching is allowed." },
+    { "find", ConfigurableClientList_find, METH_VARARGS,
+"find(zone, want_exact_match=False, want_finder=True) -> datasrc_client,\
+zone_finder, exact_match\n\
+\n\
+Look for a data source containing the given zone.\n\
+\n\
+It searches through the contained data sources and returns a data source\
+containing the zone, the zone finder of the zone and a boolean if the answer\
+is an exact match.\n\
+\n\
+The first parameter is isc.dns.Name object of a name in the zone. If the\
+want_exact_match is True, only zone with this exact origin is returned.\
+If it is False, the best matching zone is returned.\n\
+\n\
+If the want_finder is False, the returned zone_finder might be None even\
+if the data source is identified (in such case, the datasrc_client is not\
+None). Setting it to false allows the client list some optimisations, if\
+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." },
+    { NULL, NULL, 0, NULL }
+};
+
+const char* const ConfigurableClientList_doc = "\
+The list of data source clients\n\
+\n\
+The purpose is to have several data source clients of the same class\
+and then be able to search through them to identify the one containing\
+a given zone.\n\
+\n\
+Unlike the C++ version, we don't have the abstract base class. Abstract\
+classes are not needed due to the duck typing nature of python.\
+";
+
+} // end of unnamed namespace
+
+namespace isc {
+namespace datasrc {
+namespace python {
+// This defines the complete type for reflection in python and
+// parsing of PyObject* to s_ConfigurableClientList
+// Most of the functions are not actually implemented and NULL here.
+PyTypeObject configurableclientlist_type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "datasrc.ConfigurableClientList",
+    sizeof(s_ConfigurableClientList),                 // tp_basicsize
+    0,                                  // tp_itemsize
+    ConfigurableClientList_destroy,                 // tp_dealloc
+    NULL,                               // tp_print
+    NULL,                               // tp_getattr
+    NULL,                               // tp_setattr
+    NULL,                               // tp_reserved
+    NULL,                               // tp_repr
+    NULL,                               // tp_as_number
+    NULL,                               // tp_as_sequence
+    NULL,                               // tp_as_mapping
+    NULL,                               // tp_hash
+    NULL,                               // tp_call
+    NULL,                               // tp_str
+    NULL,                               // tp_getattro
+    NULL,                               // tp_setattro
+    NULL,                               // tp_as_buffer
+    Py_TPFLAGS_DEFAULT,                 // tp_flags
+    ConfigurableClientList_doc,
+    NULL,                               // tp_traverse
+    NULL,                               // tp_clear
+    NULL,                               // tp_richcompare
+    0,                                  // tp_weaklistoffset
+    NULL,                               // tp_iter
+    NULL,                               // tp_iternext
+    ConfigurableClientList_methods,                   // tp_methods
+    NULL,                               // tp_members
+    NULL,                               // tp_getset
+    NULL,                               // tp_base
+    NULL,                               // tp_dict
+    NULL,                               // tp_descr_get
+    NULL,                               // tp_descr_set
+    0,                                  // tp_dictoffset
+    ConfigurableClientList_init,                    // tp_init
+    NULL,                               // tp_alloc
+    PyType_GenericNew,                  // tp_new
+    NULL,                               // tp_free
+    NULL,                               // tp_is_gc
+    NULL,                               // tp_bases
+    NULL,                               // tp_mro
+    NULL,                               // tp_cache
+    NULL,                               // tp_subclasses
+    NULL,                               // tp_weaklist
+    NULL,                               // tp_del
+    0                                   // tp_version_tag
+};
+
+// Module Initialization, all statics are initialized here
+bool
+initModulePart_ConfigurableClientList(PyObject* mod) {
+    // We initialize the static description object with PyType_Ready(),
+    // then add it to the module. This is not just a check! (leaving
+    // this out results in segmentation faults)
+    if (PyType_Ready(&configurableclientlist_type) < 0) {
+        return (false);
+    }
+    void* p = &configurableclientlist_type;
+    if (PyModule_AddObject(mod, "ConfigurableClientList",
+                           static_cast<PyObject*>(p)) < 0) {
+        return (false);
+    }
+    Py_INCREF(&configurableclientlist_type);
+
+    return (true);
+}
+
+} // namespace python
+} // namespace datasrc
+} // namespace isc