[master] Merge branch 'trac2470'

Fixed conflicts:
	src/lib/dns/master_loader.cc
	src/lib/dns/tests/master_loader_unittest.cc

src/bin/auth/tests/testdata/example.zone

@@ -55,11 +55,11 @@ t.example.com. 3600 IN NSEC b.*.t.example.com. A NSEC RRSIG
 ;; (.no.example.com. (qname, NXDOMAIN)
 ;; ).no.example.com. (exist)
 ;; *.no.example.com. (best possible wildcard, not exist)
-).no.example.com. 3600 IN AAAA 2001:db8::53
+\).no.example.com. 3600 IN AAAA 2001:db8::53
 ;; NSEC records.
 example.com. 3600 IN NSEC cname.example.com. NS SOA NSEC RRSIG
-mx.example.com. 3600 IN NSEC ).no.example.com. MX NSEC RRSIG
-).no.example.com. 3600 IN NSEC nz.no.example.com. AAAA NSEC RRSIG
+mx.example.com. 3600 IN NSEC \).no.example.com. MX NSEC RRSIG
+\).no.example.com. 3600 IN NSEC nz.no.example.com. AAAA NSEC RRSIG
 ;; We'll also test the case where a single NSEC proves both NXDOMAIN and the
 ;; non existence of wildcard.  The following records will be used for that
 ;; test.

src/lib/datasrc/memory/zone_data_loader.cc

@@ -12,15 +12,17 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <datasrc/master_loader_callbacks.h>
 #include <datasrc/memory/zone_data_loader.h>
 #include <datasrc/memory/zone_data_updater.h>
 #include <datasrc/memory/logger.h>
 #include <datasrc/memory/segment_object_holder.h>
 #include <datasrc/memory/util_internal.h>
 
+#include <dns/master_loader.h>
+#include <dns/rrcollator.h>
 #include <dns/rdataclass.h>
 #include <dns/rrset.h>
-#include <dns/masterload.h>
 
 #include <boost/foreach.hpp>
 #include <boost/bind.hpp>
@@ -181,18 +183,25 @@ loadZoneDataInternal(util::MemorySegment& mem_sgmt,
     return (holder.release());
 }
 
-// A wrapper for dns::masterLoad used by loadZoneData() below.  Essentially it
-// converts the two callback types.  Note the mostly redundant wrapper of
+// A wrapper for dns::MasterLoader used by loadZoneData() below.  Essentially
+// it converts the two callback types.  Note the mostly redundant wrapper of
 // boost::bind.  It converts function<void(ConstRRsetPtr)> to
-// function<void(RRsetPtr)> (masterLoad() expects the latter).  SunStudio
+// function<void(RRsetPtr)> (MasterLoader expects the latter).  SunStudio
 // doesn't seem to do this conversion if we just pass 'callback'.
 void
-masterLoadWrapper(const char* const filename, const Name& origin,
-                  const RRClass& zone_class, LoadCallback callback)
+masterLoaderWrapper(const char* const filename, const Name& origin,
+                    const RRClass& zone_class, LoadCallback callback)
 {
+    bool load_ok = false;       // (we don't use it)
+    dns::RRCollator collator(boost::bind(callback, _1));
+
     try {
-        masterLoad(filename, origin, zone_class, boost::bind(callback, _1));
-    } catch (MasterLoadError& e) {
+        dns::MasterLoader(filename, origin, zone_class,
+                          createMasterLoaderCallbacks(origin, zone_class,
+                                                      &load_ok),
+                          collator.getCallback()).load();
+        collator.flush();
+    } catch (const dns::MasterLoaderError& e) {
         isc_throw(ZoneLoaderException, e.what());
     }
 }
@@ -215,7 +224,7 @@ loadZoneData(util::MemorySegment& mem_sgmt,
              const std::string& zone_file)
 {
      return (loadZoneDataInternal(mem_sgmt, rrclass, zone_name,
-                                 boost::bind(masterLoadWrapper,
+                                 boost::bind(masterLoaderWrapper,
                                              zone_file.c_str(),
                                              zone_name, rrclass,
                                              _1)));

src/lib/dns/Makefile.am

@@ -117,6 +117,7 @@ libb10_dns___la_SOURCES += rrparamregistry.h
 libb10_dns___la_SOURCES += rrset.h rrset.cc
 libb10_dns___la_SOURCES += rrttl.h rrttl.cc
 libb10_dns___la_SOURCES += rrtype.cc
+libb10_dns___la_SOURCES += rrcollator.h rrcollator.cc
 libb10_dns___la_SOURCES += question.h question.cc
 libb10_dns___la_SOURCES += serial.h serial.cc
 libb10_dns___la_SOURCES += tsig.h tsig.cc
@@ -124,7 +125,7 @@ libb10_dns___la_SOURCES += tsigerror.h tsigerror.cc
 libb10_dns___la_SOURCES += tsigkey.h tsigkey.cc
 libb10_dns___la_SOURCES += tsigrecord.h tsigrecord.cc
 libb10_dns___la_SOURCES += character_string.h character_string.cc
-libb10_dns___la_SOURCES += master_loader_callbacks.h
+libb10_dns___la_SOURCES += master_loader_callbacks.h master_loader_callbacks.cc
 libb10_dns___la_SOURCES += master_loader.h
 libb10_dns___la_SOURCES += rdata/generic/detail/char_string.h
 libb10_dns___la_SOURCES += rdata/generic/detail/char_string.cc

src/lib/dns/master_loader.cc

@@ -22,10 +22,10 @@
 #include <dns/rdata.h>
 
 #include <boost/scoped_ptr.hpp>
+#include <boost/algorithm/string/predicate.hpp> // for iequals
 
 #include <string>
 #include <memory>
-#include <boost/algorithm/string/predicate.hpp> // for iequals
 
 using std::string;
 using std::auto_ptr;

src/lib/dns/master_loader_callbacks.cc

@@ -0,0 +1,34 @@
+// 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 <dns/master_loader_callbacks.h>
+
+#include <string>
+
+namespace isc {
+namespace dns {
+
+namespace {
+void
+nullCallback(const std::string&, size_t, const std::string&) {
+}
+}
+
+MasterLoaderCallbacks
+MasterLoaderCallbacks::getNullCallbacks() {
+    return (MasterLoaderCallbacks(nullCallback, nullCallback));
+}
+
+} // end namespace dns
+} // end namespace isc

src/lib/dns/master_loader_callbacks.h

@@ -122,6 +122,16 @@ public:
         warning_(source_name, source_line, reason);
     }
 
+    /// \brief Return a callbacks instance with null callbacks
+    ///
+    /// This is a convenience wrapper to generate a
+    /// \c MasterLoaderCallbacks object with both callbacks being nothing.
+    /// This will be useful for applications that only need to run
+    /// \c MasterLoader and get the end result.
+    ///
+    /// \throw None
+    static MasterLoaderCallbacks getNullCallbacks();
+
 private:
     IssueCallback error_, warning_;
 };

src/lib/dns/rrcollator.cc

@@ -0,0 +1,110 @@
+// 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 this first to check the header is self-contained.
+#include <dns/rrcollator.h>
+
+#include <dns/name.h>
+#include <dns/rdataclass.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+#include <dns/rrttl.h>
+#include <dns/rdata.h>
+#include <dns/rrset.h>
+
+#include <boost/bind.hpp>
+
+#include <algorithm>
+
+namespace isc {
+namespace dns {
+using namespace rdata;
+
+class RRCollator::Impl {
+public:
+    Impl(const AddRRsetCallback& callback) : callback_(callback) {}
+
+    void addRR(const Name& name, const RRClass& rrclass,
+               const RRType& rrtype, const RRTTL& rrttl,
+               const RdataPtr& rdata);
+
+    RRsetPtr current_rrset_;
+    AddRRsetCallback callback_;
+};
+
+namespace {
+inline bool
+isSameType(RRType type1, const ConstRdataPtr& rdata1,
+           const ConstRRsetPtr& rrset)
+{
+    if (type1 != rrset->getType()) {
+        return (false);
+    }
+    if (type1 == RRType::RRSIG()) {
+        RdataIteratorPtr rit = rrset->getRdataIterator();
+        return (dynamic_cast<const generic::RRSIG&>(*rdata1).typeCovered()
+                == dynamic_cast<const generic::RRSIG&>(
+                    rit->getCurrent()).typeCovered());
+    }
+    return (true);
+}
+}
+
+void
+RRCollator::Impl::addRR(const Name& name, const RRClass& rrclass,
+                        const RRType& rrtype, const RRTTL& rrttl,
+                        const RdataPtr& rdata)
+{
+    if (current_rrset_ && (!isSameType(rrtype, rdata, current_rrset_) ||
+                           current_rrset_->getClass() != rrclass ||
+                           current_rrset_->getName() != name)) {
+        callback_(current_rrset_);
+        current_rrset_.reset();
+    }
+
+    if (!current_rrset_) {
+        current_rrset_ = RRsetPtr(new RRset(name, rrclass, rrtype, rrttl));
+    } else if (current_rrset_->getTTL() != rrttl) {
+        // RRs with different TTLs are given.  Smaller TTL should win.
+        current_rrset_->setTTL(std::min(current_rrset_->getTTL(), rrttl));
+    }
+    current_rrset_->addRdata(rdata);
+}
+
+RRCollator::RRCollator(const AddRRsetCallback& callback) :
+    impl_(new Impl(callback))
+{}
+
+RRCollator::~RRCollator() {
+    delete impl_;
+}
+
+AddRRCallback
+RRCollator::getCallback() {
+    return (boost::bind(&RRCollator::Impl::addRR, this->impl_,
+                        _1, _2, _3, _4, _5));
+}
+
+void
+RRCollator::flush() {
+    if (impl_->current_rrset_) {
+        impl_->callback_(impl_->current_rrset_);
+        impl_->current_rrset_.reset();
+    }
+}
+
+} // end namespace dns
+} // end namespace isc

src/lib/dns/rrcollator.h

@@ -0,0 +1,133 @@
+// 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 RRCOLLATOR_H
+#define RRCOLLATOR_H 1
+
+#include <dns/master_loader_callbacks.h>
+#include <dns/rrset.h>
+
+#include <boost/noncopyable.hpp>
+#include <boost/function.hpp>
+
+namespace isc {
+namespace dns {
+
+/// \brief A converter from a stream of RRs to a stream of collated RRsets
+///
+/// This class is mainly intended to be a helper used as an adaptor for
+/// user applications of the \c MasterLoader class; it works as a callback
+/// for \c MasterLoader, buffers given RRs from the loader, collating
+/// consecutive RRs that belong to the same RRset (ones having the same
+/// owner name, RR type and class), and produces a stream of RRsets through
+/// its own callback.  RRSIGs are also separated if their type covered fields
+/// have different values even if the owner name and RR class are the same.
+///
+/// It also "normalizes" TTLs of the RR; if collated RRs have different TTLs,
+/// this class guarantees that the TTL of the resulting RRsets has the
+/// smallest TTL among them.
+///
+/// The conversion will be useful for applications of \c MasterLoader because
+/// many of this library have interfaces that take an RRset object (or
+/// a pointer to it).  Note, however, that this class doesn't guarantee that
+/// all RRs that would belong to the same RRset are collated into the same
+/// single RRset.  In fact, it can only collate RRs that are consecutive
+/// in the original stream; once it encounters an RR of a different RRset,
+/// any subsequent RRs of the previous RRset will form a separate RRset object.
+///
+/// This class is non-copyable; it's partially for the convenience of internal
+/// implementation details, but it actually doesn't make sense to copy
+/// an object of this class, if not harmful, for the intended usage of
+/// the class.
+class RRCollator : boost::noncopyable {
+public:
+    /// \brief Callback functor type for \c RRCollator.
+    ///
+    /// This type of callback is given to an \c RRCollator object on its
+    /// construction, and will be called for each collated RRset built in
+    /// the \c RRCollator.
+    ///
+    /// \param rrset The collated RRset.
+    typedef boost::function<void(const RRsetPtr& rrset)> AddRRsetCallback;
+
+    /// \brief Constructor.
+    ///
+    /// \throw std::bad_alloc Internal memory allocation fails.  This should
+    /// be very rare.
+    ///
+    /// \param callback The callback functor to be called for each collated
+    /// RRset.
+    RRCollator(const AddRRsetCallback& callback);
+
+    /// \brief Destructor.
+    ///
+    /// It only performs trivial internal cleanup.  In particular, even if
+    /// it still has a buffered RRset it will be simply discarded.  This is
+    /// because the given callback could throw an exception, and it's
+    /// impossible to predict how this class is used (to see if it's a very
+    /// rare case where propagating an exception from a destructor is
+    /// justified).  Instead, the application needs to make sure that
+    /// \c flush() is called before the object of this class is destroyed.
+    ///
+    /// \throw None
+    ~RRCollator();
+
+    /// \brief Call the callback on the remaining RRset, if any.
+    ///
+    /// This method is expected to be called that it's supposed all RRs have
+    /// been passed to this class object.  Since there is no explicit
+    /// indicator of the end of the stream, the user of this class needs to
+    /// explicitly call this method to call the callback for the last buffered
+    /// RRset (see also the destructor's description).
+    ///
+    /// If there is no buffered RRset, this method does nothing.  It can happen
+    /// if it's called without receiving any RRs, or called more than once.
+    ///
+    /// It propagates any exception thrown from the callback; otherwise it
+    /// doesn't throw anything.
+    void flush();
+
+    /// \brief Return \c MasterLoader compatible callback.
+    ///
+    /// This method returns a functor in the form of \c AddRRCallback
+    /// that works as an adaptor between \c MasterLoader and an application
+    /// that needs to get a stream of RRsets.  When the returned callback
+    /// is called, this \c RRCollator object accepts the corresponding RR,
+    /// and collates it with other RRs of the same RRset if necessary.
+    /// Every time the \c RRCollator object encounters an RR of a different
+    /// RRset, it calls the callback passed to the constructor with the RRset
+    /// built so far.
+    ///
+    /// Like \c flush(), this \c AddRRCallback functor propagates any exception
+    /// thrown from the callback.
+    ///
+    /// This method is expected to be called only once for a given
+    /// \c RRCollator object.  It doesn't prohibit duplicate calls, but
+    /// returned functor objects internally refer to the same \c RRCollator
+    /// object, and calling the both callbacks randomly will just cause
+    /// confusion.
+    AddRRCallback getCallback();
+
+private:
+    class Impl;
+    Impl* impl_;
+};
+
+} // namespace dns
+} // namespace isc
+#endif  // RRCOLLATOR_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/tests/Makefile.am

@@ -33,6 +33,7 @@ run_unittests_SOURCES += name_unittest.cc
 run_unittests_SOURCES += nsec3hash_unittest.cc
 run_unittests_SOURCES += rrclass_unittest.cc rrtype_unittest.cc
 run_unittests_SOURCES += rrttl_unittest.cc
+run_unittests_SOURCES += rrcollator_unittest.cc
 run_unittests_SOURCES += opcode_unittest.cc
 run_unittests_SOURCES += rcode_unittest.cc
 run_unittests_SOURCES += rdata_unittest.h rdata_unittest.cc

src/lib/dns/tests/master_loader_unittest.cc

@@ -608,5 +608,4 @@ TEST_F(MasterLoaderTest, noEOLN) {
     checkRR("example.org", RRType::SOA(), "ns1.example.org. "
             "admin.example.org. 1234 3600 1800 2419200 7200");
 }
-
 }

src/lib/dns/tests/rdata_txt_like_unittest.cc

@@ -53,17 +53,11 @@ const uint8_t wiredata_txt_like[] = {
 
 const uint8_t wiredata_nulltxt[] = { 0 };
 
-// For lexer-based constructor
-void
-dummyCallback(const string&, size_t, const string&) {
-}
-
 template<class TXT_LIKE>
 class Rdata_TXT_LIKE_Test : public RdataTest {
 protected:
     Rdata_TXT_LIKE_Test() :
-        callback(boost::bind(&dummyCallback, _1, _2, _3)),
-        loader_cb(callback, callback),
+        loader_cb(MasterLoaderCallbacks::getNullCallbacks()),
         wiredata_longesttxt(256, 'a'),
         rdata_txt_like("Test-String"),
         rdata_txt_like_empty("\"\""),
@@ -72,9 +66,6 @@ protected:
         wiredata_longesttxt[0] = 255; // adjust length
     }
 
-private:
-    const MasterLoaderCallbacks::IssueCallback callback;
-
 protected:
     MasterLoaderCallbacks loader_cb;
     vector<uint8_t> wiredata_longesttxt;

src/lib/dns/tests/rdata_unittest.cc

@@ -60,10 +60,6 @@ RdataTest::rdataFactoryFromFile(const RRType& rrtype, const RRClass& rrclass,
 
 namespace test {
 
-void
-dummyCallback(const string&, size_t, const string&) {
-}
-
 RdataPtr
 createRdataUsingLexer(const RRType& rrtype, const RRClass& rrclass,
                       const std::string& str)
@@ -72,9 +68,8 @@ createRdataUsingLexer(const RRType& rrtype, const RRClass& rrclass,
     MasterLexer lexer;
     lexer.pushSource(ss);
 
-    const MasterLoaderCallbacks::IssueCallback callback
-        (boost::bind(&dummyCallback, _1, _2, _3));
-    MasterLoaderCallbacks callbacks(callback, callback);
+    MasterLoaderCallbacks callbacks =
+        MasterLoaderCallbacks::getNullCallbacks();
     const Name origin("example.org.");
 
     return (createRdata(rrtype, rrclass, lexer, &origin,

src/lib/dns/tests/rrcollator_unittest.cc

@@ -0,0 +1,214 @@
+// 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/name.h>
+#include <dns/master_loader.h>
+#include <dns/master_loader_callbacks.h>
+#include <dns/rrclass.h>
+#include <dns/rrcollator.h>
+#include <dns/rdata.h>
+#include <dns/rrset.h>
+#include <dns/rrttl.h>
+
+#include <gtest/gtest.h>
+
+#include <boost/bind.hpp>
+
+#include <sstream>
+#include <vector>
+
+using std::vector;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+
+namespace {
+
+typedef RRCollator::AddRRsetCallback AddRRsetCallback;
+
+void
+addRRset(const RRsetPtr& rrset, vector<ConstRRsetPtr>* to_append,
+         const bool* do_throw) {
+    if (*do_throw) {
+        isc_throw(isc::Unexpected, "faked failure");
+    }
+    to_append->push_back(rrset);
+}
+
+class RRCollatorTest : public ::testing::Test {
+protected:
+    RRCollatorTest() :
+        origin_("example.com"), rrclass_(RRClass::IN()), rrttl_(3600),
+        throw_from_callback_(false),
+        collator_(boost::bind(addRRset, _1, &rrsets_, &throw_from_callback_)),
+        rr_callback_(collator_.getCallback()),
+        a_rdata1_(createRdata(RRType::A(), rrclass_, "192.0.2.1")),
+        a_rdata2_(createRdata(RRType::A(), rrclass_, "192.0.2.2")),
+        txt_rdata_(createRdata(RRType::TXT(), rrclass_, "test")),
+        sig_rdata1_(createRdata(RRType::RRSIG(), rrclass_,
+                                "A 5 3 3600 20000101000000 20000201000000 "
+                                "12345 example.com. FAKE\n")),
+        sig_rdata2_(createRdata(RRType::RRSIG(), rrclass_,
+                                "NS 5 3 3600 20000101000000 20000201000000 "
+                                "12345 example.com. FAKE\n"))
+    {}
+
+    void checkRRset(const Name& expected_name, const RRClass& expected_class,
+                    const RRType& expected_type, const RRTTL& expected_ttl,
+                    const vector<ConstRdataPtr>& expected_rdatas) {
+        SCOPED_TRACE(expected_name.toText(true) + "/" +
+                     expected_class.toText() + "/" + expected_type.toText());
+
+        // This test always clears rrsets_ to confirm RRsets are added
+        // one-by-one
+        ASSERT_EQ(1, rrsets_.size());
+
+        ConstRRsetPtr actual = rrsets_[0];
+        EXPECT_EQ(expected_name, actual->getName());
+        EXPECT_EQ(expected_class, actual->getClass());
+        EXPECT_EQ(expected_type, actual->getType());
+        EXPECT_EQ(expected_ttl, actual->getTTL());
+        ASSERT_EQ(expected_rdatas.size(), actual->getRdataCount());
+        vector<ConstRdataPtr>::const_iterator it = expected_rdatas.begin();
+        for (RdataIteratorPtr rit = actual->getRdataIterator();
+             !rit->isLast();
+             rit->next()) {
+            EXPECT_EQ(0, rit->getCurrent().compare(**it));
+            ++it;
+        }
+
+        rrsets_.clear();
+    }
+
+    const Name origin_;
+    const RRClass rrclass_;
+    const RRTTL rrttl_;
+    vector<ConstRRsetPtr> rrsets_;
+    bool throw_from_callback_;
+    RRCollator collator_;
+    AddRRCallback rr_callback_;
+    const RdataPtr a_rdata1_, a_rdata2_, txt_rdata_, sig_rdata1_, sig_rdata2_;
+    vector<ConstRdataPtr> rdatas_; // placeholder for expected data
+};
+
+TEST_F(RRCollatorTest, basicCases) {
+    // Add two RRs belonging to the same RRset.  These will be buffered.
+    rr_callback_(origin_, rrclass_, RRType::A(), rrttl_, a_rdata1_);
+    EXPECT_TRUE(rrsets_.empty()); // not yet given as an RRset
+    rr_callback_(origin_, rrclass_, RRType::A(), rrttl_, a_rdata2_);
+    EXPECT_TRUE(rrsets_.empty()); // still not given
+
+    // Add another type of RR.  This completes the construction of the A RRset,
+    // which will be given via the callback.
+    rr_callback_(origin_, rrclass_, RRType::TXT(), rrttl_, txt_rdata_);
+    rdatas_.push_back(a_rdata1_);
+    rdatas_.push_back(a_rdata2_);
+    checkRRset(origin_, rrclass_, RRType::A(), rrttl_, rdatas_);
+
+    // Add the same type of RR but of different name.  This should make another
+    // callback for the previous TXT RR.
+    rr_callback_(Name("txt.example.com"), rrclass_, RRType::TXT(), rrttl_,
+                 txt_rdata_);
+    rdatas_.clear();
+    rdatas_.push_back(txt_rdata_);
+    checkRRset(origin_, rrclass_, RRType::TXT(), rrttl_, rdatas_);
+
+    // Add the same type and name of RR but of different class (rare case
+    // in practice)
+    rr_callback_(Name("txt.example.com"), RRClass::CH(), RRType::TXT(), rrttl_,
+                 txt_rdata_);
+    rdatas_.clear();
+    rdatas_.push_back(txt_rdata_);
+    checkRRset(Name("txt.example.com"), rrclass_, RRType::TXT(), rrttl_,
+               rdatas_);
+
+    // Tell the collator we are done, then we'll see the last RR as an RRset.
+    collator_.flush();
+    checkRRset(Name("txt.example.com"), RRClass::CH(), RRType::TXT(), rrttl_,
+               rdatas_);
+
+    // Redundant flush() will be no-op.
+    collator_.flush();
+    EXPECT_TRUE(rrsets_.empty());
+}
+
+TEST_F(RRCollatorTest, minTTLFirst) {
+    // RRs of the same RRset but has different TTLs.  The first RR has
+    // the smaller TTL, which should be used for the TTL of the RRset.
+    rr_callback_(origin_, rrclass_, RRType::A(), RRTTL(10), a_rdata1_);
+    rr_callback_(origin_, rrclass_, RRType::A(), RRTTL(20), a_rdata2_);
+    rdatas_.push_back(a_rdata1_);
+    rdatas_.push_back(a_rdata2_);
+    collator_.flush();
+    checkRRset(origin_, rrclass_, RRType::A(), RRTTL(10), rdatas_);
+}
+
+TEST_F(RRCollatorTest, maxTTLFirst) {
+    // RRs of the same RRset but has different TTLs.  The second RR has
+    // the smaller TTL, which should be used for the TTL of the RRset.
+    rr_callback_(origin_, rrclass_, RRType::A(), RRTTL(20), a_rdata1_);
+    rr_callback_(origin_, rrclass_, RRType::A(), RRTTL(10), a_rdata2_);
+    rdatas_.push_back(a_rdata1_);
+    rdatas_.push_back(a_rdata2_);
+    collator_.flush();
+    checkRRset(origin_, rrclass_, RRType::A(), RRTTL(10), rdatas_);
+}
+
+TEST_F(RRCollatorTest, addRRSIGs) {
+    // RRSIG is special; they are also distinguished by their covered types.
+    rr_callback_(origin_, rrclass_, RRType::RRSIG(), rrttl_, sig_rdata1_);
+    rr_callback_(origin_, rrclass_, RRType::RRSIG(), rrttl_, sig_rdata2_);
+
+    rdatas_.push_back(sig_rdata1_);
+    checkRRset(origin_, rrclass_, RRType::RRSIG(), rrttl_, rdatas_);
+}
+
+TEST_F(RRCollatorTest, emptyFlush) {
+    collator_.flush();
+    EXPECT_TRUE(rrsets_.empty());
+}
+
+TEST_F(RRCollatorTest, throwFromCallback) {
+    // Adding an A RR
+    rr_callback_(origin_, rrclass_, RRType::A(), rrttl_, a_rdata1_);
+
+    // Adding a TXT RR, which would trigger RRset callback, but in this test
+    // it throws.  The added TXT RR will be effectively lost.
+    throw_from_callback_ = true;
+    EXPECT_THROW(rr_callback_(origin_, rrclass_, RRType::TXT(), rrttl_,
+                              txt_rdata_), isc::Unexpected);
+
+    // We'll only see the A RR.
+    throw_from_callback_ = false;
+    collator_.flush();
+    rdatas_.push_back(a_rdata1_);
+    checkRRset(origin_, rrclass_, RRType::A(), rrttl_, rdatas_);
+}
+
+TEST_F(RRCollatorTest, withMasterLoader) {
+    // Test a simple case with MasterLoader.  There shouldn't be anything
+    // special, but that's the mainly intended usage of the collator, so we
+    // check it explicitly.
+    std::istringstream ss("example.com. 3600 IN A 192.0.2.1\n");
+    MasterLoader loader(ss, origin_, rrclass_,
+                        MasterLoaderCallbacks::getNullCallbacks(),
+                        collator_.getCallback());
+    loader.load();
+    collator_.flush();
+    rdatas_.push_back(a_rdata1_);
+    checkRRset(origin_, rrclass_, RRType::A(), rrttl_, rdatas_);
+}
+
+}

src/lib/dns/tests/rrparamregistry_unittest.cc

@@ -157,10 +157,6 @@ TEST_F(RRParamRegistryTest, addRemoveFactory) {
                      RRType(test_type_code)));
 }
 
-void
-dummyCallback(const string&, size_t, const string&) {
-}
-
 RdataPtr
 createRdataHelper(const std::string& str) {
     boost::scoped_ptr<AbstractRdataFactory> rdf(new TestRdataFactory);
@@ -169,9 +165,7 @@ createRdataHelper(const std::string& str) {
     MasterLexer lexer;
     lexer.pushSource(ss);
 
-    const MasterLoaderCallbacks::IssueCallback callback
-        (boost::bind(&dummyCallback, _1, _2, _3));
-    MasterLoaderCallbacks callbacks(callback, callback);
+    MasterLoaderCallbacks callbacks(MasterLoaderCallbacks::getNullCallbacks());
     const Name origin("example.org.");
 
     return (rdf->create(lexer, &origin,