BIND 10 trac1791, updated. d12134b3a9f135f9ae4317587eef31f45e6c0454 [1791] fixed one remaining regression: 'separate_rrs' case was broken.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Apr 10 22:01:21 UTC 2012
The branch, trac1791 has been updated
via d12134b3a9f135f9ae4317587eef31f45e6c0454 (commit)
from 99adadb0496046011060ec22e6834dfede9f1ce3 (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 d12134b3a9f135f9ae4317587eef31f45e6c0454
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Apr 10 14:57:38 2012 -0700
[1791] fixed one remaining regression: 'separate_rrs' case was broken.
added a test for that case to confirm the regression and the fix.
also introduced some more cleanups: removed rdata_txt_ so we simplify the
code further, made isSameType static class function (it doesn't refer to
any class attributes), constified separate_rrs_.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 39 ++++----
src/lib/datasrc/tests/database_unittest.cc | 153 ++++++++++++++++-----------
2 files changed, 109 insertions(+), 83 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 107b656..160d1da 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -1011,12 +1011,8 @@ public:
const RRType rtype(rtype_txt_);
RRsetPtr rrset(new RRset(Name(name_txt_), class_, rtype,
RRTTL(ttl_txt_)));
- // For the first RR, rdata_ is null, so we need to create it here.
- // After that, rdata_ will be updated in the while loop below.
- if (!rdata_) {
- rdata_ = rdata::createRdata(rtype, class_, rdata_txt_);
- }
- const ConstRdataPtr rdata_base = rdata_; // remember it for comparison
+ // Remember the first RDATA of the RRset for comparison:
+ const ConstRdataPtr rdata_base = rdata_;
while (true) {
// Extend the RRset with the new RDATA.
rrset->addRdata(rdata_);
@@ -1030,13 +1026,11 @@ public:
}
// Check if the next record belongs to the same RRset. If not,
- // we are done. The next RDATA is stored in rdata_, which is used
- // within this loop (if it belongs to the same RRset) or in the
- // next call.
- const RRType next_rtype(rtype_txt_);
- rdata_ = rdata::createRdata(next_rtype, class_, rdata_txt_);
+ // we are done. The next RDATA has been stored in rdata_, which
+ // is used within this loop (if it belongs to the same RRset) or
+ // in the next call.
if (Name(name_txt_) != rrset->getName() ||
- !isSameType(rtype, rdata_base, next_rtype, rdata_)) {
+ !isSameType(rtype, rdata_base, RRType(rtype_txt_), rdata_)) {
break;
}
@@ -1059,8 +1053,8 @@ private:
// Check two RDATA types are equivalent. Basically it's a trivial
// comparison, but if both are of RRSIG, we should also compare the types
// covered.
- bool isSameType(RRType type1, ConstRdataPtr rdata1,
- RRType type2, ConstRdataPtr rdata2)
+ static bool isSameType(RRType type1, ConstRdataPtr rdata1,
+ RRType type2, ConstRdataPtr rdata2)
{
if (type1 != type2) {
return (false);
@@ -1077,10 +1071,13 @@ private:
void getData() {
string data[DatabaseAccessor::COLUMN_COUNT];
data_ready_ = context_->getNext(data);
- name_txt_ = data[DatabaseAccessor::NAME_COLUMN];
- rtype_txt_ = data[DatabaseAccessor::TYPE_COLUMN];
- ttl_txt_ = data[DatabaseAccessor::TTL_COLUMN];
- rdata_txt_= data[DatabaseAccessor::RDATA_COLUMN];
+ if (data_ready_) {
+ name_txt_ = data[DatabaseAccessor::NAME_COLUMN];
+ rtype_txt_ = data[DatabaseAccessor::TYPE_COLUMN];
+ ttl_txt_ = data[DatabaseAccessor::TTL_COLUMN];
+ rdata_ = rdata::createRdata(RRType(rtype_txt_), class_,
+ data[DatabaseAccessor::RDATA_COLUMN]);
+ }
}
// The dedicated accessor
@@ -1094,12 +1091,12 @@ private:
// Status
bool ready_, data_ready_;
// Data of the next row
- string name_txt_, rtype_txt_, rdata_txt_, ttl_txt_;
- // RDATA of the next row; created from rdata_txt_.
+ string name_txt_, rtype_txt_, ttl_txt_;
+ // RDATA of the next row
ConstRdataPtr rdata_;
// Whether to modify differing TTL values, or treat a different TTL as
// a different RRset
- bool separate_rrs_;
+ const bool separate_rrs_;
};
}
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index bf4bb11..1da39c1 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -1333,7 +1333,7 @@ checkRRset(isc::dns::ConstRRsetPtr rrset,
isc::testutils::rrsetCheck(expected_rrset, rrset);
}
-// Iterate through a zone
+// Iterate through a zone, common case
TYPED_TEST(DatabaseClientTest, iterator) {
ZoneIteratorPtr it(this->client_->getIterator(Name("example.org")));
ConstRRsetPtr rrset(it->getNextRRset());
@@ -1341,71 +1341,100 @@ TYPED_TEST(DatabaseClientTest, iterator) {
// The first name should be the zone origin.
EXPECT_EQ(this->zname_, rrset->getName());
+}
- // The rest of the checks work only for the mock accessor.
- if (!this->is_mock_) {
- return;
- }
-
- this->expected_rdatas_.clear();
- this->expected_rdatas_.push_back("192.0.2.1");
- checkRRset(rrset, Name("example.org"), this->qclass_, RRType::A(),
- this->rrttl_, this->expected_rdatas_);
-
- rrset = it->getNextRRset();
- this->expected_rdatas_.clear();
- this->expected_rdatas_.push_back("ns1.example.org. admin.example.org. "
- "1234 3600 1800 2419200 7200");
- checkRRset(rrset, Name("example.org"), this->qclass_, RRType::SOA(),
- this->rrttl_, this->expected_rdatas_);
-
- rrset = it->getNextRRset();
- this->expected_rdatas_.clear();
- this->expected_rdatas_.push_back("192.0.2.1");
- this->expected_rdatas_.push_back("192.0.2.2");
- checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::A(),
- RRTTL(300), this->expected_rdatas_);
-
- rrset = it->getNextRRset();
- this->expected_rdatas_.clear();
- this->expected_rdatas_.push_back("2001:db8::1");
- this->expected_rdatas_.push_back("2001:db8::2");
- checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::AAAA(),
- RRTTL(300), this->expected_rdatas_);
-
- rrset = it->getNextRRset();
- this->expected_rdatas_.clear();
- this->expected_rdatas_.push_back(
- "A 5 3 3600 20000101000000 20000201000000 "
- "12345 example.org. FAKEFAKEFAKE");
- checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::RRSIG(),
- RRTTL(300), this->expected_rdatas_);
-
- rrset = it->getNextRRset();
- this->expected_rdatas_.clear();
- this->expected_rdatas_.push_back(
- "AAAA 5 3 3600 20000101000000 20000201000000 "
- "12345 example.org. FAKEFAKEFAKEFAKE");
- checkRRset(rrset, Name("x.example.org"), this->qclass_, RRType::RRSIG(),
- RRTTL(300), this->expected_rdatas_);
-
- rrset = it->getNextRRset();
- ASSERT_NE(ConstRRsetPtr(), rrset);
- this->expected_rdatas_.clear();
- this->expected_rdatas_.push_back("192.0.2.1");
- this->expected_rdatas_.push_back("192.0.2.2");
- checkRRset(rrset, Name("ttldiff.example.org"), this->qclass_, RRType::A(),
- RRTTL(300), this->expected_rdatas_);
-
- rrset = it->getNextRRset();
- ASSERT_NE(ConstRRsetPtr(), rrset);
- this->expected_rdatas_.clear();
- this->expected_rdatas_.push_back("2001:db8::1");
- this->expected_rdatas_.push_back("2001:db8::2");
- checkRRset(rrset, Name("ttldiff2.example.org"), this->qclass_,
- RRType::AAAA(), RRTTL(300), this->expected_rdatas_);
+// Supplemental structure used in the couple of tests below. It represents
+// parameters of an expected RRset containing up to two RDATAs. If it contains
+// only one RDATA, rdata2 is NULL.
+struct ExpectedRRset {
+ const char* const name;
+ const RRType rrtype;
+ const RRTTL rrttl;
+ const char* const rdata1;
+ const char* const rdata2;
+};
- EXPECT_EQ(ConstRRsetPtr(), it->getNextRRset());
+// Common checker for the iterator tests below. It extracts RRsets from the
+// give iterator and compare them to the expected sequence.
+void
+checkIteratorSequence(ZoneIterator& it, ExpectedRRset expected_sequence[],
+ size_t num_rrsets)
+{
+ vector<string> expected_rdatas;
+ for (size_t i = 0; i < num_rrsets; ++i) {
+ const ConstRRsetPtr rrset = it.getNextRRset();
+ ASSERT_TRUE(rrset);
+
+ expected_rdatas.clear();
+ expected_rdatas.push_back(expected_sequence[i].rdata1);
+ if (expected_sequence[i].rdata2 != NULL) {
+ expected_rdatas.push_back(expected_sequence[i].rdata2);
+ }
+ checkRRset(rrset, Name(expected_sequence[i].name), RRClass::IN(),
+ expected_sequence[i].rrtype, expected_sequence[i].rrttl,
+ expected_rdatas);
+ }
+ EXPECT_FALSE(it.getNextRRset());
+}
+
+TEST_F(MockDatabaseClientTest, iterator) {
+ // This version of test creates an iterator that combines same types of
+ // RRs into single RRsets.
+ ExpectedRRset expected_sequence[] = {
+ {"example.org", RRType::A(), rrttl_, "192.0.2.1", NULL},
+ {"example.org", RRType::SOA(), rrttl_,
+ "ns1.example.org. admin.example.org. 1234 3600 1800 2419200 7200",
+ NULL},
+ {"x.example.org", RRType::A(), RRTTL(300), "192.0.2.1", "192.0.2.2"},
+ {"x.example.org", RRType::AAAA(), RRTTL(300),
+ "2001:db8::1", "2001:db8::2"},
+ {"x.example.org", RRType::RRSIG(), RRTTL(300),
+ "A 5 3 3600 20000101000000 20000201000000 12345 example.org. "
+ "FAKEFAKEFAKE", NULL},
+ {"x.example.org", RRType::RRSIG(), RRTTL(300),
+ "AAAA 5 3 3600 20000101000000 20000201000000 12345 example.org. "
+ "FAKEFAKEFAKEFAKE", NULL},
+ {"ttldiff.example.org", RRType::A(), RRTTL(300),
+ "192.0.2.1", "192.0.2.2"},
+ {"ttldiff2.example.org", RRType::AAAA(), RRTTL(300),
+ "2001:db8::1", "2001:db8::2"}
+ };
+ checkIteratorSequence(*client_->getIterator(Name("example.org")),
+ expected_sequence,
+ sizeof(expected_sequence) /
+ sizeof(expected_sequence[0]));
+}
+
+TEST_F(MockDatabaseClientTest, iteratorSeparateRRs) {
+ // This version of test creates an iterator that separates all RRs as
+ // individual RRsets. In particular, it preserves the TTLs of an RRset
+ // even if they are different.
+ ExpectedRRset expected_sequence[] = {
+ {"example.org", RRType::A(), rrttl_, "192.0.2.1", NULL},
+ {"example.org", RRType::SOA(), rrttl_,
+ "ns1.example.org. admin.example.org. 1234 3600 1800 2419200 7200",
+ NULL},
+ {"x.example.org", RRType::A(), RRTTL(300), "192.0.2.1", NULL},
+ {"x.example.org", RRType::A(), RRTTL(300), "192.0.2.2", NULL},
+ {"x.example.org", RRType::AAAA(), RRTTL(300), "2001:db8::1", NULL},
+ {"x.example.org", RRType::AAAA(), RRTTL(300), "2001:db8::2", NULL},
+ {"x.example.org", RRType::RRSIG(), RRTTL(300),
+ "A 5 3 3600 20000101000000 20000201000000 12345 example.org. "
+ "FAKEFAKEFAKE", NULL},
+ {"x.example.org", RRType::RRSIG(), RRTTL(300),
+ "AAAA 5 3 3600 20000101000000 20000201000000 12345 example.org. "
+ "FAKEFAKEFAKEFAKE", NULL},
+ {"ttldiff.example.org", RRType::A(), RRTTL(300), "192.0.2.1", NULL},
+ {"ttldiff.example.org", RRType::A(), RRTTL(600), "192.0.2.2", NULL},
+ {"ttldiff2.example.org", RRType::AAAA(), RRTTL(600), "2001:db8::1",
+ NULL},
+ {"ttldiff2.example.org", RRType::AAAA(), RRTTL(300), "2001:db8::2",
+ NULL}
+ };
+ checkIteratorSequence(*client_->getIterator(Name("example.org"), true),
+ expected_sequence,
+ sizeof(expected_sequence) /
+ sizeof(expected_sequence[0]));
}
// This has inconsistent TTL in the set (the rest, like nonsense in
More information about the bind10-changes
mailing list