From ea528f50fddfb83618aa338bdd4607791fd788ef Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 5 Apr 2012 15:54:16 -0700 Subject: [PATCH] [1791] complete inmemory loader from iterator w/ consideration for RRSIGs. the iterator is now assumed to be created in the 'non separate' mode based on the bug fix of the previous commit. tests are updated with RRSIGs, and about how to create the iterator. doxygen comments clarify this point too. --- src/lib/datasrc/memory_datasrc.cc | 41 +++++++++++++---- src/lib/datasrc/memory_datasrc.h | 15 +++++- .../datasrc/tests/memory_datasrc_unittest.cc | 46 +++++++++++++++---- 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc index bdbba6d985..8374c2142f 100644 --- a/src/lib/datasrc/memory_datasrc.cc +++ b/src/lib/datasrc/memory_datasrc.cc @@ -1601,6 +1601,7 @@ InMemoryZoneFinder::InMemoryZoneFinderImpl::load( // And let the old data die with tmp } +namespace { // A wrapper for dns::masterLoad used by load() below. Essentially it // converts the two callback types. void @@ -1610,6 +1611,38 @@ masterLoadWrapper(const char* const filename, const Name& origin, masterLoad(filename, origin, zone_class, callback); } +// The installer called from Impl::load() for the iterator version of load(). +void +generateRRsetFromIterator(ZoneIterator* iterator, LoadCallback callback) { + ConstRRsetPtr rrset; + vector rrsigs; // placeholder for RRSIGs until "commitable". + + // The current internal implementation assumes an RRSIG is always added + // after the RRset they cover. So we store any RRSIGs in 'rrsigs' until + // it's safe to add them; based on our assumption if the owner name + // changes, all covered RRsets of the previous name should have been + // installed and any pending RRSIGs can be added at that point. RRSIGs + // of the last name from the iterator must be added separately. + while ((rrset = iterator->getNextRRset()) != NULL) { + if (!rrsigs.empty() && rrset->getName() != rrsigs[0]->getName()) { + BOOST_FOREACH(ConstRRsetPtr sig_rrset, rrsigs) { + callback(sig_rrset); + } + rrsigs.clear(); + } + if (rrset->getType() == RRType::RRSIG()) { + rrsigs.push_back(rrset); + } else { + callback(rrset); + } + } + + BOOST_FOREACH(ConstRRsetPtr sig_rrset, rrsigs) { + callback(sig_rrset); + } +} +} + void InMemoryZoneFinder::load(const string& filename) { LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_LOAD).arg(getOrigin()). @@ -1620,14 +1653,6 @@ InMemoryZoneFinder::load(const string& filename) { getClass(), _1)); } -void -generateRRsetFromIterator(ZoneIterator* iterator, LoadCallback callback) { - ConstRRsetPtr rrset; - while ((rrset = iterator->getNextRRset()) != NULL) { - callback(rrset); - } -} - void InMemoryZoneFinder::load(ZoneIterator& iterator) { impl_->load(string(), diff --git a/src/lib/datasrc/memory_datasrc.h b/src/lib/datasrc/memory_datasrc.h index bed2e83dbd..c687d1bcd6 100644 --- a/src/lib/datasrc/memory_datasrc.h +++ b/src/lib/datasrc/memory_datasrc.h @@ -192,7 +192,20 @@ public: /// /// This is similar to the other version, but zone's RRsets are provided /// by an iterator of another data source. On successful load, the - // internal filename will be cleared. + /// internal filename will be cleared. + /// + /// This implementation assumes the iterator produces combined RRsets, + /// that is, there should exactly one RRset for the same owner name and + /// RR type. This means the caller is expected to create the iterator + /// with \c separate_rrs being \c false. This implementation also assumes + /// RRsets of different names are not mixed; so if the iterator produces + /// an RRset of a different name than that of the previous RRset, that + /// previous name must never appear in the subsequent sequence of RRsets. + /// Note that the iterator API does not ensure this. If the underlying + /// implementation does not follow it, load() will fail. Note, however, + /// that this whole interface is tentative. in-memory zone loading will + /// have to be revisited fundamentally, and at that point this restriction + /// probably won't matter. void load(ZoneIterator& iterator); /// Exchanges the content of \c this zone finder with that of the given diff --git a/src/lib/datasrc/tests/memory_datasrc_unittest.cc b/src/lib/datasrc/tests/memory_datasrc_unittest.cc index 20fb376db9..5f2b0b8f12 100644 --- a/src/lib/datasrc/tests/memory_datasrc_unittest.cc +++ b/src/lib/datasrc/tests/memory_datasrc_unittest.cc @@ -290,14 +290,15 @@ setRRset(RRsetPtr rrset, vector::iterator& it) { ++it; } -ConstRRsetPtr -textToRRset(const string& text_rrset, const RRClass& rrclass = RRClass::IN()) { +RRsetPtr +textToRRset(const string& text_rrset, const RRClass& rrclass = RRClass::IN(), + const Name& origin = Name::ROOT_NAME()) +{ stringstream ss(text_rrset); RRsetPtr rrset; vector rrsets; rrsets.push_back(&rrset); - masterLoad(ss, Name::ROOT_NAME(), rrclass, - boost::bind(setRRset, _1, rrsets.begin())); + masterLoad(ss, origin, rrclass, boost::bind(setRRset, _1, rrsets.begin())); return (rrset); } @@ -552,6 +553,8 @@ public: ZoneFinder::FindOptions options = ZoneFinder::FIND_DEFAULT, bool check_wild_answer = false) { + SCOPED_TRACE("findTest for " + name.toText() + "/" + rrtype.toText()); + if (zone_finder == NULL) { zone_finder = &zone_finder_; } @@ -1108,14 +1111,39 @@ TEST_F(InMemoryZoneFinderTest, loadFromIterator) { findTest(origin_, RRType::SOA(), ZoneFinder::NXRRSET, false, ConstRRsetPtr()); - stringstream ss("example.org. 300 IN SOA . . 0 0 0 0 0"); + // The content of the new version of zone to be first installed to + // the SQLite3 data source, then to in-memory via SQLite3. The data are + // chosen to cover major check points of the implementation: + // - the previously non-existent record is added (SOA) + // - An RRSIG is given from the iterator before the RRset it covers + // (RRSIG for SOA, because they are sorted by name then rrtype as text) + // - An RRset containing multiple RRs (ns1/A) + // - RRSIGs for different owner names + stringstream ss; + const char* const soa_txt = "example.org. 300 IN SOA . . 0 0 0 0 0\n"; + const char* const soa_sig_txt = "example.org. 300 IN RRSIG SOA 5 3 300 " + "20000101000000 20000201000000 12345 example.org. FAKEFAKE\n"; + const char* const a_txt = + "ns1.example.org. 300 IN A 192.0.2.1\n" + "ns1.example.org. 300 IN A 192.0.2.2\n"; + const char* const a_sig_txt = "ns1.example.org. 300 IN RRSIG A 5 3 300 " + "20000101000000 20000201000000 12345 example.org. FAKEFAKE\n"; + ss << soa_txt << soa_sig_txt << a_txt << a_sig_txt; shared_ptr db_client = unittest::createSQLite3Client( class_, origin_, TEST_DATA_BUILDDIR "/contexttest.sqlite3.copied", ss); - zone_finder_.load(*db_client->getIterator(origin_, true)); + zone_finder_.load(*db_client->getIterator(origin_)); - // Now the zone should have an SOA. - findTest(origin_, RRType::SOA(), ZoneFinder::SUCCESS, false, - ConstRRsetPtr()); + // The new content should be visible, including the previously-nonexistent + // SOA. + RRsetPtr expected_answer = textToRRset(soa_txt, RRClass::IN(), origin_); + expected_answer->addRRsig(textToRRset(soa_sig_txt)); + findTest(origin_, RRType::SOA(), ZoneFinder::SUCCESS, true, + expected_answer); + + expected_answer = textToRRset(a_txt); + expected_answer->addRRsig(textToRRset(a_sig_txt)); + findTest(Name("ns1.example.org"), RRType::A(), ZoneFinder::SUCCESS, true, + expected_answer); // File name should be (re)set to empty. EXPECT_TRUE(zone_finder_.getFileName().empty());