[1330] Distinguish between no records for zone and none for serial number

src/lib/datasrc/database.h

@@ -319,7 +319,8 @@ public:
      *
      * \return Newly created iterator context. Must not be NULL.
      */
-    virtual IteratorContextPtr getDiffs(int id, int start, int end) const = 0;
+    virtual IteratorContextPtr
+    getDiffs(int id, uint32_t start, uint32_t end) const = 0;
 
     /// Start a transaction for updating a zone.
     ///

src/lib/datasrc/sqlite3_accessor.cc

@@ -16,6 +16,7 @@
 
 #include <string>
 #include <vector>
+#include <iostream>
 
 #include <boost/foreach.hpp>
 
@@ -54,10 +55,11 @@ enum StatementID {
     FIND_PREVIOUS = 10,
     ADD_RECORD_DIFF = 11,
     GET_RECORD_DIFF = 12,       // This is temporary for testing "add diff"
-    LOW_DIFF_ID = 13,
-    HIGH_DIFF_ID = 14,
-    DIFFS = 15,
-    NUM_STATEMENTS = 16
+    COUNT_DIFF_RECS = 13,
+    LOW_DIFF_ID = 14,
+    HIGH_DIFF_ID = 15,
+    DIFFS = 16,
+    NUM_STATEMENTS = 17
 };
 
 const char* const text_statements[NUM_STATEMENTS] = {
@@ -95,6 +97,8 @@ const char* const text_statements[NUM_STATEMENTS] = {
 
     // Two statements to select the lowest ID and highest ID in a set of
     // differences.
+    "SELECT COUNT(id) FROM diffs "  // COUNT_DIFF_RECS
+        "WHERE zone_id=?1",
     "SELECT id FROM diffs "     // LOW_DIFF_ID
         "WHERE zone_id=?1 AND version=?2 and OPERATION=0 "
         "ORDER BY id ASC LIMIT 1",
@@ -595,19 +599,22 @@ SQLite3Accessor::getAllRecords(int id) const {
 
 /// \brief Difference Iterator
 ///
-///
+/// This iterator is used to search through the differences table for the
+/// resouce records making up an IXFR between two versions of a zone.
 
 class SQLite3Accessor::DiffContext : public DatabaseAccessor::IteratorContext {
 public:
 
-    // Construct an iterator for difference records.
+    /// \brief Constructor
+    ///
+    /// \param zone_id ID of the zone (in the zone table)
     DiffContext(const boost::shared_ptr<const SQLite3Accessor>& accessor,
-                int id, int start, int end) :
+                int zone_id, uint32_t start, uint32_t end) :
         accessor_(accessor)
     {
         try {
-            int low_id = findIndex(LOW_DIFF_ID, id, start);
-            int high_id = findIndex(HIGH_DIFF_ID, id, end);
+            int low_id = findIndex(LOW_DIFF_ID, zone_id, start);
+            int high_id = findIndex(HIGH_DIFF_ID, zone_id, end);
             std::cout << "Low index is " << low_id << ", high index is " << high_id << "\n";
         } catch (...) {
             accessor_->dbparameters_->finalizeStatements();
@@ -633,7 +640,7 @@ private:
     void clearBindings(int stindex) {
         if (sqlite3_clear_bindings(
             accessor_->dbparameters_->getStatement(stindex)) != SQLITE_OK) {
-            isc_throw(SQLite3Error, "Could not clear SQL statement bindings in '" <<
+            isc_throw(SQLite3Error, "Could not clear statement bindings in '" <<
                       text_statements[stindex] << "': " << 
                       sqlite3_errmsg(accessor_->dbparameters_->db_));
         }
@@ -647,8 +654,8 @@ private:
     /// \param varindex Index of variable to which to bind
     /// \param value Value of variable to bind
     /// \exception SQLite3Error on an error
-    void bindInt(int stindex, int varindex, int value) {
-        if (sqlite3_bind_int(accessor_->dbparameters_->getStatement(stindex),
+    void bindInt(int stindex, int varindex, sqlite3_int64 value) {
+        if (sqlite3_bind_int64(accessor_->dbparameters_->getStatement(stindex),
                              varindex, value) != SQLITE_OK) {
             isc_throw(SQLite3Error, "Could not bind value to parameter " <<
                       varindex << " in statement '" <<
@@ -657,25 +664,19 @@ private:
         }
     }
 
-    /// \brief Find index
+    ///\brief Get Single Value
     ///
-    /// Executes the prepared statement locating the high or low index in
-    /// the diffs table and returns that index.
+    /// Executes a prepared statement (which has parameters bound to it)
+    /// for which the result of a single value is expected.
     ///
-    /// \param stmt_id Index of the prepared statement to execute
-    /// \param zone_id ID of the zone for which the index is being sought
-    /// \param serial Zone serial number for which an index is being sought.
+    /// \param stindex Index of prepared statement in statement table.
     ///
-    /// \return int ID of the row in the difss table corresponding to the
-    ///         statement.
+    /// \return Value of SELECT.
     ///
-    /// \exception NoSuchSerial Serial number not found
-    int findIndex(StatementID stindex, int zone_id, uint32_t serial) {
-
-        // Set up the statement
-        clearBindings(stindex);
-        bindInt(stindex, 1, zone_id);
-        bindInt(stindex, 2, serial);
+    /// \exception TooMuchData Multiple rows returned when one expected
+    /// \exception TooLittleData Zero rows returned when one expected
+    /// \exception DataSourceError SQLite3-related error
+    int getSingleValue(StatementID stindex) {
 
         // Get a pointer to the statement for brevity (does not transfer
         // resources)
@@ -687,7 +688,7 @@ private:
         if (rc == SQLITE_ROW) {
 
             // Got some data, extract the value
-            result = sqlite3_column_int(stmt, 1);
+            result = sqlite3_column_int(stmt, 0);
             int rc = sqlite3_step(stmt);
             if (rc == SQLITE_DONE) {
 
@@ -695,15 +696,15 @@ private:
                 return (result);
 
             } else if (rc == SQLITE_ROW) {
-                isc_throw(DataSourceError, "request to return one value from "
-                          "diffs table for serial " << serial << " (zone ID " <<
-                          zone_id << ") returned multiple values");
+                isc_throw(TooMuchData, "request to return one value from "
+                          "diffs table returned multiple values");
             }
         } else if (rc == SQLITE_DONE) {
 
-            // No data in the table for this zone and version.  Note that.
-            isc_throw(NoSuchSerial, "no data on serial number " << serial <<
-                      " for zone ID " << zone_id);
+            // No data in the table.  A bare exception with no explanation is
+            // thrown, as it will be replaced by a more informative one by
+            // the caller.
+            isc_throw(TooLittleData, "");
         }
 
         // We get here on an error.
@@ -714,6 +715,63 @@ private:
         return (result);
     }
 
+    /// \brief Find index
+    ///
+    /// Executes the prepared statement locating the high or low index in
+    /// the diffs table and returns that index.
+    ///
+    /// \param stmt_id Index of the prepared statement to execute
+    /// \param zone_id ID of the zone for which the index is being sought
+    /// \param serial Zone serial number for which an index is being sought.
+    ///
+    /// \return int ID of the row in the difss table corresponding to the
+    ///         statement.
+    ///
+    /// \exception NoSuchSerial Serial number not found.
+    ///
+    /// TODO: NoSuchSerial will be thrown if no records are found.  This is
+    /// most likely due to there being no match for the serial number, but it
+    /// could also be 
+    /// might also occur if there are no difference records for the zone in
+    /// the table.  We can check for this, but only at the cost of
+    /// no difference records in the table.  We can disambiguate these cases,
+    /// but only by 
+    int findIndex(StatementID stindex, int zone_id, uint32_t serial) {
+
+        // Set up the statement
+        clearBindings(stindex);
+        bindInt(stindex, 1, zone_id);
+        bindInt(stindex, 2, serial);
+
+        // Execute the statement
+        int result = -1;
+        try {
+            result = getSingleValue(stindex);
+
+        } catch (TooLittleData) {
+
+            // Why is there too little data?  Could be there is no data in
+            // the table for the zone, or there is but there is no data for
+            // that particular serial number.  Do another query to find out.
+            clearBindings(COUNT_DIFF_RECS);
+            bindInt(COUNT_DIFF_RECS, 1, zone_id);
+
+            // If this throws an exception, let it propagate - there is
+            // definitely an error.
+            result = getSingleValue(COUNT_DIFF_RECS);
+            if (result == 0) {
+                isc_throw(NoDiffRecs, "no data in differences table for "
+                          "zone ID " << zone_id);
+            } else {
+                isc_throw(NoSuchSerial, "no data in differences table for "
+                          "zone ID " << zone_id << ", serial number " <<
+                          serial);
+            }
+        }
+
+        return (result);
+    }
+
     
     boost::shared_ptr<const SQLite3Accessor> accessor_; // Accessor object
 };
@@ -721,7 +779,7 @@ private:
 // ... and return the iterator
 
 DatabaseAccessor::IteratorContextPtr
-SQLite3Accessor::getDiffs(int id, int start, int end) const {
+SQLite3Accessor::getDiffs(int id, uint32_t start, uint32_t end) const {
     return (IteratorContextPtr(new DiffContext(shared_from_this(), id, start,
                                end)));
 }

src/lib/datasrc/sqlite3_accessor.h

@@ -47,10 +47,46 @@ public:
 };
 
 /**
+ * \brief Too Much Data
+ *
+ * Thrown if a query expecting a certain number of rows back returned too
+ * many rows.
+ */
+class TooMuchData : public Exception {
+public:
+    TooMuchData(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/**
+ * \brief Too Little Data
+ *
+ * Thrown if a query expecting a certain number of rows back returned too
+ * few rows (including none).
+ */
+class TooLittleData : public Exception {
+public:
+    TooLittleData(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/**
+ * \brief No difference records
+ *
+ * Thrown if there are no difference records in the table for the requested
+ * zone.
+ */
+class NoDiffRecs : public Exception {
+public:
+    NoDiffRecs(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/**
  * \brief No such serial number when obtaining difference iterator
  *
  * Thrown if either the start or end version requested for the difference
- * iterator doe nsot exist.
+ * iterator does not exist.
  */
 class NoSuchSerial : public Exception {
 public:
@@ -157,7 +193,8 @@ public:
      *
      * \return Iterator containing difference records.
      */
-    virtual IteratorContextPtr getDiffs(int id, int start, int end) const;
+    virtual IteratorContextPtr
+    getDiffs(int id, uint32_t start, uint32_t end) const;
                                         
 
 

src/lib/datasrc/tests/database_unittest.cc

@@ -252,7 +252,7 @@ public:
                   "This database datasource can't be iterated");
     }
 
-    virtual IteratorContextPtr getDiffs(int, int, int) const {
+    virtual IteratorContextPtr getDiffs(int, uint32_t, uint32_t) const {
         isc_throw(isc::NotImplemented,
                   "This database datasource can't be iterated");
     }

src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -212,7 +212,7 @@ TEST_F(SQLite3AccessorTest, iterator) {
 // Test that at attempt to create a difference iterator for a serial that
 // does not exist throws an exception.
 
-TEST_F(SQLite3AccessorTest, diffIteratorNoVersion) {
+TEST_F(SQLite3AccessorTest, diffIteratorNoRecords) {
 
     // Our test zone is conveniently small, but not empty
     initAccessor(SQLITE_DBFILE_DIFFS, "IN");
@@ -229,6 +229,11 @@ TEST_F(SQLite3AccessorTest, diffIteratorNoVersion) {
     EXPECT_THROW(accessor->getDiffs(zone_info.second, 1231, 2234),
                  NoSuchSerial);
 
+    // Check that valid versions - but for the wrong zone which does not hold
+    // any records - throws the correct exception.
+    EXPECT_THROW(accessor->getDiffs(zone_info.second + 42, 1231, 1234),
+                 NoSuchSerial);
+
 }
 
 // Try to iterate through a valid set of differences