BIND 10 trac1253, updated. a6790c80bfcefde81e032db9d3a45c7a9e48faad [1253] make createInstance set an error instead of raising an exception
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Oct 6 09:49:12 UTC 2011
The branch, trac1253 has been updated
via a6790c80bfcefde81e032db9d3a45c7a9e48faad (commit)
from 6b206d435a3dd92ef4a18f1c4558da147016fe4f (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 a6790c80bfcefde81e032db9d3a45c7a9e48faad
Author: Jelte Jansen <jelte at isc.org>
Date: Thu Oct 6 11:48:41 2011 +0200
[1253] make createInstance set an error instead of raising an exception
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/factory.cc | 13 ++----
src/lib/datasrc/factory.h | 13 +-----
src/lib/datasrc/memory_datasrc.cc | 5 +-
src/lib/datasrc/memory_datasrc.h | 3 +-
src/lib/datasrc/sqlite3_accessor.cc | 16 +++++--
src/lib/datasrc/sqlite3_accessor.h | 3 +-
src/lib/datasrc/tests/factory_unittest.cc | 46 +++++++++++-----------
src/lib/python/isc/datasrc/client_python.cc | 7 +---
src/lib/python/isc/datasrc/tests/Makefile.am | 7 ++-
src/lib/python/isc/datasrc/tests/datasrc_test.py | 8 ++--
10 files changed, 57 insertions(+), 64 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/factory.cc b/src/lib/datasrc/factory.cc
index a272e37..9904979 100644
--- a/src/lib/datasrc/factory.cc
+++ b/src/lib/datasrc/factory.cc
@@ -70,15 +70,10 @@ DataSourceClientContainer::DataSourceClientContainer(const std::string& type,
ds_creator* ds_create = (ds_creator*)ds_lib_.getSym("createInstance");
destructor_ = (ds_destructor*)ds_lib_.getSym("destroyInstance");
- // 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);
+ std::string error;
+ instance_ = ds_create(config, error);
+ if (instance_ == NULL) {
+ isc_throw(DataSourceError, error);
}
}
diff --git a/src/lib/datasrc/factory.h b/src/lib/datasrc/factory.h
index eff8891..dffdd2d 100644
--- a/src/lib/datasrc/factory.h
+++ b/src/lib/datasrc/factory.h
@@ -44,17 +44,8 @@ public:
DataSourceError(file, line, what) {}
};
-/// \brief Raised if the given config contains bad data
-///
-/// Depending on the datasource type, the configuration may differ (for
-/// instance, the sqlite3 datasource needs a database file).
-class DataSourceConfigError : public DataSourceError {
-public:
- DataSourceConfigError(const char* file, size_t line, const char* what) :
- DataSourceError(file, line, what) {}
-};
-
-typedef DataSourceClient* ds_creator(isc::data::ConstElementPtr config);
+typedef DataSourceClient* ds_creator(isc::data::ConstElementPtr config,
+ std::string& error);
typedef void ds_destructor(DataSourceClient* instance);
/// \brief Container class for dynamically loaded libraries
diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc
index 4c9e53f..0b96121 100644
--- a/src/lib/datasrc/memory_datasrc.cc
+++ b/src/lib/datasrc/memory_datasrc.cc
@@ -931,10 +931,11 @@ checkConfig(ConstElementPtr config, ElementPtr errors) {
} // end anonymous namespace
DataSourceClient *
-createInstance(isc::data::ConstElementPtr config) {
+createInstance(isc::data::ConstElementPtr config, std::string& error) {
ElementPtr errors(Element::createList());
if (!checkConfig(config, errors)) {
- isc_throw(DataSourceConfigError, errors->str());
+ error = "Configuration error: " + errors->str();
+ return (NULL);
}
return (new InMemoryClient());
}
diff --git a/src/lib/datasrc/memory_datasrc.h b/src/lib/datasrc/memory_datasrc.h
index cf467a2..c95d0b3 100644
--- a/src/lib/datasrc/memory_datasrc.h
+++ b/src/lib/datasrc/memory_datasrc.h
@@ -310,7 +310,8 @@ private:
///
/// This configuration setup is currently under discussion and will change in
/// the near future.
-extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config);
+extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config,
+ std::string& error);
/// \brief Destroy the instance created by createInstance()
extern "C" void destroyInstance(DataSourceClient* instance);
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index 3607227..177c2cf 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -760,15 +760,21 @@ checkConfig(ConstElementPtr config, ElementPtr errors) {
} // end anonymous namespace
DataSourceClient *
-createInstance(isc::data::ConstElementPtr config) {
+createInstance(isc::data::ConstElementPtr config, std::string& error) {
ElementPtr errors(Element::createList());
if (!checkConfig(config, errors)) {
- isc_throw(DataSourceConfigError, errors->str());
+ error = std::string("Configuration error: " + errors->str());
+ return (NULL);
}
std::string dbfile = config->get(CONFIG_ITEM_DATABASE_FILE)->stringValue();
- boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
- new SQLite3Accessor(dbfile, isc::dns::RRClass::IN()));
- return (new DatabaseClient(isc::dns::RRClass::IN(), sqlite3_accessor));
+ try {
+ boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
+ new SQLite3Accessor(dbfile, isc::dns::RRClass::IN()));
+ return (new DatabaseClient(isc::dns::RRClass::IN(), sqlite3_accessor));
+ } catch (const std::exception& exc) {
+ error = exc.what();
+ return (NULL);
+ }
}
void destroyInstance(DataSourceClient* instance) {
diff --git a/src/lib/datasrc/sqlite3_accessor.h b/src/lib/datasrc/sqlite3_accessor.h
index 3286f3b..b0c09f3 100644
--- a/src/lib/datasrc/sqlite3_accessor.h
+++ b/src/lib/datasrc/sqlite3_accessor.h
@@ -200,7 +200,8 @@ private:
///
/// This configuration setup is currently under discussion and will change in
/// the near future.
-extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config);
+extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config,
+ std::string& error);
/// \brief Destroy the instance created by createInstance()
extern "C" void destroyInstance(DataSourceClient* instance);
diff --git a/src/lib/datasrc/tests/factory_unittest.cc b/src/lib/datasrc/tests/factory_unittest.cc
index 94d1118..0133508 100644
--- a/src/lib/datasrc/tests/factory_unittest.cc
+++ b/src/lib/datasrc/tests/factory_unittest.cc
@@ -37,43 +37,43 @@ TEST(FactoryTest, sqlite3ClientBadConfig) {
// tests are left to the implementation-specific backends)
ElementPtr config;
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config = Element::create("asdf");
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config = Element::createMap();
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("class", ElementPtr());
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("class", Element::create(1));
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("class", Element::create("FOO"));
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("class", Element::create("IN"));
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("database_file", ElementPtr());
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("database_file", Element::create(1));
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("database_file", Element::create("/foo/bar/doesnotexist"));
ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
- SQLite3Error);
+ DataSourceError);
config->set("database_file", Element::create(SQLITE_DBFILE_EXAMPLE_ORG));
DataSourceClientContainer dsc("sqlite3", config);
@@ -100,55 +100,55 @@ TEST(FactoryTest, memoryClient) {
// tests are left to the implementation-specific backends)
ElementPtr config;
ASSERT_THROW(DataSourceClientContainer client("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config = Element::create("asdf");
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config = Element::createMap();
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("type", ElementPtr());
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("type", Element::create(1));
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("type", Element::create("FOO"));
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("type", Element::create("memory"));
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("class", ElementPtr());
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("class", Element::create(1));
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("class", Element::create("FOO"));
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("class", Element::create("IN"));
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("zones", ElementPtr());
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("zones", Element::create(1));
ASSERT_THROW(DataSourceClientContainer("memory", config),
- DataSourceConfigError);
+ DataSourceError);
config->set("zones", Element::createList());
DataSourceClientContainer dsc("memory", config);
diff --git a/src/lib/python/isc/datasrc/client_python.cc b/src/lib/python/isc/datasrc/client_python.cc
index a229c98..162993e 100644
--- a/src/lib/python/isc/datasrc/client_python.cc
+++ b/src/lib/python/isc/datasrc/client_python.cc
@@ -184,12 +184,7 @@ DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
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());
+ PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
return (-1);
} catch (const DataSourceError& dse) {
const string ex_what = "Failed to create DataSourceClient of type " +
diff --git a/src/lib/python/isc/datasrc/tests/Makefile.am b/src/lib/python/isc/datasrc/tests/Makefile.am
index be30dfa..33b9c74 100644
--- a/src/lib/python/isc/datasrc/tests/Makefile.am
+++ b/src/lib/python/isc/datasrc/tests/Makefile.am
@@ -10,9 +10,12 @@ CLEANFILES = $(abs_builddir)/rwtest.sqlite3.copied
# If necessary (rare cases), explicitly specify paths to dynamic libraries
# required by loadable python modules.
-LIBRARY_PATH_PLACEHOLDER =
+# We always add one, the location of the data source modules
+# We may want to add an API method for this to the ds factory, but that is out
+# of scope for this ticket
+LIBRARY_PATH_PLACEHOLDER = $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/datasrc/.libs
if SET_ENV_LIBRARY_PATH
-LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/datasrc/.libs:$$$(ENV_LIBRARY_PATH)
+LIBRARY_PATH_PLACEHOLDER += :$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$$$(ENV_LIBRARY_PATH)
endif
# test using command-line arguments, so use check-local target instead of TESTS
diff --git a/src/lib/python/isc/datasrc/tests/datasrc_test.py b/src/lib/python/isc/datasrc/tests/datasrc_test.py
index 6551d60..97601f5 100644
--- a/src/lib/python/isc/datasrc/tests/datasrc_test.py
+++ b/src/lib/python/isc/datasrc/tests/datasrc_test.py
@@ -68,14 +68,14 @@ class DataSrcClient(unittest.TestCase):
self.assertRaises(TypeError, isc.datasrc.DataSourceClient, "sqlite3", 1)
self.assertRaises(isc.datasrc.Error,
isc.datasrc.DataSourceClient, "foo", "{}")
- self.assertRaises(isc.datasrc.ConfigError,
+ self.assertRaises(isc.datasrc.Error,
isc.datasrc.DataSourceClient, "sqlite3", "")
- self.assertRaises(isc.datasrc.ConfigError,
+ self.assertRaises(isc.datasrc.Error,
isc.datasrc.DataSourceClient, "sqlite3", "{}")
- self.assertRaises(isc.datasrc.ConfigError,
+ self.assertRaises(isc.datasrc.Error,
isc.datasrc.DataSourceClient, "sqlite3",
"{ \"foo\": 1 }")
- self.assertRaises(isc.datasrc.ConfigError,
+ self.assertRaises(isc.datasrc.Error,
isc.datasrc.DataSourceClient, "memory",
"{ \"foo\": 1 }")
More information about the bind10-changes
mailing list