[1067] Unify the interface

src/lib/datasrc/database.cc

@@ -361,12 +361,12 @@ public:
 private:
     // Load next row of data
     void getData() {
-        string data[4];
-        data_ready_ = context_->getNext(data);
-        name_ = data[0];
-        rtype_ = data[1];
-        ttl_ = data[2];
-        rdata_ = data[3];
+        string data[DatabaseAccessor::COLUMN_COUNT];
+        data_ready_ = context_->getNext(data, DatabaseAccessor::COLUMN_COUNT);
+        name_ = data[DatabaseAccessor::NAME_COLUMN];
+        rtype_ = data[DatabaseAccessor::TYPE_COLUMN];
+        ttl_ = data[DatabaseAccessor::TTL_COLUMN];
+        rdata_ = data[DatabaseAccessor::RDATA_COLUMN];
     }
 
     // The context

src/lib/datasrc/database.h

@@ -108,13 +108,14 @@ public:
          * RRset must not be interleaved with any other RRs (eg. RRsets must be
          * "together").
          *
-         * \param data The data are to be returned by this parameter. They are
-         *     (in order) the name, rrtype, TTL and the rdata.
-         * \todo Unify with the interface in #1062 eventually.
+         * \param columns The data will be returned through here. The order
+         *     is specified by the RecordColumns enum.
+         * \param Size of the columns array. Must be equal to COLUMN_COUNT,
+         *     otherwise DataSourceError is thrown.
          * \todo Do we consider databases where it is stored in binary blob
          *     format?
          */
-        virtual bool getNext(std::string data[4]) = 0;
+        virtual bool getNext(std::string columns[], size_t column_data) = 0;
     };
 
     typedef boost::shared_ptr<IteratorContext> IteratorContextPtr;
@@ -222,7 +223,7 @@ public:
                             ///< the RRSIG covers. In the current implementation,
                             ///< this field is ignored.
         RDATA_COLUMN = 3,   ///< Full text representation of the record's RDATA
-        DOMAIN_NAME = 4     ///< The domain name of this RR
+        NAME_COLUMN = 4     ///< The domain name of this RR
     };
 
     /// The number of fields the columns array passed to getNextRecord should have

src/lib/datasrc/sqlite3_accessor.cc

@@ -141,7 +141,7 @@ const char* const q_zone_str = "SELECT id FROM zones WHERE name=?1 AND rdclass =
 const char* const q_any_str = "SELECT rdtype, ttl, sigtype, rdata, name "
     "FROM records WHERE zone_id=?1 AND name=?2";
 
-const char* const q_iterate_str = "SELECT name, rdtype, ttl, rdata FROM records "
+const char* const q_iterate_str = "SELECT rdtype, ttl, sigtype, rdata, name FROM records "
                                   "WHERE zone_id = ?1 "
                                   "ORDER BY name, rdtype";
 
@@ -362,6 +362,10 @@ convertToPlainChar(const unsigned char* ucp,
 }
 }
 
+// TODO: Once we want to have iterator returned from searchForRecords, this
+// class can be reused. It should be modified to take the sqlite3 statement
+// instead of creating it in constructor, it doesn't have to care which one
+// it is, just provide data from it.
 class SQLite3Database::Context : public DatabaseAccessor::IteratorContext {
 public:
     Context(const boost::shared_ptr<const SQLite3Database>& database, int id) :
@@ -375,18 +379,18 @@ public:
                       " to SQL statement (iterate)");
         }
     }
-    bool getNext(std::string data[4]) {
+    bool getNext(std::string data[], size_t size) {
+        if (size != COLUMN_COUNT) {
+            isc_throw(DataSourceError, "getNext received size of " << size <<
+                      ", not " << COLUMN_COUNT);
+        }
         // If there's another row, get it
         int rc(sqlite3_step(statement));
         if (rc == SQLITE_ROW) {
-            data[0] = convertToPlainChar(sqlite3_column_text(statement, 0),
-                                         database_->dbparameters_);
-            data[1] = convertToPlainChar(sqlite3_column_text(statement, 1),
-                                         database_->dbparameters_);
-            data[2] = boost::lexical_cast<std::string>(
-                sqlite3_column_int(statement, 2));
-            data[3] = convertToPlainChar(sqlite3_column_text(statement, 3),
-                                         database_->dbparameters_);
+            for (size_t i(0); i < size; ++ i) {
+                data[i] = convertToPlainChar(sqlite3_column_text(statement, i),
+                                             database_->dbparameters_);
+            }
             return (true);
         } else if (rc != SQLITE_DONE) {
             isc_throw(DataSourceError,

src/lib/datasrc/tests/database_unittest.cc

@@ -94,38 +94,41 @@ private:
         MockIteratorContext() :
             step(0)
         { }
-        virtual bool getNext(string data[4]) {
+        virtual bool getNext(string data[], size_t size) {
+            if (size != DatabaseAccessor::COLUMN_COUNT) {
+                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
+            }
             switch (step ++) {
                 case 0:
-                    data[0] = "example.org";
-                    data[1] = "SOA";
-                    data[2] = "300";
-                    data[3] = "ns1.example.org. admin.example.org. "
+                    data[DatabaseAccessor::NAME_COLUMN] = "example.org";
+                    data[DatabaseAccessor::TYPE_COLUMN] = "SOA";
+                    data[DatabaseAccessor::TTL_COLUMN] = "300";
+                    data[DatabaseAccessor::RDATA_COLUMN] = "ns1.example.org. admin.example.org. "
                         "1234 3600 1800 2419200 7200";
                     return (true);
                 case 1:
-                    data[0] = "x.example.org";
-                    data[1] = "A";
-                    data[2] = "300";
-                    data[3] = "192.0.2.1";
+                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
+                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
+                    data[DatabaseAccessor::TTL_COLUMN] = "300";
+                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
                     return (true);
                 case 2:
-                    data[0] = "x.example.org";
-                    data[1] = "A";
-                    data[2] = "300";
-                    data[3] = "192.0.2.2";
+                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
+                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
+                    data[DatabaseAccessor::TTL_COLUMN] = "300";
+                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.2";
                     return (true);
                 case 3:
-                    data[0] = "x.example.org";
-                    data[1] = "AAAA";
-                    data[2] = "300";
-                    data[3] = "2001:db8::1";
+                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
+                    data[DatabaseAccessor::TYPE_COLUMN] = "AAAA";
+                    data[DatabaseAccessor::TTL_COLUMN] = "300";
+                    data[DatabaseAccessor::RDATA_COLUMN] = "2001:db8::1";
                     return (true);
                 case 4:
-                    data[0] = "x.example.org";
-                    data[1] = "AAAA";
-                    data[2] = "300";
-                    data[3] = "2001:db8::2";
+                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
+                    data[DatabaseAccessor::TYPE_COLUMN] = "AAAA";
+                    data[DatabaseAccessor::TTL_COLUMN] = "300";
+                    data[DatabaseAccessor::RDATA_COLUMN] = "2001:db8::2";
                     return (true);
                 default:
                     ADD_FAILURE() <<
@@ -137,7 +140,10 @@ private:
     };
     class EmptyIteratorContext : public IteratorContext {
     public:
-        virtual bool getNext(string[4]) {
+        virtual bool getNext(string[], size_t size) {
+            if (size != DatabaseAccessor::COLUMN_COUNT) {
+                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
+            }
             return (false);
         }
     };
@@ -148,19 +154,22 @@ private:
         BadIteratorContext() :
             step(0)
         { }
-        virtual bool getNext(string data[4]) {
+        virtual bool getNext(string data[], size_t size) {
+            if (size != DatabaseAccessor::COLUMN_COUNT) {
+                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
+            }
             switch (step ++) {
                 case 0:
-                    data[0] = "x.example.org";
-                    data[1] = "A";
-                    data[2] = "300";
-                    data[3] = "192.0.2.1";
+                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
+                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
+                    data[DatabaseAccessor::TTL_COLUMN] = "300";
+                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.1";
                     return (true);
                 case 1:
-                    data[0] = "x.example.org";
-                    data[1] = "A";
-                    data[2] = "301";
-                    data[3] = "192.0.2.2";
+                    data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
+                    data[DatabaseAccessor::TYPE_COLUMN] = "A";
+                    data[DatabaseAccessor::TTL_COLUMN] = "301";
+                    data[DatabaseAccessor::RDATA_COLUMN] = "192.0.2.2";
                     return (true);
                 default:
                     ADD_FAILURE() <<

src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -114,16 +114,34 @@ TEST_F(SQLite3Access, iterator) {
     ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
               context);
 
-    std::string data[4];
+    size_t size(5);
+    std::string data[size];
     // Get and check the first and only record
-    EXPECT_TRUE(context->getNext(data));
-    EXPECT_EQ("example2.com.", data[0]);
-    EXPECT_EQ("SOA", data[1]);
+    EXPECT_TRUE(context->getNext(data, size));
+    EXPECT_EQ("example2.com.", data[4]);
+    EXPECT_EQ("SOA", data[0]);
     EXPECT_EQ("master.example2.com. admin.example2.com. "
               "1234 3600 1800 2419200 7200", data[3]);
-    EXPECT_EQ("3600", data[2]);
+    EXPECT_EQ("3600", data[1]);
     // Check there's no other
-    EXPECT_FALSE(context->getNext(data));
+    EXPECT_FALSE(context->getNext(data, size));
+}
+
+TEST_F(SQLite3Access, iteratorColumnCount) {
+    // Our test zone is conveniently small, but not empty
+    initAccessor(SQLITE_DBFILE_EXAMPLE2, RRClass::IN());
+
+    // Get the iterator context
+    DatabaseAccessor::IteratorContextPtr
+        context(db->getAllRecords(Name("example2.com"), 1));
+    ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
+              context);
+
+    EXPECT_THROW(context->getNext(NULL, 0), DataSourceError);
+    std::string data[6];
+    EXPECT_THROW(context->getNext(data, 4), DataSourceError);
+    EXPECT_THROW(context->getNext(data, 6), DataSourceError);
+    EXPECT_NO_THROW(context->getNext(data, 5));
 }
 
 TEST(SQLite3Open, getDBNameExample2) {