BIND 10 trac2470, updated. 92ac02b80cb94846eff4cd136d5c426526f5145a [2470] Revert "[2470] added a framework to report different TTLs via an optional callback."

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Dec 18 08:14:56 UTC 2012


The branch, trac2470 has been updated
       via  92ac02b80cb94846eff4cd136d5c426526f5145a (commit)
       via  2a8f04d9e437cc209ebb0921d0269ac4fdf3c760 (commit)
      from  15d0b77ba78bc2507b65072a74294a85ee7e5440 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 92ac02b80cb94846eff4cd136d5c426526f5145a
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Dec 18 00:14:57 2012 -0800

    [2470] Revert "[2470] added a framework to report different TTLs via an optional callback."
    
    This reverts commit 2c50eecd72f4e59aaea65cab950ae058179ae1bd.
    
    Conflicts:
    	src/lib/dns/rrcollator.cc
    	src/lib/dns/rrcollator.h

commit 2a8f04d9e437cc209ebb0921d0269ac4fdf3c760
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Dec 18 00:10:32 2012 -0800

    [2470] Revert "[2470] provide RR collator with issue callbacks"
    
    This reverts commit 822fa31524fc37af1ffdd265f8554fb9d85471f1.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/datasrc/memory/zone_data_loader.cc |    8 ++--
 src/lib/dns/rrcollator.cc                  |   24 ++----------
 src/lib/dns/rrcollator.h                   |   22 +----------
 src/lib/dns/tests/rrcollator_unittest.cc   |   57 ++--------------------------
 4 files changed, 14 insertions(+), 97 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/zone_data_loader.cc b/src/lib/datasrc/memory/zone_data_loader.cc
index 0238093..e224224 100644
--- a/src/lib/datasrc/memory/zone_data_loader.cc
+++ b/src/lib/datasrc/memory/zone_data_loader.cc
@@ -193,12 +193,12 @@ masterLoaderWrapper(const char* const filename, const Name& origin,
                     const RRClass& zone_class, LoadCallback callback)
 {
     bool load_ok = false;       // (we don't use it)
-    const MasterLoaderCallbacks issue_callbacks =
-        createMasterLoaderCallbacks(origin, zone_class, &load_ok);
-    dns::RRCollator collator(boost::bind(callback, _1), issue_callbacks);
+    dns::RRCollator collator(boost::bind(callback, _1));
 
     try {
-        dns::MasterLoader(filename, origin, zone_class, issue_callbacks,
+        dns::MasterLoader(filename, origin, zone_class,
+                          createMasterLoaderCallbacks(origin, zone_class,
+                                                      &load_ok),
                           collator.getCallback()).load();
         collator.flush();
     } catch (const dns::MasterLoaderError& e) {
diff --git a/src/lib/dns/rrcollator.cc b/src/lib/dns/rrcollator.cc
index d8cc55b..4b12222 100644
--- a/src/lib/dns/rrcollator.cc
+++ b/src/lib/dns/rrcollator.cc
@@ -35,14 +35,7 @@ using namespace rdata;
 
 class RRCollator::Impl {
 public:
-    Impl(const AddRRsetCallback& callback,
-         const MasterLoaderCallbacks& issue_callback) :
-        callback_(callback), issue_callback_(issue_callback)
-    {
-        if (!callback_) {
-            isc_throw(InvalidParameter, "Empty add RRset callback");
-        }
-    }
+    Impl(const AddRRsetCallback& callback) : callback_(callback) {}
 
     void addRR(const Name& name, const RRClass& rrclass,
                const RRType& rrtype, const RRTTL& rrttl,
@@ -50,8 +43,6 @@ public:
 
     RRsetPtr current_rrset_;
     AddRRsetCallback callback_;
-private:
-    MasterLoaderCallbacks issue_callback_;
 };
 
 namespace {
@@ -88,20 +79,13 @@ RRCollator::Impl::addRR(const Name& name, const RRClass& rrclass,
         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.
-        const RRTTL min_ttl(std::min(current_rrset_->getTTL(), rrttl));
-        issue_callback_.warning("<unknown source>", 0,
-                                "Different TTLs for the same RRset: " +
-                                name.toText(true) + "/" +
-                                rrclass.toText() + "/" + rrtype.toText() +
-                                ", set to " + min_ttl.toText());
-        current_rrset_->setTTL(min_ttl);
+        current_rrset_->setTTL(std::min(current_rrset_->getTTL(), rrttl));
     }
     current_rrset_->addRdata(rdata);
 }
 
-RRCollator::RRCollator(const AddRRsetCallback& callback,
-                       const MasterLoaderCallbacks& issue_callback) :
-    impl_(new Impl(callback, issue_callback))
+RRCollator::RRCollator(const AddRRsetCallback& callback) :
+    impl_(new Impl(callback))
 {}
 
 RRCollator::~RRCollator() {
diff --git a/src/lib/dns/rrcollator.h b/src/lib/dns/rrcollator.h
index 97b7642..3a9e0aa 100644
--- a/src/lib/dns/rrcollator.h
+++ b/src/lib/dns/rrcollator.h
@@ -63,30 +63,12 @@ public:
 
     /// \brief Constructor.
     ///
-    /// \c callback must not be an empty functor.
-    ///
-    /// If the optional issue_callback parameter is given, it will be used
-    /// to report any errors and non fatal warnings found in the collator's
-    /// operation.  By default special callbacks that do nothing are used.
-    ///
-    /// \note Since the \c RRCollator does not have any information on the
-    /// source of the given RRs (which is normally a DNS master file in the
-    /// intended usage) it cannot provide the actual source name or the line
-    /// number via the callback.  Instead, it passes a special string of
-    /// "<unknown source>" for the source name and the line number of 0 via
-    /// the callback.
-    ///
-    /// \throw isc::InvalidParameter Empty RRset callback is given.
     /// \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.
-    /// \param issue_callback The callbacks to be called on any issue found
-    /// in the collator.
-    RRCollator(const AddRRsetCallback& callback,
-               const MasterLoaderCallbacks& issue_callback =
-               MasterLoaderCallbacks::getNullCallbacks());
+    RRCollator(const AddRRsetCallback& callback);
 
     /// \brief Destructor.
     ///
@@ -103,7 +85,7 @@ public:
 
     /// \brief Call the callback on the remaining RRset, if any.
     ///
-    /// This method is expected to be called when it's supposed all RRs have
+    /// 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
diff --git a/src/lib/dns/tests/rrcollator_unittest.cc b/src/lib/dns/tests/rrcollator_unittest.cc
index 155edb9..e66f87c 100644
--- a/src/lib/dns/tests/rrcollator_unittest.cc
+++ b/src/lib/dns/tests/rrcollator_unittest.cc
@@ -25,16 +25,12 @@
 
 #include <gtest/gtest.h>
 
-#include <boost/lexical_cast.hpp>
 #include <boost/bind.hpp>
 
-#include <string>
 #include <sstream>
 #include <vector>
 
-using std::string;
 using std::vector;
-using boost::lexical_cast;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 
@@ -54,14 +50,9 @@ addRRset(const RRsetPtr& rrset, vector<ConstRRsetPtr>* to_append,
 class RRCollatorTest : public ::testing::Test {
 protected:
     RRCollatorTest() :
-        issue_callbacks_(boost::bind(&RRCollatorTest::issueCallback, this,
-                                     _1, _2, _3, true),
-                         boost::bind(&RRCollatorTest::issueCallback, this,
-                                     _1, _2, _3, false)),
         origin_("example.com"), rrclass_(RRClass::IN()), rrttl_(3600),
         throw_from_callback_(false),
-        collator_(boost::bind(addRRset, _1, &rrsets_, &throw_from_callback_),
-                  issue_callbacks_),
+        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")),
@@ -74,12 +65,6 @@ protected:
                                 "12345 example.com. FAKE\n"))
     {}
 
-    virtual void TearDown() {
-        // (The current implementation of) RRCollator should never report an
-        // error.
-        EXPECT_TRUE(errors_.empty());
-    }
-
     void checkRRset(const Name& expected_name, const RRClass& expected_class,
                     const RRType& expected_type, const RRTTL& expected_ttl,
                     const vector<ConstRdataPtr>& expected_rdatas) {
@@ -107,20 +92,6 @@ protected:
         rrsets_.clear();
     }
 
-    void issueCallback(const string& src_name, size_t src_line,
-                       const string& reason, bool is_error) {
-        const string msg(src_name + ":" + lexical_cast<string>(src_line) +
-                         ": " + reason);
-        if (is_error) {
-            errors_.push_back(msg);
-        } else {
-            warnings_.push_back(msg);
-        }
-    }
-
-private:
-    MasterLoaderCallbacks issue_callbacks_;
-protected:
     const Name origin_;
     const RRClass rrclass_;
     const RRTTL rrttl_;
@@ -130,7 +101,6 @@ protected:
     AddRRCallback rr_callback_;
     const RdataPtr a_rdata1_, a_rdata2_, txt_rdata_, sig_rdata1_, sig_rdata2_;
     vector<ConstRdataPtr> rdatas_; // placeholder for expected data
-    vector<string> warnings_, errors_;
 };
 
 TEST_F(RRCollatorTest, basicCases) {
@@ -164,9 +134,6 @@ TEST_F(RRCollatorTest, basicCases) {
     checkRRset(Name("txt.example.com"), rrclass_, RRType::TXT(), rrttl_,
                rdatas_);
 
-    // There should have been no warnings.
-    EXPECT_EQ(0, warnings_.size());
-
     // 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_,
@@ -180,16 +147,12 @@ TEST_F(RRCollatorTest, basicCases) {
 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.
-    // There should be a warning callback about this.
     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_);
-    EXPECT_EQ(1, warnings_.size());
-    EXPECT_EQ("<unknown source>:0: Different TTLs for the same RRset: "
-              "example.com/IN/A, set to 10", warnings_.at(0));
 }
 
 TEST_F(RRCollatorTest, maxTTLFirst) {
@@ -201,9 +164,6 @@ TEST_F(RRCollatorTest, maxTTLFirst) {
     rdatas_.push_back(a_rdata2_);
     collator_.flush();
     checkRRset(origin_, rrclass_, RRType::A(), RRTTL(10), rdatas_);
-    EXPECT_EQ(1, warnings_.size());
-    EXPECT_EQ("<unknown source>:0: Different TTLs for the same RRset: "
-              "example.com/IN/A, set to 10", warnings_.at(0));
 }
 
 TEST_F(RRCollatorTest, addRRSIGs) {
@@ -237,25 +197,16 @@ TEST_F(RRCollatorTest, throwFromCallback) {
     checkRRset(origin_, rrclass_, RRType::A(), rrttl_, rdatas_);
 }
 
-TEST_F(RRCollatorTest, emptyCallback) {
-    const AddRRsetCallback empty_callback;
-    EXPECT_THROW(RRCollator collator(empty_callback), isc::InvalidParameter);
-}
-
 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.  Also, this test uses a different local collator,
-    // just for checking it works with omitting the issue callback (using
-    // the default).
-    RRCollator collator(boost::bind(addRRset, _1, &rrsets_,
-                                    &throw_from_callback_));
+    // 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());
+                        collator_.getCallback());
     loader.load();
-    collator.flush();
+    collator_.flush();
     rdatas_.push_back(a_rdata1_);
     checkRRset(origin_, rrclass_, RRType::A(), rrttl_, rdatas_);
 }



More information about the bind10-changes mailing list