BIND 10 trac1062, updated. 8ce8e05a403440e7f2323e9d43dca08be1cf8a94 [1062] more review updates (comment:14)
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Aug 11 20:52:53 UTC 2011
The branch, trac1062 has been updated
via 8ce8e05a403440e7f2323e9d43dca08be1cf8a94 (commit)
from 414b25d4bfa89e0609cd3c8c3a6e610681f4c929 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 8ce8e05a403440e7f2323e9d43dca08be1cf8a94
Author: Jelte Jansen <jelte at isc.org>
Date: Thu Aug 11 22:52:21 2011 +0200
[1062] more review updates (comment:14)
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 21 +++++++---
src/lib/datasrc/database.h | 18 +++++++-
src/lib/datasrc/datasrc_messages.mes | 2 +-
src/lib/datasrc/sqlite3_connection.cc | 5 +-
src/lib/datasrc/tests/database_unittest.cc | 43 +++++++++++--------
.../datasrc/tests/sqlite3_connection_unittest.cc | 2 +-
6 files changed, 60 insertions(+), 31 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 7bbf285..2494cad 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -83,12 +83,18 @@ namespace {
// Raises a DataSourceError if the type does not
// match, or if the given rdata string does not
// parse correctly for the given type and class
+//
+// The DatabaseConnection is passed to print the
+// database name in the log message if the TTL is
+// modified
void addOrCreate(isc::dns::RRsetPtr& rrset,
const isc::dns::Name& name,
const isc::dns::RRClass& cls,
const isc::dns::RRType& type,
const isc::dns::RRTTL& ttl,
- const std::string& rdata_str)
+ const std::string& rdata_str,
+ const DatabaseConnection& conn
+ )
{
if (!rrset) {
rrset.reset(new isc::dns::RRset(name, cls, type, ttl));
@@ -100,7 +106,8 @@ void addOrCreate(isc::dns::RRsetPtr& rrset,
rrset->setTTL(ttl);
}
logger.info(DATASRC_DATABASE_FIND_TTL_MISMATCH)
- .arg(name).arg(cls).arg(type).arg(rrset->getTTL());
+ .arg(conn.getDBName()).arg(name).arg(cls)
+ .arg(type).arg(rrset->getTTL());
}
}
try {
@@ -174,9 +181,9 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
try {
connection_->searchForRecords(zone_id_, name.toText());
- std::string columns[DatabaseConnection::RECORDCOLUMNCOUNT];
+ std::string columns[DatabaseConnection::COLUMN_COUNT];
while (connection_->getNextRecord(columns,
- DatabaseConnection::RECORDCOLUMNCOUNT)) {
+ DatabaseConnection::COLUMN_COUNT)) {
if (!records_found) {
records_found = true;
}
@@ -203,7 +210,8 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
"the only record for " + name.toText());
}
addOrCreate(result_rrset, name, getClass(), cur_type,
- cur_ttl, columns[DatabaseConnection::RDATA_COLUMN]);
+ cur_ttl, columns[DatabaseConnection::RDATA_COLUMN],
+ *connection_);
} else if (cur_type == isc::dns::RRType::CNAME()) {
// There should be no other data, so result_rrset should
// be empty.
@@ -212,7 +220,8 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
"the only record for " + name.toText());
}
addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl,
- columns[DatabaseConnection::RDATA_COLUMN]);
+ columns[DatabaseConnection::RDATA_COLUMN],
+ *connection_);
result_status = CNAME;
} else if (cur_type == isc::dns::RRType::RRSIG()) {
// If we get signatures before we get the actual data, we
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index fc4057a..05b966c 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -93,7 +93,7 @@ public:
* In the case of a database error, a DatasourceError is thrown.
*
* The columns passed is an array of std::strings consisting of
- * DatabaseConnection::RECORDCOLUMNCOUNT elements, the elements of which
+ * DatabaseConnection::COLUMN_COUNT elements, the elements of which
* are defined in DatabaseConnection::RecordColumns, in their basic
* string representation.
*
@@ -146,7 +146,7 @@ public:
};
/// The number of fields the columns array passed to getNextRecord should have
- static const size_t RECORDCOLUMNCOUNT = 4;
+ static const size_t COLUMN_COUNT = 4;
/**
* \brief Returns a string identifying this dabase backend
@@ -237,6 +237,20 @@ public:
* different. It is left in for compatibility at the moment.
* \note options are ignored at this moment
*
+ * \note Maybe counter intuitively, this method is not a const member
+ * function. This is intentional; some of the underlying implementations
+ * are expected to use a database backend, and would internally contain
+ * some abstraction of "database connection". In the most strict sense
+ * any (even read only) operation might change the internal state of
+ * such a connection, and in that sense the operation cannot be considered
+ * "const". In order to avoid giving a false sense of safety to the
+ * caller, we indicate a call to this method may have a surprising
+ * side effect. That said, this view may be too strict and it may
+ * make sense to say the internal database connection doesn't affect
+ * external behavior in terms of the interface of this method. As
+ * we gain more experiences with various kinds of backends we may
+ * revisit the constness.
+ *
* \exception DataSourceError when there is a problem reading
* the data from the dabase backend.
* This can be a connection, code, or
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index 5a63b0d..ce2545f 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -73,7 +73,7 @@ The error message contains specific information about the error.
Debug information. The database data source is looking up records with the given
name and type in the database.
-% DATASRC_DATABASE_FIND_TTL_MISMATCH TTL values differ for elements of %1/%2/%3, setting to %4
+% DATASRC_DATABASE_FIND_TTL_MISMATCH TTL values differ in %1 for elements of %2/%3/%4, setting to %5
The datasource backend provided resource records for the given RRset with
different TTL values. The TTL of the RRSET is set to the lowest value, which
is printed in the log message.
diff --git a/src/lib/datasrc/sqlite3_connection.cc b/src/lib/datasrc/sqlite3_connection.cc
index 4a90b71..b996bd5 100644
--- a/src/lib/datasrc/sqlite3_connection.cc
+++ b/src/lib/datasrc/sqlite3_connection.cc
@@ -370,11 +370,10 @@ convertToPlainChar(const unsigned char* ucp,
bool
SQLite3Connection::getNextRecord(std::string columns[], size_t column_count) {
- if (column_count != RECORDCOLUMNCOUNT) {
+ if (column_count != COLUMN_COUNT) {
isc_throw(DataSourceError,
"Datasource backend caller did not pass a column array "
- "of size " << RECORDCOLUMNCOUNT <<
- " to getNextRecord()");
+ "of size " << COLUMN_COUNT << " to getNextRecord()");
}
sqlite3_stmt* current_stmt = dbparameters_->q_any_;
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index d0ab1a0..c2eee03 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -92,7 +92,7 @@ public:
throw std::exception();
}
- if (column_count != DatabaseConnection::RECORDCOLUMNCOUNT) {
+ if (column_count != DatabaseConnection::COLUMN_COUNT) {
isc_throw(DataSourceError, "Wrong column count in getNextRecord");
}
if (cur_record < cur_name.size()) {
@@ -324,6 +324,25 @@ TEST_F(DatabaseClientTest, noConnException) {
}
namespace {
+// checks if the given rrset matches the
+// given name, class, type and rdatas
+void
+checkRRset(isc::dns::ConstRRsetPtr rrset,
+ const isc::dns::Name& name,
+ const isc::dns::RRClass& rrclass,
+ const isc::dns::RRType& rrtype,
+ const isc::dns::RRTTL& rrttl,
+ const std::vector<std::string> rdatas) {
+ isc::dns::RRsetPtr expected_rrset(
+ new isc::dns::RRset(name, rrclass, rrtype, rrttl));
+ for (unsigned int i = 0; i < rdatas.size(); ++i) {
+ expected_rrset->addRdata(
+ isc::dns::rdata::createRdata(rrtype, rrclass,
+ rdatas[i]));
+ }
+ isc::testutils::rrsetCheck(expected_rrset, rrset);
+}
+
void
doFindTest(shared_ptr<DatabaseClient::Finder> finder,
const isc::dns::Name& name,
@@ -338,25 +357,13 @@ doFindTest(shared_ptr<DatabaseClient::Finder> finder,
finder->find(name, type, NULL, ZoneFinder::FIND_DEFAULT);
ASSERT_EQ(expected_result, result.code) << name << " " << type;
if (expected_rdatas.size() > 0) {
- EXPECT_EQ(expected_rdatas.size(), result.rrset->getRdataCount());
- EXPECT_EQ(expected_ttl, result.rrset->getTTL());
- EXPECT_EQ(expected_type, result.rrset->getType());
-
- isc::dns::RRsetPtr expected_rrset(
- new isc::dns::RRset(name, finder->getClass(),
- expected_type, expected_ttl));
- for (unsigned int i = 0; i < expected_rdatas.size(); ++i) {
- expected_rrset->addRdata(
- isc::dns::rdata::createRdata(expected_type,
- finder->getClass(),
- expected_rdatas[i]));
- }
- isc::testutils::rrsetCheck(expected_rrset, result.rrset);
+ checkRRset(result.rrset, name, finder->getClass(),
+ expected_type, expected_ttl, expected_rdatas);
if (expected_sig_rdatas.size() > 0) {
- // TODO same for sigrrset
- EXPECT_EQ(expected_sig_rdatas.size(),
- result.rrset->getRRsig()->getRdataCount());
+ checkRRset(result.rrset->getRRsig(), name,
+ finder->getClass(), isc::dns::RRType::RRSIG(),
+ expected_ttl, expected_sig_rdatas);
} else {
EXPECT_EQ(isc::dns::RRsetPtr(), result.rrset->getRRsig());
}
diff --git a/src/lib/datasrc/tests/sqlite3_connection_unittest.cc b/src/lib/datasrc/tests/sqlite3_connection_unittest.cc
index e864e25..6423a96 100644
--- a/src/lib/datasrc/tests/sqlite3_connection_unittest.cc
+++ b/src/lib/datasrc/tests/sqlite3_connection_unittest.cc
@@ -134,7 +134,7 @@ TEST_F(SQLite3Conn, getRecords) {
const int zone_id = zone_info.second;
ASSERT_EQ(1, zone_id);
- const size_t column_count = DatabaseConnection::RECORDCOLUMNCOUNT;
+ const size_t column_count = DatabaseConnection::COLUMN_COUNT;
std::string columns[column_count];
// without search, getNext() should return false
More information about the bind10-changes
mailing list