[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

@@ -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<ConstRRsetPtr> 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()).
@@ -1621,14 +1654,6 @@ InMemoryZoneFinder::load(const string& filename) {
 }
 
 void
-generateRRsetFromIterator(ZoneIterator* iterator, LoadCallback callback) {
-    ConstRRsetPtr rrset;
-    while ((rrset = iterator->getNextRRset()) != NULL) {
-        callback(rrset);
-    }
-}
-
-void
 InMemoryZoneFinder::load(ZoneIterator& iterator) {
     impl_->load(string(),
                 boost::bind(generateRRsetFromIterator, &iterator, _1));

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

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -290,14 +290,15 @@ setRRset(RRsetPtr rrset, vector<RRsetPtr*>::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<RRsetPtr*> 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<DataSourceClient> db_client = unittest::createSQLite3Client(
         class_, origin_, TEST_DATA_BUILDDIR "/contexttest.sqlite3.copied", ss);
-    zone_finder_.load(*db_client->getIterator(origin_, true));
-
-    // Now the zone should have an SOA.
-    findTest(origin_, RRType::SOA(), ZoneFinder::SUCCESS, false,
-             ConstRRsetPtr());
+    zone_finder_.load(*db_client->getIterator(origin_));
+
+    // 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());