[master] Merge branch 'trac1791'

src/lib/datasrc/database.cc

@@ -1005,28 +1005,44 @@ public:
             // At the end of zone
             accessor_->commit();
             ready_ = false;
-            LOG_DEBUG(logger, DBG_TRACE_DETAILED,
-                      DATASRC_DATABASE_ITERATE_END);
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE_END);
             return (ConstRRsetPtr());
         }
-        const string name_str(name_), rtype_str(rtype_), ttl(ttl_);
-        const Name name(name_str);
-        const RRType rtype(rtype_str);
-        RRsetPtr rrset(new RRset(name, class_, rtype, RRTTL(ttl)));
-        while (data_ready_ && name_ == name_str && rtype_str == rtype_) {
-            if (ttl_ != ttl) {
-                if (ttl < ttl_) {
-                    ttl_ = ttl;
-                    rrset->setTTL(RRTTL(ttl));
-                }
-                LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH).
-                    arg(name_).arg(class_).arg(rtype_).arg(rrset->getTTL());
-            }
-            rrset->addRdata(rdata::createRdata(rtype, class_, rdata_));
+        const RRType rtype(rtype_txt_);
+        RRsetPtr rrset(new RRset(Name(name_txt_), class_, rtype,
+                                 RRTTL(ttl_txt_)));
+        // 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_);
+
+            // Retrieve the next record from the database.  If we reach the
+            // end of the zone, done; if we were requested to separate all RRs,
+            // just remember this record and return the single RR.
             getData();
-            if (separate_rrs_) {
+            if (separate_rrs_ || !data_ready_) {
                 break;
             }
+
+            // Check if the next record belongs to the same RRset.  If not,
+            // 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, RRType(rtype_txt_), rdata_)) {
+                break;
+            }
+
+            // Adjust TTL if necessary
+            const RRTTL next_ttl(ttl_txt_);
+            if (next_ttl != rrset->getTTL()) {
+                if (next_ttl < rrset->getTTL()) {
+                    rrset->setTTL(next_ttl);
+                }
+                LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH).
+                    arg(name_txt_).arg(class_).arg(rtype).arg(rrset->getTTL());
+            }
         }
         LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE_NEXT).
             arg(rrset->getName()).arg(rrset->getType());
@@ -1034,14 +1050,34 @@ public:
     }
 
 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.
+    static bool isSameType(RRType type1, ConstRdataPtr rdata1,
+                           RRType type2, ConstRdataPtr rdata2)
+    {
+        if (type1 != type2) {
+            return (false);
+        }
+        if (type1 == RRType::RRSIG()) {
+            return (dynamic_cast<const generic::RRSIG&>(*rdata1).typeCovered()
+                    == dynamic_cast<const generic::RRSIG&>(*rdata2).
+                    typeCovered());
+        }
+        return (true);
+    }
+
     // Load next row of data
     void getData() {
         string data[DatabaseAccessor::COLUMN_COUNT];
         data_ready_ = context_->getNext(data);
-        name_ = data[DatabaseAccessor::NAME_COLUMN];
-        rtype_ = data[DatabaseAccessor::TYPE_COLUMN];
-        ttl_ = data[DatabaseAccessor::TTL_COLUMN];
-        rdata_ = 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
@@ -1055,10 +1091,12 @@ private:
     // Status
     bool ready_, data_ready_;
     // Data of the next row
-    string name_, rtype_, rdata_, ttl_;
+    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_;
 };
 
 }

src/lib/datasrc/datasrc_messages.mes

@@ -145,10 +145,12 @@ While iterating through the zone, the program extracted next RRset from it.
 The name and RRtype of the RRset is indicated in the message.
 
 % DATASRC_DATABASE_ITERATE_TTL_MISMATCH TTL values differ for RRs of %1/%2/%3, setting to %4
-While iterating through the zone, the time to live for RRs of the given RRset
-were found to be different. This isn't allowed on the wire and is considered
-an error, so we set it to the lowest value we found (but we don't modify the
-database). The data in database should be checked and fixed.
+While iterating through the zone, the time to live for RRs of the
+given RRset were found to be different. Since an RRset cannot have
+multiple TTLs, we set it to the lowest value we found (but we don't
+modify the database). This is what the client would do when such RRs
+were given in a DNS response according to RFC2181. The data in
+database should be checked and fixed.
 
 % DATASRC_DATABASE_JOURNALREADER_END %1/%2 on %3 from %4 to %5
 This is a debug message indicating that the program (successfully)

src/lib/datasrc/memory_datasrc.cc

@@ -29,6 +29,7 @@
 #include <datasrc/data_source.h>
 #include <datasrc/factory.h>
 
+#include <boost/function.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <boost/bind.hpp>
@@ -53,6 +54,9 @@ using namespace internal;
 namespace {
 // Some type aliases
 
+// A functor type used for loading.
+typedef boost::function<void(ConstRRsetPtr)> LoadCallback;
+
 // RRset specified for this implementation
 typedef boost::shared_ptr<internal::RBNodeRRset> RBNodeRRsetPtr;
 typedef boost::shared_ptr<const internal::RBNodeRRset> ConstRBNodeRRsetPtr;
@@ -761,6 +765,17 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
     // The actual zone data
     scoped_ptr<ZoneData> zone_data_;
 
+    // Common process for zone load.
+    // rrset_installer is a functor that takes another functor as an argument,
+    // and expected to call the latter for each RRset of the zone.  How the
+    // sequence of the RRsets is generated depends on the internal
+    // details  of the loader: either from a textual master file or from
+    // another data source.
+    // filename is the file name of the master file or empty if the zone is
+    // loaded from another data source.
+    void load(const string& filename,
+              boost::function<void(LoadCallback)> rrset_installer);
+
     // Add the necessary magic for any wildcard contained in 'name'
     // (including itself) to be found in the zone.
     //
@@ -1551,24 +1566,16 @@ addWildAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
 }
 
 void
-InMemoryZoneFinder::load(const string& filename) {
-    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_LOAD).arg(getOrigin()).
-        arg(filename);
-    // Load it into temporary zone data.  As we build the zone, we record
-    // the (RBNode)RRsets that needs to be associated with additional
-    // information in 'need_additionals'.
+InMemoryZoneFinder::InMemoryZoneFinderImpl::load(
+    const string& filename,
+    boost::function<void(LoadCallback)> rrset_installer)
+{
     vector<RBNodeRRset*> need_additionals;
-    scoped_ptr<ZoneData> tmp(new ZoneData(getOrigin()));
+    scoped_ptr<ZoneData> tmp(new ZoneData(origin_));
 
-    masterLoad(filename.c_str(), getOrigin(), getClass(),
-               boost::bind(&InMemoryZoneFinderImpl::addFromLoad, impl_,
-                           _1, tmp.get(), &need_additionals));
+    rrset_installer(boost::bind(&InMemoryZoneFinderImpl::addFromLoad, this,
+                                _1, tmp.get(), &need_additionals));
 
-    // For each RRset in need_additionals, identify the corresponding
-    // RBnode for additional processing and associate it in the RRset.
-    // If some additional names in an RRset RDATA as additional need wildcard
-    // expansion, we'll remember them in a separate vector, and handle them
-    // with addWildAdditional.
     vector<RBNodeRRset*> wild_additionals;
     for_each(need_additionals.begin(), need_additionals.end(),
              boost::bind(addAdditional, _1, tmp.get(), &wild_additionals));
@@ -1584,16 +1591,74 @@ InMemoryZoneFinder::load(const string& filename) {
         if (tmp->origin_data_->getData()->find(RRType::NSEC3PARAM()) ==
             tmp->origin_data_->getData()->end()) {
             LOG_WARN(logger, DATASRC_MEM_NO_NSEC3PARAM).
-                arg(getOrigin()).arg(getClass());
+                arg(origin_).arg(zone_class_);
         }
     }
 
     // If it went well, put it inside
-    impl_->file_name_ = filename;
-    tmp.swap(impl_->zone_data_);
+    file_name_ = filename;
+    tmp.swap(zone_data_);
     // 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
+masterLoadWrapper(const char* const filename, const Name& origin,
+                  const RRClass& zone_class, LoadCallback callback)
+{
+    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()).
+        arg(filename);
+
+    impl_->load(filename,
+                boost::bind(masterLoadWrapper, filename.c_str(), getOrigin(),
+                            getClass(), _1));
+}
+
+void
+InMemoryZoneFinder::load(ZoneIterator& iterator) {
+    impl_->load(string(),
+                boost::bind(generateRRsetFromIterator, &iterator, _1));
+}
+
 void
 InMemoryZoneFinder::swap(InMemoryZoneFinder& zone_finder) {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_SWAP).arg(getOrigin()).

src/lib/datasrc/memory_datasrc.h

@@ -188,6 +188,26 @@ public:
     ///     configuration reloading is written.
     void load(const std::string& filename);
 
+    /// \brief Load zone from another data source.
+    ///
+    /// 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.
+    ///
+    /// 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
     /// \c zone_finder.
     ///

src/lib/datasrc/tests/Makefile.am

@@ -48,6 +48,7 @@ run_unittests_SOURCES += datasrc_unittest.cc
 run_unittests_SOURCES += static_unittest.cc
 run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += cache_unittest.cc
+run_unittests_SOURCES += test_client.h test_client.cc
 run_unittests_SOURCES += test_datasrc.h test_datasrc.cc
 run_unittests_SOURCES += rbtree_unittest.cc
 run_unittests_SOURCES += logger_unittest.cc

src/lib/datasrc/tests/database_unittest.cc

@@ -501,62 +501,46 @@ private:
             }
 
             // Return faked data for tests
-            switch (step ++) {
-                case 0:
-                    data[DatabaseAccessor::NAME_COLUMN] = "example.org";
-                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
-                    data[DatabaseAccessor::TTL_COLUMN] = "3600";
-                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
-                    return (true);
-                case 1:
-                    data[DatabaseAccessor::NAME_COLUMN] = "example.org";
-                    data[DatabaseAccessor::TYPE_COLUMN] = "SOA";
-                    data[DatabaseAccessor::TTL_COLUMN] = "3600";
-                    data[DatabaseAccessor::RDATA_COLUMN] = "ns1.example.org. admin.example.org. "
-                        "1234 3600 1800 2419200 7200";
-                    return (true);
-                case 2:
-                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
-                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
-                    data[DatabaseAccessor::TTL_COLUMN] = "300";
-                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
-                    return (true);
-                case 3:
-                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
-                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
-                    data[DatabaseAccessor::TTL_COLUMN] = "300";
-                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.2";
-                    return (true);
-                case 4:
-                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
-                    data[DatabaseAccessor::TYPE_COLUMN] = "AAAA";
-                    data[DatabaseAccessor::TTL_COLUMN] = "300";
-                    data[DatabaseAccessor::RDATA_COLUMN] = "2001:db8::1";
-                    return (true);
-                case 5:
-                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
-                    data[DatabaseAccessor::TYPE_COLUMN] = "AAAA";
-                    data[DatabaseAccessor::TTL_COLUMN] = "300";
-                    data[DatabaseAccessor::RDATA_COLUMN] = "2001:db8::2";
-                    return (true);
-                case 6:
-                    data[DatabaseAccessor::NAME_COLUMN] = "ttldiff.example.org";
-                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
-                    data[DatabaseAccessor::TTL_COLUMN] = "300";
-                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
-                    return (true);
-                case 7:
-                    data[DatabaseAccessor::NAME_COLUMN] = "ttldiff.example.org";
-                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
-                    data[DatabaseAccessor::TTL_COLUMN] = "600";
-                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.2";
-                    return (true);
-                default:
-                    ADD_FAILURE() <<
-                        "Request past the end of iterator context";
-                case 8:
-                    return (false);
+            // This is the sequence of zone data in the order of appearance
+            // in the returned sequence from this iterator.
+            typedef const char* ColumnText[4];
+            const ColumnText zone_data[] = {
+                // A couple of basic RRs at the zone origin.
+                {"example.org", "A", "3600", "192.0.2.1"},
+                {"example.org", "SOA", "3600", "ns1.example.org. "
+                 "admin.example.org. 1234 3600 1800 2419200 7200"},
+                // RRsets sharing the same owner name with multiple RRs.
+                {"x.example.org", "A", "300", "192.0.2.1"},
+                {"x.example.org", "A", "300", "192.0.2.2"},
+                {"x.example.org", "AAAA", "300", "2001:db8::1"},
+                {"x.example.org", "AAAA", "300", "2001:db8::2"},
+                // RRSIGs.  Covered types are different and these two should
+                // be distinguished.
+                {"x.example.org", "RRSIG", "300",
+                 "A 5 3 3600 20000101000000 20000201000000 12345 "
+                 "example.org. FAKEFAKEFAKE"},
+                {"x.example.org", "RRSIG", "300",
+                 "AAAA 5 3 3600 20000101000000 20000201000000 12345 "
+                 "example.org. FAKEFAKEFAKEFAKE"},
+                // Mixture of different TTLs.  Covering both cases of small
+                // then large and large then small.  In either case the smaller
+                // TTL should win.
+                {"ttldiff.example.org", "A", "300", "192.0.2.1"},
+                {"ttldiff.example.org", "A", "600", "192.0.2.2"},
+                {"ttldiff2.example.org", "AAAA", "600", "2001:db8::1"},
+                {"ttldiff2.example.org", "AAAA", "300", "2001:db8::2"}};
+            const size_t num_rrs = sizeof(zone_data) / sizeof(zone_data[0]);
+            if (step > num_rrs) {
+                ADD_FAILURE() << "Request past the end of iterator context";
+            } else if (step < num_rrs) {
+                data[DatabaseAccessor::NAME_COLUMN] = zone_data[step][0];
+                data[DatabaseAccessor::TYPE_COLUMN] = zone_data[step][1];
+                data[DatabaseAccessor::TTL_COLUMN] = zone_data[step][2];
+                data[DatabaseAccessor::RDATA_COLUMN] = zone_data[step][3];
+                ++step;
+                return (true);
             }
+            return (false);
         }
     };
     class EmptyIteratorContext : public IteratorContext {
@@ -1349,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());
@@ -1357,47 +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();
-    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_);
+// 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

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -12,12 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <sstream>
-#include <vector>
-
-#include <boost/bind.hpp>
-#include <boost/foreach.hpp>
-
 #include <exceptions/exceptions.h>
 
 #include <dns/masterload.h>
@@ -30,19 +24,30 @@
 #include <dns/rrttl.h>
 #include <dns/masterload.h>
 
+#include <datasrc/client.h>
 #include <datasrc/memory_datasrc.h>
 #include <datasrc/data_source.h>
 #include <datasrc/iterator.h>
 
+#include "test_client.h"
+
 #include <testutils/dnsmessage_test.h>
 
 #include <gtest/gtest.h>
 
+#include <boost/bind.hpp>
+#include <boost/foreach.hpp>
+#include <boost/shared_ptr.hpp>
+
+#include <sstream>
+#include <vector>
+
 using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 using namespace isc::datasrc;
 using namespace isc::testutils;
+using boost::shared_ptr;
 
 namespace {
 // Commonly used result codes (Who should write the prefix all the time)
@@ -285,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);
 }
 
@@ -398,6 +404,8 @@ public:
         // Build test RRsets.  Below, we construct an RRset for
         // each textual RR(s) of zone_data, and assign it to the corresponding
         // rr_xxx.
+        // Note that this contains an out-of-zone RR, and due to the
+        // validation check of masterLoad() used below, we cannot add SOA.
         const RRsetData zone_data[] = {
             {"example.org. 300 IN NS ns.example.org.", &rr_ns_},
             {"example.org. 300 IN A 192.0.2.1", &rr_a_},
@@ -545,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_;
         }
@@ -1093,7 +1103,70 @@ TEST_F(InMemoryZoneFinderTest, load) {
 
     // Try loading zone that is wrong in a different way
     EXPECT_THROW(zone_finder_.load(TEST_DATA_DIR "/duplicate_rrset.zone"),
-        MasterLoadError);
+                 MasterLoadError);
+}
+
+TEST_F(InMemoryZoneFinderTest, loadFromIterator) {
+    // The initial test set doesn't have SOA at the apex.
+    findTest(origin_, RRType::SOA(), ZoneFinder::NXRRSET, false,
+             ConstRRsetPtr());
+
+    // 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_));
+
+    // 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());
+
+    // Loading the zone with an iterator separating RRs of the same RRset
+    // will fail because the resulting sequence doesn't meet assumptions of
+    // the (current) in-memory implementation.
+    EXPECT_THROW(zone_finder_.load(*db_client->getIterator(origin_, true)),
+                 MasterLoadError);
+
+    // Load the zone from a file that contains more realistic data (borrowed
+    // from a different test).  There's nothing special in this case for the
+    // purpose of this test, so it should just succeed.
+    db_client = unittest::createSQLite3Client(
+        class_, origin_, TEST_DATA_BUILDDIR "/contexttest.sqlite3.copied",
+        TEST_DATA_DIR "/contexttest.zone");
+    zone_finder_.load(*db_client->getIterator(origin_));
+
+    // just checking a couple of RRs in the new version of zone.
+    findTest(Name("mx1.example.org"), RRType::A(), ZoneFinder::SUCCESS, true,
+             textToRRset("mx1.example.org. 3600 IN A 192.0.2.10"));
+    findTest(Name("ns1.example.org"), RRType::AAAA(), ZoneFinder::SUCCESS,
+             true, textToRRset("ns1.example.org. 3600 IN AAAA 2001:db8::1"));
 }
 
 /*

src/lib/datasrc/tests/test_client.cc

@@ -0,0 +1,92 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <exceptions/exceptions.h>
+
+#include <dns/masterload.h>
+#include <dns/name.h>
+#include <dns/rrclass.h>
+
+#include <datasrc/client.h>
+#include <datasrc/zone.h>
+#include <datasrc/sqlite3_accessor.h>
+
+#include "test_client.h"
+
+#include <boost/bind.hpp>
+#include <boost/shared_ptr.hpp>
+
+#include <cstdlib>
+#include <istream>
+#include <fstream>
+
+using namespace std;
+using boost::shared_ptr;
+
+using namespace isc::dns;
+
+namespace isc {
+namespace datasrc {
+namespace unittest {
+
+namespace {
+// A helper subroutine for the SQLite3Client creator.
+void
+addRRset(ZoneUpdaterPtr updater, ConstRRsetPtr rrset) {
+    updater->addRRset(*rrset);
+}
+}
+
+shared_ptr<DataSourceClient>
+createSQLite3Client(RRClass zclass, const Name& zname,
+                    const char* const db_file, const char* const zone_file)
+{
+    ifstream ifs(zone_file, ios_base::in);
+    if (ifs.fail()) {
+        isc_throw(isc::Unexpected, "Failed to open test zone file: "
+                  << zone_file);
+    }
+    return (createSQLite3Client(zclass, zname, db_file, ifs));
+}
+
+shared_ptr<DataSourceClient>
+createSQLite3Client(RRClass zclass, const Name& zname,
+                    const char* const db_file, istream& rr_stream)
+{
+    // We always begin with an empty template SQLite3 DB file and install
+    // the zone data from the zone file to ensure that the data source contains
+    // the exact given data, and only that data.
+    const char* const install_cmd_prefix = INSTALL_PROG " " TEST_DATA_DIR
+        "/rwtest.sqlite3 ";
+    const string install_cmd = string(install_cmd_prefix) + db_file;
+    if (system(install_cmd.c_str()) != 0) {
+        isc_throw(isc::Unexpected,
+                  "Error setting up; command failed: " << install_cmd);
+    }
+
+    shared_ptr<SQLite3Accessor> accessor(
+        new SQLite3Accessor(db_file, zclass.toText()));
+    shared_ptr<DatabaseClient> client(new DatabaseClient(zclass, accessor));
+
+    ZoneUpdaterPtr updater = client->getUpdater(zname, true);
+    masterLoad(rr_stream, zname, zclass, boost::bind(addRRset, updater, _1));
+
+    updater->commit();
+
+    return (client);
+}
+
+}
+}
+}

src/lib/datasrc/tests/test_client.h

@@ -0,0 +1,71 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __TEST_DATA_SOURCE_CLIENT_H
+#define __TEST_DATA_SOURCE_CLIENT_H 1
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+
+#include <boost/shared_ptr.hpp>
+
+#include <istream>
+
+namespace isc {
+namespace datasrc {
+namespace unittest {
+
+// Here we define utility modules for the convenience of tests that create
+// a data source client according to the specified conditions.
+
+/// \brief Create an SQLite3 data source client from a zone file.
+///
+/// This function creates an SQLite3 client for the specified zone containing
+/// RRs in the specified zone file.  The zone will be created in the given
+/// SQLite3 database file.  The database file does not have to exist; this
+/// function will automatically create a new file for the test; if the given
+/// file already exists this function overrides the content (so basically the
+/// file must be an ephemeral one only for that test case).
+///
+/// The zone file must be formatted so it's accepted by the dns::masterLoad()
+/// function.
+///
+/// \param zclass The RR class of the zone
+/// \param zname The origin name of the zone
+/// \param db_file The SQLite3 data base file in which the zone data should be
+/// installed.
+/// \param zone_file The filename of the zone data in the textual format.
+/// \return Newly created \c DataSourceClient using the SQLite3 data source
+boost::shared_ptr<DataSourceClient>
+createSQLite3Client(dns::RRClass zclass, const dns::Name& zname,
+                    const char* const db_file, const char* const zone_file);
+
+/// \brief Create an SQLite3 data source client from a stream.
+///
+/// This is similar to the other version of the function, but takes an input
+/// stream for the zone data.  The stream produces strings as the corresponding
+/// dns::masterLoad() function expects.
+boost::shared_ptr<DataSourceClient>
+createSQLite3Client(dns::RRClass zclass, const dns::Name& zname,
+                    const char* const db_file, std::istream& rr_stream);
+
+} // end of unittest
+} // end of datasrc
+} // end of isc
+
+#endif  // __TEST_DATA_SOURCE_CLIENT_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
 #include <dns/masterload.h>
 #include <dns/name.h>
 #include <dns/rrclass.h>
@@ -21,6 +23,7 @@
 #include <datasrc/database.h>
 #include <datasrc/sqlite3_accessor.h>
 
+#include "test_client.h"
 #include <testutils/dnsmessage_test.h>
 
 #include <gtest/gtest.h>
@@ -29,7 +32,8 @@
 #include <boost/foreach.hpp>
 #include <boost/shared_ptr.hpp>
 
-#include <cstdlib>
+#include <fstream>
+#include <sstream>
 #include <vector>
 
 using namespace std;
@@ -66,8 +70,6 @@ createInMemoryClient(RRClass zclass, const Name& zname) {
     return (client);
 }
 
-// Creator for the SQLite3 client to be tested.  addRRset() is a helper
-// subroutine.
 void
 addRRset(ZoneUpdaterPtr updater, ConstRRsetPtr rrset) {
     updater->addRRset(*rrset);
@@ -78,25 +80,14 @@ createSQLite3Client(RRClass zclass, const Name& zname) {
     // We always begin with an empty template SQLite3 DB file and install
     // the zone data from the zone file to ensure both cases have the
     // same test data.
+    DataSourceClientPtr client = unittest::createSQLite3Client(
+        zclass, zname, TEST_DATA_BUILDDIR "/contexttest.sqlite3.copied",
+        TEST_ZONE_FILE);
 
-    const char* const install_cmd = INSTALL_PROG " " TEST_DATA_DIR
-        "/rwtest.sqlite3 " TEST_DATA_BUILDDIR "/contexttest.sqlite3.copied";
-    if (system(install_cmd) != 0) {
-        isc_throw(isc::Unexpected,
-                  "Error setting up; command failed: " << install_cmd);
-    }
-
-    shared_ptr<SQLite3Accessor> accessor(
-        new SQLite3Accessor(TEST_DATA_BUILDDIR "/contexttest.sqlite3.copied",
-                            zclass.toText()));
-    shared_ptr<DatabaseClient> client(new DatabaseClient(zclass, accessor));
-
-    ZoneUpdaterPtr updater = client->getUpdater(zname, true);
-    masterLoad(TEST_ZONE_FILE, zname, zclass, boost::bind(addRRset, updater,
-                                                          _1));
     // Insert an out-of-zone name to test if it's incorrectly returned.
     // Note that neither updater nor SQLite3 accessor checks this condition,
     // so this should succeed.
+    ZoneUpdaterPtr updater = client->getUpdater(zname, false);
     stringstream ss("ns.example.com. 3600 IN A 192.0.2.7");
     masterLoad(ss, Name::ROOT_NAME(), zclass,
                boost::bind(addRRset, updater, _1));

src/lib/python/isc/datasrc/tests/datasrc_test.py

@@ -262,16 +262,16 @@ class DataSrcClient(unittest.TestCase):
         rrets = dsc.get_iterator(isc.dns.Name("example.com"))
         # there are more than 80 RRs in this zone... let's just count them
         # (already did a full check of the smaller zone above)
-        self.assertEqual(55, len(list(rrets)))
+        # There are 40 non-RRSIG RRsets and 32 dinstinct RRSIGs.
+        self.assertEqual(72, len(list(rrets)))
 
         # same test, but now with explicit False argument for separate_rrs
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
         rrets = dsc.get_iterator(isc.dns.Name("example.com"), False)
         # there are more than 80 RRs in this zone... let's just count them
         # (already did a full check of the smaller zone above)
-        self.assertEqual(55, len(list(rrets)))
+        self.assertEqual(72, len(list(rrets)))
 
-        # Count should be 71 if we request individual rrsets for differing ttls
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
         rrets = dsc.get_iterator(isc.dns.Name("example.com"), True)
         # there are more than 80 RRs in this zone... let's just count them