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());