[2573] count added RR within datasrc loader, and deprecate MasterLoader method

in fact, the master loader's method is redundant because its user can count
them the same way.  its own method might be of some use in the future,
but until then I'd simpy remove it based on YAGNI.

Also made some unrelated cleanup based on recent clarification of
null tests on pointer/shared pointers.

src/lib/datasrc/zone_loader.cc

@@ -25,6 +25,8 @@
 #include <dns/master_loader.h>
 #include <dns/master_lexer.h>
 
+#include <boost/bind.hpp>
+
 using isc::dns::Name;
 using isc::dns::ConstRRsetPtr;
 using isc::dns::MasterLoader;
@@ -62,6 +64,21 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
     }
 }
 
+namespace {
+// Unified callback to install RR and increment RR count at the same time.
+void
+addRR(const dns::Name& name, const dns::RRClass& rrclass,
+      const dns::RRType& type, const dns::RRTTL& ttl,
+      const dns::rdata::RdataPtr& data, ZoneUpdater* updater,
+      size_t* rr_count)
+{
+    isc::dns::BasicRRset rrset(name, rrclass, type, ttl);
+    rrset.addRdata(data);
+    updater->addRRset(rrset);
+    ++*rr_count;
+}
+}
+
 ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
                        const char* filename) :
     updater_(destination.getUpdater(zone_name, true, false)),
@@ -79,7 +96,8 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
                                    createMasterLoaderCallbacks(zone_name,
                                        updater_->getFinder().getClass(),
                                        &loaded_ok_),
-                                   createMasterLoaderAddCallback(*updater_)));
+                                   boost::bind(addRR, _1, _2, _3, _4, _5,
+                                               updater_.get(), &rr_count_)));
     }
 }
 
@@ -114,8 +132,8 @@ ZoneLoader::loadIncremental(size_t limit) {
                   "Loading has been completed previously");
     }
 
-    if (iterator_ == ZoneIteratorPtr()) {
-        assert(loader_.get() != NULL);
+    if (!iterator_) {
+        assert(loader_);
         try {
             complete_ = loader_->loadIncremental(limit);
         } catch (const isc::dns::MasterLoaderError& e) {
@@ -124,7 +142,6 @@ ZoneLoader::loadIncremental(size_t limit) {
         if (complete_ && !loaded_ok_) {
             isc_throw(MasterFileError, "Error while loading master file");
         }
-        rr_count_ = loader_->getRRCount();
     } else {
         complete_ = copyRRsets(updater_, iterator_, limit, rr_count_);
     }

src/lib/dns/master_loader.cc

@@ -638,11 +638,6 @@ MasterLoader::loadedSucessfully() const {
 }
 
 size_t
-MasterLoader::getRRCount() const {
-    return (impl_->rr_count_);
-}
-
-size_t
 MasterLoader::getSize() const {
     return (impl_->getSize());
 }

src/lib/dns/master_loader.h

@@ -142,16 +142,6 @@ public:
     ///     finishing the load.
     bool loadedSucessfully() const;
 
-    /// \brief Return the number of RRs loaded.
-    ///
-    /// This method returns the number of RRs loaded via this loader by the
-    /// time of the call.  Before starting the load it will return 0.
-    /// It will return the total number of RRs of the zone on and after
-    /// completing the load.
-    ///
-    /// \throw None
-    size_t getRRCount() const;
-
     /// \brief Return the total size of the zone files and streams.
     ///
     /// This method returns the size of the source of the zone to be loaded

src/lib/dns/tests/master_loader_unittest.cc

@@ -156,7 +156,6 @@ TEST_F(MasterLoaderTest, basicLoad) {
 
     // The following three should be set to 0 initially in case the loader
     // is constructed from a file name.
-    EXPECT_EQ(0, loader_->getRRCount());
     EXPECT_EQ(0, loader_->getSize());
     EXPECT_EQ(0, loader_->getPosition());
 
@@ -168,7 +167,6 @@ TEST_F(MasterLoaderTest, basicLoad) {
 
     // Hardcode expected values taken from the test data file, assuming it
     // won't change too often.
-    EXPECT_EQ(4, loader_->getRRCount());
     EXPECT_EQ(549, loader_->getSize());
     EXPECT_EQ(549, loader_->getPosition());
 
@@ -323,7 +321,6 @@ TEST_F(MasterLoaderTest, popAfterError) {
     EXPECT_FALSE(loader_->loadedSucessfully());
     EXPECT_EQ(1, errors_.size()); // For the broken RR
     EXPECT_EQ(1, warnings_.size()); // For missing EOLN
-    EXPECT_EQ(1, loader_->getRRCount()); // broken RR shouldn't be counted
 
     // The included file doesn't contain anything usable, but the
     // line after the include should be there.
@@ -342,7 +339,6 @@ TEST_F(MasterLoaderTest, streamConstructor) {
     // Unlike the basicLoad test, if we construct the loader from a stream
     // getSize() returns the data size in the stream immediately after the
     // construction.
-    EXPECT_EQ(0, loader_->getRRCount());
     EXPECT_EQ(zone_data.size(), loader_->getSize());
     EXPECT_EQ(0, loader_->getPosition());
 
@@ -356,9 +352,7 @@ TEST_F(MasterLoaderTest, streamConstructor) {
     checkRR("correct.example.org", RRType::A(), "192.0.2.2");
 
     // On completion of the load, both getSize() and getPosition() return the
-    // size of the data, and getRRCount() returns the number of loaded RRs,
-    // which is 2 in this case.
-    EXPECT_EQ(2, loader_->getRRCount());
+    // size of the data.
     EXPECT_EQ(zone_data.size(), loader_->getSize());
     EXPECT_EQ(zone_data.size(), loader_->getPosition());
 }
@@ -368,10 +362,8 @@ TEST_F(MasterLoaderTest, incrementalLoad) {
     setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
               RRClass::IN(), MasterLoader::MANY_ERRORS);
 
-    EXPECT_EQ(0, loader_->getRRCount()); // initial count should be 2
     EXPECT_FALSE(loader_->loadedSucessfully());
     EXPECT_FALSE(loader_->loadIncremental(2));
-    EXPECT_EQ(2, loader_->getRRCount()); // number of loaded RR so far
     EXPECT_FALSE(loader_->loadedSucessfully());
 
     EXPECT_TRUE(errors_.empty());
@@ -394,8 +386,6 @@ TEST_F(MasterLoaderTest, incrementalLoad) {
 
     checkRR("www.example.org", RRType::A(), "192.0.2.1");
     checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
-
-    EXPECT_EQ(4, loader_->getRRCount()); // on completion it's # of zone's RRs.
 }
 
 // Try loading from file that doesn't exist. There should be single error