[1332] handled the "no such serial" case. To distinguish various errors

change the return value of the journal reader factory to a pair of result
code and an object pointer.  added some more tests.

src/lib/datasrc/client.h

@@ -15,6 +15,8 @@
 #ifndef __DATA_SOURCE_CLIENT_H
 #define __DATA_SOURCE_CLIENT_H 1
 
+#include <utility>
+
 #include <boost/noncopyable.hpp>
 #include <boost/shared_ptr.hpp>
 
@@ -310,7 +312,7 @@ public:
                                       bool replace, bool journaling = false)
         const = 0;
 
-    virtual ZoneJournalReaderPtr
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
     getJournalReader(const isc::dns::Name& zone, uint32_t begin_serial,
                      uint32_t end_serial) const = 0;
 };

src/lib/datasrc/database.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <string>
+#include <utility>
 #include <vector>
 
 #include <datasrc/database.h>
@@ -1137,15 +1138,26 @@ private:
 };
 
 // The JournalReader factory
-ZoneJournalReaderPtr
+pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
 DatabaseClient::getJournalReader(const isc::dns::Name& zone,
                                  uint32_t begin_serial,
                                  uint32_t end_serial) const
 {
-    return (ZoneJournalReaderPtr(new DatabaseJournalReader(accessor_, zone,
+    try {
+        const pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> ret(
+            ZoneJournalReader::SUCCESS,
+            ZoneJournalReaderPtr(new DatabaseJournalReader(accessor_, zone,
                                                            rrclass_,
                                                            begin_serial,
                                                            end_serial)));
+        return (ret);
+    } catch (const NoSuchSerial&) {
+        return (pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>(
+                    ZoneJournalReader::NO_SUCH_SERIAL,
+                    ZoneJournalReaderPtr()));
+    } catch (...) {
+        throw;
+    }
 }
 }
 }

src/lib/datasrc/database.h

@@ -23,6 +23,8 @@
 #include <dns/rrclass.h>
 #include <dns/rrset.h>
 
+#include <datasrc/data_source.h>
+#include <datasrc/client.h>
 #include <datasrc/client.h>
 
 #include <dns/name.h>
@@ -35,6 +37,19 @@ namespace isc {
 namespace datasrc {
 
 /**
+ * \brief No such serial number when obtaining difference iterator
+ *
+ * This exception can be thrown from \c DatabaseAccessor::getDiffs().
+ * It's thrown if either the zone/start version or zone/end version
+ * combination does not exist in the differences table.
+ */
+class NoSuchSerial : public DataSourceError {
+public:
+    NoSuchSerial(const char* file, size_t line, const char* what) :
+        DataSourceError(file, line, what) {}
+};
+
+/**
  * \brief Abstraction of lowlevel database with DNS data
  *
  * This class is defines interface to databases. Each supported database
@@ -932,7 +947,7 @@ public:
                                       bool journaling = false) const;
 
     /// TBD
-    virtual ZoneJournalReaderPtr
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
     getJournalReader(const isc::dns::Name& zone, uint32_t begin_serial,
                      uint32_t end_serial) const;
 

src/lib/datasrc/memory_datasrc.cc

@@ -819,7 +819,7 @@ InMemoryClient::getUpdater(const isc::dns::Name&, bool, bool) const {
     isc_throw(isc::NotImplemented, "Update attempt on in memory data source");
 }
 
-ZoneJournalReaderPtr
+pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
 InMemoryClient::getJournalReader(const isc::dns::Name&, uint32_t,
                                  uint32_t) const
 {

src/lib/datasrc/memory_datasrc.h

@@ -287,7 +287,7 @@ public:
                                       bool replace, bool journaling = false)
         const;
 
-    virtual ZoneJournalReaderPtr
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
     getJournalReader(const isc::dns::Name& zone, uint32_t begin_serial,
                      uint32_t end_serial) const;
 

src/lib/datasrc/sqlite3_accessor.h

@@ -71,19 +71,6 @@ public:
         DataSourceError(file, line, what) {}
 };
 
-/**
- * \brief No such serial number when obtaining difference iterator
- *
- * Thrown if either the zone/start version or zone/end version combination
- * does not exist in the differences table.
- */
-class NoSuchSerial : public DataSourceError {
-public:
-    NoSuchSerial(const char* file, size_t line, const char* what) :
-        DataSourceError(file, line, what) {}
-};
-
-
 struct SQLite3Parameters;
 
 /**

src/lib/datasrc/tests/client_unittest.cc

@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <utility>
+
 #include <datasrc/client.h>
 
 #include <dns/name.h>
@@ -37,9 +39,11 @@ public:
     {
         return (ZoneUpdaterPtr());
     }
-    virtual ZoneJournalReaderPtr
+    virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
     getJournalReader(const isc::dns::Name&, uint32_t, uint32_t) const {
-        return (ZoneJournalReaderPtr());
+        isc_throw(isc::NotImplemented, "Journaling isn't supported for "
+                  "in Nop data source");
+        //return (ZoneJournalReaderPtr());
     }
 };
 

src/lib/datasrc/tests/database_unittest.cc

@@ -766,14 +766,33 @@ public:
                  journal_entries_->begin();
              it != journal_entries_->end(); ++it)
         {
-            // For simplicity we don't take into account serial number
-            // arithmetic; compare serials as bare unsigned integers.
-            if (id != READONLY_ZONE_ID || (*it).serial_ < start ||
-                (*it).serial_ > end) {
+            // For simplicity we assume this method is called for the
+            // "readonly" zone possibly after making updates on the "writable"
+            // copy and committing them.
+            if (id != READONLY_ZONE_ID) {
+                continue;
+            }
+
+            // Note: the following logic is not 100% accurate in terms of
+            // serial number arithmetic; we prefer brevity for testing.
+            // Skip until we see the starting serial.  Once we started
+            // recording this condition is ignored (to support wrap-around
+            // case)
+            if ((*it).serial_ < start && selected_jnl.empty()) {
                 continue;
             }
             selected_jnl.push_back(*it);
+            if ((*it).serial_ > end) { // gone over the end serial. we're done.
+                break;
+            }
         }
+
+        // Check if we've found the requested range.  If not, throw.
+        if (selected_jnl.empty() || selected_jnl.front().serial_ != start ||
+            selected_jnl.back().serial_ != end) {
+            isc_throw(NoSuchSerial, "requested diff range is not found");
+        }
+
         return (IteratorContextPtr(new MockDiffIteratorContext(selected_jnl)));
     }
 
@@ -3071,7 +3090,7 @@ TYPED_TEST(DatabaseClientTest, journalReader) {
     this->updater_->commit();
 
     ZoneJournalReaderPtr jnl_reader(this->client_->getJournalReader(
-                                        this->zname_, 1234, 1235));
+                                        this->zname_, 1234, 1235).second);
     ConstRRsetPtr rrset = jnl_reader->getNextDiff();
     ASSERT_TRUE(rrset);
     isc::testutils::rrsetCheck(this->soa_, rrset);
@@ -3082,6 +3101,65 @@ TYPED_TEST(DatabaseClientTest, journalReader) {
     ASSERT_FALSE(rrset);
 }
 
+TYPED_TEST(DatabaseClientTest, readLargeJournal) {
+    // Similar to journalMultiple, but check that at a higher level.
+
+    this->updater_ = this->client_->getUpdater(this->zname_, false, true);
+
+    vector<ConstRRsetPtr> expected;
+    for (size_t i = 0; i < 100; ++i) {
+        // Create the old SOA and remove it, and record it in the expected list
+        RRsetPtr rrset1(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                                  this->rrttl_));
+        string soa_rdata = "ns1.example.org. admin.example.org. " +
+            lexical_cast<std::string>(1234 + i) + " 3600 1800 2419200 7200";
+        rrset1->addRdata(rdata::createRdata(RRType::SOA(), this->qclass_,
+                                            soa_rdata));
+        this->updater_->deleteRRset(*rrset1);
+        expected.push_back(rrset1);
+
+        // Create a new SOA, add it, and record it.
+        RRsetPtr rrset2(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                                  this->rrttl_));
+        soa_rdata = "ns1.example.org. admin.example.org. " +
+            lexical_cast<std::string>(1234 + i + 1) +
+            " 3600 1800 2419200 7200";
+        rrset2->addRdata(rdata::createRdata(RRType::SOA(), this->qclass_,
+                                            soa_rdata));
+        this->updater_->addRRset(*rrset2);
+        expected.push_back(rrset2);
+    }
+    this->updater_->commit();
+
+    ZoneJournalReaderPtr jnl_reader(this->client_->getJournalReader(
+                                        this->zname_, 1234, 1334).second);
+    ConstRRsetPtr actual;
+    int i = 0;
+    while ((actual = jnl_reader->getNextDiff()) != NULL) {
+        isc::testutils::rrsetCheck(expected.at(i++), actual);
+    }
+    EXPECT_EQ(expected.size(), i); // we should have eaten all expected data
+}
+
+TYPED_TEST(DatabaseClientTest, readJournalForNoRange) {
+    this->updater_ = this->client_->getUpdater(this->zname_, false, true);
+    this->updater_->deleteRRset(*this->soa_);
+    RRsetPtr soa_end(new RRset(this->zname_, this->qclass_, RRType::SOA(),
+                               this->rrttl_));
+    soa_end->addRdata(rdata::createRdata(RRType::SOA(), this->qclass_,
+                                         "ns1.example.org. admin.example.org. "
+                                         "1300 3600 1800 2419200 7200"));
+    this->updater_->addRRset(*soa_end);
+    this->updater_->commit();
+
+    // The specified range does not exist in the diff storage.  The factory
+    // method should result in NO_SUCH_SERIAL
+    pair<ZoneJournalReader::Result, ZoneJournalReaderPtr> result =
+        this->client_->getJournalReader(this->zname_, 1234, 1235);
+    EXPECT_EQ(ZoneJournalReader::NO_SUCH_SERIAL, result.first);
+    EXPECT_FALSE(result.second);
+}
+
 TYPED_TEST(DatabaseClientTest, journalReaderForNXZone) {
     EXPECT_THROW(this->client_->getJournalReader(Name("nosuchzone"), 0, 1),
                  DataSourceError);

src/lib/datasrc/zone.h

@@ -562,6 +562,14 @@ typedef boost::shared_ptr<ZoneUpdater> ZoneUpdaterPtr;
 
 /// The base class to retrieve differences between two versions of a zone.
 class ZoneJournalReader {
+public:
+    /// TBD: describe this
+    enum Result {
+        SUCCESS,
+        NO_SUCH_ZONE,
+        NO_SUCH_SERIAL
+    };
+
 protected:
     /// The default constructor.
     ///