diff --git a/src/lib/dns/rrcollator.cc b/src/lib/dns/rrcollator.cc index d8cc55b1e107d39ed3f6447dc45710bf7b93af7b..4b1222204ab4d8d0180dc1dadb875a9cf4586004 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("", 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 97b764261b408ba01d9f580462ea7bcab45239a9..3a9e0aa019b410ba4937df2463951c75a5c9295f 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 - /// "" 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 155edb92ebac33e4c796c32f71e8786b976d814b..e66f87c03a23719e56b64de3179311fb8e2704a4 100644 --- a/src/lib/dns/tests/rrcollator_unittest.cc +++ b/src/lib/dns/tests/rrcollator_unittest.cc @@ -25,16 +25,12 @@ #include -#include #include -#include #include #include -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* 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& 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(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 rdatas_; // placeholder for expected data - vector 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(":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(":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_); }