[1253] better exception handling, tests and docs

src/lib/datasrc/factory.cc

@@ -70,7 +70,16 @@ 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);
+    // We catch and reraise copies of known-to-be-thrown exceptions
+    // Since otherwise, if the constructor fails, the stack unroll loop may
+    // want access to the then-destroyed library for info
+    try {
+        instance_ = ds_create(config);
+    } catch (const DataSourceConfigError& dsce) {
+        throw DataSourceConfigError(dsce);
+    } catch (const DataSourceError& dse) {
+        throw DataSourceError(dse);
+    }
 }
 
 DataSourceClientContainer::~DataSourceClientContainer() {

src/lib/datasrc/factory.h

@@ -52,10 +52,6 @@ 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);

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\

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

@@ -164,20 +164,38 @@ DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
     // 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)
+    char* ds_type_str;
+    char* ds_config_str;
     try {
-        // Turn the given argument into config Element; then simply call factory class to do its magic
-        char *ds_type_str;
-        char* ds_config_str;
+        // 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);
+            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("ConfigError"), ex_what.c_str());
+        return (-1);
+    } catch (const DataSourceConfigError& dsce) {
+        const string ex_what = "Bad data source configuration data for type " +
+                               string(ds_type_str) + ":" + string(dsce.what());
+        PyErr_SetString(getDataSourceException("ConfigError"), 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());

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

@@ -163,6 +163,7 @@ initModulePart_ZoneUpdater(PyObject* mod) {
 
 
 PyObject* po_DataSourceError;
+PyObject* po_DataSourceConfigError;
 PyObject* po_NotImplemented;
 
 PyModuleDef iscDataSrc = {
@@ -212,6 +213,10 @@ PyInit_datasrc(void) {
         po_DataSourceError = PyErr_NewException("isc.datasrc.Error", NULL,
                                                 NULL);
         PyObjectContainer(po_DataSourceError).installToModule(mod, "Error");
+        po_DataSourceConfigError = PyErr_NewException("isc.datasrc.ConfigError",
+                                                      NULL, NULL);
+        PyObjectContainer(po_DataSourceConfigError).installToModule(mod,
+                                                                "ConfigError");
         po_NotImplemented = PyErr_NewException("isc.datasrc.NotImplemented",
                                                NULL, NULL);
         PyObjectContainer(po_NotImplemented).installToModule(mod,

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,24 @@ 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.ConfigError,
+                          isc.datasrc.DataSourceClient, "sqlite3", "")
+        self.assertRaises(isc.datasrc.ConfigError,
+                          isc.datasrc.DataSourceClient, "sqlite3", "{}")
+        self.assertRaises(isc.datasrc.ConfigError,
+                          isc.datasrc.DataSourceClient, "sqlite3",
+                          "{ \"foo\": 1 }")
 
     def test_iterate(self):
-        dsc = isc.datasrc.DataSourceClient("sqlite3", "{ \"database_file\": \"" + 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 +188,7 @@ class DataSrcClient(unittest.TestCase):
         self.assertRaises(TypeError, isc.datasrc.ZoneFinder)
 
     def test_find(self):
-        dsc = isc.datasrc.DataSourceClient("sqlite3", "{ \"database_file\": \"" + 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 +272,7 @@ class DataSrcUpdater(unittest.TestCase):
 
     def test_update_delete_commit(self):
 
-        dsc = isc.datasrc.DataSourceClient("sqlite3", "{ \"database_file\": \"" + 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 +346,7 @@ class DataSrcUpdater(unittest.TestCase):
                          rrset.to_text())
 
     def test_update_delete_abort(self):
-        dsc = isc.datasrc.DataSourceClient("sqlite3", "{ \"database_file\": \"" + 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"))