BIND 10 trac1175, updated. 07c1b5ec226758101a9ec540df7f3ef0c9f6ab6d [1175] fix wrong list-type handling in the function get_spec_defaults and add more tests into test_get_spec_defaults
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Aug 26 08:43:30 UTC 2011
The branch, trac1175 has been updated
discards ee6d95d9acf3957aa57949e34d17a12760878c14 (commit)
discards 36df7f4a05a2f07cdb6cddd80a476fa53fdb3bf9 (commit)
discards 16358d7c1107b414d47f77589aaf1b14546c23b5 (commit)
discards 2ab98e51df00f361fb1bf4aa8c7d05aa7f446399 (commit)
discards c9435f0c08ddb8ecf57f1c1ce56b3f208c33b20b (commit)
discards 040223d6d2a3c988e6d1470c37852aa5e7165922 (commit)
discards 38bde9a0483c2a606696167241c2199b3f4589f0 (commit)
discards 1abcb12d544e891248fad911daf96c57316e6137 (commit)
discards ec9e075afc95bae71f52b08051598744aac75b9f (commit)
discards 9b2fc2fd64d2c0c10eb388322cd508edb70288c3 (commit)
discards 25a104224a45b93b95fe85c7e0491bd88d2d0491 (commit)
via 07c1b5ec226758101a9ec540df7f3ef0c9f6ab6d (commit)
via 17e8027e14920b76366117cdb90f6d6eccd17eb3 (commit)
via 5ed59a00c0e4f5f0f8e964612f5af281f419bbac (commit)
via 9761b4df2e6c746e93f7220ae52e4e6c124cb576 (commit)
via 64f41ca526ad5858604815980ec1a8bb6e770ee6 (commit)
via 3d5e8a58b5379118787b736172bb005f525d5d0c (commit)
via 496714b338e777d373542ada473303f03dfaf163 (commit)
via 77f3ef9ea62ce35288af257b9204b19ef7840c7e (commit)
via 156adf368e66dccc0929a71bf565ea4bc06c94ad (commit)
via 5dac7a791f02498f3971c95f46747488d08e8999 (commit)
via bc12dc101aa75269ed1308278792f7c35fcf794a (commit)
via 255bf5b18e2b0e28a65062e87dc2d1212376bfc2 (commit)
via e2ada81cd2a090f707147abdb73a90d44db2f2b0 (commit)
via 0953b3f5d7ed1b4a25362f9a2d1a41eeeda8efa6 (commit)
via 30df43575158b0cb294ec49a8463fe8b49593e62 (commit)
via 4c0accf0a591b0422c84216150e1b9b4e008609e (commit)
via 1f051716bce3d7aa2545722ba41958df9758cadc (commit)
via 10553ed4ebb5b949ae74d277d398d2e8a3909ea5 (commit)
via d916aef6af6bb8506b1ff4756054a1697410982f (commit)
via 4700bada6282f5ad10b53cd8ca7cc03b8fea791d (commit)
via ef64723fe9638f8d56f58fba44a149ac620eadd9 (commit)
via 5de6f9658f745e05361242042afd518b444d7466 (commit)
via 3f847f9d35bf2bf9ee0d957ea1aa9ffb27a32cdb (commit)
via df047c5ccb5c81f9a3d36f7fc38a19bc7c8f2ac2 (commit)
via a7346d50ae5389ce37e35a7131f0f218663b8c68 (commit)
via ad91831c938430b6d4a8fd7bfae517a0f1e327c1 (commit)
via 43da3c6c1cc7cb5fcb1dbe2f983a53e883408d1b (commit)
via 27b3488b71a5c3b95652eab2720497d6d055346e (commit)
via 087c6def9087019640a437b63c782a5c22de1feb (commit)
via 1921e1297dfcb878b9417edefe4d87639c827948 (commit)
via 4ff5e524a7f79ad7f4513ebed3ca0990392263af (commit)
via 38d1a8aa943424e1a0de0503ee8aa961a95d0e14 (commit)
via 4579a2a9f43a38144539447bb5076bfcbaf8b6d8 (commit)
via 08b5add9a6e405342c0c8bc3bdf5d552ed45df0e (commit)
via a176724d99c073f8e547dea2675a5b7d1df70515 (commit)
via a9b769b8bf12e2922e385c62ce337fb723731699 (commit)
via ad36c799ff07d47ebd5c861c63e9feef50408e34 (commit)
via c5e0ebf85ef50e61457f3b99a05109a92b328573 (commit)
via 8216d5dbe1ef23d56ba589fe1de619a601bada4b (commit)
via 1c834de994f51a1fb98add648dad49abfea2c403 (commit)
via dc9318330acbd36e07ad5a4e8a68c9a6e2430543 (commit)
via e4a17a0630a6460090c5cdb562e02ba992a74fa8 (commit)
via b4ae924f504e9749989059a14e6a5dc830c99e81 (commit)
via 2384bcf387e93435658ec1ab92addbf28c9ab640 (commit)
via 1d314b2544b8af8a936c90e00a0dbbb605410952 (commit)
via e3c81bd07046903b4b3bff8325024aafcdb35cba (commit)
via 9001f1db99dfff10957dc2a971e7466a496f0f2f (commit)
via 616fb3be8c0b3c266eaf0aa4ae399918fc7992ef (commit)
via 1351cb42c92cd415003adf6234d96507c8a6d2db (commit)
via 575bd3ec2fe918257cb448eee8ebbff269d85431 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (ee6d95d9acf3957aa57949e34d17a12760878c14)
\
N -- N -- N (07c1b5ec226758101a9ec540df7f3ef0c9f6ab6d)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
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 07c1b5ec226758101a9ec540df7f3ef0c9f6ab6d
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Fri Aug 26 12:01:47 2011 +0900
[1175] fix wrong list-type handling in the function get_spec_defaults and add
more tests into test_get_spec_defaults
commit 17e8027e14920b76366117cdb90f6d6eccd17eb3
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Wed Aug 24 17:28:29 2011 +0900
[1175] fix typo and correct changes from trac519
commit 5ed59a00c0e4f5f0f8e964612f5af281f419bbac
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Wed Aug 24 16:51:11 2011 +0900
[1175] deadlock will be killed afer 20 secs
commit 9761b4df2e6c746e93f7220ae52e4e6c124cb576
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Wed Aug 24 15:55:21 2011 +0900
[1175] fix conflicts with trac519
commit 64f41ca526ad5858604815980ec1a8bb6e770ee6
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Tue Aug 23 16:42:18 2011 +0900
[1175]
- A hostname (canonical name of host) is not acceptable in listen_on
configuration.
- A default port number(starting number for search) is added in args of the
function get_availaddr.
commit 3d5e8a58b5379118787b736172bb005f525d5d0c
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Tue Aug 23 11:22:03 2011 +0900
[1175] set msgq verbose off
commit 496714b338e777d373542ada473303f03dfaf163
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Mon Aug 22 18:20:39 2011 +0900
[1175] add #1175
commit 77f3ef9ea62ce35288af257b9204b19ef7840c7e
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Mon Aug 22 18:18:47 2011 +0900
[1175]
- don't use time.sleep for waiting threads are starting or finishing
- correct shutting down of mock modules
- use _started (threading.Event) where command_handler is invoked
- add implementation to changing contents of specfile of MyStatsHttpd
- set "BIND10_MSGQ_SOCKET_FILE" only when it's not set yet
commit 156adf368e66dccc0929a71bf565ea4bc06c94ad
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Mon Aug 22 18:10:25 2011 +0900
[1175]
- add function get_availaddr to get available address and port on the platform
- add function is_ipv6enabled to check ipv6 enabled on the platform
- add miscellaneous changes to refactor unittest
commit 5dac7a791f02498f3971c95f46747488d08e8999
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Mon Aug 22 18:05:57 2011 +0900
[1175]
- don't use DEFAULT_CONFIG
- move up mccs.start and open_httpd to __init__(). It takes time to do these
functions, and an extra sleep is needed in unittests.
- set running to False in http stopping
- use validate_config in module_spec class
- don't close/open http before it's opened
commit bc12dc101aa75269ed1308278792f7c35fcf794a
Author: Naoki Kambe <kambe at jprs.co.jp>
Date: Wed Aug 17 13:37:45 2011 +0900
Revert "[master] Revert trac930 because of failures on biuldbots:"
This reverts commit 5de7909a21a077238567b64e489ed5345824b2a0.
Conflicts:
ChangeLog
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 6 +
configure.ac | 1 +
src/bin/bind10/tests/Makefile.am | 2 +
src/bin/cfgmgr/tests/Makefile.am | 4 +-
src/bin/tests/Makefile.am | 1 +
src/bin/xfrout/tests/Makefile.am | 3 +-
src/lib/datasrc/database.cc | 183 ++++++--
src/lib/datasrc/database.h | 257 ++++++++++-
src/lib/datasrc/datasrc_messages.mes | 27 +
src/lib/datasrc/sqlite3_accessor.cc | 500 +++++++++++++-------
src/lib/datasrc/sqlite3_accessor.h | 45 ++-
src/lib/datasrc/sqlite3_datasrc.cc | 95 ++++-
src/lib/datasrc/tests/Makefile.am | 6 +-
src/lib/datasrc/tests/database_unittest.cc | 186 +++++++-
src/lib/datasrc/tests/sqlite3_accessor_unittest.cc | 431 +++++++++++++++--
src/lib/datasrc/tests/testdata/Makefile.am | 1 +
src/lib/python/isc/datasrc/sqlite3_ds.py | 84 ++--
.../python/isc/datasrc/tests/sqlite3_ds_test.py | 50 ++-
src/lib/python/isc/log/tests/Makefile.am | 18 +-
19 files changed, 1562 insertions(+), 338 deletions(-)
create mode 100644 src/lib/datasrc/tests/testdata/Makefile.am
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 2785371..ce367c2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,12 @@ xxx. [func] naokikambe
schema by each module via both bindctl and HTTP/XML.
(Trac #928,#929,#930,#1175, git TBD)
+285. [bug] jelte
+ sqlite3 data source: fixed a race condition on initial startup,
+ when the database has not been initialized yet, and multiple
+ processes are trying to do so, resulting in one of them failing.
+ (Trac #326, git 5de6f9658f745e05361242042afd518b444d7466)
+
284. [bug] jerry
b10-zonemgr: zonemgr will not terminate on empty zones, it will
log a warning and try to do zone transfer for them.
diff --git a/configure.ac b/configure.ac
index ee990eb..bbf1d80 100644
--- a/configure.ac
+++ b/configure.ac
@@ -849,6 +849,7 @@ AC_CONFIG_FILES([Makefile
src/lib/exceptions/tests/Makefile
src/lib/datasrc/Makefile
src/lib/datasrc/tests/Makefile
+ src/lib/datasrc/tests/testdata/Makefile
src/lib/xfr/Makefile
src/lib/log/Makefile
src/lib/log/compiler/Makefile
diff --git a/src/bin/bind10/tests/Makefile.am b/src/bin/bind10/tests/Makefile.am
index d9e012f..f388ba1 100644
--- a/src/bin/bind10/tests/Makefile.am
+++ b/src/bin/bind10/tests/Makefile.am
@@ -2,6 +2,7 @@ PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
#PYTESTS = args_test.py bind10_test.py
# NOTE: this has a generated test found in the builddir
PYTESTS = bind10_test.py
+noinst_SCRIPTS = $(PYTESTS)
# If necessary (rare cases), explicitly specify paths to dynamic libraries
# required by loadable python modules.
@@ -19,6 +20,7 @@ if ENABLE_PYTHON_COVERAGE
endif
for pytest in $(PYTESTS) ; do \
echo Running test: $$pytest ; \
+ chmod +x $(abs_builddir)/$$pytest ; \
$(LIBRARY_PATH_PLACEHOLDER) \
env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_srcdir)/src/bin:$(abs_top_builddir)/src/bin/bind10:$(abs_top_builddir)/src/lib/util/io/.libs \
BIND10_MSGQ_SOCKET_FILE=$(abs_top_builddir)/msgq_socket \
diff --git a/src/bin/cfgmgr/tests/Makefile.am b/src/bin/cfgmgr/tests/Makefile.am
index bd67241..99f8cc9 100644
--- a/src/bin/cfgmgr/tests/Makefile.am
+++ b/src/bin/cfgmgr/tests/Makefile.am
@@ -1,7 +1,8 @@
PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
PYTESTS = b10-cfgmgr_test.py
-EXTRA_DIST = $(PYTESTS) testdata/plugins/testplugin.py
+noinst_SCRIPTS = $(PYTESTS)
+EXTRA_DIST = testdata/plugins/testplugin.py
# If necessary (rare cases), explicitly specify paths to dynamic libraries
# required by loadable python modules.
@@ -19,6 +20,7 @@ if ENABLE_PYTHON_COVERAGE
endif
for pytest in $(PYTESTS) ; do \
echo Running test: $$pytest ; \
+ chmod +x $(abs_builddir)/$$pytest ; \
env TESTDATA_PATH=$(abs_srcdir)/testdata \
$(LIBRARY_PATH_PLACEHOLDER) \
env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/bin/cfgmgr:$(abs_top_builddir)/src/lib/python/isc/config \
diff --git a/src/bin/tests/Makefile.am b/src/bin/tests/Makefile.am
index 0dc5021..23a9060 100644
--- a/src/bin/tests/Makefile.am
+++ b/src/bin/tests/Makefile.am
@@ -20,6 +20,7 @@ if ENABLE_PYTHON_COVERAGE
endif
for pytest in $(PYTESTS) ; do \
echo Running test: $$pytest ; \
+ chmod +x $(abs_builddir)/$$pytest ; \
$(LIBRARY_PATH_PLACEHOLDER) \
env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/dns/python/.libs \
$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
diff --git a/src/bin/xfrout/tests/Makefile.am b/src/bin/xfrout/tests/Makefile.am
index 99f4843..2f1e2ea 100644
--- a/src/bin/xfrout/tests/Makefile.am
+++ b/src/bin/xfrout/tests/Makefile.am
@@ -1,6 +1,6 @@
PYCOVERAGE_RUN=@PYCOVERAGE_RUN@
PYTESTS = xfrout_test.py
-EXTRA_DIST = $(PYTESTS)
+noinst_SCRIPTS = $(PYTESTS)
# If necessary (rare cases), explicitly specify paths to dynamic libraries
# required by loadable python modules.
@@ -18,6 +18,7 @@ if ENABLE_PYTHON_COVERAGE
endif
for pytest in $(PYTESTS) ; do \
echo Running test: $$pytest ; \
+ chmod +x $(abs_builddir)/$$pytest ; \
$(LIBRARY_PATH_PLACEHOLDER) \
env PYTHONPATH=$(abs_top_builddir)/src/bin/xfrout:$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/util/io/.libs \
$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index c2c0a4b..d9b5458 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -38,10 +38,10 @@ namespace isc {
namespace datasrc {
DatabaseClient::DatabaseClient(boost::shared_ptr<DatabaseAccessor>
- database) :
- database_(database)
+ accessor) :
+ accessor_(accessor)
{
- if (database_.get() == NULL) {
+ if (!accessor_) {
isc_throw(isc::InvalidParameter,
"No database provided to DatabaseClient");
}
@@ -49,21 +49,21 @@ DatabaseClient::DatabaseClient(boost::shared_ptr<DatabaseAccessor>
DataSourceClient::FindResult
DatabaseClient::findZone(const Name& name) const {
- std::pair<bool, int> zone(database_->getZone(name));
+ std::pair<bool, int> zone(accessor_->getZone(name.toText()));
// Try exact first
if (zone.first) {
return (FindResult(result::SUCCESS,
- ZoneFinderPtr(new Finder(database_,
+ ZoneFinderPtr(new Finder(accessor_,
zone.second, name))));
}
// Then super domains
// Start from 1, as 0 is covered above
for (size_t i(1); i < name.getLabelCount(); ++i) {
isc::dns::Name superdomain(name.split(i));
- zone = database_->getZone(superdomain);
+ zone = accessor_->getZone(superdomain.toText());
if (zone.first) {
return (FindResult(result::PARTIALMATCH,
- ZoneFinderPtr(new Finder(database_,
+ ZoneFinderPtr(new Finder(accessor_,
zone.second,
superdomain))));
}
@@ -72,10 +72,9 @@ DatabaseClient::findZone(const Name& name) const {
return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
}
-DatabaseClient::Finder::Finder(boost::shared_ptr<DatabaseAccessor>
- database, int zone_id,
- const isc::dns::Name& origin) :
- database_(database),
+DatabaseClient::Finder::Finder(boost::shared_ptr<DatabaseAccessor> accessor,
+ int zone_id, const isc::dns::Name& origin) :
+ accessor_(accessor),
zone_id_(zone_id),
origin_(origin)
{ }
@@ -175,7 +174,8 @@ std::pair<bool, isc::dns::RRsetPtr>
DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
const isc::dns::RRType* type,
bool want_cname, bool want_dname,
- bool want_ns)
+ bool want_ns,
+ const isc::dns::Name* construct_name)
{
RRsigStore sig_store;
bool records_found = false;
@@ -183,7 +183,7 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
// Request the context
DatabaseAccessor::IteratorContextPtr
- context(database_->getRecords(name.toText(), zone_id_));
+ context(accessor_->getRecords(name.toText(), zone_id_));
// It must not return NULL, that's a bug of the implementation
if (!context) {
isc_throw(isc::Unexpected, "Iterator context null at " +
@@ -191,6 +191,9 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
}
std::string columns[DatabaseAccessor::COLUMN_COUNT];
+ if (construct_name == NULL) {
+ construct_name = &name;
+ }
while (context->getNext(columns)) {
if (!records_found) {
records_found = true;
@@ -225,9 +228,10 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
isc_throw(DataSourceError, "NS found together with data"
" in non-apex domain " + name.toText());
}
- addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl,
+ addOrCreate(result_rrset, *construct_name, getClass(),
+ cur_type, cur_ttl,
columns[DatabaseAccessor::RDATA_COLUMN],
- *database_);
+ *accessor_);
} else if (type != NULL && cur_type == *type) {
if (result_rrset &&
result_rrset->getType() == isc::dns::RRType::CNAME()) {
@@ -238,9 +242,10 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
isc_throw(DataSourceError, "NS found together with data"
" in non-apex domain " + name.toText());
}
- addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl,
+ addOrCreate(result_rrset, *construct_name, getClass(),
+ cur_type, cur_ttl,
columns[DatabaseAccessor::RDATA_COLUMN],
- *database_);
+ *accessor_);
} else if (want_cname && cur_type == isc::dns::RRType::CNAME()) {
// There should be no other data, so result_rrset should
// be empty.
@@ -248,9 +253,10 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
isc_throw(DataSourceError, "CNAME found but it is not "
"the only record for " + name.toText());
}
- addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl,
+ addOrCreate(result_rrset, *construct_name, getClass(),
+ cur_type, cur_ttl,
columns[DatabaseAccessor::RDATA_COLUMN],
- *database_);
+ *accessor_);
} else if (want_dname && cur_type == isc::dns::RRType::DNAME()) {
// There should be max one RR of DNAME present
if (result_rrset &&
@@ -258,9 +264,10 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
isc_throw(DataSourceError, "DNAME with multiple RRs in " +
name.toText());
}
- addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl,
+ addOrCreate(result_rrset, *construct_name, getClass(),
+ cur_type, cur_ttl,
columns[DatabaseAccessor::RDATA_COLUMN],
- *database_);
+ *accessor_);
} else if (cur_type == isc::dns::RRType::RRSIG()) {
// If we get signatures before we get the actual data, we
// can't know which ones to keep and which to drop...
@@ -292,6 +299,20 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
return (std::pair<bool, isc::dns::RRsetPtr>(records_found, result_rrset));
}
+bool
+DatabaseClient::Finder::hasSubdomains(const std::string& name) {
+ // Request the context
+ DatabaseAccessor::IteratorContextPtr
+ context(accessor_->getRecords(name, zone_id_, true));
+ // It must not return NULL, that's a bug of the implementation
+ if (!context) {
+ isc_throw(isc::Unexpected, "Iterator context null at " + name);
+ }
+
+ std::string columns[DatabaseAccessor::COLUMN_COUNT];
+ return (context->getNext(columns));
+}
+
ZoneFinder::FindResult
DatabaseClient::Finder::find(const isc::dns::Name& name,
const isc::dns::RRType& type,
@@ -306,21 +327,36 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
ZoneFinder::Result result_status = SUCCESS;
std::pair<bool, isc::dns::RRsetPtr> found;
logger.debug(DBG_TRACE_DETAILED, DATASRC_DATABASE_FIND_RECORDS)
- .arg(database_->getDBName()).arg(name).arg(type);
+ .arg(accessor_->getDBName()).arg(name).arg(type);
+ // In case we are in GLUE_OK mode and start matching wildcards,
+ // we can't do it under NS, so we store it here to check
+ isc::dns::RRsetPtr first_ns;
// First, do we have any kind of delegation (NS/DNAME) here?
- const Name origin(getOrigin());
- const size_t origin_label_count(origin.getLabelCount());
- const size_t current_label_count(name.getLabelCount());
+ Name origin(getOrigin());
+ size_t origin_label_count(origin.getLabelCount());
+ // Number of labels in the last known non-empty domain
+ size_t last_known(origin_label_count);
+ size_t current_label_count(name.getLabelCount());
// This is how many labels we remove to get origin
- const size_t remove_labels(current_label_count - origin_label_count);
+ size_t remove_labels(current_label_count - origin_label_count);
// Now go trough all superdomains from origin down
for (int i(remove_labels); i > 0; --i) {
- const Name superdomain(name.split(i));
+ Name superdomain(name.split(i));
// Look if there's NS or DNAME (but ignore the NS in origin)
found = getRRset(superdomain, NULL, false, true,
i != remove_labels && !glue_ok);
+ if (found.first) {
+ // It contains some RRs, so it exists.
+ last_known = superdomain.getLabelCount();
+ // In case we are in GLUE_OK, we want to store the highest
+ // encountered RRset.
+ if (glue_ok && !first_ns && i != remove_labels) {
+ first_ns = getRRset(superdomain, NULL, false, false,
+ true).second;
+ }
+ }
if (found.second) {
// We found something redirecting somewhere else
// (it can be only NS or DNAME here)
@@ -328,12 +364,12 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
if (result_rrset->getType() == isc::dns::RRType::NS()) {
LOG_DEBUG(logger, DBG_TRACE_DETAILED,
DATASRC_DATABASE_FOUND_DELEGATION).
- arg(database_->getDBName()).arg(superdomain);
+ arg(accessor_->getDBName()).arg(superdomain);
result_status = DELEGATION;
} else {
LOG_DEBUG(logger, DBG_TRACE_DETAILED,
DATASRC_DATABASE_FOUND_DNAME).
- arg(database_->getDBName()).arg(superdomain);
+ arg(accessor_->getDBName()).arg(superdomain);
result_status = DNAME;
}
// Don't search more
@@ -352,7 +388,7 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
result_rrset->getType() == isc::dns::RRType::NS()) {
LOG_DEBUG(logger, DBG_TRACE_DETAILED,
DATASRC_DATABASE_FOUND_DELEGATION_EXACT).
- arg(database_->getDBName()).arg(name);
+ arg(accessor_->getDBName()).arg(name);
result_status = DELEGATION;
} else if (result_rrset && type != isc::dns::RRType::CNAME() &&
result_rrset->getType() == isc::dns::RRType::CNAME()) {
@@ -360,18 +396,73 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
}
if (!result_rrset && !records_found) {
- // Request the context
- DatabaseAccessor::IteratorContextPtr
- context(database_->getRecords(name.toText(), zone_id_, true));
- // It must not return NULL, that's a bug of the implementation
- if (!context) {
- isc_throw(isc::Unexpected, "Iterator context null at " +
- name.toText());
- }
-
- std::string columns[DatabaseAccessor::COLUMN_COUNT];
- if (context->getNext(columns)) {
+ // Nothing lives here.
+ // But check if something lives below this
+ // domain and if so, pretend something is here as well.
+ if (hasSubdomains(name.toText())) {
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+ DATASRC_DATABASE_FOUND_EMPTY_NONTERMINAL).
+ arg(accessor_->getDBName()).arg(name);
records_found = true;
+ } else {
+ // It's not empty non-terminal. So check for wildcards.
+ // We remove labels one by one and look for the wildcard there.
+ // Go up to first non-empty domain.
+
+ remove_labels = current_label_count - last_known;
+ Name star("*");
+ for (size_t i(1); i <= remove_labels; ++ i) {
+ // Construct the name with *
+ // TODO: Once the underlying DatabaseAccessor takes
+ // string, do the concatenation on strings, not
+ // Names
+ Name superdomain(name.split(i));
+ Name wildcard(star.concatenate(superdomain));
+ // TODO What do we do about DNAME here?
+ found = getRRset(wildcard, &type, true, false, true,
+ &name);
+ if (found.first) {
+ if (first_ns) {
+ // In case we are under NS, we don't
+ // wildcard-match, but return delegation
+ result_rrset = first_ns;
+ result_status = DELEGATION;
+ records_found = true;
+ // We pretend to switch to non-glue_ok mode
+ glue_ok = false;
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+ DATASRC_DATABASE_WILDCARD_CANCEL_NS).
+ arg(accessor_->getDBName()).arg(wildcard).
+ arg(first_ns->getName());
+ } else if (!hasSubdomains(name.split(i - 1).toText()))
+ {
+ // Nothing we added as part of the * can exist
+ // directly, as we go up only to first existing
+ // domain, but it could be empty non-terminal. In
+ // that case, we need to cancel the match.
+ records_found = true;
+ result_rrset = found.second;
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+ DATASRC_DATABASE_WILDCARD).
+ arg(accessor_->getDBName()).arg(wildcard).
+ arg(name);
+ } else {
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+ DATASRC_DATABASE_WILDCARD_CANCEL_SUB).
+ arg(accessor_->getDBName()).arg(wildcard).
+ arg(name).arg(superdomain);
+ }
+ break;
+ } else if (hasSubdomains(wildcard.toText())) {
+ // Empty non-terminal asterisk
+ records_found = true;
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+ DATASRC_DATABASE_WILDCARD_EMPTY).
+ arg(accessor_->getDBName()).arg(wildcard).
+ arg(name);
+ break;
+ }
+ }
}
}
}
@@ -380,20 +471,20 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
if (records_found) {
logger.debug(DBG_TRACE_DETAILED,
DATASRC_DATABASE_FOUND_NXRRSET)
- .arg(database_->getDBName()).arg(name)
+ .arg(accessor_->getDBName()).arg(name)
.arg(getClass()).arg(type);
result_status = NXRRSET;
} else {
logger.debug(DBG_TRACE_DETAILED,
DATASRC_DATABASE_FOUND_NXDOMAIN)
- .arg(database_->getDBName()).arg(name)
+ .arg(accessor_->getDBName()).arg(name)
.arg(getClass()).arg(type);
result_status = NXDOMAIN;
}
} else {
logger.debug(DBG_TRACE_DETAILED,
DATASRC_DATABASE_FOUND_RRSET)
- .arg(database_->getDBName()).arg(*result_rrset);
+ .arg(accessor_->getDBName()).arg(*result_rrset);
}
return (FindResult(result_status, result_rrset));
}
@@ -488,7 +579,7 @@ private:
ZoneIteratorPtr
DatabaseClient::getIterator(const isc::dns::Name& name) const {
// Get the zone
- std::pair<bool, int> zone(database_->getZone(name));
+ std::pair<bool, int> zone(accessor_->getZone(name.toText()));
if (!zone.first) {
// No such zone, can't continue
isc_throw(DataSourceError, "Zone " + name.toText() +
@@ -497,7 +588,7 @@ DatabaseClient::getIterator(const isc::dns::Name& name) const {
}
// Request the context
DatabaseAccessor::IteratorContextPtr
- context(database_->getAllRecords(zone.second));
+ context(accessor_->getAllRecords(zone.second));
// It must not return NULL, that's a bug of the implementation
if (context == DatabaseAccessor::IteratorContextPtr()) {
isc_throw(isc::Unexpected, "Iterator context null at " +
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index 16d742d..d9e2343 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -71,6 +71,38 @@ public:
};
/**
+ * Definitions of the fields to be passed to addRecordToZone().
+ *
+ * Each derived implementation of addRecordToZone() should expect
+ * the "columns" vector to be filled with the values as described in this
+ * enumeration, in this order.
+ */
+ enum AddRecordColumns {
+ ADD_NAME = 0, ///< The owner name of the record (a domain name)
+ ADD_REV_NAME = 1, ///< Reversed name of NAME (used for DNSSEC)
+ ADD_TTL = 2, ///< The TTL of the record (in numeric form)
+ ADD_TYPE = 3, ///< The RRType of the record (A/NS/TXT etc.)
+ ADD_SIGTYPE = 4, ///< For RRSIG records, this contains the RRTYPE
+ ///< the RRSIG covers.
+ ADD_RDATA = 5, ///< Full text representation of the record's RDATA
+ ADD_COLUMN_COUNT = 6 ///< Number of columns
+ };
+
+ /**
+ * Definitions of the fields to be passed to deleteRecordInZone().
+ *
+ * Each derived implementation of deleteRecordInZone() should expect
+ * the "params" vector to be filled with the values as described in this
+ * enumeration, in this order.
+ */
+ enum DeleteRecordParams {
+ DEL_NAME = 0, ///< The owner name of the record (a domain name)
+ DEL_TYPE = 1, ///< The RRType of the record (A/NS/TXT etc.)
+ DEL_RDATA = 2, ///< Full text representation of the record's RDATA
+ DEL_PARAM_COUNT = 3 ///< Number of parameters
+ };
+
+ /**
* \brief Destructor
*
* It is empty, but needs a virtual one, since we will use the derived
@@ -88,7 +120,8 @@ public:
* It is not specified if and what implementation of this method may throw,
* so code should expect anything.
*
- * \param name The name of the zone's apex to be looked up.
+ * \param name The (fully qualified) domain name of the zone's apex to be
+ * looked up.
* \return The first part of the result indicates if a matching zone
* was found. In case it was, the second part is internal zone ID.
* This one will be passed to methods finding data in the zone.
@@ -96,7 +129,7 @@ public:
* be returned - the ID is only passed back to the database as
* an opaque handle.
*/
- virtual std::pair<bool, int> getZone(const isc::dns::Name& name) const = 0;
+ virtual std::pair<bool, int> getZone(const std::string& name) const = 0;
/**
* \brief This holds the internal context of ZoneIterator for databases
@@ -136,6 +169,11 @@ public:
* definition already known to the caller (it already passes it as
* an argument to getRecords()).
*
+ * Once this function returns false, any subsequent call to it should
+ * result in false. The implementation of a derived class must ensure
+ * it doesn't cause any disruption due to that such as a crash or
+ * exception.
+ *
* \note The order of RRs is not strictly set, but the RRs for single
* RRset must not be interleaved with any other RRs (eg. RRsets must be
* "together").
@@ -199,6 +237,189 @@ public:
*/
virtual IteratorContextPtr getAllRecords(int id) const = 0;
+ /// Start a transaction for updating a zone.
+ ///
+ /// Each derived class version of this method starts a database
+ /// transaction to make updates to the given name of zone (whose class was
+ /// specified at the construction of the class).
+ ///
+ /// If \c replace is true, any existing records of the zone will be
+ /// deleted on successful completion of updates (after
+ /// \c commitUpdateZone()); if it's false, the existing records will be
+ /// intact unless explicitly deleted by \c deleteRecordInZone().
+ ///
+ /// A single \c DatabaseAccessor instance can perform at most one update
+ /// transaction; a duplicate call to this method before
+ /// \c commitUpdateZone() or \c rollbackUpdateZone() will result in
+ /// a \c DataSourceError exception. If multiple update attempts need
+ /// to be performed concurrently (and if the underlying database allows
+ /// such operation), separate \c DatabaseAccessor instance must be
+ /// created.
+ ///
+ /// \note The underlying database may not allow concurrent updates to
+ /// the same database instance even if different "connections" (or
+ /// something similar specific to the database implementation) are used
+ /// for different sets of updates. For example, it doesn't seem to be
+ /// possible for SQLite3 unless different databases are used. MySQL
+ /// allows concurrent updates to different tables of the same database,
+ /// but a specific operation may block others. As such, this interface
+ /// doesn't require derived classes to allow concurrent updates with
+ /// multiple \c DatabaseAccessor instances; however, the implementation
+ /// is encouraged to do the best for making it more likely to succeed
+ /// as long as the underlying database system allows concurrent updates.
+ ///
+ /// This method returns a pair of \c bool and \c int. Its first element
+ /// indicates whether the given name of zone is found. If it's false,
+ /// the transaction isn't considered to be started; a subsequent call to
+ /// this method with an existing zone name should succeed. Likewise,
+ /// if a call to this method results in an exception, the transaction
+ /// isn't considered to be started. Note also that if the zone is not
+ /// found this method doesn't try to create a new one in the database.
+ /// It must have been created by some other means beforehand.
+ ///
+ /// The second element is the internal zone ID used for subsequent
+ /// updates. Depending on implementation details of the actual derived
+ /// class method, it may be different from the one returned by
+ /// \c getZone(); for example, a specific implementation may use a
+ /// completely new zone ID when \c replace is true.
+ ///
+ /// \exception DataSourceError Duplicate call to this method, or some
+ /// internal database related error.
+ ///
+ /// \param zone_name A string representation of the zone name to be updated
+ /// \param replace Whether to replace the entire zone (see above)
+ ///
+ /// \return A pair of bool and int, indicating whether the specified zone
+ /// exists and (if so) the zone ID to be used for the update, respectively.
+ virtual std::pair<bool, int> startUpdateZone(const std::string& zone_name,
+ bool replace) = 0;
+
+ /// Add a single record to the zone to be updated.
+ ///
+ /// This method provides a simple interface to insert a new record
+ /// (a database "row") to the zone in the update context started by
+ /// \c startUpdateZone(). The zone to which the record to be added
+ /// is the one specified at the time of the \c startUpdateZone() call.
+ ///
+ /// A successful call to \c startUpdateZone() must have preceded to
+ /// this call; otherwise a \c DataSourceError exception will be thrown.
+ ///
+ /// The row is defined as a vector of strings that has exactly
+ /// ADD_COLUMN_COUNT number of elements. See AddRecordColumns for
+ /// the semantics of each element.
+ ///
+ /// Derived class methods are not required to check whether the given
+ /// values in \c columns are valid in terms of the expected semantics;
+ /// in general, it's the caller's responsibility.
+ /// For example, TTLs would normally be expected to be a textual
+ /// representation of decimal numbers, but this interface doesn't require
+ /// the implementation to perform this level of validation. It may check
+ /// the values, however, and in that case if it detects an error it
+ /// should throw a \c DataSourceError exception.
+ ///
+ /// Likewise, derived class methods are not required to detect any
+ /// duplicate record that is already in the zone.
+ ///
+ /// \note The underlying database schema may not have a trivial mapping
+ /// from this style of definition of rows to actual database records.
+ /// It's the implementation's responsibility to implement the mapping
+ /// in the actual derived method.
+ ///
+ /// \exception DataSourceError Invalid call without starting a transaction,
+ /// or other internal database error.
+ ///
+ /// \param columns An array of strings that defines a record to be added
+ /// to the zone.
+ virtual void addRecordToZone(
+ const std::string (&columns)[ADD_COLUMN_COUNT]) = 0;
+
+ /// Delete a single record from the zone to be updated.
+ ///
+ /// This method provides a simple interface to delete a record
+ /// (a database "row") from the zone in the update context started by
+ /// \c startUpdateZone(). The zone from which the record to be deleted
+ /// is the one specified at the time of the \c startUpdateZone() call.
+ ///
+ /// A successful call to \c startUpdateZone() must have preceded to
+ /// this call; otherwise a \c DataSourceError exception will be thrown.
+ ///
+ /// The record to be deleted is specified by a vector of strings that has
+ /// exactly DEL_PARAM_COUNT number of elements. See DeleteRecordParams
+ /// for the semantics of each element.
+ ///
+ /// \note In IXFR, TTL may also be specified, but we intentionally
+ /// ignore that in this interface, because it's not guaranteed
+ /// that all records have the same TTL (unlike the RRset
+ /// assumption) and there can even be multiple records for the
+ /// same name, type and rdata with different TTLs. If we only
+ /// delete one of them, subsequent lookup will still return a
+ /// positive answer, which would be confusing. It's a higher
+ /// layer's responsibility to check if there is at least one
+ /// record in the database that has the given TTL.
+ ///
+ /// Like \c addRecordToZone, derived class methods are not required to
+ /// validate the semantics of the given parameters or to check if there
+ /// is a record that matches the specified parameter; if there isn't
+ /// it simply ignores the result.
+ ///
+ /// \exception DataSourceError Invalid call without starting a transaction,
+ /// or other internal database error.
+ ///
+ /// \param params An array of strings that defines a record to be deleted
+ /// from the zone.
+ virtual void deleteRecordInZone(
+ const std::string (¶ms)[DEL_PARAM_COUNT]) = 0;
+
+ /// Commit updates to the zone.
+ ///
+ /// This method completes a transaction of making updates to the zone
+ /// in the context started by startUpdateZone.
+ ///
+ /// A successful call to \c startUpdateZone() must have preceded to
+ /// this call; otherwise a \c DataSourceError exception will be thrown.
+ /// Once this method successfully completes, the transaction isn't
+ /// considered to exist any more. So a new transaction can now be
+ /// started. On the other hand, a duplicate call to this method after
+ /// a successful completion of it is invalid and should result in
+ /// a \c DataSourceError exception.
+ ///
+ /// If some internal database error happens, a \c DataSourceError
+ /// exception must be thrown. In that case the transaction is still
+ /// considered to be valid; the caller must explicitly rollback it
+ /// or (if it's confident that the error is temporary) try to commit it
+ /// again.
+ ///
+ /// \exception DataSourceError Call without a transaction, duplicate call
+ /// to the method or internal database error.
+ virtual void commitUpdateZone() = 0;
+
+ /// Rollback updates to the zone made so far.
+ ///
+ /// This method rollbacks a transaction of making updates to the zone
+ /// in the context started by startUpdateZone. When it succeeds
+ /// (it normally should, but see below), the underlying database should
+ /// be reverted to the point before performing the corresponding
+ /// \c startUpdateZone().
+ ///
+ /// A successful call to \c startUpdateZone() must have preceded to
+ /// this call; otherwise a \c DataSourceError exception will be thrown.
+ /// Once this method successfully completes, the transaction isn't
+ /// considered to exist any more. So a new transaction can now be
+ /// started. On the other hand, a duplicate call to this method after
+ /// a successful completion of it is invalid and should result in
+ /// a \c DataSourceError exception.
+ ///
+ /// Normally this method should not fail. But it may not always be
+ /// possible to guarantee it depending on the characteristics of the
+ /// underlying database system. So this interface doesn't require the
+ /// actual implementation for the error free property. But if a specific
+ /// implementation of this method can fail, it is encouraged to document
+ /// when that can happen with its implication.
+ ///
+ /// \exception DataSourceError Call without a transaction, duplicate call
+ /// to the method or internal database error.
+ virtual void rollbackUpdateZone() = 0;
+
/**
* \brief Returns a string identifying this dabase backend
*
@@ -332,18 +553,19 @@ public:
* applications shouldn't need it.
*/
int zone_id() const { return (zone_id_); }
+
/**
- * \brief The database.
+ * \brief The database accessor.
*
- * This function provides the database stored inside as
+ * This function provides the database accessor stored inside as
* passed to the constructor. This is meant for testing purposes and
* normal applications shouldn't need it.
*/
- const DatabaseAccessor& database() const {
- return (*database_);
+ const DatabaseAccessor& getAccessor() const {
+ return (*accessor_);
}
private:
- boost::shared_ptr<DatabaseAccessor> database_;
+ boost::shared_ptr<DatabaseAccessor> accessor_;
const int zone_id_;
const isc::dns::Name origin_;
/**
@@ -369,6 +591,9 @@ public:
* DataSourceError.
* \param want_ns This allows redirection by NS to be returned. If
* any other data is met as well, DataSourceError is thrown.
+ * \param construct_name If set to non-NULL, the resulting RRset will
+ * be constructed for this name instead of the queried one. This
+ * is useful for wildcards.
* \note It may happen that some of the above error conditions are not
* detected in some circumstances. The goal here is not to validate
* the domain in DB, but to avoid bad behaviour resulting from
@@ -386,8 +611,20 @@ public:
type,
bool want_cname,
bool want_dname,
- bool want_ns);
+ bool want_ns, const
+ isc::dns::Name*
+ construct_name = NULL);
+ /**
+ * \brief Checks if something lives below this domain.
+ *
+ * This looks if there's any subdomain of the given name. It can be
+ * used to test if domain is empty non-terminal.
+ *
+ * \param name The domain to check.
+ */
+ bool hasSubdomains(const std::string& name);
};
+
/**
* \brief Find a zone in the database
*
@@ -424,8 +661,8 @@ public:
virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name) const;
private:
- /// \brief Our database.
- const boost::shared_ptr<DatabaseAccessor> database_;
+ /// \brief The accessor to our database.
+ const boost::shared_ptr<DatabaseAccessor> accessor_;
};
}
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index 659d2bd..5f92407 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -87,6 +87,11 @@ When searching for a domain, the program met a DNAME redirection to a different
place in the domain space at the given domain name. It will return that one
instead.
+% DATASRC_DATABASE_FOUND_EMPTY_NONTERMINAL empty non-terminal %2 in %1
+The domain name doesn't have any RRs, so it doesn't exist in the database.
+However, it has a subdomain, so it exists in the DNS address space. So we
+return NXRRSET instead of NXDOMAIN.
+
% DATASRC_DATABASE_FOUND_NXDOMAIN search in datasource %1 resulted in NXDOMAIN for %2/%3/%4
The data returned by the database backend did not contain any data for the given
domain name, class and type.
@@ -117,6 +122,28 @@ were found to be different. This isn't allowed on the wire and is considered
an error, so we set it to the lowest value we found (but we don't modify the
database). The data in database should be checked and fixed.
+% DATASRC_DATABASE_WILDCARD constructing RRset %3 from wildcard %2 in %1
+The database doesn't contain directly matching domain, but it does contain a
+wildcard one which is being used to synthesize the answer.
+
+% DATASRC_DATABASE_WILDCARD_CANCEL_NS canceled wildcard match on %2 because %3 contains NS in %1
+The database was queried to provide glue data and it didn't find direct match.
+It could create it from given wildcard, but matching wildcards is forbidden
+under a zone cut, which was found. Therefore the delegation will be returned
+instead.
+
+% DATASRC_DATABASE_WILDCARD_CANCEL_SUB wildcard %2 can't be used to construct %3 because %4 exists in %1
+The answer could be constructed using the wildcard, but the given subdomain
+exists, therefore this name is something like empty non-terminal (actually,
+from the protocol point of view, it is empty non-terminal, but the code
+discovers it differently).
+
+% DATASRC_DATABASE_WILDCARD_EMPTY implicit wildcard %2 used to construct %3 in %1
+The given wildcard exists implicitly in the domainspace, as empty nonterminal
+(eg. there's something like subdomain.*.example.org, so *.example.org exists
+implicitly, but is empty). This will produce NXRRSET, because the constructed
+domain is empty as well as the wildcard.
+
% DATASRC_DO_QUERY handling query for '%1/%2'
A debug message indicating that a query for the given name and RR type is being
processed.
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index 092d919..ee81655 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -14,27 +14,118 @@
#include <sqlite3.h>
+#include <string>
+#include <vector>
+
+#include <boost/foreach.hpp>
+
#include <datasrc/sqlite3_accessor.h>
#include <datasrc/logger.h>
#include <datasrc/data_source.h>
#include <util/filename.h>
-#include <boost/lexical_cast.hpp>
+using namespace std;
+
+#define SQLITE_SCHEMA_VERSION 1
namespace isc {
namespace datasrc {
+// The following enum and char* array define the SQL statements commonly
+// used in this implementation. Corresponding prepared statements (of
+// type sqlite3_stmt*) are maintained in the statements_ array of the
+// SQLite3Parameters structure.
+
+enum StatementID {
+ ZONE = 0,
+ ANY = 1,
+ ANY_SUB = 2,
+ BEGIN = 3,
+ COMMIT = 4,
+ ROLLBACK = 5,
+ DEL_ZONE_RECORDS = 6,
+ ADD_RECORD = 7,
+ DEL_RECORD = 8,
+ ITERATE = 9,
+ NUM_STATEMENTS = 10
+};
+
+const char* const text_statements[NUM_STATEMENTS] = {
+ // note for ANY and ITERATE: the order of the SELECT values is
+ // specifically chosen to match the enum values in RecordColumns
+ "SELECT id FROM zones WHERE name=?1 AND rdclass = ?2", // ZONE
+ "SELECT rdtype, ttl, sigtype, rdata FROM records " // ANY
+ "WHERE zone_id=?1 AND name=?2",
+ "SELECT rdtype, ttl, sigtype, rdata " // ANY_SUB
+ "FROM records WHERE zone_id=?1 AND name LIKE (\"%.\" || ?2)",
+ "BEGIN", // BEGIN
+ "COMMIT", // COMMIT
+ "ROLLBACK", // ROLLBACK
+ "DELETE FROM records WHERE zone_id=?1", // DEL_ZONE_RECORDS
+ "INSERT INTO records " // ADD_RECORD
+ "(zone_id, name, rname, ttl, rdtype, sigtype, rdata) "
+ "VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)",
+ "DELETE FROM records WHERE zone_id=?1 AND name=?2 " // DEL_RECORD
+ "AND rdtype=?3 AND rdata=?4",
+ "SELECT rdtype, ttl, sigtype, rdata, name FROM records " // ITERATE
+ "WHERE zone_id = ?1 ORDER BY name, rdtype"
+};
+
struct SQLite3Parameters {
SQLite3Parameters() :
- db_(NULL), version_(-1),
- q_zone_(NULL)
- {}
+ db_(NULL), version_(-1), updating_zone(false), updated_zone_id(-1)
+ {
+ for (int i = 0; i < NUM_STATEMENTS; ++i) {
+ statements_[i] = NULL;
+ }
+ }
+
sqlite3* db_;
int version_;
- sqlite3_stmt* q_zone_;
+ sqlite3_stmt* statements_[NUM_STATEMENTS];
+ bool updating_zone; // whether or not updating the zone
+ int updated_zone_id; // valid only when updating_zone is true
};
-SQLite3Database::SQLite3Database(const std::string& filename,
+// This is a helper class to encapsulate the code logic of executing
+// a specific SQLite3 statement, ensuring the corresponding prepared
+// statement is always reset whether the execution is completed successfully
+// or it results in an exception.
+// Note that an object of this class is intended to be used for "ephemeral"
+// statement, which is completed with a single "step" (normally within a
+// single call to an SQLite3Database method). In particular, it cannot be
+// used for "SELECT" variants, which generally expect multiple matching rows.
+class StatementProcessor {
+public:
+ // desc will be used on failure in the what() message of the resulting
+ // DataSourceError exception.
+ StatementProcessor(SQLite3Parameters& dbparameters, StatementID stmt_id,
+ const char* desc) :
+ dbparameters_(dbparameters), stmt_id_(stmt_id), desc_(desc)
+ {
+ sqlite3_clear_bindings(dbparameters_.statements_[stmt_id_]);
+ }
+
+ ~StatementProcessor() {
+ sqlite3_reset(dbparameters_.statements_[stmt_id_]);
+ }
+
+ void exec() {
+ if (sqlite3_step(dbparameters_.statements_[stmt_id_]) !=
+ SQLITE_DONE) {
+ sqlite3_reset(dbparameters_.statements_[stmt_id_]);
+ isc_throw(DataSourceError, "failed to " << desc_ << ": " <<
+ sqlite3_errmsg(dbparameters_.db_));
+ }
+ }
+
+private:
+ SQLite3Parameters& dbparameters_;
+ const StatementID stmt_id_;
+ const char* const desc_;
+};
+
+SQLite3Accessor::SQLite3Accessor(const std::string& filename,
const isc::dns::RRClass& rrclass) :
dbparameters_(new SQLite3Parameters),
class_(rrclass.toText()),
@@ -58,34 +149,10 @@ namespace {
class Initializer {
public:
~Initializer() {
- if (params_.q_zone_ != NULL) {
- sqlite3_finalize(params_.q_zone_);
- }
- // we do NOT finalize q_current_ - that is just a pointer to one of
- // the other statements, not a separate one.
- /*
- if (params_.q_record_ != NULL) {
- sqlite3_finalize(params_.q_record_);
- }
- if (params_.q_addrs_ != NULL) {
- sqlite3_finalize(params_.q_addrs_);
- }
- if (params_.q_referral_ != NULL) {
- sqlite3_finalize(params_.q_referral_);
- }
- if (params_.q_count_ != NULL) {
- sqlite3_finalize(params_.q_count_);
- }
- if (params_.q_previous_ != NULL) {
- sqlite3_finalize(params_.q_previous_);
- }
- if (params_.q_nsec3_ != NULL) {
- sqlite3_finalize(params_.q_nsec3_);
+ for (int i = 0; i < NUM_STATEMENTS; ++i) {
+ sqlite3_finalize(params_.statements_[i]);
}
- if (params_.q_prevnsec3_ != NULL) {
- sqlite3_finalize(params_.q_prevnsec3_);
- }
- */
+
if (params_.db_ != NULL) {
sqlite3_close(params_.db_);
}
@@ -121,52 +188,6 @@ const char* const SCHEMA_LIST[] = {
NULL
};
-const char* const q_zone_str = "SELECT id FROM zones WHERE name=?1 AND rdclass = ?2";
-
-// note that the order of the SELECT values is specifically chosen to match
-// the enum values in RecordColumns
-const char* const q_any_str = "SELECT rdtype, ttl, sigtype, rdata "
- "FROM records WHERE zone_id=?1 AND name=?2";
-
-const char* const q_any_sub_str = "SELECT rdtype, ttl, sigtype, rdata "
- "FROM records WHERE zone_id=?1 AND name LIKE (\"%.\" || ?2)";
-
-// note that the order of the SELECT values is specifically chosen to match
-// the enum values in RecordColumns
-const char* const q_iterate_str = "SELECT rdtype, ttl, sigtype, rdata, name FROM records "
- "WHERE zone_id = ?1 "
- "ORDER BY name, rdtype";
-
-/* TODO: Prune the statements, not everything will be needed maybe?
-const char* const q_record_str = "SELECT rdtype, ttl, sigtype, rdata "
- "FROM records WHERE zone_id=?1 AND name=?2 AND "
- "((rdtype=?3 OR sigtype=?3) OR "
- "(rdtype='CNAME' OR sigtype='CNAME') OR "
- "(rdtype='NS' OR sigtype='NS'))";
-
-const char* const q_addrs_str = "SELECT rdtype, ttl, sigtype, rdata "
- "FROM records WHERE zone_id=?1 AND name=?2 AND "
- "(rdtype='A' OR sigtype='A' OR rdtype='AAAA' OR sigtype='AAAA')";
-
-const char* const q_referral_str = "SELECT rdtype, ttl, sigtype, rdata FROM "
- "records WHERE zone_id=?1 AND name=?2 AND"
- "(rdtype='NS' OR sigtype='NS' OR rdtype='DS' OR sigtype='DS' OR "
- "rdtype='DNAME' OR sigtype='DNAME')";
-
-const char* const q_count_str = "SELECT COUNT(*) FROM records "
- "WHERE zone_id=?1 AND rname LIKE (?2 || '%');";
-
-const char* const q_previous_str = "SELECT name FROM records "
- "WHERE zone_id=?1 AND rdtype = 'NSEC' AND "
- "rname < $2 ORDER BY rname DESC LIMIT 1";
-
-const char* const q_nsec3_str = "SELECT rdtype, ttl, rdata FROM nsec3 "
- "WHERE zone_id = ?1 AND hash = $2";
-
-const char* const q_prevnsec3_str = "SELECT hash FROM nsec3 "
- "WHERE zone_id = ?1 AND hash <= $2 ORDER BY hash DESC LIMIT 1";
- */
-
sqlite3_stmt*
prepare(sqlite3* const db, const char* const statement) {
sqlite3_stmt* prepared = NULL;
@@ -177,46 +198,101 @@ prepare(sqlite3* const db, const char* const statement) {
return (prepared);
}
-void
-checkAndSetupSchema(Initializer* initializer) {
- sqlite3* const db = initializer->params_.db_;
+// small function to sleep for 0.1 seconds, needed when waiting for
+// exclusive database locks (which should only occur on startup, and only
+// when the database has not been created yet)
+void doSleep() {
+ struct timespec req;
+ req.tv_sec = 0;
+ req.tv_nsec = 100000000;
+ nanosleep(&req, NULL);
+}
+// returns the schema version if the schema version table exists
+// returns -1 if it does not
+int checkSchemaVersion(sqlite3* db) {
sqlite3_stmt* prepared = NULL;
- if (sqlite3_prepare_v2(db, "SELECT version FROM schema_version", -1,
- &prepared, NULL) == SQLITE_OK &&
- sqlite3_step(prepared) == SQLITE_ROW) {
- initializer->params_.version_ = sqlite3_column_int(prepared, 0);
- sqlite3_finalize(prepared);
- } else {
- logger.info(DATASRC_SQLITE_SETUP);
- if (prepared != NULL) {
- sqlite3_finalize(prepared);
+ // At this point in time, the database might be exclusively locked, in
+ // which case even prepare() will return BUSY, so we may need to try a
+ // few times
+ for (size_t i = 0; i < 50; ++i) {
+ int rc = sqlite3_prepare_v2(db, "SELECT version FROM schema_version",
+ -1, &prepared, NULL);
+ if (rc == SQLITE_ERROR) {
+ // this is the error that is returned when the table does not
+ // exist
+ return (-1);
+ } else if (rc == SQLITE_OK) {
+ break;
+ } else if (rc != SQLITE_BUSY || i == 50) {
+ isc_throw(SQLite3Error, "Unable to prepare version query: "
+ << rc << " " << sqlite3_errmsg(db));
+ }
+ doSleep();
+ }
+ if (sqlite3_step(prepared) != SQLITE_ROW) {
+ isc_throw(SQLite3Error,
+ "Unable to query version: " << sqlite3_errmsg(db));
+ }
+ int version = sqlite3_column_int(prepared, 0);
+ sqlite3_finalize(prepared);
+ return (version);
+}
+
+// return db version
+int create_database(sqlite3* db) {
+ // try to get an exclusive lock. Once that is obtained, do the version
+ // check *again*, just in case this process was racing another
+ //
+ // try for 5 secs (50*0.1)
+ int rc;
+ logger.info(DATASRC_SQLITE_SETUP);
+ for (size_t i = 0; i < 50; ++i) {
+ rc = sqlite3_exec(db, "BEGIN EXCLUSIVE TRANSACTION", NULL, NULL,
+ NULL);
+ if (rc == SQLITE_OK) {
+ break;
+ } else if (rc != SQLITE_BUSY || i == 50) {
+ isc_throw(SQLite3Error, "Unable to acquire exclusive lock "
+ "for database creation: " << sqlite3_errmsg(db));
}
+ doSleep();
+ }
+ int schema_version = checkSchemaVersion(db);
+ if (schema_version == -1) {
for (int i = 0; SCHEMA_LIST[i] != NULL; ++i) {
if (sqlite3_exec(db, SCHEMA_LIST[i], NULL, NULL, NULL) !=
SQLITE_OK) {
isc_throw(SQLite3Error,
- "Failed to set up schema " << SCHEMA_LIST[i]);
+ "Failed to set up schema " << SCHEMA_LIST[i]);
}
}
+ sqlite3_exec(db, "COMMIT TRANSACTION", NULL, NULL, NULL);
+ return (SQLITE_SCHEMA_VERSION);
+ } else {
+ return (schema_version);
}
+}
- initializer->params_.q_zone_ = prepare(db, q_zone_str);
- /* TODO: Yet unneeded statements
- initializer->params_.q_record_ = prepare(db, q_record_str);
- initializer->params_.q_addrs_ = prepare(db, q_addrs_str);
- initializer->params_.q_referral_ = prepare(db, q_referral_str);
- initializer->params_.q_count_ = prepare(db, q_count_str);
- initializer->params_.q_previous_ = prepare(db, q_previous_str);
- initializer->params_.q_nsec3_ = prepare(db, q_nsec3_str);
- initializer->params_.q_prevnsec3_ = prepare(db, q_prevnsec3_str);
- */
+void
+checkAndSetupSchema(Initializer* initializer) {
+ sqlite3* const db = initializer->params_.db_;
+
+ int schema_version = checkSchemaVersion(db);
+ if (schema_version != SQLITE_SCHEMA_VERSION) {
+ schema_version = create_database(db);
+ }
+ initializer->params_.version_ = schema_version;
+
+ for (int i = 0; i < NUM_STATEMENTS; ++i) {
+ initializer->params_.statements_[i] = prepare(db, text_statements[i]);
+ }
}
}
void
-SQLite3Database::open(const std::string& name) {
+SQLite3Accessor::open(const std::string& name) {
LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_SQLITE_CONNOPEN).arg(name);
if (dbparameters_->db_ != NULL) {
// There shouldn't be a way to trigger this anyway
@@ -233,7 +309,7 @@ SQLite3Database::open(const std::string& name) {
initializer.move(dbparameters_.get());
}
-SQLite3Database::~SQLite3Database() {
+SQLite3Accessor::~SQLite3Accessor() {
LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_SQLITE_DROPCONN);
if (dbparameters_->db_ != NULL) {
close();
@@ -241,7 +317,7 @@ SQLite3Database::~SQLite3Database() {
}
void
-SQLite3Database::close(void) {
+SQLite3Accessor::close(void) {
LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_SQLITE_CONNCLOSE);
if (dbparameters_->db_ == NULL) {
isc_throw(DataSourceError,
@@ -249,104 +325,83 @@ SQLite3Database::close(void) {
}
// XXX: sqlite3_finalize() could fail. What should we do in that case?
- sqlite3_finalize(dbparameters_->q_zone_);
- dbparameters_->q_zone_ = NULL;
-
- /* TODO: Once they are needed or not, uncomment or drop
- sqlite3_finalize(dbparameters->q_record_);
- dbparameters->q_record_ = NULL;
-
- sqlite3_finalize(dbparameters->q_addrs_);
- dbparameters->q_addrs_ = NULL;
-
- sqlite3_finalize(dbparameters->q_referral_);
- dbparameters->q_referral_ = NULL;
-
- sqlite3_finalize(dbparameters->q_count_);
- dbparameters->q_count_ = NULL;
-
- sqlite3_finalize(dbparameters->q_previous_);
- dbparameters->q_previous_ = NULL;
-
- sqlite3_finalize(dbparameters->q_prevnsec3_);
- dbparameters->q_prevnsec3_ = NULL;
-
- sqlite3_finalize(dbparameters->q_nsec3_);
- dbparameters->q_nsec3_ = NULL;
- */
+ for (int i = 0; i < NUM_STATEMENTS; ++i) {
+ sqlite3_finalize(dbparameters_->statements_[i]);
+ dbparameters_->statements_[i] = NULL;
+ }
sqlite3_close(dbparameters_->db_);
dbparameters_->db_ = NULL;
}
std::pair<bool, int>
-SQLite3Database::getZone(const isc::dns::Name& name) const {
+SQLite3Accessor::getZone(const std::string& name) const {
int rc;
+ sqlite3_stmt* const stmt = dbparameters_->statements_[ZONE];
// Take the statement (simple SELECT id FROM zones WHERE...)
// and prepare it (bind the parameters to it)
- sqlite3_reset(dbparameters_->q_zone_);
- rc = sqlite3_bind_text(dbparameters_->q_zone_, 1, name.toText().c_str(),
- -1, SQLITE_TRANSIENT);
+ sqlite3_reset(stmt);
+ rc = sqlite3_bind_text(stmt, 1, name.c_str(), -1, SQLITE_STATIC);
if (rc != SQLITE_OK) {
isc_throw(SQLite3Error, "Could not bind " << name <<
" to SQL statement (zone)");
}
- rc = sqlite3_bind_text(dbparameters_->q_zone_, 2, class_.c_str(), -1,
- SQLITE_STATIC);
+ rc = sqlite3_bind_text(stmt, 2, class_.c_str(), -1, SQLITE_STATIC);
if (rc != SQLITE_OK) {
isc_throw(SQLite3Error, "Could not bind " << class_ <<
" to SQL statement (zone)");
}
// Get the data there and see if it found anything
- rc = sqlite3_step(dbparameters_->q_zone_);
- std::pair<bool, int> result;
+ rc = sqlite3_step(stmt);
if (rc == SQLITE_ROW) {
- result = std::pair<bool, int>(true,
- sqlite3_column_int(dbparameters_->
- q_zone_, 0));
- return (result);
+ const int zone_id = sqlite3_column_int(stmt, 0);
+ sqlite3_reset(stmt);
+ return (pair<bool, int>(true, zone_id));
} else if (rc == SQLITE_DONE) {
- result = std::pair<bool, int>(false, 0);
// Free resources
- sqlite3_reset(dbparameters_->q_zone_);
- return (result);
+ sqlite3_reset(stmt);
+ return (pair<bool, int>(false, 0));
}
+ sqlite3_reset(stmt);
isc_throw(DataSourceError, "Unexpected failure in sqlite3_step: " <<
- sqlite3_errmsg(dbparameters_->db_));
+ sqlite3_errmsg(dbparameters_->db_));
// Compilers might not realize isc_throw always throws
return (std::pair<bool, int>(false, 0));
}
-class SQLite3Database::Context : public DatabaseAccessor::IteratorContext {
+class SQLite3Accessor::Context : public DatabaseAccessor::IteratorContext {
public:
// Construct an iterator for all records. When constructed this
// way, the getNext() call will copy all fields
- Context(const boost::shared_ptr<const SQLite3Database>& database, int id) :
+ Context(const boost::shared_ptr<const SQLite3Accessor>& accessor, int id) :
iterator_type_(ITT_ALL),
- database_(database),
+ accessor_(accessor),
statement_(NULL),
name_("")
{
// We create the statement now and then just keep getting data from it
- statement_ = prepare(database->dbparameters_->db_, q_iterate_str);
+ statement_ = prepare(accessor->dbparameters_->db_,
+ text_statements[ITERATE]);
bindZoneId(id);
}
// Construct an iterator for records with a specific name. When constructed
// this way, the getNext() call will copy all fields except name
- Context(const boost::shared_ptr<const SQLite3Database>& database, int id,
+ Context(const boost::shared_ptr<const SQLite3Accessor>& accessor, int id,
const std::string& name, bool subdomains) :
iterator_type_(ITT_NAME),
- database_(database),
+ accessor_(accessor),
statement_(NULL),
name_(name)
+
{
// We create the statement now and then just keep getting data from it
- statement_ = prepare(database->dbparameters_->db_,
- subdomains ? q_any_sub_str : q_any_str);
+ statement_ = prepare(accessor->dbparameters_->db_,
+ subdomains ? text_statements[ANY_SUB] :
+ text_statements[ANY]);
bindZoneId(id);
bindName(name_);
}
@@ -373,7 +428,7 @@ public:
} else if (rc != SQLITE_DONE) {
isc_throw(DataSourceError,
"Unexpected failure in sqlite3_step: " <<
- sqlite3_errmsg(database_->dbparameters_->db_));
+ sqlite3_errmsg(accessor_->dbparameters_->db_));
}
finalize();
return (false);
@@ -402,14 +457,14 @@ private:
finalize();
isc_throw(SQLite3Error, "Could not bind int " << zone_id <<
" to SQL statement: " <<
- sqlite3_errmsg(database_->dbparameters_->db_));
+ sqlite3_errmsg(accessor_->dbparameters_->db_));
}
}
void bindName(const std::string& name) {
if (sqlite3_bind_text(statement_, 2, name.c_str(), -1,
- SQLITE_STATIC) != SQLITE_OK) {
- const char* errmsg = sqlite3_errmsg(database_->dbparameters_->db_);
+ SQLITE_TRANSIENT) != SQLITE_OK) {
+ const char* errmsg = sqlite3_errmsg(accessor_->dbparameters_->db_);
finalize();
isc_throw(SQLite3Error, "Could not bind text '" << name <<
"' to SQL statement: " << errmsg);
@@ -431,7 +486,7 @@ private:
// The field can really be NULL, in which case we return an
// empty string, or sqlite may have run out of memory, in
// which case we raise an error
- if (sqlite3_errcode(database_->dbparameters_->db_)
+ if (sqlite3_errcode(accessor_->dbparameters_->db_)
== SQLITE_NOMEM) {
isc_throw(DataSourceError,
"Sqlite3 backend encountered a memory allocation "
@@ -445,13 +500,13 @@ private:
}
const IteratorType iterator_type_;
- boost::shared_ptr<const SQLite3Database> database_;
+ boost::shared_ptr<const SQLite3Accessor> accessor_;
sqlite3_stmt *statement_;
const std::string name_;
};
DatabaseAccessor::IteratorContextPtr
-SQLite3Database::getRecords(const std::string& name, int id,
+SQLite3Accessor::getRecords(const std::string& name, int id,
bool subdomains) const
{
return (IteratorContextPtr(new Context(shared_from_this(), id, name,
@@ -459,9 +514,130 @@ SQLite3Database::getRecords(const std::string& name, int id,
}
DatabaseAccessor::IteratorContextPtr
-SQLite3Database::getAllRecords(int id) const {
+SQLite3Accessor::getAllRecords(int id) const {
return (IteratorContextPtr(new Context(shared_from_this(), id)));
}
+pair<bool, int>
+SQLite3Accessor::startUpdateZone(const string& zone_name, const bool replace) {
+ if (dbparameters_->updating_zone) {
+ isc_throw(DataSourceError,
+ "duplicate zone update on SQLite3 data source");
+ }
+
+ const pair<bool, int> zone_info(getZone(zone_name));
+ if (!zone_info.first) {
+ return (zone_info);
+ }
+
+ StatementProcessor(*dbparameters_, BEGIN,
+ "start an SQLite3 transaction").exec();
+
+ if (replace) {
+ try {
+ StatementProcessor delzone_exec(*dbparameters_, DEL_ZONE_RECORDS,
+ "delete zone records");
+
+ sqlite3_clear_bindings(
+ dbparameters_->statements_[DEL_ZONE_RECORDS]);
+ if (sqlite3_bind_int(dbparameters_->statements_[DEL_ZONE_RECORDS],
+ 1, zone_info.second) != SQLITE_OK) {
+ isc_throw(DataSourceError,
+ "failed to bind SQLite3 parameter: " <<
+ sqlite3_errmsg(dbparameters_->db_));
+ }
+
+ delzone_exec.exec();
+ } catch (const DataSourceError&) {
+ // Once we start a transaction, if something unexpected happens
+ // we need to rollback the transaction so that a subsequent update
+ // is still possible with this accessor.
+ StatementProcessor(*dbparameters_, ROLLBACK,
+ "rollback an SQLite3 transaction").exec();
+ throw;
+ }
+ }
+
+ dbparameters_->updating_zone = true;
+ dbparameters_->updated_zone_id = zone_info.second;
+
+ return (zone_info);
+}
+
+void
+SQLite3Accessor::commitUpdateZone() {
+ if (!dbparameters_->updating_zone) {
+ isc_throw(DataSourceError, "committing zone update on SQLite3 "
+ "data source without transaction");
+ }
+
+ StatementProcessor(*dbparameters_, COMMIT,
+ "commit an SQLite3 transaction").exec();
+ dbparameters_->updating_zone = false;
+ dbparameters_->updated_zone_id = -1;
+}
+
+void
+SQLite3Accessor::rollbackUpdateZone() {
+ if (!dbparameters_->updating_zone) {
+ isc_throw(DataSourceError, "rolling back zone update on SQLite3 "
+ "data source without transaction");
+ }
+
+ StatementProcessor(*dbparameters_, ROLLBACK,
+ "rollback an SQLite3 transaction").exec();
+ dbparameters_->updating_zone = false;
+ dbparameters_->updated_zone_id = -1;
+}
+
+namespace {
+// Commonly used code sequence for adding/deleting record
+template <typename COLUMNS_TYPE>
+void
+doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id,
+ COLUMNS_TYPE update_params, const char* exec_desc)
+{
+ sqlite3_stmt* const stmt = dbparams.statements_[stmt_id];
+ StatementProcessor executer(dbparams, stmt_id, exec_desc);
+
+ int param_id = 0;
+ if (sqlite3_bind_int(stmt, ++param_id, dbparams.updated_zone_id)
+ != SQLITE_OK) {
+ isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
+ sqlite3_errmsg(dbparams.db_));
+ }
+ const size_t column_count =
+ sizeof(update_params) / sizeof(update_params[0]);
+ for (int i = 0; i < column_count; ++i) {
+ if (sqlite3_bind_text(stmt, ++param_id, update_params[i].c_str(), -1,
+ SQLITE_TRANSIENT) != SQLITE_OK) {
+ isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " <<
+ sqlite3_errmsg(dbparams.db_));
+ }
+ }
+ executer.exec();
+}
+}
+
+void
+SQLite3Accessor::addRecordToZone(const string (&columns)[ADD_COLUMN_COUNT]) {
+ if (!dbparameters_->updating_zone) {
+ isc_throw(DataSourceError, "adding record to SQLite3 "
+ "data source without transaction");
+ }
+ doUpdate<const string (&)[DatabaseAccessor::ADD_COLUMN_COUNT]>(
+ *dbparameters_, ADD_RECORD, columns, "add record to zone");
+}
+
+void
+SQLite3Accessor::deleteRecordInZone(const string (¶ms)[DEL_PARAM_COUNT]) {
+ if (!dbparameters_->updating_zone) {
+ isc_throw(DataSourceError, "deleting record in SQLite3 "
+ "data source without transaction");
+ }
+ doUpdate<const string (&)[DatabaseAccessor::DEL_PARAM_COUNT]>(
+ *dbparameters_, DEL_RECORD, params, "delete record from zone");
+}
+
}
}
diff --git a/src/lib/datasrc/sqlite3_accessor.h b/src/lib/datasrc/sqlite3_accessor.h
index cc5c657..8f993c1 100644
--- a/src/lib/datasrc/sqlite3_accessor.h
+++ b/src/lib/datasrc/sqlite3_accessor.h
@@ -53,8 +53,8 @@ struct SQLite3Parameters;
* According to the design, it doesn't interpret the data in any way, it just
* provides unified access to the DB.
*/
-class SQLite3Database : public DatabaseAccessor,
- public boost::enable_shared_from_this<SQLite3Database> {
+class SQLite3Accessor : public DatabaseAccessor,
+ public boost::enable_shared_from_this<SQLite3Accessor> {
public:
/**
* \brief Constructor
@@ -69,14 +69,14 @@ public:
* file can contain multiple classes of data, single database can
* provide only one class).
*/
- SQLite3Database(const std::string& filename,
+ SQLite3Accessor(const std::string& filename,
const isc::dns::RRClass& rrclass);
/**
* \brief Destructor
*
* Closes the database.
*/
- ~SQLite3Database();
+ ~SQLite3Accessor();
/**
* \brief Look up a zone
@@ -87,11 +87,11 @@ public:
*
* \exception SQLite3Error if something about the database is broken.
*
- * \param name The name of zone to look up
+ * \param name The (fully qualified) domain name of zone to look up
* \return The pair contains if the lookup was successful in the first
* element and the zone id in the second if it was.
*/
- virtual std::pair<bool, int> getZone(const isc::dns::Name& name) const;
+ virtual std::pair<bool, int> getZone(const std::string& name) const;
/** \brief Look up all resource records for a name
*
@@ -121,6 +121,33 @@ public:
*/
virtual IteratorContextPtr getAllRecords(int id) const;
+ virtual std::pair<bool, int> startUpdateZone(const std::string& zone_name,
+ bool replace);
+
+ /// \note we are quite impatient here: it's quite possible that the COMMIT
+ /// fails due to other process performing SELECT on the same database
+ /// (consider the case where COMMIT is done by xfrin or dynamic update
+ /// server while an authoritative server is busy reading the DB).
+ /// In a future version we should probably need to introduce some retry
+ /// attempt and/or increase timeout before giving up the COMMIT, even
+ /// if it still doesn't guarantee 100% success. Right now this
+ /// implementation throws a \c DataSourceError exception in such a case.
+ virtual void commitUpdateZone();
+
+ /// \note In SQLite3 rollback can fail if there's another unfinished
+ /// statement is performed for the same database structure.
+ /// Although it's not expected to happen in our expected usage, it's not
+ /// guaranteed to be prevented at the API level. If it ever happens, this
+ /// method throws a \c DataSourceError exception. It should be
+ /// considered a bug of the higher level application program.
+ virtual void rollbackUpdateZone();
+
+ virtual void addRecordToZone(
+ const std::string (&columns)[ADD_COLUMN_COUNT]);
+
+ virtual void deleteRecordInZone(
+ const std::string (¶ms)[DEL_PARAM_COUNT]);
+
/// The SQLite3 implementation of this method returns a string starting
/// with a fixed prefix of "sqlite3_" followed by the DB file name
/// removing any path name. For example, for the DB file
@@ -146,4 +173,8 @@ private:
}
}
-#endif
+#endif // __DATASRC_SQLITE3_CONNECTION_H
+
+// Local Variables:
+// mode: c++
+// End:
diff --git a/src/lib/datasrc/sqlite3_datasrc.cc b/src/lib/datasrc/sqlite3_datasrc.cc
index 18ee929..03b057c 100644
--- a/src/lib/datasrc/sqlite3_datasrc.cc
+++ b/src/lib/datasrc/sqlite3_datasrc.cc
@@ -26,6 +26,8 @@
#include <dns/rrset.h>
#include <dns/rrsetlist.h>
+#define SQLITE_SCHEMA_VERSION 1
+
using namespace std;
using namespace isc::dns;
using namespace isc::dns::rdata;
@@ -77,6 +79,8 @@ const char* const SCHEMA_LIST[] = {
NULL
};
+const char* const q_version_str = "SELECT version FROM schema_version";
+
const char* const q_zone_str = "SELECT id FROM zones WHERE name=?1";
const char* const q_record_str = "SELECT rdtype, ttl, sigtype, rdata "
@@ -254,7 +258,7 @@ Sqlite3DataSrc::findRecords(const Name& name, const RRType& rdtype,
}
break;
}
-
+
sqlite3_reset(query);
sqlite3_clear_bindings(query);
@@ -295,7 +299,7 @@ Sqlite3DataSrc::findRecords(const Name& name, const RRType& rdtype,
//
sqlite3_reset(dbparameters->q_count_);
sqlite3_clear_bindings(dbparameters->q_count_);
-
+
rc = sqlite3_bind_int(dbparameters->q_count_, 1, zone_id);
if (rc != SQLITE_OK) {
isc_throw(Sqlite3Error, "Could not bind zone ID " << zone_id <<
@@ -653,29 +657,90 @@ prepare(sqlite3* const db, const char* const statement) {
return (prepared);
}
-void
-checkAndSetupSchema(Sqlite3Initializer* initializer) {
- sqlite3* const db = initializer->params_.db_;
+// small function to sleep for 0.1 seconds, needed when waiting for
+// exclusive database locks (which should only occur on startup, and only
+// when the database has not been created yet)
+void do_sleep() {
+ struct timespec req;
+ req.tv_sec = 0;
+ req.tv_nsec = 100000000;
+ nanosleep(&req, NULL);
+}
+// returns the schema version if the schema version table exists
+// returns -1 if it does not
+int check_schema_version(sqlite3* db) {
sqlite3_stmt* prepared = NULL;
- if (sqlite3_prepare_v2(db, "SELECT version FROM schema_version", -1,
- &prepared, NULL) == SQLITE_OK &&
- sqlite3_step(prepared) == SQLITE_ROW) {
- initializer->params_.version_ = sqlite3_column_int(prepared, 0);
- sqlite3_finalize(prepared);
- } else {
- logger.info(DATASRC_SQLITE_SETUP);
- if (prepared != NULL) {
- sqlite3_finalize(prepared);
+ // At this point in time, the database might be exclusively locked, in
+ // which case even prepare() will return BUSY, so we may need to try a
+ // few times
+ for (size_t i = 0; i < 50; ++i) {
+ int rc = sqlite3_prepare_v2(db, q_version_str, -1, &prepared, NULL);
+ if (rc == SQLITE_ERROR) {
+ // this is the error that is returned when the table does not
+ // exist
+ return (-1);
+ } else if (rc == SQLITE_OK) {
+ break;
+ } else if (rc != SQLITE_BUSY || i == 50) {
+ isc_throw(Sqlite3Error, "Unable to prepare version query: "
+ << rc << " " << sqlite3_errmsg(db));
}
+ do_sleep();
+ }
+ if (sqlite3_step(prepared) != SQLITE_ROW) {
+ isc_throw(Sqlite3Error,
+ "Unable to query version: " << sqlite3_errmsg(db));
+ }
+ int version = sqlite3_column_int(prepared, 0);
+ sqlite3_finalize(prepared);
+ return (version);
+}
+
+// return db version
+int create_database(sqlite3* db) {
+ // try to get an exclusive lock. Once that is obtained, do the version
+ // check *again*, just in case this process was racing another
+ //
+ // try for 5 secs (50*0.1)
+ int rc;
+ logger.info(DATASRC_SQLITE_SETUP);
+ for (size_t i = 0; i < 50; ++i) {
+ rc = sqlite3_exec(db, "BEGIN EXCLUSIVE TRANSACTION", NULL, NULL,
+ NULL);
+ if (rc == SQLITE_OK) {
+ break;
+ } else if (rc != SQLITE_BUSY || i == 50) {
+ isc_throw(Sqlite3Error, "Unable to acquire exclusive lock "
+ "for database creation: " << sqlite3_errmsg(db));
+ }
+ do_sleep();
+ }
+ int schema_version = check_schema_version(db);
+ if (schema_version == -1) {
for (int i = 0; SCHEMA_LIST[i] != NULL; ++i) {
if (sqlite3_exec(db, SCHEMA_LIST[i], NULL, NULL, NULL) !=
SQLITE_OK) {
isc_throw(Sqlite3Error,
- "Failed to set up schema " << SCHEMA_LIST[i]);
+ "Failed to set up schema " << SCHEMA_LIST[i]);
}
}
+ sqlite3_exec(db, "COMMIT TRANSACTION", NULL, NULL, NULL);
+ return (SQLITE_SCHEMA_VERSION);
+ } else {
+ return (schema_version);
+ }
+}
+
+void
+checkAndSetupSchema(Sqlite3Initializer* initializer) {
+ sqlite3* const db = initializer->params_.db_;
+
+ int schema_version = check_schema_version(db);
+ if (schema_version != SQLITE_SCHEMA_VERSION) {
+ schema_version = create_database(db);
}
+ initializer->params_.version_ = schema_version;
initializer->params_.q_zone_ = prepare(db, q_zone_str);
initializer->params_.q_record_ = prepare(db, q_record_str);
diff --git a/src/lib/datasrc/tests/Makefile.am b/src/lib/datasrc/tests/Makefile.am
index a913818..a1ef054 100644
--- a/src/lib/datasrc/tests/Makefile.am
+++ b/src/lib/datasrc/tests/Makefile.am
@@ -1,8 +1,12 @@
+SUBDIRS = . testdata
+
AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
AM_CPPFLAGS += -I$(top_builddir)/src/lib/dns -I$(top_srcdir)/src/lib/dns
AM_CPPFLAGS += $(BOOST_INCLUDES)
AM_CPPFLAGS += $(SQLITE_CFLAGS)
-AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(srcdir)/testdata\"
+AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(abs_srcdir)/testdata\"
+AM_CPPFLAGS += -DTEST_DATA_BUILDDIR=\"$(abs_builddir)/testdata\"
+AM_CPPFLAGS += -DINSTALL_PROG=\"$(abs_top_srcdir)/install-sh\"
AM_CXXFLAGS = $(B10_CXXFLAGS)
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index ac6cbf7..1fb79bc 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -44,20 +44,29 @@ public:
NopAccessor() : database_name_("mock_database")
{ }
- virtual std::pair<bool, int> getZone(const Name& name) const {
- if (name == Name("example.org")) {
+ virtual std::pair<bool, int> getZone(const std::string& name) const {
+ if (name == "example.org.") {
return (std::pair<bool, int>(true, 42));
- } else if (name == Name("null.example.org")) {
+ } else if (name == "null.example.org.") {
return (std::pair<bool, int>(true, 13));
- } else if (name == Name("empty.example.org")) {
+ } else if (name == "empty.example.org.") {
return (std::pair<bool, int>(true, 0));
- } else if (name == Name("bad.example.org")) {
+ } else if (name == "bad.example.org.") {
return (std::pair<bool, int>(true, -1));
} else {
return (std::pair<bool, int>(false, 0));
}
}
+ virtual std::pair<bool, int> startUpdateZone(const std::string&, bool) {
+ // return dummy value. unused anyway.
+ return (pair<bool, int>(true, 0));
+ }
+ virtual void commitUpdateZone() {}
+ virtual void rollbackUpdateZone() {}
+ virtual void addRecordToZone(const string (&)[ADD_COLUMN_COUNT]) {}
+ virtual void deleteRecordInZone(const string (&)[DEL_PARAM_COUNT]) {}
+
virtual const std::string& getDBName() const {
return (database_name_);
}
@@ -276,8 +285,6 @@ public:
private:
typedef std::map<std::string, std::vector< std::vector<std::string> > >
Domains;
- // used as internal index for getNextRecord()
- size_t cur_record;
// used as temporary storage during the building of the fake data
Domains records;
// used as temporary storage after searchForRecord() and during
@@ -460,6 +467,20 @@ private:
// This is because of empty domain test
addRecord("A", "3600", "", "192.0.2.1");
addCurName("a.b.example.org.");
+
+ // Something for wildcards
+ addRecord("A", "3600", "", "192.0.2.5");
+ addCurName("*.wild.example.org.");
+ addRecord("AAAA", "3600", "", "2001:db8::5");
+ addCurName("cancel.here.wild.example.org.");
+ addRecord("NS", "3600", "", "ns.example.com.");
+ addCurName("delegatedwild.example.org.");
+ addRecord("A", "3600", "", "192.0.2.5");
+ addCurName("*.delegatedwild.example.org.");
+ addRecord("A", "3600", "", "192.0.2.5");
+ addCurName("wild.*.foo.example.org.");
+ addRecord("A", "3600", "", "192.0.2.5");
+ addCurName("wild.*.foo.*.bar.example.org.");
}
};
@@ -486,12 +507,12 @@ public:
* times per test.
*/
void createClient() {
- current_database_ = new MockAccessor();
+ current_accessor_ = new MockAccessor();
client_.reset(new DatabaseClient(shared_ptr<DatabaseAccessor>(
- current_database_)));
+ current_accessor_)));
}
// Will be deleted by client_, just keep the current value for comparison.
- MockAccessor* current_database_;
+ MockAccessor* current_accessor_;
shared_ptr<DatabaseClient> client_;
const std::string database_name_;
@@ -506,7 +527,7 @@ public:
ASSERT_NE(shared_ptr<DatabaseClient::Finder>(), finder) <<
"Wrong type of finder";
EXPECT_EQ(42, finder->zone_id());
- EXPECT_EQ(current_database_, &finder->database());
+ EXPECT_EQ(current_accessor_, &finder->getAccessor());
}
shared_ptr<DatabaseClient::Finder> getFinder() {
@@ -672,12 +693,12 @@ doFindTest(shared_ptr<DatabaseClient::Finder> finder,
ZoneFinder::FindResult result =
finder->find(name, type, NULL, options);
ASSERT_EQ(expected_result, result.code) << name << " " << type;
- if (expected_rdatas.size() > 0) {
+ if (!expected_rdatas.empty()) {
checkRRset(result.rrset, expected_name != Name(".") ? expected_name :
name, finder->getClass(), expected_type, expected_ttl,
expected_rdatas);
- if (expected_sig_rdatas.size() > 0) {
+ if (!expected_sig_rdatas.empty()) {
checkRRset(result.rrset->getRRsig(), expected_name != Name(".") ?
expected_name : name, finder->getClass(),
isc::dns::RRType::RRSIG(), expected_ttl,
@@ -836,7 +857,6 @@ TEST_F(DatabaseClientTest, find) {
ZoneFinder::CNAME,
expected_rdatas_, expected_sig_rdatas_);
-
expected_rdatas_.clear();
expected_sig_rdatas_.clear();
expected_rdatas_.push_back("192.0.2.1");
@@ -887,7 +907,6 @@ TEST_F(DatabaseClientTest, find) {
ZoneFinder::SUCCESS,
expected_rdatas_, expected_sig_rdatas_);
-
EXPECT_THROW(finder->find(isc::dns::Name("badcname1.example.org."),
isc::dns::RRType::A(),
NULL, ZoneFinder::FIND_DEFAULT),
@@ -930,7 +949,6 @@ TEST_F(DatabaseClientTest, find) {
isc::dns::RRType::A(),
NULL, ZoneFinder::FIND_DEFAULT),
std::exception);
-
EXPECT_THROW(finder->find(isc::dns::Name("dsexception.in.getnext."),
isc::dns::RRType::A(),
NULL, ZoneFinder::FIND_DEFAULT),
@@ -1076,6 +1094,16 @@ TEST_F(DatabaseClientTest, findDelegation) {
DataSourceError);
}
+TEST_F(DatabaseClientTest, emptyDomain) {
+ shared_ptr<DatabaseClient::Finder> finder(getFinder());
+
+ // This domain doesn't exist, but a subdomain of it does.
+ // Therefore we should pretend the domain is there, but contains no RRsets
+ doFindTest(finder, isc::dns::Name("b.example.org."), isc::dns::RRType::A(),
+ isc::dns::RRType::A(), isc::dns::RRTTL(3600),
+ ZoneFinder::NXRRSET, expected_rdatas_, expected_sig_rdatas_);
+}
+
// Glue-OK mode. Just go trough NS delegations down (but not trough
// DNAME) and pretend it is not there.
TEST_F(DatabaseClientTest, glueOK) {
@@ -1135,15 +1163,127 @@ TEST_F(DatabaseClientTest, glueOK) {
ZoneFinder::FIND_GLUE_OK);
}
-TEST_F(DatabaseClientTest, empty) {
+TEST_F(DatabaseClientTest, wildcard) {
shared_ptr<DatabaseClient::Finder> finder(getFinder());
- // Check empty domain
- // This domain doesn't exist, but a subdomain of it does.
- // Therefore we should pretend the domain is there, but contains no RRsets
- doFindTest(finder, isc::dns::Name("b.example.org."), isc::dns::RRType::A(),
- isc::dns::RRType::A(), isc::dns::RRTTL(3600),
- ZoneFinder::NXRRSET, expected_rdatas_, expected_sig_rdatas_);
+ // First, simple wildcard match
+ expected_rdatas_.push_back("192.0.2.5");
+ doFindTest(finder, isc::dns::Name("a.wild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ isc::dns::RRTTL(3600), ZoneFinder::SUCCESS, expected_rdatas_,
+ expected_sig_rdatas_);
+ doFindTest(finder, isc::dns::Name("b.a.wild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ isc::dns::RRTTL(3600), ZoneFinder::SUCCESS, expected_rdatas_,
+ expected_sig_rdatas_);
+ expected_rdatas_.clear();
+ doFindTest(finder, isc::dns::Name("a.wild.example.org"),
+ isc::dns::RRType::AAAA(), isc::dns::RRType::AAAA(),
+ isc::dns::RRTTL(3600), ZoneFinder::NXRRSET, expected_rdatas_,
+ expected_sig_rdatas_);
+ doFindTest(finder, isc::dns::Name("b.a.wild.example.org"),
+ isc::dns::RRType::AAAA(), isc::dns::RRType::AAAA(),
+ isc::dns::RRTTL(3600), ZoneFinder::NXRRSET, expected_rdatas_,
+ expected_sig_rdatas_);
+
+ // Direct request for thi wildcard
+ expected_rdatas_.push_back("192.0.2.5");
+ doFindTest(finder, isc::dns::Name("*.wild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ isc::dns::RRTTL(3600), ZoneFinder::SUCCESS, expected_rdatas_,
+ expected_sig_rdatas_);
+ expected_rdatas_.clear();
+ doFindTest(finder, isc::dns::Name("*.wild.example.org"),
+ isc::dns::RRType::AAAA(), isc::dns::RRType::AAAA(),
+ isc::dns::RRTTL(3600), ZoneFinder::NXRRSET, expected_rdatas_,
+ expected_sig_rdatas_);
+ // This is nonsense, but check it doesn't match by some stupid accident
+ doFindTest(finder, isc::dns::Name("a.*.wild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ isc::dns::RRTTL(3600), ZoneFinder::NXDOMAIN,
+ expected_rdatas_, expected_sig_rdatas_);
+ // These should be canceled, since it is below a domain which exitsts
+ doFindTest(finder, isc::dns::Name("nothing.here.wild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ isc::dns::RRTTL(3600), ZoneFinder::NXDOMAIN,
+ expected_rdatas_, expected_sig_rdatas_);
+ doFindTest(finder, isc::dns::Name("cancel.here.wild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ isc::dns::RRTTL(3600), ZoneFinder::NXRRSET,
+ expected_rdatas_, expected_sig_rdatas_);
+ doFindTest(finder,
+ isc::dns::Name("below.cancel.here.wild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ isc::dns::RRTTL(3600), ZoneFinder::NXDOMAIN,
+ expected_rdatas_, expected_sig_rdatas_);
+ // And this should be just plain empty non-terminal domain, check
+ // the wildcard doesn't hurt it
+ doFindTest(finder, isc::dns::Name("here.wild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ isc::dns::RRTTL(3600), ZoneFinder::NXRRSET, expected_rdatas_,
+ expected_sig_rdatas_);
+ // Also make sure that the wildcard doesn't hurt the original data
+ // below the wildcard
+ expected_rdatas_.push_back("2001:db8::5");
+ doFindTest(finder, isc::dns::Name("cancel.here.wild.example.org"),
+ isc::dns::RRType::AAAA(), isc::dns::RRType::AAAA(),
+ isc::dns::RRTTL(3600), ZoneFinder::SUCCESS,
+ expected_rdatas_, expected_sig_rdatas_);
+ expected_rdatas_.clear();
+
+ // How wildcard go together with delegation
+ expected_rdatas_.push_back("ns.example.com.");
+ doFindTest(finder, isc::dns::Name("below.delegatedwild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::NS(),
+ isc::dns::RRTTL(3600), ZoneFinder::DELEGATION, expected_rdatas_,
+ expected_sig_rdatas_,
+ isc::dns::Name("delegatedwild.example.org"));
+ // FIXME: This doesn't look logically OK, GLUE_OK should make it transparent,
+ // so the match should either work or be canceled, but return NXDOMAIN
+ doFindTest(finder, isc::dns::Name("below.delegatedwild.example.org"),
+ isc::dns::RRType::A(), isc::dns::RRType::NS(),
+ isc::dns::RRTTL(3600), ZoneFinder::DELEGATION, expected_rdatas_,
+ expected_sig_rdatas_,
+ isc::dns::Name("delegatedwild.example.org"),
+ ZoneFinder::FIND_GLUE_OK);
+
+ expected_rdatas_.clear();
+ expected_rdatas_.push_back("192.0.2.5");
+ // These are direct matches
+ const char* positive_names[] = {
+ "wild.*.foo.example.org.",
+ "wild.*.foo.*.bar.example.org.",
+ NULL
+ };
+ for (const char** name(positive_names); *name != NULL; ++ name) {
+ doFindTest(finder, isc::dns::Name(*name), isc::dns::RRType::A(),
+ isc::dns::RRType::A(), isc::dns::RRTTL(3600),
+ ZoneFinder::SUCCESS, expected_rdatas_,
+ expected_sig_rdatas_);
+ }
+
+ // These are wildcard matches against empty nonterminal asterisk
+ expected_rdatas_.clear();
+ const char* negative_names[] = {
+ "a.foo.example.org.",
+ "*.foo.example.org.",
+ "foo.example.org.",
+ "wild.bar.foo.example.org.",
+ "baz.foo.*.bar.example.org",
+ "baz.foo.baz.bar.example.org",
+ "*.foo.baz.bar.example.org",
+ "*.foo.*.bar.example.org",
+ "foo.*.bar.example.org",
+ "*.bar.example.org",
+ "bar.example.org",
+ NULL
+ };
+ for (const char** name(negative_names); *name != NULL; ++ name) {
+ doFindTest(finder, isc::dns::Name(*name), isc::dns::RRType::A(),
+ isc::dns::RRType::A(), isc::dns::RRTTL(3600),
+ ZoneFinder::NXRRSET, expected_rdatas_,
+ expected_sig_rdatas_);
+ }
}
TEST_F(DatabaseClientTest, getOrigin) {
diff --git a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
index 2526f98..427ee71 100644
--- a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
+++ b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
@@ -11,6 +11,10 @@
// 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.
+
+#include <algorithm>
+#include <vector>
+
#include <datasrc/sqlite3_accessor.h>
#include <datasrc/data_source.h>
@@ -19,8 +23,12 @@
#include <gtest/gtest.h>
#include <boost/scoped_ptr.hpp>
+#include <fstream>
+#include <sqlite3.h>
+using namespace std;
using namespace isc::datasrc;
+using boost::shared_ptr;
using isc::data::ConstElementPtr;
using isc::data::Element;
using isc::dns::RRClass;
@@ -43,80 +51,83 @@ std::string SQLITE_DBFILE_EXAMPLE_ORG = TEST_DATA_DIR "/example.org.sqlite3";
// The "nodir", a non existent directory, is inserted for this purpose.
std::string SQLITE_DBFILE_NOTEXIST = TEST_DATA_DIR "/nodir/notexist";
+// new db file, we don't need this to be a std::string, and given the
+// raw calls we use it in a const char* is more convenient
+const char* SQLITE_NEW_DBFILE = TEST_DATA_BUILDDIR "/newdb.sqlite3";
+
// Opening works (the content is tested in different tests)
TEST(SQLite3Open, common) {
- EXPECT_NO_THROW(SQLite3Database db(SQLITE_DBFILE_EXAMPLE,
- RRClass::IN()));
+ EXPECT_NO_THROW(SQLite3Accessor accessor(SQLITE_DBFILE_EXAMPLE,
+ RRClass::IN()));
}
// The file can't be opened
TEST(SQLite3Open, notExist) {
- EXPECT_THROW(SQLite3Database db(SQLITE_DBFILE_NOTEXIST,
- RRClass::IN()), SQLite3Error);
+ EXPECT_THROW(SQLite3Accessor accessor(SQLITE_DBFILE_NOTEXIST,
+ RRClass::IN()), SQLite3Error);
}
// It rejects broken DB
TEST(SQLite3Open, brokenDB) {
- EXPECT_THROW(SQLite3Database db(SQLITE_DBFILE_BROKENDB,
- RRClass::IN()), SQLite3Error);
+ EXPECT_THROW(SQLite3Accessor accessor(SQLITE_DBFILE_BROKENDB,
+ RRClass::IN()), SQLite3Error);
}
// Test we can create the schema on the fly
TEST(SQLite3Open, memoryDB) {
- EXPECT_NO_THROW(SQLite3Database db(SQLITE_DBFILE_MEMORY,
- RRClass::IN()));
+ EXPECT_NO_THROW(SQLite3Accessor accessor(SQLITE_DBFILE_MEMORY,
+ RRClass::IN()));
}
// Test fixture for querying the db
-class SQLite3Access : public ::testing::Test {
+class SQLite3AccessorTest : public ::testing::Test {
public:
- SQLite3Access() {
+ SQLite3AccessorTest() {
initAccessor(SQLITE_DBFILE_EXAMPLE, RRClass::IN());
}
// So it can be re-created with different data
void initAccessor(const std::string& filename, const RRClass& rrclass) {
- db.reset(new SQLite3Database(filename, rrclass));
+ accessor.reset(new SQLite3Accessor(filename, rrclass));
}
- // The tested db
- boost::shared_ptr<SQLite3Database> db;
+ // The tested accessor
+ boost::shared_ptr<SQLite3Accessor> accessor;
};
// This zone exists in the data, so it should be found
-TEST_F(SQLite3Access, getZone) {
- std::pair<bool, int> result(db->getZone(Name("example.com")));
+TEST_F(SQLite3AccessorTest, getZone) {
+ std::pair<bool, int> result(accessor->getZone("example.com."));
EXPECT_TRUE(result.first);
EXPECT_EQ(1, result.second);
}
// But it should find only the zone, nothing below it
-TEST_F(SQLite3Access, subZone) {
- EXPECT_FALSE(db->getZone(Name("sub.example.com")).first);
+TEST_F(SQLite3AccessorTest, subZone) {
+ EXPECT_FALSE(accessor->getZone("sub.example.com.").first);
}
// This zone is not there at all
-TEST_F(SQLite3Access, noZone) {
- EXPECT_FALSE(db->getZone(Name("example.org")).first);
+TEST_F(SQLite3AccessorTest, noZone) {
+ EXPECT_FALSE(accessor->getZone("example.org.").first);
}
// This zone is there, but in different class
-TEST_F(SQLite3Access, noClass) {
+TEST_F(SQLite3AccessorTest, noClass) {
initAccessor(SQLITE_DBFILE_EXAMPLE, RRClass::CH());
- EXPECT_FALSE(db->getZone(Name("example.com")).first);
+ EXPECT_FALSE(accessor->getZone("example.com.").first);
}
// This tests the iterator context
-TEST_F(SQLite3Access, iterator) {
+TEST_F(SQLite3AccessorTest, iterator) {
// Our test zone is conveniently small, but not empty
initAccessor(SQLITE_DBFILE_EXAMPLE_ORG, RRClass::IN());
- const std::pair<bool, int> zone_info(db->getZone(Name("example.org")));
+ const std::pair<bool, int> zone_info(accessor->getZone("example.org."));
ASSERT_TRUE(zone_info.first);
// Get the iterator context
DatabaseAccessor::IteratorContextPtr
- context(db->getAllRecords(zone_info.second));
- ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
- context);
+ context(accessor->getAllRecords(zone_info.second));
+ ASSERT_NE(DatabaseAccessor::IteratorContextPtr(), context);
std::string data[DatabaseAccessor::COLUMN_COUNT];
// Get and check the first and only record
@@ -196,13 +207,13 @@ TEST_F(SQLite3Access, iterator) {
}
TEST(SQLite3Open, getDBNameExample2) {
- SQLite3Database db(SQLITE_DBFILE_EXAMPLE2, RRClass::IN());
- EXPECT_EQ(SQLITE_DBNAME_EXAMPLE2, db.getDBName());
+ SQLite3Accessor accessor(SQLITE_DBFILE_EXAMPLE2, RRClass::IN());
+ EXPECT_EQ(SQLITE_DBNAME_EXAMPLE2, accessor.getDBName());
}
TEST(SQLite3Open, getDBNameExampleROOT) {
- SQLite3Database db(SQLITE_DBFILE_EXAMPLE_ROOT, RRClass::IN());
- EXPECT_EQ(SQLITE_DBNAME_EXAMPLE_ROOT, db.getDBName());
+ SQLite3Accessor accessor(SQLITE_DBFILE_EXAMPLE_ROOT, RRClass::IN());
+ EXPECT_EQ(SQLITE_DBNAME_EXAMPLE_ROOT, accessor.getDBName());
}
// Simple function to cound the number of records for
@@ -222,8 +233,8 @@ checkRecordRow(const std::string columns[],
EXPECT_EQ(field4, columns[DatabaseAccessor::NAME_COLUMN]);
}
-TEST_F(SQLite3Access, getRecords) {
- const std::pair<bool, int> zone_info(db->getZone(Name("example.com")));
+TEST_F(SQLite3AccessorTest, getRecords) {
+ const std::pair<bool, int> zone_info(accessor->getZone("example.com."));
ASSERT_TRUE(zone_info.first);
const int zone_id = zone_info.second;
@@ -232,14 +243,14 @@ TEST_F(SQLite3Access, getRecords) {
std::string columns[DatabaseAccessor::COLUMN_COUNT];
DatabaseAccessor::IteratorContextPtr
- context(db->getRecords("foo.bar", 1));
+ context(accessor->getRecords("foo.bar", 1));
ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
context);
EXPECT_FALSE(context->getNext(columns));
checkRecordRow(columns, "", "", "", "", "");
// now try some real searches
- context = db->getRecords("foo.example.com.", zone_id);
+ context = accessor->getRecords("foo.example.com.", zone_id);
ASSERT_TRUE(context->getNext(columns));
checkRecordRow(columns, "CNAME", "3600", "",
"cnametest.example.org.", "");
@@ -255,12 +266,13 @@ TEST_F(SQLite3Access, getRecords) {
"NSEC 5 3 7200 20100322084538 20100220084538 33495 "
"example.com. FAKEFAKEFAKEFAKE", "");
EXPECT_FALSE(context->getNext(columns));
+
// with no more records, the array should not have been modified
checkRecordRow(columns, "RRSIG", "7200", "NSEC",
"NSEC 5 3 7200 20100322084538 20100220084538 33495 "
"example.com. FAKEFAKEFAKEFAKE", "");
- context = db->getRecords("example.com.", zone_id);
+ context = accessor->getRecords("example.com.", zone_id);
ASSERT_TRUE(context->getNext(columns));
checkRecordRow(columns, "SOA", "3600", "",
"master.example.com. admin.example.com. "
@@ -330,13 +342,358 @@ TEST_F(SQLite3Access, getRecords) {
// Try searching for subdomain
// There's foo.bar.example.com in the data
- context = db->getRecords("bar.example.com.", zone_id, true);
+ context = accessor->getRecords("bar.example.com.", zone_id, true);
ASSERT_TRUE(context->getNext(columns));
checkRecordRow(columns, "A", "3600", "", "192.0.2.1", "");
EXPECT_FALSE(context->getNext(columns));
// But we shouldn't match mix.example.com here
- context = db->getRecords("ix.example.com.", zone_id, true);
+ context = accessor->getRecords("ix.example.com.", zone_id, true);
EXPECT_FALSE(context->getNext(columns));
}
+// Test fixture for creating a db that automatically deletes it before start,
+// and when done
+class SQLite3Create : public ::testing::Test {
+public:
+ SQLite3Create() {
+ remove(SQLITE_NEW_DBFILE);
+ }
+
+ ~SQLite3Create() {
+ remove(SQLITE_NEW_DBFILE);
+ }
+};
+
+bool isReadable(const char* filename) {
+ return (std::ifstream(filename).is_open());
+}
+
+TEST_F(SQLite3Create, creationtest) {
+ ASSERT_FALSE(isReadable(SQLITE_NEW_DBFILE));
+ // Should simply be created
+ SQLite3Accessor accessor(SQLITE_NEW_DBFILE, RRClass::IN());
+ ASSERT_TRUE(isReadable(SQLITE_NEW_DBFILE));
+}
+
+TEST_F(SQLite3Create, emptytest) {
+ ASSERT_FALSE(isReadable(SQLITE_NEW_DBFILE));
+
+ // open one manualle
+ sqlite3* db;
+ ASSERT_EQ(SQLITE_OK, sqlite3_open(SQLITE_NEW_DBFILE, &db));
+
+ // empty, but not locked, so creating it now should work
+ SQLite3Accessor accessor2(SQLITE_NEW_DBFILE, RRClass::IN());
+
+ sqlite3_close(db);
+
+ // should work now that we closed it
+ SQLite3Accessor accessor3(SQLITE_NEW_DBFILE, RRClass::IN());
+}
+
+TEST_F(SQLite3Create, lockedtest) {
+ ASSERT_FALSE(isReadable(SQLITE_NEW_DBFILE));
+
+ // open one manually
+ sqlite3* db;
+ ASSERT_EQ(SQLITE_OK, sqlite3_open(SQLITE_NEW_DBFILE, &db));
+ sqlite3_exec(db, "BEGIN EXCLUSIVE TRANSACTION", NULL, NULL, NULL);
+
+ // should not be able to open it
+ EXPECT_THROW(SQLite3Accessor accessor2(SQLITE_NEW_DBFILE, RRClass::IN()),
+ SQLite3Error);
+
+ sqlite3_exec(db, "ROLLBACK TRANSACTION", NULL, NULL, NULL);
+
+ // should work now that we closed it
+ SQLite3Accessor accessor3(SQLITE_NEW_DBFILE, RRClass::IN());
+}
+
+//
+// Commonly used data for update tests
+//
+const char* const common_expected_data[] = {
+ // Test record already stored in the tested sqlite3 DB file.
+ "foo.bar.example.com.", "com.example.bar.foo.", "3600", "A", "",
+ "192.0.2.1"
+};
+const char* const new_data[] = {
+ // Newly added data commonly used by some of the tests below
+ "newdata.example.com.", "com.example.newdata.", "3600", "A", "",
+ "192.0.2.1"
+};
+const char* const deleted_data[] = {
+ // Existing data to be removed commonly used by some of the tests below
+ "foo.bar.example.com.", "A", "192.0.2.1"
+};
+
+class SQLite3Update : public SQLite3AccessorTest {
+protected:
+ SQLite3Update() {
+ // Note: if "installing" the test file fails some of the subsequent
+ // tests will fail and we should be able to notice that.
+ system(INSTALL_PROG " " TEST_DATA_DIR
+ "/test.sqlite3 " TEST_DATA_BUILDDIR "/test.sqlite3.copied");
+ initAccessor(TEST_DATA_BUILDDIR "/test.sqlite3.copied", RRClass::IN());
+ zone_id = accessor->getZone("example.com.").second;
+ another_accessor.reset(new SQLite3Accessor(
+ TEST_DATA_BUILDDIR "/test.sqlite3.copied",
+ RRClass::IN()));
+ expected_stored.push_back(common_expected_data);
+ }
+
+ int zone_id;
+ std::string get_columns[DatabaseAccessor::COLUMN_COUNT];
+ std::string add_columns[DatabaseAccessor::ADD_COLUMN_COUNT];
+ std::string del_params[DatabaseAccessor::DEL_PARAM_COUNT];
+
+ vector<const char* const*> expected_stored; // placeholder for checkRecords
+ vector<const char* const*> empty_stored; // indicate no corresponding data
+
+ // Another accessor, emulating one running on a different process/thread
+ shared_ptr<SQLite3Accessor> another_accessor;
+ DatabaseAccessor::IteratorContextPtr iterator;
+};
+
+void
+checkRecords(SQLite3Accessor& accessor, int zone_id, const std::string& name,
+ vector<const char* const*> expected_rows)
+{
+ DatabaseAccessor::IteratorContextPtr iterator =
+ accessor.getRecords(name, zone_id);
+ std::string columns[DatabaseAccessor::COLUMN_COUNT];
+ vector<const char* const*>::const_iterator it = expected_rows.begin();
+ while (iterator->getNext(columns)) {
+ ASSERT_TRUE(it != expected_rows.end());
+ checkRecordRow(columns, (*it)[3], (*it)[2], (*it)[4], (*it)[5], "");
+ ++it;
+ }
+ EXPECT_TRUE(it == expected_rows.end());
+}
+
+TEST_F(SQLite3Update, emptyUpdate) {
+ // If we do nothing between start and commit, the zone content
+ // should be intact.
+
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+ zone_id = accessor->startUpdateZone("example.com.", false).second;
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+ accessor->commitUpdateZone();
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+}
+
+TEST_F(SQLite3Update, flushZone) {
+ // With 'replace' being true startUpdateZone() will flush the existing
+ // zone content.
+
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+ zone_id = accessor->startUpdateZone("example.com.", true).second;
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
+ accessor->commitUpdateZone();
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
+}
+
+TEST_F(SQLite3Update, readWhileUpdate) {
+ zone_id = accessor->startUpdateZone("example.com.", true).second;
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
+
+ // Until commit is done, the other accessor should see the old data
+ checkRecords(*another_accessor, zone_id, "foo.bar.example.com.",
+ expected_stored);
+
+ // Once the changes are committed, the other accessor will see the new
+ // data.
+ accessor->commitUpdateZone();
+ checkRecords(*another_accessor, zone_id, "foo.bar.example.com.",
+ empty_stored);
+}
+
+TEST_F(SQLite3Update, rollback) {
+ zone_id = accessor->startUpdateZone("example.com.", true).second;
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
+
+ // Rollback will revert the change made by startUpdateZone(, true).
+ accessor->rollbackUpdateZone();
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+}
+
+TEST_F(SQLite3Update, rollbackFailure) {
+ // This test emulates a rare scenario of making rollback attempt fail.
+ // The iterator is paused in the middle of getting records, which prevents
+ // the rollback operation at the end of the test.
+
+ string columns[DatabaseAccessor::COLUMN_COUNT];
+ iterator = accessor->getRecords("example.com.", zone_id);
+ EXPECT_TRUE(iterator->getNext(columns));
+
+ accessor->startUpdateZone("example.com.", true);
+ EXPECT_THROW(accessor->rollbackUpdateZone(), DataSourceError);
+}
+
+TEST_F(SQLite3Update, commitConflict) {
+ // Start reading the DB by another accessor. We should stop at a single
+ // call to getNextRecord() to keep holding the lock.
+ iterator = another_accessor->getRecords("foo.example.com.", zone_id);
+ EXPECT_TRUE(iterator->getNext(get_columns));
+
+ // Due to getNextRecord() above, the other accessor holds a DB lock,
+ // which will prevent commit.
+ zone_id = accessor->startUpdateZone("example.com.", true).second;
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
+ EXPECT_THROW(accessor->commitUpdateZone(), DataSourceError);
+ accessor->rollbackUpdateZone(); // rollback should still succeed
+
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+}
+
+TEST_F(SQLite3Update, updateConflict) {
+ // Similar to the previous case, but this is a conflict with another
+ // update attempt. Note that these two accessors modify disjoint sets
+ // of data; sqlite3 only has a coarse-grained lock so we cannot allow
+ // these updates to run concurrently.
+ EXPECT_TRUE(another_accessor->startUpdateZone("sql1.example.com.",
+ true).first);
+ EXPECT_THROW(accessor->startUpdateZone("example.com.", true),
+ DataSourceError);
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+
+ // Once we rollback the other attempt of change, we should be able to
+ // start and commit the transaction using the main accessor.
+ another_accessor->rollbackUpdateZone();
+ accessor->startUpdateZone("example.com.", true);
+ accessor->commitUpdateZone();
+}
+
+TEST_F(SQLite3Update, duplicateUpdate) {
+ accessor->startUpdateZone("example.com.", false);
+ EXPECT_THROW(accessor->startUpdateZone("example.com.", false),
+ DataSourceError);
+}
+
+TEST_F(SQLite3Update, commitWithoutTransaction) {
+ EXPECT_THROW(accessor->commitUpdateZone(), DataSourceError);
+}
+
+TEST_F(SQLite3Update, rollbackWithoutTransaction) {
+ EXPECT_THROW(accessor->rollbackUpdateZone(), DataSourceError);
+}
+
+TEST_F(SQLite3Update, addRecord) {
+ // Before update, there should be no record for this name
+ checkRecords(*accessor, zone_id, "newdata.example.com.", empty_stored);
+
+ zone_id = accessor->startUpdateZone("example.com.", false).second;
+ copy(new_data, new_data + DatabaseAccessor::ADD_COLUMN_COUNT,
+ add_columns);
+ accessor->addRecordToZone(add_columns);
+
+ expected_stored.clear();
+ expected_stored.push_back(new_data);
+ checkRecords(*accessor, zone_id, "newdata.example.com.", expected_stored);
+
+ // Commit the change, and confirm the new data is still there.
+ accessor->commitUpdateZone();
+ checkRecords(*accessor, zone_id, "newdata.example.com.", expected_stored);
+}
+
+TEST_F(SQLite3Update, addThenRollback) {
+ zone_id = accessor->startUpdateZone("example.com.", false).second;
+ copy(new_data, new_data + DatabaseAccessor::ADD_COLUMN_COUNT,
+ add_columns);
+ accessor->addRecordToZone(add_columns);
+
+ expected_stored.clear();
+ expected_stored.push_back(new_data);
+ checkRecords(*accessor, zone_id, "newdata.example.com.", expected_stored);
+
+ accessor->rollbackUpdateZone();
+ checkRecords(*accessor, zone_id, "newdata.example.com.", empty_stored);
+}
+
+TEST_F(SQLite3Update, duplicateAdd) {
+ const char* const dup_data[] = {
+ "foo.bar.example.com.", "com.example.bar.foo.", "3600", "A", "",
+ "192.0.2.1"
+ };
+ expected_stored.clear();
+ expected_stored.push_back(dup_data);
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+
+ // Adding exactly the same data. As this backend is "dumb", another
+ // row of the same content will be inserted.
+ copy(dup_data, dup_data + DatabaseAccessor::ADD_COLUMN_COUNT,
+ add_columns);
+ zone_id = accessor->startUpdateZone("example.com.", false).second;
+ accessor->addRecordToZone(add_columns);
+ expected_stored.push_back(dup_data);
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+}
+
+TEST_F(SQLite3Update, invalidAdd) {
+ // An attempt of add before an explicit start of transaction
+ EXPECT_THROW(accessor->addRecordToZone(add_columns), DataSourceError);
+}
+
+TEST_F(SQLite3Update, deleteRecord) {
+ zone_id = accessor->startUpdateZone("example.com.", false).second;
+
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+
+ copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+ del_params);
+ accessor->deleteRecordInZone(del_params);
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
+
+ // Commit the change, and confirm the deleted data still isn't there.
+ accessor->commitUpdateZone();
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
+}
+
+TEST_F(SQLite3Update, deleteThenRollback) {
+ zone_id = accessor->startUpdateZone("example.com.", false).second;
+
+ copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+ del_params);
+ accessor->deleteRecordInZone(del_params);
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
+
+ // Rollback the change, and confirm the data still exists.
+ accessor->rollbackUpdateZone();
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+}
+
+TEST_F(SQLite3Update, deleteNonexistent) {
+ zone_id = accessor->startUpdateZone("example.com.", false).second;
+ copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+ del_params);
+
+ // Replace the name with a non existent one, then try to delete it.
+ // nothing should happen.
+ del_params[DatabaseAccessor::DEL_NAME] = "no-such-name.example.com.";
+ checkRecords(*accessor, zone_id, "no-such-name.example.com.",
+ empty_stored);
+ accessor->deleteRecordInZone(del_params);
+ checkRecords(*accessor, zone_id, "no-such-name.example.com.",
+ empty_stored);
+
+ // Name exists but the RR type is different. Delete attempt shouldn't
+ // delete only by name.
+ copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+ del_params);
+ del_params[DatabaseAccessor::DEL_TYPE] = "AAAA";
+ accessor->deleteRecordInZone(del_params);
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+
+ // Similar to the previous case, but RDATA is different.
+ copy(deleted_data, deleted_data + DatabaseAccessor::DEL_PARAM_COUNT,
+ del_params);
+ del_params[DatabaseAccessor::DEL_RDATA] = "192.0.2.2";
+ accessor->deleteRecordInZone(del_params);
+ checkRecords(*accessor, zone_id, "foo.bar.example.com.", expected_stored);
+}
+
+TEST_F(SQLite3Update, invalidDelete) {
+ // An attempt of delete before an explicit start of transaction
+ EXPECT_THROW(accessor->deleteRecordInZone(del_params), DataSourceError);
+}
} // end anonymous namespace
diff --git a/src/lib/datasrc/tests/testdata/Makefile.am b/src/lib/datasrc/tests/testdata/Makefile.am
new file mode 100644
index 0000000..6a35fe3
--- /dev/null
+++ b/src/lib/datasrc/tests/testdata/Makefile.am
@@ -0,0 +1 @@
+CLEANFILES = *.copied
diff --git a/src/lib/python/isc/datasrc/sqlite3_ds.py b/src/lib/python/isc/datasrc/sqlite3_ds.py
index a77645a..fd63741 100644
--- a/src/lib/python/isc/datasrc/sqlite3_ds.py
+++ b/src/lib/python/isc/datasrc/sqlite3_ds.py
@@ -33,44 +33,63 @@ def create(cur):
Arguments:
cur - sqlite3 cursor.
"""
- cur.execute("CREATE TABLE schema_version (version INTEGER NOT NULL)")
- cur.execute("INSERT INTO schema_version VALUES (1)")
- cur.execute("""CREATE TABLE zones (id INTEGER PRIMARY KEY,
- name STRING NOT NULL COLLATE NOCASE,
- rdclass STRING NOT NULL COLLATE NOCASE DEFAULT 'IN',
- dnssec BOOLEAN NOT NULL DEFAULT 0)""")
- cur.execute("CREATE INDEX zones_byname ON zones (name)")
- cur.execute("""CREATE TABLE records (id INTEGER PRIMARY KEY,
- zone_id INTEGER NOT NULL,
- name STRING NOT NULL COLLATE NOCASE,
- rname STRING NOT NULL COLLATE NOCASE,
- ttl INTEGER NOT NULL,
- rdtype STRING NOT NULL COLLATE NOCASE,
- sigtype STRING COLLATE NOCASE,
- rdata STRING NOT NULL)""")
- cur.execute("CREATE INDEX records_byname ON records (name)")
- cur.execute("CREATE INDEX records_byrname ON records (rname)")
- cur.execute("""CREATE TABLE nsec3 (id INTEGER PRIMARY KEY,
- zone_id INTEGER NOT NULL,
- hash STRING NOT NULL COLLATE NOCASE,
- owner STRING NOT NULL COLLATE NOCASE,
- ttl INTEGER NOT NULL,
- rdtype STRING NOT NULL COLLATE NOCASE,
- rdata STRING NOT NULL)""")
- cur.execute("CREATE INDEX nsec3_byhash ON nsec3 (hash)")
-
-def open(dbfile):
+ # We are creating the database because it apparently had not been at
+ # the time we tried to read from it. However, another process may have
+ # had the same idea, resulting in a potential race condition.
+ # Therefore, we obtain an exclusive lock before we create anything
+ # When we have it, we check *again* whether the database has been
+ # initialized. If not, we do so.
+
+ # If the database is perpetually locked, it'll time out automatically
+ # and we just let it fail.
+ cur.execute("BEGIN EXCLUSIVE TRANSACTION")
+ try:
+ cur.execute("SELECT version FROM schema_version")
+ row = cur.fetchone()
+ except sqlite3.OperationalError:
+ cur.execute("CREATE TABLE schema_version (version INTEGER NOT NULL)")
+ cur.execute("INSERT INTO schema_version VALUES (1)")
+ cur.execute("""CREATE TABLE zones (id INTEGER PRIMARY KEY,
+ name STRING NOT NULL COLLATE NOCASE,
+ rdclass STRING NOT NULL COLLATE NOCASE DEFAULT 'IN',
+ dnssec BOOLEAN NOT NULL DEFAULT 0)""")
+ cur.execute("CREATE INDEX zones_byname ON zones (name)")
+ cur.execute("""CREATE TABLE records (id INTEGER PRIMARY KEY,
+ zone_id INTEGER NOT NULL,
+ name STRING NOT NULL COLLATE NOCASE,
+ rname STRING NOT NULL COLLATE NOCASE,
+ ttl INTEGER NOT NULL,
+ rdtype STRING NOT NULL COLLATE NOCASE,
+ sigtype STRING COLLATE NOCASE,
+ rdata STRING NOT NULL)""")
+ cur.execute("CREATE INDEX records_byname ON records (name)")
+ cur.execute("CREATE INDEX records_byrname ON records (rname)")
+ cur.execute("""CREATE TABLE nsec3 (id INTEGER PRIMARY KEY,
+ zone_id INTEGER NOT NULL,
+ hash STRING NOT NULL COLLATE NOCASE,
+ owner STRING NOT NULL COLLATE NOCASE,
+ ttl INTEGER NOT NULL,
+ rdtype STRING NOT NULL COLLATE NOCASE,
+ rdata STRING NOT NULL)""")
+ cur.execute("CREATE INDEX nsec3_byhash ON nsec3 (hash)")
+ row = [1]
+ cur.execute("COMMIT TRANSACTION")
+ return row
+
+def open(dbfile, connect_timeout=5.0):
""" Open a database, if the database is not yet set up, call create
to do so. It may raise Sqlite3DSError if failed to open sqlite3
database file or find bad database schema version in the database.
Arguments:
dbfile - the filename for the sqlite3 database.
+ connect_timeout - timeout for opening the database or acquiring locks
+ defaults to sqlite3 module's default of 5.0 seconds
Return sqlite3 connection, sqlite3 cursor.
"""
try:
- conn = sqlite3.connect(dbfile)
+ conn = sqlite3.connect(dbfile, timeout=connect_timeout)
cur = conn.cursor()
except Exception as e:
fail = "Failed to open " + dbfile + ": " + e.args[0]
@@ -80,10 +99,13 @@ def open(dbfile):
try:
cur.execute("SELECT version FROM schema_version")
row = cur.fetchone()
- except:
- create(cur)
- conn.commit()
- row = [1]
+ except sqlite3.OperationalError:
+ # temporarily disable automatic transactions so
+ # we can do our own
+ iso_lvl = conn.isolation_level
+ conn.isolation_level = None
+ row = create(cur)
+ conn.isolation_level = iso_lvl
if row == None or row[0] != 1:
raise Sqlite3DSError("Bad database schema version")
diff --git a/src/lib/python/isc/datasrc/tests/sqlite3_ds_test.py b/src/lib/python/isc/datasrc/tests/sqlite3_ds_test.py
index 707994f..10c61cf 100644
--- a/src/lib/python/isc/datasrc/tests/sqlite3_ds_test.py
+++ b/src/lib/python/isc/datasrc/tests/sqlite3_ds_test.py
@@ -23,8 +23,9 @@ 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"
-WRITE_ZONE_DB_FILE = TESTDATA_WRITE_PATH + "example.com.out.sqlite3"
BROKEN_DB_FILE = TESTDATA_PATH + "brokendb.sqlite3"
+WRITE_ZONE_DB_FILE = TESTDATA_WRITE_PATH + "example.com.out.sqlite3"
+NEW_DB_FILE = TESTDATA_WRITE_PATH + "new_db.sqlite3"
def example_reader():
my_zone = [
@@ -91,5 +92,52 @@ class TestSqlite3_ds(unittest.TestCase):
# and make sure lock does not stay
sqlite3_ds.load(WRITE_ZONE_DB_FILE, ".", example_reader)
+class NewDBFile(unittest.TestCase):
+ def tearDown(self):
+ # remove the created database after every test
+ if (os.path.exists(NEW_DB_FILE)):
+ os.remove(NEW_DB_FILE)
+
+ def setUp(self):
+ # remove the created database before every test too, just
+ # in case a test got aborted half-way, and cleanup didn't occur
+ if (os.path.exists(NEW_DB_FILE)):
+ os.remove(NEW_DB_FILE)
+
+ def test_new_db(self):
+ self.assertFalse(os.path.exists(NEW_DB_FILE))
+ sqlite3_ds.open(NEW_DB_FILE)
+ self.assertTrue(os.path.exists(NEW_DB_FILE))
+
+ def test_new_db_locked(self):
+ self.assertFalse(os.path.exists(NEW_DB_FILE))
+ con = sqlite3.connect(NEW_DB_FILE);
+ con.isolation_level = None
+ cur = con.cursor()
+ cur.execute("BEGIN IMMEDIATE TRANSACTION")
+
+ # load should now fail, since the database is locked,
+ # and the open() call needs an exclusive lock
+ self.assertRaises(sqlite3.OperationalError,
+ sqlite3_ds.open, NEW_DB_FILE, 0.1)
+
+ con.rollback()
+ cur.close()
+ con.close()
+ self.assertTrue(os.path.exists(NEW_DB_FILE))
+
+ # now that we closed our connection, load should work again
+ sqlite3_ds.open(NEW_DB_FILE)
+
+ # the database should now have been created, and a new load should
+ # not require an exclusive lock anymore, so we lock it again
+ con = sqlite3.connect(NEW_DB_FILE);
+ cur = con.cursor()
+ cur.execute("BEGIN IMMEDIATE TRANSACTION")
+ sqlite3_ds.open(NEW_DB_FILE, 0.1)
+ con.rollback()
+ cur.close()
+ con.close()
+
if __name__ == '__main__':
unittest.main()
diff --git a/src/lib/python/isc/log/tests/Makefile.am b/src/lib/python/isc/log/tests/Makefile.am
index 6bb67de..a23887c 100644
--- a/src/lib/python/isc/log/tests/Makefile.am
+++ b/src/lib/python/isc/log/tests/Makefile.am
@@ -1,6 +1,8 @@
PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
-PYTESTS = log_test.py
-EXTRA_DIST = $(PYTESTS) log_console.py.in console.out check_output.sh
+PYTESTS_GEN = log_console.py
+PYTESTS_NOGEN = log_test.py
+noinst_SCRIPTS = $(PYTESTS_GEN)
+EXTRA_DIST = console.out check_output.sh $(PYTESTS_NOGEN)
# If necessary (rare cases), explicitly specify paths to dynamic libraries
# required by loadable python modules.
@@ -10,7 +12,9 @@ LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cc/.
endif
# test using command-line arguments, so use check-local target instead of TESTS
+# We need to run the cycle twice, because once the files are in builddir, once in srcdir
check-local:
+ chmod +x $(abs_builddir)/log_console.py
$(LIBRARY_PATH_PLACEHOLDER) \
env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/python/isc/log \
$(abs_srcdir)/check_output.sh $(abs_builddir)/log_console.py $(abs_srcdir)/console.out
@@ -19,10 +23,18 @@ if ENABLE_PYTHON_COVERAGE
rm -f .coverage
${LN_S} $(abs_top_srcdir)/.coverage .coverage
endif
- for pytest in $(PYTESTS) ; do \
+ for pytest in $(PYTESTS_NOGEN) ; do \
echo Running test: $$pytest ; \
$(LIBRARY_PATH_PLACEHOLDER) \
env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/python/isc/log:$(abs_top_builddir)/src/lib/log/python/.libs \
B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \
$(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \
+ done ; \
+ for pytest in $(PYTESTS_GEN) ; do \
+ echo Running test: $$pytest ; \
+ chmod +x $(abs_builddir)/$$pytest ; \
+ $(LIBRARY_PATH_PLACEHOLDER) \
+ env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/python/isc/log:$(abs_top_builddir)/src/lib/log/python/.libs \
+ B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \
+ $(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
done
More information about the bind10-changes
mailing list