[master] Merge branch 'trac2573'

fixed Conflicts:
	src/lib/datasrc/zone_loader.cc

src/lib/datasrc/tests/zone_loader_unittest.cc

@@ -246,6 +246,11 @@ TEST_F(ZoneLoaderTest, copyUnsigned) {
     // It gets the updater directly in the constructor
     ASSERT_EQ(1, destination_client_.provided_updaters_.size());
     EXPECT_EQ(Name::ROOT_NAME(), destination_client_.provided_updaters_[0]);
+
+    // Counter is initialized to 0, progress is "unknown" in case of copy.
+    EXPECT_EQ(0, loader.getRRCount());
+    EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
+
     // Now load the whole zone
     loader.load();
     EXPECT_TRUE(destination_client_.commit_called_);
@@ -254,6 +259,12 @@ TEST_F(ZoneLoaderTest, copyUnsigned) {
 
     // The count is 34 because we expect the RRs to be separated.
     EXPECT_EQ(34, destination_client_.rrsets_.size());
+
+    // Check various counters.  getRRCount should be identical of the RRs
+    // we've seen. Progress is still "unknown" in the copy operation.
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
+
     // Ensure known order.
     std::sort(destination_client_.rrset_texts_.begin(),
               destination_client_.rrset_texts_.end());
@@ -281,6 +292,11 @@ TEST_F(ZoneLoaderTest, copyUnsignedIncremental) {
     // Not committed yet, we didn't complete the loading
     EXPECT_FALSE(destination_client_.commit_called_);
 
+    // Check we can get intermediate counters. Progress is always "unknown"
+    // in case of copy.
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    EXPECT_EQ(ZoneLoader::PROGRESS_UNKNOWN, loader.getProgress());
+
     // This is unusual, but allowed. Check it doesn't do anything
     loader.loadIncremental(0);
     EXPECT_EQ(10, destination_client_.rrsets_.size());
@@ -349,6 +365,11 @@ TEST_F(ZoneLoaderTest, classMismatch) {
 TEST_F(ZoneLoaderTest, loadUnsigned) {
     ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
                       TEST_DATA_DIR "/root.zone");
+
+    // Counter and progress are initialized to 0.
+    EXPECT_EQ(0, loader.getRRCount());
+    EXPECT_EQ(0, loader.getProgress());
+
     // It gets the updater directly in the constructor
     ASSERT_EQ(1, destination_client_.provided_updaters_.size());
     EXPECT_EQ(Name::ROOT_NAME(), destination_client_.provided_updaters_[0]);
@@ -360,6 +381,12 @@ TEST_F(ZoneLoaderTest, loadUnsigned) {
 
     // The count is 34 because we expect the RRs to be separated.
     EXPECT_EQ(34, destination_client_.rrsets_.size());
+
+    // getRRCount should be identical of the RRs we've seen.  progress
+    // should reach 100% (= 1).
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    EXPECT_EQ(1, loader.getProgress());
+
     // Ensure known order.
     std::sort(destination_client_.rrset_texts_.begin(),
               destination_client_.rrset_texts_.end());
@@ -380,6 +407,10 @@ TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
     ZoneLoader loader(destination_client_, Name::ROOT_NAME(),
                       TEST_DATA_DIR "/root.zone");
 
+    // Counters are initialized to 0.
+    EXPECT_EQ(0, loader.getRRCount());
+    EXPECT_EQ(0, loader.getProgress());
+
     // Try loading few RRs first.
     loader.loadIncremental(10);
     // We should get the 10 we asked for
@@ -390,11 +421,26 @@ TEST_F(ZoneLoaderTest, loadUnsignedIncremental) {
     EXPECT_EQ(10, destination_client_.rrsets_.size());
     EXPECT_FALSE(destination_client_.commit_called_);
 
+    // Check we can get intermediate counters. Expected progress is calculated
+    // based on the size of the zone file and the offset to the end of 10th RR
+    // (subject to future changes to the file, but we assume it's a rare
+    // event.).  The expected value should be the exact expression that
+    // getProgress() should do internally, so EXPECT_EQ() should work here,
+    // but floating-point comparison can be always tricky we use
+    // EXPECT_DOUBLE_EQ just in case.
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    // file size = 1541, offset = 428 (27.77%).
+    EXPECT_DOUBLE_EQ(static_cast<double>(428) / 1541, loader.getProgress());
+
     // We can finish the rest
     loader.loadIncremental(30);
     EXPECT_EQ(34, destination_client_.rrsets_.size());
     EXPECT_TRUE(destination_client_.commit_called_);
 
+    // Counters are updated accordingly. Progress should reach 100%.
+    EXPECT_EQ(destination_client_.rrsets_.size(), loader.getRRCount());
+    EXPECT_EQ(1, loader.getProgress());
+
     // No more loading now
     EXPECT_THROW(loader.load(), isc::InvalidOperation);
     EXPECT_THROW(loader.loadIncremental(1), isc::InvalidOperation);

src/lib/datasrc/zone_loader.cc

@@ -34,17 +34,20 @@
 using isc::dns::Name;
 using isc::dns::ConstRRsetPtr;
 using isc::dns::MasterLoader;
+using isc::dns::MasterLexer;
 
 namespace isc {
 namespace datasrc {
 
+const double ZoneLoader::PROGRESS_UNKNOWN = -1;
+
 ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
                        DataSourceClient& source) :
     // Separate the RRsets as that is possibly faster (the data source doesn't
     // have to aggregate them) and also because our limit semantics.
     iterator_(source.getIterator(zone_name, true)),
     updater_(destination.getUpdater(zone_name, true, false)),
-    complete_(false)
+    complete_(false), rr_count_(0)
 {
     // The getIterator should never return NULL. So we check it.
     // Or should we throw instead?
@@ -65,11 +68,25 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
     }
 }
 
+namespace {
+// Unified callback to install RR and increment RR count at the same time.
+void
+addRR(ZoneUpdater* updater, size_t* rr_count,
+      const dns::Name& name, const dns::RRClass& rrclass,
+      const dns::RRType& type, const dns::RRTTL& ttl,
+      const dns::rdata::RdataPtr& data)
+{
+    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)),
-    complete_(false),
-    loaded_ok_(true)
+    complete_(false), loaded_ok_(true), rr_count_(0)
 {
     if (updater_ == ZoneUpdaterPtr()) {
         isc_throw(DataSourceError, "Zone " << zone_name << " not found in "
@@ -83,7 +100,9 @@ ZoneLoader::ZoneLoader(DataSourceClient& destination, const Name& zone_name,
                                    createMasterLoaderCallbacks(zone_name,
                                        updater_->getFinder().getClass(),
                                        &loaded_ok_),
-                                   createMasterLoaderAddCallback(*updater_)));
+                                   boost::bind(addRR,
+                                               updater_.get(), &rr_count_,
+                                               _1, _2, _3, _4, _5)));
     }
 }
 
@@ -92,7 +111,7 @@ namespace {
 // Copy up to limit RRsets from source to destination
 bool
 copyRRsets(const ZoneUpdaterPtr& destination, const ZoneIteratorPtr& source,
-           size_t limit)
+           size_t limit, size_t& rr_count_)
 {
     size_t loaded = 0;
     while (loaded < limit) {
@@ -104,6 +123,7 @@ copyRRsets(const ZoneUpdaterPtr& destination, const ZoneIteratorPtr& source,
             destination->addRRset(*rrset);
         }
         ++loaded;
+        rr_count_ += rrset->getRdataCount();
     }
     return (false); // Not yet, there may be more
 }
@@ -133,8 +153,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) {
@@ -144,7 +164,7 @@ ZoneLoader::loadIncremental(size_t limit) {
             isc_throw(MasterFileError, "Error while loading master file");
         }
     } else {
-        complete_ = copyRRsets(updater_, iterator_, limit);
+        complete_ = copyRRsets(updater_, iterator_, limit, rr_count_);
     }
 
     if (complete_) {
@@ -166,5 +186,36 @@ ZoneLoader::loadIncremental(size_t limit) {
     return (complete_);
 }
 
+size_t
+ZoneLoader::getRRCount() const {
+    return (rr_count_);
+}
+
+double
+ZoneLoader::getProgress() const {
+    if (!loader_) {
+        return (PROGRESS_UNKNOWN);
+    }
+
+    const size_t pos = loader_->getPosition();
+    const size_t total_size = loader_->getSize();
+
+    // If the current position is 0, progress should definitely be 0; we
+    // don't bother to check the total size even if it's "unknown".
+    if (pos == 0) {
+        return (0);
+    }
+
+    // These cases shouldn't happen with our usage of MasterLoader.  So, in
+    // theory, we could throw here; however, since this method is expected
+    // to be used for informational purposes only, that's probably too harsh.
+    // So we return "unknown" instead.
+    if (total_size == MasterLexer::SOURCE_SIZE_UNKNOWN || total_size == 0) {
+        return (PROGRESS_UNKNOWN);
+    }
+
+    return (static_cast<double>(pos) / total_size);
+}
+
 } // end namespace datasrc
 } // end namespace isc

src/lib/datasrc/zone_loader.h

@@ -154,10 +154,66 @@ public:
     /// \throw ZoneContentError when the zone doesn't pass sanity check.
     /// \note If the limit is exactly the number of RRs available to be loaded,
     ///     the method still returns false and true'll be returned on the next
-    ///     call (which will load 0 RRs). This is because the end of iterator or
-    ///     master file is detected when reading past the end, not when the last
-    ///     one is read.
+    ///     call (which will load 0 RRs). This is because the end of iterator
+    ///     or master file is detected when reading past the end, not when the
+    ///     last one is read.
     bool loadIncremental(size_t limit);
+
+    /// \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 current progress of the loader.
+    ///
+    /// This method returns the current estimated progress of loader as a
+    /// value between 0 and 1 (inclusive); it's 0 before starting the load,
+    /// and 1 at the completion, and a value between these (exclusive) in the
+    /// middle of loading.  It's an implementation detail how to calculate
+    /// the progress, which may vary depending on how the loader is
+    /// constructed and may even be impossible to detect effectively.
+    ///
+    /// If the progress cannot be determined, this method returns a special
+    /// value of PROGRESS_UNKNOWN, which is not included in the range between
+    /// 0 and 1.
+    ///
+    /// As such, the application should use the return value only for
+    /// informational purposes such as logging.  For example, it shouldn't
+    /// be used to determine whether loading is completed by comparing it
+    /// to 1.  It should also expect the possibility of getting
+    /// \c PROGRESS_UNKNOWN at any call to this method; it shouldn't assume
+    /// the specific way of internal implementation as described below (which
+    /// is provided for informational purposes only).
+    ///
+    /// In this implementation, if the loader is constructed with a file
+    /// name, the progress value is measured by the number of characters
+    /// read from the zone file divided by the size of the zone file
+    /// (with taking into account any included files).  Note that due to
+    /// the possibility of intermediate included files, the total file size
+    /// cannot be fully fixed until the completion of the load.  And, due to
+    /// this possibility, return values from this method may not always
+    /// increase monotonically.
+    ///
+    /// If it's constructed with another data source client, this method
+    /// always returns \c PROGRESS_UNKNOWN; in future, however, it may become
+    /// possible to return something more useful, e.g, based on the result
+    /// of \c getRRCount() and the total number of RRs if the underlying data
+    /// source can provide the latter value efficiently.
+    ///
+    /// \throw None
+    double getProgress() const;
+
+    /// \brief A special value for \c getProgress, meaning the progress is
+    /// unknown.
+    ///
+    /// See the method description for details.
+    static const double PROGRESS_UNKNOWN;
+
 private:
     /// \brief The iterator used as source of data in case of the copy mode.
     const ZoneIteratorPtr iterator_;
@@ -169,9 +225,14 @@ private:
     bool complete_;
     /// \brief Was the loading successful?
     bool loaded_ok_;
+    size_t rr_count_;
 };
 
 }
 }
 
-#endif
+#endif  // DATASRC_ZONE_LOADER_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/master_lexer.cc

@@ -48,6 +48,7 @@ using namespace master_lexer_internal;
 
 struct MasterLexer::MasterLexerImpl {
     MasterLexerImpl() : source_(NULL), token_(MasterToken::NOT_STARTED),
+                        total_size_(0), popped_size_(0),
                         paren_count_(0), last_was_eol_(true),
                         has_previous_(false),
                         previous_paren_count_(0),
@@ -91,11 +92,28 @@ struct MasterLexer::MasterLexerImpl {
                 separators_.test(c & 0x7f));
     }
 
+    void setTotalSize() {
+        assert(source_ != NULL);
+        if (total_size_ != SOURCE_SIZE_UNKNOWN) {
+            const size_t current_size = source_->getSize();
+            if (current_size != SOURCE_SIZE_UNKNOWN) {
+                total_size_ += current_size;
+            } else {
+                total_size_ = SOURCE_SIZE_UNKNOWN;
+            }
+        }
+    }
+
     std::vector<InputSourcePtr> sources_;
     InputSource* source_;       // current source (NULL if sources_ is empty)
     MasterToken token_;         // currently recognized token (set by a state)
     std::vector<char> data_;    // placeholder for string data
 
+    // Keep track of the total size of all sources and characters that have
+    // been read from sources already popped.
+    size_t total_size_;         // accumulated size (# of chars) of sources
+    size_t popped_size_;        // total size of sources that have been popped
+
     // These are used in states, and defined here only as a placeholder.
     // The main lexer class does not need these members.
     size_t paren_count_;        // nest count of the parentheses
@@ -139,6 +157,7 @@ MasterLexer::pushSource(const char* filename, std::string* error) {
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
     impl_->last_was_eol_ = true;
+    impl_->setTotalSize();
     return (true);
 }
 
@@ -154,6 +173,7 @@ MasterLexer::pushSource(std::istream& input) {
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
     impl_->last_was_eol_ = true;
+    impl_->setTotalSize();
 }
 
 void
@@ -162,6 +182,7 @@ MasterLexer::popSource() {
         isc_throw(InvalidOperation,
                   "MasterLexer::popSource on an empty source");
     }
+    impl_->popped_size_ += impl_->source_->getPosition();
     impl_->sources_.pop_back();
     impl_->source_ = impl_->sources_.empty() ? NULL :
         impl_->sources_.back().get();
@@ -191,22 +212,12 @@ MasterLexer::getSourceLine() const {
 
 size_t
 MasterLexer::getTotalSourceSize() const {
-    size_t total_size = 0;
-    BOOST_FOREACH(InputSourcePtr& src, impl_->sources_) {
-        // If the size of any pushed source is unknown, the total is also
-        // considered unknown.
-        if (src->getSize() == SOURCE_SIZE_UNKNOWN) {
-            return (SOURCE_SIZE_UNKNOWN);
-        }
-
-        total_size += src->getSize();
-    }
-    return (total_size);
+    return (impl_->total_size_);
 }
 
 size_t
 MasterLexer::getPosition() const {
-    size_t position = 0;
+    size_t position = impl_->popped_size_;
     BOOST_FOREACH(InputSourcePtr& src, impl_->sources_) {
         position += src->getPosition();
     }

src/lib/dns/master_lexer.h

@@ -477,7 +477,7 @@ public:
     ///
     /// This method returns the sum of the size of sources that have been
     /// pushed to the lexer by the time of the call.  It would give the
-    /// caller of some hint about the amount of data the lexer is working on.
+    /// caller some hint about the amount of data the lexer is working on.
     ///
     /// The size of a normal file is equal to the file size at the time of
     /// the source is pushed.  The size of other type of input stream is
@@ -488,25 +488,42 @@ public:
     /// stream is unknown.  It happens, for example, if the standard input
     /// is associated with a pipe from the output of another process and it's
     /// specified as an input source.  If the size of some of the pushed
-    /// pushed source is unknown, this method returns SOURCE_SIZE_UNKNOWN.
+    /// source is unknown, this method returns SOURCE_SIZE_UNKNOWN.
     ///
-    /// If there is no source pushed in the lexer, it returns 0.
+    /// The total size won't change when a source is popped.  So the return
+    /// values of this method will monotonically increase or
+    /// \c SOURCE_SIZE_UNKNOWN; once it returns \c SOURCE_SIZE_UNKNOWN,
+    /// any subsequent call will also result in that value, by the above
+    /// definition.
+    ///
+    /// Before pushing any source, it returns 0.
     ///
     /// \throw None
     size_t getTotalSourceSize() const;
 
-    /// \brief Return the position of lexer in the currently pushed sources.
+    /// \brief Return the position of lexer in the pushed sources so far.
     ///
     /// This method returns the position in terms of the number of recognized
-    /// characters from all sources.  Roughly speaking, the position in a
-    /// single source is the offset from the beginning of the file or stream
-    /// to the current "read cursor" of the lexer, and the return value of
-    /// this method is the sum of the position in all the pushed sources.
+    /// characters from all sources that have been pushed by the time of the
+    /// call.  Conceptually, the position in a single source is the offset
+    /// from the beginning of the file or stream to the current "read cursor"
+    /// of the lexer.  The return value of this method is the sum of the
+    /// positions in all the pushed sources.  If any of the sources has
+    /// already been popped, the position of the source at the time of the
+    /// pop operation will be used for the calculation.
     ///
     /// If the lexer reaches the end for each of all the pushed sources,
     /// the return value should be equal to that of \c getTotalSourceSize().
+    /// It's generally expected that a source is popped when the lexer
+    /// reaches the end of the source.  So, when the application of this
+    /// class parses all contents of all sources, possibly with multiple
+    /// pushes and pops, the return value of this method and
+    /// \c getTotalSourceSize() should be identical (unless the latter
+    /// returns SOURCE_SIZE_UNKNOWN).  But this is not necessarily
+    /// guaranteed as the application can pop a source in the middle of
+    /// parsing it.
     ///
-    /// If there is no source pushed in the lexer, it returns 0.
+    /// Before pushing any source, it returns 0.
     ///
     /// The return values of this method and \c getTotalSourceSize() would
     /// give the caller an idea of the progress of the lexer at the time of
@@ -515,11 +532,10 @@ public:
     /// this way may not make much sense; it can only give an informational
     /// hint of the progress.
     ///
-    /// Note also that if a source is popped, this method will normally return
-    /// a smaller number by definition (and so will \c getTotalSourceSize()).
-    /// Likewise, the conceptual "read cursor" would move backward after a
+    /// Note that the conceptual "read cursor" would move backward after a
     /// call to \c ungetToken(), in which case this method will return a
-    /// smaller value, too.
+    /// smaller value.  That is, unlike \c getTotalSourceSize(), return
+    /// values of this method may not always monotonically increase.
     ///
     /// \throw None
     size_t getPosition() const;

src/lib/dns/master_loader.cc

@@ -76,7 +76,8 @@ public:
         previous_name_(false),
         complete_(false),
         seen_error_(false),
-        warn_rfc1035_ttl_(true)
+        warn_rfc1035_ttl_(true),
+        rr_count_(0)
     {}
 
     void pushSource(const std::string& filename, const Name& current_origin) {
@@ -103,6 +104,9 @@ public:
 
     bool loadIncremental(size_t count_limit);
 
+    size_t getSize() const { return (lexer_.getTotalSourceSize()); }
+    size_t getPosition() const { return (lexer_.getPosition()); }
+
 private:
     void reportError(const std::string& filename, size_t line,
                      const std::string& reason)
@@ -418,12 +422,14 @@ private:
     vector<IncludeInfo> include_info_;
     bool previous_name_; // True if there was a previous name in this file
                          // (false at the beginning or after an $INCLUDE line)
+
 public:
     bool complete_;             // All work done.
     bool seen_error_;           // Was there at least one error during the
                                 // load?
     bool warn_rfc1035_ttl_;     // should warn if implicit TTL determination
                                 // from the previous RR is used.
+    size_t rr_count_;    // number of RRs successfully loaded
 };
 
 // A helper method of loadIncremental, parsing the first token of a new line.
@@ -554,6 +560,7 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
                               rdata);
                 // Good, we loaded another one
                 ++count;
+                ++rr_count_;
             } else {
                 seen_error_ = true;
                 if (!many_errors_) {
@@ -630,5 +637,15 @@ MasterLoader::loadedSucessfully() const {
     return (impl_->complete_ && !impl_->seen_error_);
 }
 
+size_t
+MasterLoader::getSize() const {
+    return (impl_->getSize());
+}
+
+size_t
+MasterLoader::getPosition() const {
+    return (impl_->getPosition());
+}
+
 } // end namespace dns
 } // end namespace isc

src/lib/dns/master_loader.h

@@ -142,6 +142,44 @@ public:
     ///     finishing the load.
     bool loadedSucessfully() 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
+    /// (master zone files or streams) that is known at the time of the call.
+    /// For a zone file, it's the size of the file; for a stream, it's the
+    /// size of the data (in bytes) available at the start of the load.
+    /// If separate zone files are included via the $INCLUDE directive, the
+    /// sum of the sizes of these files are added.
+    ///
+    /// If the loader is constructed with a stream, the size can be
+    /// "unknown" as described for \c MasterLexer::getTotalSourceSize().
+    /// In this case this method always returns
+    /// \c MasterLexer::SOURCE_SIZE_UNKNOWN.
+    ///
+    /// If the loader is constructed with a zone file, this method
+    /// initially returns 0.  So until either \c load() or \c loadIncremental()
+    /// is called, the value is meaningless.
+    ///
+    /// Note that when the source includes separate files, this method
+    /// cannot take into account the included files that the loader has not
+    /// recognized at the time of call.  So it's possible that this method
+    /// returns different values at different times of call.
+    ///
+    /// \throw None
+    size_t getSize() const;
+
+    /// \brief Return the position of the loader in zone.
+    ///
+    /// This method returns a conceptual "position" of the loader in the
+    /// zone to be loaded.  Specifically, it returns the total number of
+    /// characters contained in the zone files and streams and recognized
+    /// by the loader.  Before starting the load it returns 0; on successful
+    /// completion it will be equal to the return value of \c getSize()
+    /// (unless the latter returns \c MasterLexer::SOURCE_SIZE_UNKNOWN).
+    ///
+    /// \throw None
+    size_t getPosition() const;
+
 private:
     class MasterLoaderImpl;
     MasterLoaderImpl* impl_;
@@ -151,3 +189,7 @@ private:
 } // end namespace isc
 
 #endif // MASTER_LOADER_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/tests/master_lexer_unittest.cc

@@ -52,7 +52,6 @@ void
 checkEmptySource(const MasterLexer& lexer) {
     EXPECT_TRUE(lexer.getSourceName().empty());
     EXPECT_EQ(0, lexer.getSourceLine());
-    EXPECT_EQ(0, lexer.getTotalSourceSize());
     EXPECT_EQ(0, lexer.getPosition());
 }
 
@@ -78,6 +77,7 @@ TEST_F(MasterLexerTest, pushStream) {
     lexer.popSource();
     EXPECT_EQ(0, lexer.getSourceCount());
     checkEmptySource(lexer);
+    EXPECT_EQ(4, lexer.getTotalSourceSize()); // this shouldn't change
 }
 
 TEST_F(MasterLexerTest, pushStreamFail) {
@@ -105,6 +105,7 @@ TEST_F(MasterLexerTest, pushFile) {
     lexer.popSource();
     checkEmptySource(lexer);
     EXPECT_EQ(0, lexer.getSourceCount());
+    EXPECT_EQ(143, lexer.getTotalSourceSize()); // this shouldn't change
 
     // If we give a non NULL string pointer, its content will be intact
     // if pushSource succeeds.
@@ -133,22 +134,44 @@ TEST_F(MasterLexerTest, pushFileFail) {
 }
 
 TEST_F(MasterLexerTest, nestedPush) {
-    ss << "test";
+    const string test_txt = "test";
+    ss << test_txt;
     lexer.pushSource(ss);
+
+    EXPECT_EQ(test_txt.size(), lexer.getTotalSourceSize());
+    EXPECT_EQ(0, lexer.getPosition());
+
     EXPECT_EQ(expected_stream_name, lexer.getSourceName());
 
+    // Read the string; getPosition() should reflect that.
+    EXPECT_EQ(MasterToken::STRING, lexer.getNextToken().getType());
+    EXPECT_EQ(test_txt.size(), lexer.getPosition());
+
     // We can push another source without popping the previous one.
     lexer.pushSource(TEST_DATA_SRCDIR "/masterload.txt");
     EXPECT_EQ(TEST_DATA_SRCDIR "/masterload.txt", lexer.getSourceName());
-    EXPECT_EQ(143 + 4, lexer.getTotalSourceSize()); // see above for magic nums
+    EXPECT_EQ(143 + test_txt.size(),
+              lexer.getTotalSourceSize()); // see above for magic nums
+
+    // the next token should be the EOL (skipping a comment line), its
+    // position in the file is 35 (hardcoded).
+    EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType());
+    EXPECT_EQ(test_txt.size() + 35, lexer.getPosition());
 
     // popSource() works on the "topmost" (last-pushed) source
     lexer.popSource();
     EXPECT_EQ(expected_stream_name, lexer.getSourceName());
-    EXPECT_EQ(4, lexer.getTotalSourceSize());
+
+    // pop shouldn't change the total size and the current position
+    EXPECT_EQ(143 + test_txt.size(), lexer.getTotalSourceSize());
+    EXPECT_EQ(test_txt.size() + 35, lexer.getPosition());
 
     lexer.popSource();
     EXPECT_TRUE(lexer.getSourceName().empty());
+
+    // size and position still shouldn't change
+    EXPECT_EQ(143 + test_txt.size(), lexer.getTotalSourceSize());
+    EXPECT_EQ(test_txt.size() + 35, lexer.getPosition());
 }
 
 TEST_F(MasterLexerTest, unknownSourceSize) {
@@ -164,9 +187,9 @@ TEST_F(MasterLexerTest, unknownSourceSize) {
     // Then the total size is also unknown.
     EXPECT_EQ(MasterLexer::SOURCE_SIZE_UNKNOWN, lexer.getTotalSourceSize());
 
-    // If we pop that source, the size becomes known again.
+    // Even if we pop that source, the size is still unknown.
     lexer.popSource();
-    EXPECT_EQ(4, lexer.getTotalSourceSize());
+    EXPECT_EQ(MasterLexer::SOURCE_SIZE_UNKNOWN, lexer.getTotalSourceSize());
 }
 
 TEST_F(MasterLexerTest, invalidPop) {
@@ -317,7 +340,7 @@ TEST_F(MasterLexerTest, includeAndInitialWS) {
     lexer.pushSource(ss2);
     EXPECT_EQ(MasterToken::INITIAL_WS,
               lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
-    EXPECT_EQ(2, lexer.getPosition()); // should be sum of position positions.
+    EXPECT_EQ(2, lexer.getPosition()); // should be sum of pushed positions.
 }
 
 // Test only one token can be ungotten

src/lib/dns/tests/master_loader_unittest.cc

@@ -153,12 +153,23 @@ TEST_F(MasterLoaderTest, basicLoad) {
               RRClass::IN(), MasterLoader::MANY_ERRORS);
 
     EXPECT_FALSE(loader_->loadedSucessfully());
+
+    // The following three should be set to 0 initially in case the loader
+    // is constructed from a file name.
+    EXPECT_EQ(0, loader_->getSize());
+    EXPECT_EQ(0, loader_->getPosition());
+
     loader_->load();
     EXPECT_TRUE(loader_->loadedSucessfully());
 
     EXPECT_TRUE(errors_.empty());
     EXPECT_TRUE(warnings_.empty());
 
+    // Hardcode expected values taken from the test data file, assuming it
+    // won't change too often.
+    EXPECT_EQ(549, loader_->getSize());
+    EXPECT_EQ(549, loader_->getPosition());
+
     checkBasicRRs();
 }
 
@@ -195,6 +206,47 @@ TEST_F(MasterLoaderTest, include) {
     }
 }
 
+TEST_F(MasterLoaderTest, includeAndIncremental) {
+    // Check getSize() and getPosition() are adjusted before and after
+    // $INCLUDE.
+    const string first_rr = "before.example.org. 0 A 192.0.2.1\n";
+    const string include_str = "$INCLUDE " TEST_DATA_SRCDIR "/example.org";
+    const string zone_data = first_rr + include_str + "\n" +
+        "www 3600 IN AAAA 2001:db8::1\n";
+    stringstream ss(zone_data);
+    setLoader(ss, Name("example.org."), RRClass::IN(), MasterLoader::DEFAULT);
+
+    // On construction, getSize() returns the size of the data (exclude the
+    // the file to be included); position is set to 0.
+    EXPECT_EQ(zone_data.size(), loader_->getSize());
+    EXPECT_EQ(0, loader_->getPosition());
+
+    // Read the first RR.  getSize() doesn't change; position should be
+    // at the end of the first line.
+    loader_->loadIncremental(1);
+    EXPECT_EQ(zone_data.size(), loader_->getSize());
+    EXPECT_EQ(first_rr.size(), loader_->getPosition());
+
+    // Read next 4.  It includes $INCLUDE processing.  Magic number of 549
+    // is the size of the test zone file (see above); 506 is the position in
+    // the file at the end of 4th RR (due to extra comments it's smaller than
+    // the file size).
+    loader_->loadIncremental(4);
+    EXPECT_EQ(zone_data.size() + 549, loader_->getSize());
+    EXPECT_EQ(first_rr.size() + include_str.size() + 506,
+              loader_->getPosition());
+
+    // Read the last one.  At this point getSize and getPosition return
+    // the same value, indicating progress of 100%.
+    loader_->loadIncremental(1);
+    EXPECT_EQ(zone_data.size() + 549, loader_->getSize());
+    EXPECT_EQ(zone_data.size() + 549, loader_->getPosition());
+
+    // we were not interested in checking RRs in this test.  clear them to
+    // not confuse TearDown().
+    rrsets_.clear();
+}
+
 // A commonly used helper to check callback message.
 void
 checkCallbackMessage(const string& actual_msg, const string& expected_msg,
@@ -260,7 +312,7 @@ TEST_F(MasterLoaderTest, popAfterError) {
     const string include_str = "$include " TEST_DATA_SRCDIR
         "/broken.zone\nwww 3600 IN AAAA 2001:db8::1\n";
     stringstream ss(include_str);
-    // We don't test without MANY_ERRORS, we want to see what happens
+    // We perform the test with MANY_ERRORS, we want to see what happens
     // after the error.
     setLoader(ss, Name("example.org."), RRClass::IN(),
               MasterLoader::MANY_ERRORS);
@@ -277,11 +329,19 @@ TEST_F(MasterLoaderTest, popAfterError) {
 
 // Check it works the same when created based on a stream, not filename
 TEST_F(MasterLoaderTest, streamConstructor) {
-    stringstream zone_stream(prepareZone("", true));
+    const string zone_data(prepareZone("", true));
+    stringstream zone_stream(zone_data);
     setLoader(zone_stream, Name("example.org."), RRClass::IN(),
               MasterLoader::MANY_ERRORS);
 
     EXPECT_FALSE(loader_->loadedSucessfully());
+
+    // 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(zone_data.size(), loader_->getSize());
+    EXPECT_EQ(0, loader_->getPosition());
+
     loader_->load();
     EXPECT_TRUE(loader_->loadedSucessfully());
 
@@ -290,6 +350,11 @@ TEST_F(MasterLoaderTest, streamConstructor) {
     checkRR("example.org", RRType::SOA(), "ns1.example.org. "
             "admin.example.org. 1234 3600 1800 2419200 7200");
     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.
+    EXPECT_EQ(zone_data.size(), loader_->getSize());
+    EXPECT_EQ(zone_data.size(), loader_->getPosition());
 }
 
 // Try loading data incrementally.

src/lib/python/isc/datasrc/datasrc.cc

@@ -21,6 +21,7 @@
 #include <datasrc/client.h>
 #include <datasrc/database.h>
 #include <datasrc/sqlite3_accessor.h>
+#include <datasrc/zone_loader.h>
 
 #include "datasrc.h"
 #include "client_python.h"
@@ -34,6 +35,9 @@
 #include <util/python/pycppwrapper_util.h>
 #include <dns/python/pydnspp_common.h>
 
+#include <stdexcept>
+#include <string>
+
 using namespace isc::datasrc;
 using namespace isc::datasrc::python;
 using namespace isc::util::python;
@@ -195,6 +199,21 @@ initModulePart_ZoneLoader(PyObject* mod) {
     }
     Py_INCREF(&zone_loader_type);
 
+    try {
+        installClassVariable(zone_loader_type, "PROGRESS_UNKNOWN",
+                             Py_BuildValue("d", ZoneLoader::PROGRESS_UNKNOWN));
+    } catch (const std::exception& ex) {
+        const std::string ex_what =
+            "Unexpected failure in ZoneLoader initialization: " +
+            std::string(ex.what());
+        PyErr_SetString(po_IscException, ex_what.c_str());
+        return (false);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError,
+                        "Unexpected failure in ZoneLoader initialization");
+        return (false);
+    }
+
     return (true);
 }
 

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

@@ -38,7 +38,7 @@ ORIG_SOA_TXT = 'example.com. 3600 IN SOA master.example.com. ' +\
                'admin.example.com. 1234 3600 1800 2419200 7200\n'
 NEW_SOA_TXT = 'example.com. 1000 IN SOA a.dns.example.com. ' +\
               'mail.example.com. 1 1 1 1 1\n'
-
+PROGRESS_UNKNOWN = isc.datasrc.ZoneLoader.PROGRESS_UNKNOWN
 
 class ZoneLoaderTests(unittest.TestCase):
     def setUp(self):
@@ -111,22 +111,50 @@ class ZoneLoaderTests(unittest.TestCase):
     def test_load_from_file(self):
         self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
                                              self.test_file)
+        self.assertEqual(0, self.loader.get_rr_count())
+        self.assertEqual(0, self.loader.get_progress())
+
         self.check_load()
 
+        # Expected values are hardcoded, taken from the test zone file,
+        # assuming it won't change too often.  progress should reach 100% (=1).
+        self.assertEqual(8, self.loader.get_rr_count())
+        self.assertEqual(1, self.loader.get_progress())
+
     def test_load_from_client(self):
         self.source_client = isc.datasrc.DataSourceClient('sqlite3',
                                     DB_SOURCE_CLIENT_CONFIG)
         self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
                                              self.source_client)
+
+        self.assertEqual(0, self.loader.get_rr_count())
+        self.assertEqual(PROGRESS_UNKNOWN, self.loader.get_progress())
+
         self.check_load()
 
-    def check_load_incremental(self):
+        # In case of loading from another data source, progress is unknown.
+        self.assertEqual(8, self.loader.get_rr_count())
+        self.assertEqual(PROGRESS_UNKNOWN, self.loader.get_progress())
+
+    def check_load_incremental(self, from_file=True):
         # New zone has 8 RRs
         # After 5, it should return False
         self.assertFalse(self.loader.load_incremental(5))
         # New zone should not have been loaded yet
         self.check_zone_soa(ORIG_SOA_TXT)
 
+        # In case it's from a zone file, get_progress should be in the middle
+        # of (0, 1).  expected value is taken from the test zone file
+        # (total size = 422, current position = 288)
+        if from_file:
+            # To avoid any false positive due to rounding errors, we convert
+            # them to near integers between 0 and 100.
+            self.assertEqual(int((288 * 100) / 422),
+                             int(self.loader.get_progress() * 100))
+            # Also check the return value has higher precision.
+            self.assertNotEqual(int(288 * 100 / 422),
+                                100 * self.loader.get_progress())
+
         # After 5 more, it should return True (only having read 3)
         self.assertTrue(self.loader.load_incremental(5))
         # New zone should now be loaded
@@ -147,7 +175,7 @@ class ZoneLoaderTests(unittest.TestCase):
                                             DB_SOURCE_CLIENT_CONFIG)
         self.loader = isc.datasrc.ZoneLoader(self.client, self.test_name,
                                              self.source_client)
-        self.check_load_incremental()
+        self.check_load_incremental(False)
 
     def test_bad_file(self):
         self.check_zone_soa(ORIG_SOA_TXT)

src/lib/python/isc/datasrc/zone_loader_inc.cc

@@ -113,4 +113,64 @@ on the next call (which will load 0 RRs). This is because the end of\n\
 iterator or master file is detected when reading past the end, not\n\
 when the last one is read.\n\
 ";
+
+const char* const ZoneLoader_getRRCount_doc = "\
+get_rr_count() -> integer\n\
+\n\
+Return the number of RRs loaded.\n\
+\n\
+This method returns the number of RRs loaded via this loader by the\n\
+time of the call. Before starting the load it will return 0. It will\n\
+return the total number of RRs of the zone on and after completing the\n\
+load.\n\
+\n\
+Exceptions:\n\
+  None\n\
+\n\
+";
+
+// Modifications
+// - double => float
+const char* const ZoneLoader_getProgress_doc = "\
+get_progress() -> float\n\
+\n\
+Return the current progress of the loader.\n\
+\n\
+This method returns the current estimated progress of loader as a\n\
+value between 0 and 1 (inclusive); it's 0 before starting the load,\n\
+and 1 at the completion, and a value between these (exclusive) in the\n\
+middle of loading. It's an implementation detail how to calculate the\n\
+progress, which may vary depending on how the loader is constructed\n\
+and may even be impossible to detect effectively.\n\
+\n\
+If the progress cannot be determined, this method returns a special\n\
+value of PROGRESS_UNKNOWN, which is not included in the range between\n\
+0 and 1.\n\
+\n\
+As such, the application should use the return value only for\n\
+informational purposes such as logging. For example, it shouldn't be\n\
+used to determine whether loading is completed by comparing it to 1.\n\
+It should also expect the possibility of getting PROGRESS_UNKNOWN at\n\
+any call to this method; it shouldn't assume the specific way of\n\
+internal implementation as described below (which is provided for\n\
+informational purposes only).\n\
+\n\
+In this implementation, if the loader is constructed with a file name,\n\
+the progress value is measured by the number of characters read from\n\
+the zone file divided by the size of the zone file (with taking into\n\
+account any included files). Note that due to the possibility of\n\
+intermediate included files, the total file size cannot be fully fixed\n\
+until the completion of the load. And, due to this possibility, return\n\
+values from this method may not always increase monotonically.\n\
+\n\
+If it's constructed with another data source client, this method\n\
+always returns PROGRESS_UNKNOWN; in future, however, it may become\n\
+possible to return something more useful, e.g, based on the result of\n\
+get_rr_count() and the total number of RRs if the underlying data\n\
+source can provide the latter value efficiently.\n\
+\n\
+Exceptions:\n\
+  None\n\
+\n\
+";
 } // unnamed namespace

src/lib/python/isc/datasrc/zone_loader_python.cc

@@ -187,6 +187,18 @@ ZoneLoader_loadIncremental(PyObject* po_self, PyObject* args) {
     }
 }
 
+PyObject*
+ZoneLoader_getRRCount(PyObject* po_self, PyObject*) {
+    s_ZoneLoader* self = static_cast<s_ZoneLoader*>(po_self);
+    return (Py_BuildValue("I", self->cppobj->getRRCount()));
+}
+
+PyObject*
+ZoneLoader_getProgress(PyObject* po_self, PyObject*) {
+    s_ZoneLoader* self = static_cast<s_ZoneLoader*>(po_self);
+    return (Py_BuildValue("d", self->cppobj->getProgress()));
+}
+
 // This list contains the actual set of functions we have in
 // python. Each entry has
 // 1. Python method name
@@ -197,6 +209,10 @@ PyMethodDef ZoneLoader_methods[] = {
     { "load", ZoneLoader_load, METH_NOARGS, ZoneLoader_load_doc },
     { "load_incremental", ZoneLoader_loadIncremental, METH_VARARGS,
       ZoneLoader_loadIncremental_doc },
+    { "get_rr_count", ZoneLoader_getRRCount, METH_NOARGS,
+      ZoneLoader_getRRCount_doc },
+    { "get_progress", ZoneLoader_getProgress, METH_NOARGS,
+      ZoneLoader_getProgress_doc },
     { NULL, NULL, 0, NULL }
 };