Browse Source

Merge branch 'trac1183'

Jelte Jansen 13 years ago
parent
commit
2b755575c9

+ 57 - 70
src/lib/datasrc/database.cc

@@ -178,12 +178,20 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
                                  bool want_ns)
 {
     RRsigStore sig_store;
-    database_->searchForRecords(zone_id_, name.toText());
     bool records_found = false;
     isc::dns::RRsetPtr result_rrset;
 
+    // Request the context
+    DatabaseAccessor::IteratorContextPtr
+        context(database_->getRecords(name.toText(), zone_id_));
+    // It must not return NULL, that's a bug of the implementation
+    if (!context) {
+        isc_throw(isc::Unexpected, "Iterator context null at " +
+                  name.toText());
+    }
+
     std::string columns[DatabaseAccessor::COLUMN_COUNT];
-    while (database_->getNextRecord(columns, DatabaseAccessor::COLUMN_COUNT)) {
+    while (context->getNext(columns)) {
         if (!records_found) {
             records_found = true;
         }
@@ -284,7 +292,6 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
     return (std::pair<bool, isc::dns::RRsetPtr>(records_found, result_rrset));
 }
 
-
 ZoneFinder::FindResult
 DatabaseClient::Finder::find(const isc::dns::Name& name,
                              const isc::dns::RRType& type,
@@ -301,76 +308,56 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
     logger.debug(DBG_TRACE_DETAILED, DATASRC_DATABASE_FIND_RECORDS)
         .arg(database_->getDBName()).arg(name).arg(type);
 
-    try {
-        // First, do we have any kind of delegation (NS/DNAME) here?
-        Name origin(getOrigin());
-        size_t origin_label_count(origin.getLabelCount());
-        size_t current_label_count(name.getLabelCount());
-        // This is how many labels we remove to get origin
-        size_t remove_labels(current_label_count - origin_label_count);
-
-        // Now go trough all superdomains from origin down
-        for (int i(remove_labels); i > 0; --i) {
-            Name superdomain(name.split(i));
-            // Look if there's NS or DNAME (but ignore the NS in origin)
-            found = getRRset(superdomain, NULL, false, true,
-                             i != remove_labels && !glue_ok);
-            if (found.second) {
-                // We found something redirecting somewhere else
-                // (it can be only NS or DNAME here)
-                result_rrset = found.second;
-                if (result_rrset->getType() == isc::dns::RRType::NS()) {
-                    LOG_DEBUG(logger, DBG_TRACE_DETAILED,
-                              DATASRC_DATABASE_FOUND_DELEGATION).
-                        arg(database_->getDBName()).arg(superdomain);
-                    result_status = DELEGATION;
-                } else {
-                    LOG_DEBUG(logger, DBG_TRACE_DETAILED,
-                              DATASRC_DATABASE_FOUND_DNAME).
-                        arg(database_->getDBName()).arg(superdomain);
-                    result_status = DNAME;
-                }
-                // Don't search more
-                break;
-            }
-        }
-
-        if (!result_rrset) { // Only if we didn't find a redirect already
-            // Try getting the final result and extract it
-            // It is special if there's a CNAME or NS, DNAME is ignored here
-            // And we don't consider the NS in origin
-            found = getRRset(name, &type, true, false,
-                             name != origin && !glue_ok);
-            records_found = found.first;
+    // First, do we have any kind of delegation (NS/DNAME) here?
+    const Name origin(getOrigin());
+    const size_t origin_label_count(origin.getLabelCount());
+    const size_t current_label_count(name.getLabelCount());
+    // This is how many labels we remove to get origin
+    const size_t remove_labels(current_label_count - origin_label_count);
+
+    // Now go trough all superdomains from origin down
+    for (int i(remove_labels); i > 0; --i) {
+        const Name superdomain(name.split(i));
+        // Look if there's NS or DNAME (but ignore the NS in origin)
+        found = getRRset(superdomain, NULL, false, true,
+                         i != remove_labels && !glue_ok);
+        if (found.second) {
+            // We found something redirecting somewhere else
+            // (it can be only NS or DNAME here)
             result_rrset = found.second;
-            if (result_rrset && name != origin && !glue_ok &&
-                result_rrset->getType() == isc::dns::RRType::NS()) {
+            if (result_rrset->getType() == isc::dns::RRType::NS()) {
                 LOG_DEBUG(logger, DBG_TRACE_DETAILED,
-                          DATASRC_DATABASE_FOUND_DELEGATION_EXACT).
-                    arg(database_->getDBName()).arg(name);
+                          DATASRC_DATABASE_FOUND_DELEGATION).
+                    arg(database_->getDBName()).arg(superdomain);
                 result_status = DELEGATION;
-            } else if (result_rrset && type != isc::dns::RRType::CNAME() &&
-                       result_rrset->getType() == isc::dns::RRType::CNAME()) {
-                result_status = CNAME;
+            } else {
+                LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+                          DATASRC_DATABASE_FOUND_DNAME).
+                    arg(database_->getDBName()).arg(superdomain);
+                result_status = DNAME;
             }
+            // Don't search more
+            break;
+        }
+    }
+
+    if (!result_rrset) { // Only if we didn't find a redirect already
+        // Try getting the final result and extract it
+        // It is special if there's a CNAME or NS, DNAME is ignored here
+        // And we don't consider the NS in origin
+        found = getRRset(name, &type, true, false, name != origin && !glue_ok);
+        records_found = found.first;
+        result_rrset = found.second;
+        if (result_rrset && name != origin && !glue_ok &&
+            result_rrset->getType() == isc::dns::RRType::NS()) {
+            LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+                      DATASRC_DATABASE_FOUND_DELEGATION_EXACT).
+                arg(database_->getDBName()).arg(name);
+            result_status = DELEGATION;
+        } else if (result_rrset && type != isc::dns::RRType::CNAME() &&
+                   result_rrset->getType() == isc::dns::RRType::CNAME()) {
+            result_status = CNAME;
         }
-    } catch (const DataSourceError& dse) {
-        logger.error(DATASRC_DATABASE_FIND_ERROR)
-            .arg(database_->getDBName()).arg(dse.what());
-        // call cleanup and rethrow
-        database_->resetSearch();
-        throw;
-    } catch (const isc::Exception& isce) {
-        logger.error(DATASRC_DATABASE_FIND_UNCAUGHT_ISC_ERROR)
-            .arg(database_->getDBName()).arg(isce.what());
-        // cleanup, change it to a DataSourceError and rethrow
-        database_->resetSearch();
-        isc_throw(DataSourceError, isce.what());
-    } catch (const std::exception& ex) {
-        logger.error(DATASRC_DATABASE_FIND_UNCAUGHT_ERROR)
-            .arg(database_->getDBName()).arg(ex.what());
-        database_->resetSearch();
-        throw;
     }
 
     if (!result_rrset) {
@@ -463,7 +450,7 @@ private:
     // Load next row of data
     void getData() {
         string data[DatabaseAccessor::COLUMN_COUNT];
-        data_ready_ = context_->getNext(data, DatabaseAccessor::COLUMN_COUNT);
+        data_ready_ = context_->getNext(data);
         name_ = data[DatabaseAccessor::NAME_COLUMN];
         rtype_ = data[DatabaseAccessor::TYPE_COLUMN];
         ttl_ = data[DatabaseAccessor::TTL_COLUMN];
@@ -494,7 +481,7 @@ DatabaseClient::getIterator(const isc::dns::Name& name) const {
     }
     // Request the context
     DatabaseAccessor::IteratorContextPtr
-        context(database_->getAllRecords(name, zone.second));
+        context(database_->getAllRecords(zone.second));
     // It must not return NULL, that's a bug of the implementation
     if (context == DatabaseAccessor::IteratorContextPtr()) {
         isc_throw(isc::Unexpected, "Iterator context null at " +

+ 61 - 99
src/lib/datasrc/database.h

@@ -49,6 +49,28 @@ namespace datasrc {
 class DatabaseAccessor : boost::noncopyable {
 public:
     /**
+     * Definitions of the fields as they are required to be filled in
+     * by IteratorContext::getNext()
+     *
+     * When implementing getNext(), the columns array should
+     * be filled with the values as described in this enumeration,
+     * in this order, i.e. TYPE_COLUMN should be the first element
+     * (index 0) of the array, TTL_COLUMN should be the second element
+     * (index 1), etc.
+     */
+    enum RecordColumns {
+        TYPE_COLUMN = 0,    ///< The RRType of the record (A/NS/TXT etc.)
+        TTL_COLUMN = 1,     ///< The TTL of the record (a
+        SIGTYPE_COLUMN = 2, ///< For RRSIG records, this contains the RRTYPE
+                            ///< the RRSIG covers. In the current implementation,
+                            ///< this field is ignored.
+        RDATA_COLUMN = 3,   ///< Full text representation of the record's RDATA
+        NAME_COLUMN = 4,    ///< The domain name of this RR
+        COLUMN_COUNT = 5    ///< The total number of columns, MUST be value of
+                            ///< the largest other element in this enum plus 1.
+    };
+
+    /**
      * \brief Destructor
      *
      * It is empty, but needs a virtual one, since we will use the derived
@@ -103,135 +125,75 @@ public:
          * \brief Function to provide next resource record
          *
          * This function should provide data about the next resource record
-         * from the iterated zone. The data are not converted yet.
+         * from the data that is searched. The data is not converted yet.
+         *
+         * Depending on how the iterator was constructed, there is a difference
+         * in behaviour; for a 'full zone iterator', created with
+         * getAllRecords(), all COLUMN_COUNT elements of the array are
+         * overwritten.
+         * For a 'name iterator', created with getRecords(), the column
+         * NAME_COLUMN is untouched, since what would be added here is by
+         * definition already known to the caller (it already passes it as
+         * an argument to getRecords()).
          *
          * \note The order of RRs is not strictly set, but the RRs for single
          * RRset must not be interleaved with any other RRs (eg. RRsets must be
          * "together").
          *
          * \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.
+         *     is specified by the RecordColumns enum, and the size must be
+         *     COLUMN_COUNT
          * \todo Do we consider databases where it is stored in binary blob
          *     format?
          * \throw DataSourceError if there's database-related error. If the
          *     exception (or any other in case of derived class) is thrown,
          *     the iterator can't be safely used any more.
+         * \return true if a record was found, and the columns array was
+         *         updated. false if there was no more data, in which case
+         *         the columns array is untouched.
          */
-        virtual bool getNext(std::string columns[], size_t column_data) = 0;
+        virtual bool getNext(std::string (&columns)[COLUMN_COUNT]) = 0;
     };
 
     typedef boost::shared_ptr<IteratorContext> IteratorContextPtr;
 
     /**
-     * \brief Creates an iterator context for the whole zone.
+     * \brief Creates an iterator context for a specific name.
+     *
+     * Returns an IteratorContextPtr that contains all records of the
+     * given name from the given zone.
      *
-     * This should create a new iterator context to be used by
-     * DatabaseConnection's ZoneIterator. It can be created based on the name
-     * or the ID (returned from getZone()), what is more comfortable for the
-     * database implementation. Both are provided (and are guaranteed to match,
-     * the DatabaseClient first looks up the zone ID and then calls this).
+     * The implementation of the iterator that is returned may leave the
+     * NAME_COLUMN column of the array passed to getNext() untouched, as that
+     * data is already known (it is the same as the name argument here)
      *
-     * The default implementation throws isc::NotImplemented, to allow
-     * "minimal" implementations of the connection not supporting optional
-     * functionality.
+     * \exception any Since any implementation can be used, the caller should
+     *            expect any exception to be thrown.
      *
-     * \param name The name of the zone.
+     * \param name The name to search for. This should be a FQDN.
      * \param id The ID of the zone, returned from getZone().
      * \return Newly created iterator context. Must not be NULL.
      */
-    virtual IteratorContextPtr getAllRecords(const isc::dns::Name& name,
-                                             int id) const
-    {
-        /*
-         * This is a compromise. We need to document the parameters in doxygen,
-         * so they need a name, but then it complains about unused parameter.
-         * This is a NOP that "uses" the parameters.
-         */
-        static_cast<void>(name);
-        static_cast<void>(id);
-
-        isc_throw(isc::NotImplemented,
-                  "This database datasource can't be iterated");
-    }
-
-    /**
-     * \brief Starts a new search for records of the given name in the given zone
-     *
-     * The data searched by this call can be retrieved with subsequent calls to
-     * getNextRecord().
-     *
-     * \exception DataSourceError if there is a problem connecting to the
-     *                            backend database
-     *
-     * \param zone_id The zone to search in, as returned by getZone()
-     * \param name The name of the records to find
-     */
-    virtual void searchForRecords(int zone_id, const std::string& name) = 0;
+    virtual IteratorContextPtr getRecords(const std::string& name,
+                                          int id) const = 0;
 
     /**
-     * \brief Retrieves the next record from the search started with searchForRecords()
-     *
-     * Returns a boolean specifying whether or not there was more data to read.
-     * In the case of a database error, a DatasourceError is thrown.
-     *
-     * The columns passed is an array of std::strings consisting of
-     * DatabaseConnection::COLUMN_COUNT elements, the elements of which
-     * are defined in DatabaseConnection::RecordColumns, in their basic
-     * string representation.
-     *
-     * If you are implementing a derived database connection class, you
-     * should have this method check the column_count value, and fill the
-     * array with strings conforming to their description in RecordColumn.
-     *
-     * \exception DatasourceError if there was an error reading from the database
-     *
-     * \param columns The elements of this array will be filled with the data
-     *                for one record as defined by RecordColumns
-     *                If there was no data, the array is untouched.
-     * \return true if there was a next record, false if there was not
-     */
-    virtual bool getNextRecord(std::string columns[], size_t column_count) = 0;
-
-    /**
-     * \brief Resets the current search initiated with searchForRecords()
+     * \brief Creates an iterator context for the whole zone.
      *
-     * This method will be called when the called of searchForRecords() and
-     * getNextRecord() finds bad data, and aborts the current search.
-     * It should clean up whatever handlers searchForRecords() created, and
-     * any other state modified or needed by getNextRecord()
+     * Returns an IteratorContextPtr that contains all records of the
+     * zone with the given zone id.
      *
-     * Of course, the implementation of getNextRecord may also use it when
-     * it is done with a search. If it does, the implementation of this
-     * method should make sure it can handle being called multiple times.
+     * Each call to getNext() on the returned iterator should copy all
+     * column fields of the array that is passed, as defined in the
+     * RecordColumns enum.
      *
-     * The implementation for this method should make sure it never throws.
-     */
-    virtual void resetSearch() = 0;
-
-    /**
-     * Definitions of the fields as they are required to be filled in
-     * by getNextRecord()
+     * \exception any Since any implementation can be used, the caller should
+     *            expect any exception to be thrown.
      *
-     * When implementing getNextRecord(), the columns array should
-     * be filled with the values as described in this enumeration,
-     * in this order, i.e. TYPE_COLUMN should be the first element
-     * (index 0) of the array, TTL_COLUMN should be the second element
-     * (index 1), etc.
+     * \param id The ID of the zone, returned from getZone().
+     * \return Newly created iterator context. Must not be NULL.
      */
-    enum RecordColumns {
-        TYPE_COLUMN = 0,    ///< The RRType of the record (A/NS/TXT etc.)
-        TTL_COLUMN = 1,     ///< The TTL of the record (a
-        SIGTYPE_COLUMN = 2, ///< For RRSIG records, this contains the RRTYPE
-                            ///< the RRSIG covers. In the current implementation,
-                            ///< this field is ignored.
-        RDATA_COLUMN = 3,   ///< Full text representation of the record's RDATA
-        NAME_COLUMN = 4     ///< The domain name of this RR
-    };
-
-    /// The number of fields the columns array passed to getNextRecord should have
-    static const size_t COLUMN_COUNT = 5;
+    virtual IteratorContextPtr getAllRecords(int id) const = 0;
 
     /**
      * \brief Returns a string identifying this dabase backend

+ 0 - 18
src/lib/datasrc/datasrc_messages.mes

@@ -63,12 +63,6 @@ The maximum allowed number of items of the hotspot cache is set to the given
 number. If there are too many, some of them will be dropped. The size of 0
 means no limit.
 
-% DATASRC_DATABASE_FIND_ERROR error retrieving data from datasource %1: %2
-This was an internal error while reading data from a datasource. This can either
-mean the specific data source implementation is not behaving correctly, or the
-data it provides is invalid. The current search is aborted.
-The error message contains specific information about the error.
-
 % DATASRC_DATABASE_FIND_RECORDS looking in datasource %1 for record %2/%3
 Debug information. The database data source is looking up records with the given
 name and type in the database.
@@ -79,18 +73,6 @@ different TTL values. This isn't allowed on the wire and is considered
 an error, so we set it to the lowest value we found (but we don't modify the
 database). The data in database should be checked and fixed.
 
-% DATASRC_DATABASE_FIND_UNCAUGHT_ERROR uncaught general error retrieving data from datasource %1: %2
-There was an uncaught general exception while reading data from a datasource.
-This most likely points to a logic error in the code, and can be considered a
-bug. The current search is aborted. Specific information about the exception is
-printed in this error message.
-
-% DATASRC_DATABASE_FIND_UNCAUGHT_ISC_ERROR uncaught error retrieving data from datasource %1: %2
-There was an uncaught ISC exception while reading data from a datasource. This
-most likely points to a logic error in the code, and can be considered a bug.
-The current search is aborted. Specific information about the exception is
-printed in this error message.
-
 % DATASRC_DATABASE_FOUND_DELEGATION Found delegation at %2 in %1
 When searching for a domain, the program met a delegation to a different zone
 at the given domain name. It will return that one instead.

+ 111 - 115
src/lib/datasrc/sqlite3_accessor.cc

@@ -27,7 +27,7 @@ namespace datasrc {
 struct SQLite3Parameters {
     SQLite3Parameters() :
         db_(NULL), version_(-1),
-        q_zone_(NULL), q_any_(NULL)
+        q_zone_(NULL)
         /*q_record_(NULL), q_addrs_(NULL), q_referral_(NULL),
         q_count_(NULL), q_previous_(NULL), q_nsec3_(NULL),
         q_prevnsec3_(NULL) */
@@ -35,7 +35,6 @@ struct SQLite3Parameters {
     sqlite3* db_;
     int version_;
     sqlite3_stmt* q_zone_;
-    sqlite3_stmt* q_any_;
     /*
     TODO: Yet unneeded statements
     sqlite3_stmt* q_record_;
@@ -49,7 +48,7 @@ struct SQLite3Parameters {
 };
 
 SQLite3Database::SQLite3Database(const std::string& filename,
-                                     const isc::dns::RRClass& rrclass) :
+                                 const isc::dns::RRClass& rrclass) :
     dbparameters_(new SQLite3Parameters),
     class_(rrclass.toText()),
     database_name_("sqlite3_" +
@@ -75,9 +74,6 @@ public:
         if (params_.q_zone_ != NULL) {
             sqlite3_finalize(params_.q_zone_);
         }
-        if (params_.q_any_ != NULL) {
-            sqlite3_finalize(params_.q_any_);
-        }
         /*
         if (params_.q_record_ != NULL) {
             sqlite3_finalize(params_.q_record_);
@@ -138,9 +134,13 @@ const char* const SCHEMA_LIST[] = {
 
 const char* const q_zone_str = "SELECT id FROM zones WHERE name=?1 AND rdclass = ?2";
 
-const char* const q_any_str = "SELECT rdtype, ttl, sigtype, rdata, name "
+// note that the order of the SELECT values is specifically chosen to match
+// the enum values in RecordColumns
+const char* const q_any_str = "SELECT rdtype, ttl, sigtype, rdata "
     "FROM records WHERE zone_id=?1 AND name=?2";
 
+// note that the order of the SELECT values is specifically chosen to match
+// the enum values in RecordColumns
 const char* const q_iterate_str = "SELECT rdtype, ttl, sigtype, rdata, name FROM records "
                                   "WHERE zone_id = ?1 "
                                   "ORDER BY name, rdtype";
@@ -210,7 +210,6 @@ checkAndSetupSchema(Initializer* initializer) {
     }
 
     initializer->params_.q_zone_ = prepare(db, q_zone_str);
-    initializer->params_.q_any_ = prepare(db, q_any_str);
     /* TODO: Yet unneeded statements
     initializer->params_.q_record_ = prepare(db, q_record_str);
     initializer->params_.q_addrs_ = prepare(db, q_addrs_str);
@@ -239,7 +238,7 @@ SQLite3Database::open(const std::string& name) {
     }
 
     checkAndSetupSchema(&initializer);
-    initializer.move(dbparameters_);
+    initializer.move(dbparameters_.get());
 }
 
 SQLite3Database::~SQLite3Database() {
@@ -247,7 +246,6 @@ SQLite3Database::~SQLite3Database() {
     if (dbparameters_->db_ != NULL) {
         close();
     }
-    delete dbparameters_;
 }
 
 void
@@ -262,9 +260,6 @@ SQLite3Database::close(void) {
     sqlite3_finalize(dbparameters_->q_zone_);
     dbparameters_->q_zone_ = NULL;
 
-    sqlite3_finalize(dbparameters_->q_any_);
-    dbparameters_->q_any_ = NULL;
-
     /* TODO: Once they are needed or not, uncomment or drop
     sqlite3_finalize(dbparameters->q_record_);
     dbparameters->q_record_ = NULL;
@@ -333,63 +328,54 @@ SQLite3Database::getZone(const isc::dns::Name& name) const {
     return (std::pair<bool, int>(false, 0));
 }
 
-namespace {
-// This helper function converts from the unsigned char* type (used by
-// sqlite3) to char* (wanted by std::string). Technically these types
-// might not be directly convertable
-// In case sqlite3_column_text() returns NULL, we just make it an
-// empty string.
-// The sqlite3parameters value is only used to check the error code if
-// ucp == NULL
-const char*
-convertToPlainChar(const unsigned char* ucp,
-                   SQLite3Parameters* dbparameters) {
-    if (ucp == NULL) {
-        // The field can really be NULL, in which case we return an
-        // empty string, or sqlite may have run out of memory, in
-        // which case we raise an error
-        if (dbparameters != NULL &&
-            sqlite3_errcode(dbparameters->db_) == SQLITE_NOMEM) {
-            isc_throw(DataSourceError,
-                      "Sqlite3 backend encountered a memory allocation "
-                      "error in sqlite3_column_text()");
-        } else {
-            return ("");
-        }
-    }
-    const void* p = ucp;
-    return (static_cast<const char*>(p));
-}
-}
 
-// 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:
+    // Construct an iterator for all records. When constructed this
+    // way, the getNext() call will copy all fields
     Context(const boost::shared_ptr<const SQLite3Database>& database, int id) :
+        iterator_type_(ITT_ALL),
         database_(database),
-        statement(NULL)
+        statement_(NULL),
+        name_("")
     {
         // We create the statement now and then just keep getting data from it
-        statement = prepare(database->dbparameters_->db_, q_iterate_str);
-        if (sqlite3_bind_int(statement, 1, id) != SQLITE_OK) {
-            isc_throw(SQLite3Error, "Could not bind " << id <<
-                      " to SQL statement (iterate)");
-        }
+        statement_ = prepare(database->dbparameters_->db_, q_iterate_str);
+        bindZoneId(id);
     }
-    bool getNext(std::string data[], size_t size) {
-        if (size != COLUMN_COUNT) {
-            isc_throw(DataSourceError, "getNext received size of " << size <<
-                      ", not " << COLUMN_COUNT);
-        }
+
+    // Construct an iterator for records with a specific name. When constructed
+    // this way, the getNext() call will copy all fields except name
+    Context(const boost::shared_ptr<const SQLite3Database>& database, int id,
+            const std::string& name) :
+        iterator_type_(ITT_NAME),
+        database_(database),
+        statement_(NULL),
+        name_(name)
+    {
+        // We create the statement now and then just keep getting data from it
+        statement_ = prepare(database->dbparameters_->db_, q_any_str);
+        bindZoneId(id);
+        bindName(name_);
+    }
+
+    bool getNext(std::string (&data)[COLUMN_COUNT]) {
         // If there's another row, get it
-        int rc(sqlite3_step(statement));
+        // If finalize has been called (e.g. when previous getNext() got
+        // SQLITE_DONE), directly return false
+        if (statement_ == NULL) {
+            return false;
+        }
+        const int rc(sqlite3_step(statement_));
         if (rc == SQLITE_ROW) {
-            for (size_t i(0); i < size; ++ i) {
-                data[i] = convertToPlainChar(sqlite3_column_text(statement, i),
-                                             database_->dbparameters_);
+            // For both types, we copy the first four columns
+            copyColumn(data, TYPE_COLUMN);
+            copyColumn(data, TTL_COLUMN);
+            copyColumn(data, SIGTYPE_COLUMN);
+            copyColumn(data, RDATA_COLUMN);
+            // Only copy Name if we are iterating over every record
+            if (iterator_type_ == ITT_ALL) {
+                copyColumn(data, NAME_COLUMN);
             }
             return (true);
         } else if (rc != SQLITE_DONE) {
@@ -397,79 +383,89 @@ public:
                       "Unexpected failure in sqlite3_step: " <<
                       sqlite3_errmsg(database_->dbparameters_->db_));
         }
+        finalize();
         return (false);
     }
+
     virtual ~Context() {
-        if (statement) {
-            sqlite3_finalize(statement);
-        }
+        finalize();
     }
 
 private:
-    boost::shared_ptr<const SQLite3Database> database_;
-    sqlite3_stmt *statement;
-};
-
-DatabaseAccessor::IteratorContextPtr
-SQLite3Database::getAllRecords(const isc::dns::Name&, int id) const {
-    return (IteratorContextPtr(new Context(shared_from_this(), id)));
-}
-
-void
-SQLite3Database::searchForRecords(int zone_id, const std::string& name) {
-    resetSearch();
-    if (sqlite3_bind_int(dbparameters_->q_any_, 1, zone_id) != SQLITE_OK) {
-        isc_throw(DataSourceError,
-                  "Error in sqlite3_bind_int() for zone_id " <<
-                  zone_id << ": " << sqlite3_errmsg(dbparameters_->db_));
+    // Depending on which constructor is called, behaviour is slightly
+    // different. We keep track of what to do with the iterator type
+    // See description of getNext() and the constructors
+    enum IteratorType {
+        ITT_ALL,
+        ITT_NAME
+    };
+
+    void copyColumn(std::string (&data)[COLUMN_COUNT], int column) {
+        data[column] = convertToPlainChar(sqlite3_column_text(statement_,
+                                                              column));
     }
-    // use transient since name is a ref and may disappear
-    if (sqlite3_bind_text(dbparameters_->q_any_, 2, name.c_str(), -1,
-                               SQLITE_TRANSIENT) != SQLITE_OK) {
-        isc_throw(DataSourceError,
-                  "Error in sqlite3_bind_text() for name " <<
-                  name << ": " << sqlite3_errmsg(dbparameters_->db_));
+
+    void bindZoneId(const int zone_id) {
+        if (sqlite3_bind_int(statement_, 1, zone_id) != SQLITE_OK) {
+            finalize();
+            isc_throw(SQLite3Error, "Could not bind int " << zone_id <<
+                      " to SQL statement: " <<
+                      sqlite3_errmsg(database_->dbparameters_->db_));
+        }
     }
-}
 
-bool
-SQLite3Database::getNextRecord(std::string columns[], size_t column_count) {
-    if (column_count != COLUMN_COUNT) {
-            isc_throw(DataSourceError,
-                    "Datasource backend caller did not pass a column array "
-                    "of size " << COLUMN_COUNT << " to getNextRecord()");
+    void bindName(const std::string& name) {
+        if (sqlite3_bind_text(statement_, 2, name.c_str(), -1,
+                              SQLITE_STATIC) != SQLITE_OK) {
+            const char* errmsg = sqlite3_errmsg(database_->dbparameters_->db_);
+            finalize();
+            isc_throw(SQLite3Error, "Could not bind text '" << name <<
+                      "' to SQL statement: " << errmsg);
+        }
     }
 
-    sqlite3_stmt* current_stmt = dbparameters_->q_any_;
-    const int rc = sqlite3_step(current_stmt);
+    void finalize() {
+        sqlite3_finalize(statement_);
+        statement_ = NULL;
+    }
 
-    if (rc == SQLITE_ROW) {
-        for (int column = 0; column < column_count; ++column) {
-            try {
-                columns[column] = convertToPlainChar(sqlite3_column_text(
-                                                     current_stmt, column),
-                                                     dbparameters_);
-            } catch (const std::bad_alloc&) {
+    // This helper method converts from the unsigned char* type (used by
+    // sqlite3) to char* (wanted by std::string). Technically these types
+    // might not be directly convertable
+    // In case sqlite3_column_text() returns NULL, we just make it an
+    // empty string, unless it was caused by a memory error
+    const char* convertToPlainChar(const unsigned char* ucp) {
+        if (ucp == NULL) {
+            // The field can really be NULL, in which case we return an
+            // empty string, or sqlite may have run out of memory, in
+            // which case we raise an error
+            if (sqlite3_errcode(database_->dbparameters_->db_)
+                                == SQLITE_NOMEM) {
                 isc_throw(DataSourceError,
-                        "bad_alloc in Sqlite3Connection::getNextRecord");
+                        "Sqlite3 backend encountered a memory allocation "
+                        "error in sqlite3_column_text()");
+            } else {
+                return ("");
             }
         }
-        return (true);
-    } else if (rc == SQLITE_DONE) {
-        // reached the end of matching rows
-        resetSearch();
-        return (false);
+        const void* p = ucp;
+        return (static_cast<const char*>(p));
     }
-    isc_throw(DataSourceError, "Unexpected failure in sqlite3_step: " <<
-                               sqlite3_errmsg(dbparameters_->db_));
-    // Compilers might not realize isc_throw always throws
-    return (false);
+
+    const IteratorType iterator_type_;
+    boost::shared_ptr<const SQLite3Database> database_;
+    sqlite3_stmt *statement_;
+    const std::string name_;
+};
+
+DatabaseAccessor::IteratorContextPtr
+SQLite3Database::getRecords(const std::string& name, int id) const {
+    return (IteratorContextPtr(new Context(shared_from_this(), id, name)));
 }
 
-void
-SQLite3Database::resetSearch() {
-    sqlite3_reset(dbparameters_->q_any_);
-    sqlite3_clear_bindings(dbparameters_->q_any_);
+DatabaseAccessor::IteratorContextPtr
+SQLite3Database::getAllRecords(int id) const {
+    return (IteratorContextPtr(new Context(shared_from_this(), id)));
 }
 
 }

+ 19 - 40
src/lib/datasrc/sqlite3_accessor.h

@@ -21,6 +21,7 @@
 #include <exceptions/exceptions.h>
 
 #include <boost/enable_shared_from_this.hpp>
+#include <boost/scoped_ptr.hpp>
 #include <string>
 
 namespace isc {
@@ -76,6 +77,7 @@ public:
      * Closes the database.
      */
     ~SQLite3Database();
+
     /**
      * \brief Look up a zone
      *
@@ -91,54 +93,31 @@ public:
      */
     virtual std::pair<bool, int> getZone(const isc::dns::Name& name) const;
 
-    /// \brief Implementation of DatabaseAbstraction::getAllRecords
-    virtual IteratorContextPtr getAllRecords(const isc::dns::Name&,
-                                                  int id) const;
-    /**
-     * \brief Start a new search for the given name in the given zone.
+    /** \brief Look up all resource records for a name
      *
-     * This implements the searchForRecords from DatabaseConnection.
-     * This particular implementation does not raise DataSourceError.
+     * This implements the getRecords() method from DatabaseAccessor
      *
-     * \exception DataSourceError when sqlite3_bind_int() or
-     *                            sqlite3_bind_text() fails
+     * \exception SQLite3Error if there is an sqlite3 error when performing
+     *                         the query
      *
-     * \param zone_id The zone to seach in, as returned by getZone()
-     * \param name The name to find records for
+     * \param name the name to look up
+     * \param id the zone id, as returned by getZone()
+     * \return Iterator that contains all records with the given name
      */
-    virtual void searchForRecords(int zone_id, const std::string& name);
+    virtual IteratorContextPtr getRecords(const std::string& name,
+                                          int id) const;
 
-    /**
-     * \brief Retrieve the next record from the search started with
-     *        searchForRecords
-     *
-     * This implements the getNextRecord from DatabaseConnection.
-     * See the documentation there for more information.
+    /** \brief Look up all resource records for a zone
      *
-     * If this method raises an exception, the contents of columns are undefined.
-     *
-     * \exception DataSourceError if there is an error returned by sqlite_step()
-     *                            When this exception is raised, the current
-     *                            search as initialized by searchForRecords() is
-     *                            NOT reset, and the caller is expected to take
-     *                            care of that.
-     * \param columns This vector will be cleared, and the fields of the record will
-     *                be appended here as strings (in the order rdtype, ttl, sigtype,
-     *                and rdata). If there was no data (i.e. if this call returns
-     *                false), the vector is untouched.
-     * \return true if there was a next record, false if there was not
-     */
-    virtual bool getNextRecord(std::string columns[], size_t column_count);
-
-    /**
-     * \brief Resets any state created by searchForRecords
+     * This implements the getRecords() method from DatabaseAccessor
      *
-     * This implements the resetSearch from DatabaseConnection.
-     * See the documentation there for more information.
+     * \exception SQLite3Error if there is an sqlite3 error when performing
+     *                         the query
      *
-     * This function never throws.
+     * \param id the zone id, as returned by getZone()
+     * \return Iterator that contains all records in the given zone
      */
-    virtual void resetSearch();
+    virtual IteratorContextPtr getAllRecords(int id) const;
 
     /// The SQLite3 implementation of this method returns a string starting
     /// with a fixed prefix of "sqlite3_" followed by the DB file name
@@ -149,7 +128,7 @@ public:
 
 private:
     /// \brief Private database data
-    SQLite3Parameters* dbparameters_;
+    boost::scoped_ptr<SQLite3Parameters> dbparameters_;
     /// \brief The class for which the queries are done
     const std::string class_;
     /// \brief Opens the database

+ 85 - 145
src/lib/datasrc/tests/database_unittest.cc

@@ -62,12 +62,15 @@ public:
         return (database_name_);
     }
 
-    // These are just to compile, they won't be called
-    virtual void searchForRecords(int, const std::string&) { }
-    virtual bool getNextRecord(string*, size_t) {
-        return (false);
-    }
-    virtual void resetSearch() { }
+    virtual IteratorContextPtr getRecords(const std::string&, int) const {
+        isc_throw(isc::NotImplemented,
+                  "This database datasource can't be iterated");
+    };
+
+    virtual IteratorContextPtr getAllRecords(int) const {
+        isc_throw(isc::NotImplemented,
+                  "This database datasource can't be iterated");
+    };
 private:
     const std::string database_name_;
 
@@ -82,11 +85,67 @@ private:
  */
 class MockAccessor : public NopAccessor {
 public:
-    MockAccessor() : cur_record(0), search_running_(false)
+    MockAccessor()
     {
         fillData();
     }
 private:
+    class MockNameIteratorContext : public IteratorContext {
+    public:
+        MockNameIteratorContext(const MockAccessor& mock_accessor, int zone_id,
+                                const std::string& name) :
+            searched_name_(name), cur_record_(0)
+        {
+            // 'hardcoded' names to trigger exceptions
+            // On these names some exceptions are thrown, to test the robustness
+            // of the find() method.
+            if (searched_name_ == "dsexception.in.search.") {
+                isc_throw(DataSourceError, "datasource exception on search");
+            } else if (searched_name_ == "iscexception.in.search.") {
+                isc_throw(isc::Exception, "isc exception on search");
+            } else if (searched_name_ == "basicexception.in.search.") {
+                throw std::exception();
+            }
+
+            // we're not aiming for efficiency in this test, simply
+            // copy the relevant vector from records
+            if (zone_id == 42) {
+                if (mock_accessor.records.count(searched_name_) > 0) {
+                    cur_name = mock_accessor.records.find(searched_name_)->second;
+                } else {
+                    cur_name.clear();
+                }
+            } else {
+                cur_name.clear();
+            }
+        }
+
+        virtual bool getNext(std::string (&columns)[COLUMN_COUNT]) {
+            if (searched_name_ == "dsexception.in.getnext.") {
+                isc_throw(DataSourceError, "datasource exception on getnextrecord");
+            } else if (searched_name_ == "iscexception.in.getnext.") {
+                isc_throw(isc::Exception, "isc exception on getnextrecord");
+            } else if (searched_name_ == "basicexception.in.getnext.") {
+                throw std::exception();
+            }
+
+            if (cur_record_ < cur_name.size()) {
+                for (size_t i = 0; i < COLUMN_COUNT; ++i) {
+                    columns[i] = cur_name[cur_record_][i];
+                }
+                cur_record_++;
+                return (true);
+            } else {
+                return (false);
+            }
+        }
+
+    private:
+        const std::string searched_name_;
+        int cur_record_;
+        std::vector< std::vector<std::string> > cur_name;
+    };
+
     class MockIteratorContext : public IteratorContext {
     private:
         int step;
@@ -94,10 +153,7 @@ private:
         MockIteratorContext() :
             step(0)
         { }
-        virtual bool getNext(string data[], size_t size) {
-            if (size != DatabaseAccessor::COLUMN_COUNT) {
-                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
-            }
+        virtual bool getNext(string (&data)[COLUMN_COUNT]) {
             switch (step ++) {
                 case 0:
                     data[DatabaseAccessor::NAME_COLUMN] = "example.org";
@@ -140,10 +196,7 @@ private:
     };
     class EmptyIteratorContext : public IteratorContext {
     public:
-        virtual bool getNext(string[], size_t size) {
-            if (size != DatabaseAccessor::COLUMN_COUNT) {
-                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
-            }
+        virtual bool getNext(string(&)[COLUMN_COUNT]) {
             return (false);
         }
     };
@@ -154,10 +207,7 @@ private:
         BadIteratorContext() :
             step(0)
         { }
-        virtual bool getNext(string data[], size_t size) {
-            if (size != DatabaseAccessor::COLUMN_COUNT) {
-                isc_throw(DataSourceError, "Wrong column count in getNextRecord");
-            }
+        virtual bool getNext(string (&data)[COLUMN_COUNT]) {
             switch (step ++) {
                 case 0:
                     data[DatabaseAccessor::NAME_COLUMN] = "x.example.org";
@@ -180,7 +230,7 @@ private:
         }
     };
 public:
-    virtual IteratorContextPtr getAllRecords(const Name&, int id) const {
+    virtual IteratorContextPtr getAllRecords(int id) const {
         if (id == 42) {
             return (IteratorContextPtr(new MockIteratorContext()));
         } else if (id == 13) {
@@ -194,83 +244,19 @@ public:
         }
     }
 
-    virtual void searchForRecords(int zone_id, const std::string& name) {
-        search_running_ = true;
-
-        // 'hardcoded' name to trigger exceptions (for testing
-        // the error handling of find() (the other on is below in
-        // if the name is "exceptiononsearch" it'll raise an exception here
-        if (name == "dsexception.in.search.") {
-            isc_throw(DataSourceError, "datasource exception on search");
-        } else if (name == "iscexception.in.search.") {
-            isc_throw(isc::Exception, "isc exception on search");
-        } else if (name == "basicexception.in.search.") {
-            throw std::exception();
-        }
-        searched_name_ = name;
-
-        // we're not aiming for efficiency in this test, simply
-        // copy the relevant vector from records
-        cur_record = 0;
-        if (zone_id == 42) {
-            if (records.count(name) > 0) {
-                cur_name = records.find(name)->second;
-            } else {
-                cur_name.clear();
-            }
-        } else {
-            cur_name.clear();
-        }
-    };
-
-    virtual bool getNextRecord(std::string columns[], size_t column_count) {
-        if (searched_name_ == "dsexception.in.getnext.") {
-            isc_throw(DataSourceError, "datasource exception on getnextrecord");
-        } else if (searched_name_ == "iscexception.in.getnext.") {
-            isc_throw(isc::Exception, "isc exception on getnextrecord");
-        } else if (searched_name_ == "basicexception.in.getnext.") {
-            throw std::exception();
-        }
-
-        if (column_count != DatabaseAccessor::COLUMN_COUNT) {
-            isc_throw(DataSourceError, "Wrong column count in getNextRecord");
-        }
-        if (cur_record < cur_name.size()) {
-            for (size_t i = 0; i < column_count; ++i) {
-                columns[i] = cur_name[cur_record][i];
-            }
-            cur_record++;
-            return (true);
+    virtual IteratorContextPtr getRecords(const std::string& name, int id) const {
+        if (id == 42) {
+            return (IteratorContextPtr(new MockNameIteratorContext(*this, id, name)));
         } else {
-            resetSearch();
-            return (false);
+            isc_throw(isc::Unexpected, "Unknown zone ID");
         }
-    };
-
-    virtual void resetSearch() {
-        search_running_ = false;
-    };
-
-    bool searchRunning() const {
-        return (search_running_);
     }
+
 private:
     std::map<std::string, std::vector< std::vector<std::string> > > records;
-    // used as internal index for getNextRecord()
-    size_t cur_record;
-    // used as temporary storage after searchForRecord() and during
-    // getNextRecord() calls, as well as during the building of the
-    // fake data
+    // used as temporary storage during the building of the fake data
     std::vector< std::vector<std::string> > cur_name;
 
-    // This boolean is used to make sure find() calls resetSearch
-    // when it encounters an error
-    bool search_running_;
-
-    // We store the name passed to searchForRecords, so we can
-    // hardcode some exceptions into getNextRecord
-    std::string searched_name_;
-
     // Adds one record to the current name in the database
     // The actual data will not be added to 'records' until
     // addCurName() is called
@@ -445,10 +431,16 @@ private:
     }
 };
 
+// This tests the default getRecords behaviour, throwing NotImplemented
+TEST(DatabaseConnectionTest, getRecords) {
+    EXPECT_THROW(NopAccessor().getRecords(".", 1),
+                 isc::NotImplemented);
+}
+
 // This tests the default getAllRecords behaviour, throwing NotImplemented
 TEST(DatabaseConnectionTest, getAllRecords) {
     // The parameters don't matter
-    EXPECT_THROW(NopAccessor().getAllRecords(Name("."), 1),
+    EXPECT_THROW(NopAccessor().getAllRecords(1),
                  isc::NotImplemented);
 }
 
@@ -492,7 +484,6 @@ public:
         shared_ptr<DatabaseClient::Finder> finder(
             dynamic_pointer_cast<DatabaseClient::Finder>(zone.zone_finder));
         EXPECT_EQ(42, finder->zone_id());
-        EXPECT_FALSE(current_database_->searchRunning());
 
         return (finder);
     }
@@ -678,7 +669,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -689,7 +679,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -700,7 +689,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -709,7 +697,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::NXRRSET,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -719,7 +706,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::CNAME,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -729,7 +715,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -738,7 +723,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::NXDOMAIN,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -750,7 +734,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -762,7 +745,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -771,7 +753,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::NXRRSET,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -782,7 +763,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::CNAME,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -794,7 +774,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -806,7 +785,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -815,7 +793,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::NXRRSET,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -826,7 +803,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::CNAME,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
 
     expected_rdatas_.clear();
@@ -838,7 +814,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -849,7 +824,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -860,7 +834,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -871,7 +844,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(360),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_sig_rdatas_.clear();
@@ -882,77 +854,63 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(360),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
 
     EXPECT_THROW(finder->find(isc::dns::Name("badcname1.example.org."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("badcname2.example.org."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("badcname3.example.org."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("badrdata.example.org."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("badtype.example.org."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("badttl.example.org."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("badsig.example.org."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // Trigger the hardcoded exceptions and see if find() has cleaned up
     EXPECT_THROW(finder->find(isc::dns::Name("dsexception.in.search."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("iscexception.in.search."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
-                 DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
+                 isc::Exception);
     EXPECT_THROW(finder->find(isc::dns::Name("basicexception.in.search."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  std::exception);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     EXPECT_THROW(finder->find(isc::dns::Name("dsexception.in.getnext."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("iscexception.in.getnext."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
-                 DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
+                 isc::Exception);
     EXPECT_THROW(finder->find(isc::dns::Name("basicexception.in.getnext."),
                                               isc::dns::RRType::A(),
                                               NULL, ZoneFinder::FIND_DEFAULT),
                  std::exception);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // This RRSIG has the wrong sigtype field, which should be
     // an error if we decide to keep using that field
@@ -966,7 +924,6 @@ TEST_F(DatabaseClientTest, find) {
                isc::dns::RRTTL(3600),
                ZoneFinder::SUCCESS,
                expected_rdatas_, expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 }
 
 TEST_F(DatabaseClientTest, findDelegation) {
@@ -981,7 +938,6 @@ TEST_F(DatabaseClientTest, findDelegation) {
                isc::dns::RRType::A(), isc::dns::RRType::A(),
                isc::dns::RRTTL(3600), ZoneFinder::SUCCESS, expected_rdatas_,
                expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     expected_rdatas_.clear();
     expected_rdatas_.push_back("ns.example.com.");
@@ -991,7 +947,6 @@ TEST_F(DatabaseClientTest, findDelegation) {
                isc::dns::RRType::NS(), isc::dns::RRType::NS(),
                isc::dns::RRTTL(3600), ZoneFinder::SUCCESS, expected_rdatas_,
                expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // Check when we ask for something below delegation point, we get the NS
     // (Both when the RRset there exists and doesn't)
@@ -1006,7 +961,6 @@ TEST_F(DatabaseClientTest, findDelegation) {
                isc::dns::RRTTL(3600), ZoneFinder::DELEGATION, expected_rdatas_,
                expected_sig_rdatas_,
                isc::dns::Name("delegation.example.org."));
-    EXPECT_FALSE(current_database_->searchRunning());
     doFindTest(finder, isc::dns::Name("ns.delegation.example.org."),
                isc::dns::RRType::AAAA(), isc::dns::RRType::NS(),
                isc::dns::RRTTL(3600), ZoneFinder::DELEGATION, expected_rdatas_,
@@ -1017,7 +971,6 @@ TEST_F(DatabaseClientTest, findDelegation) {
                isc::dns::RRTTL(3600), ZoneFinder::DELEGATION, expected_rdatas_,
                expected_sig_rdatas_,
                isc::dns::Name("delegation.example.org."));
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // Even when we check directly at the delegation point, we should get
     // the NS
@@ -1025,14 +978,12 @@ TEST_F(DatabaseClientTest, findDelegation) {
                isc::dns::RRType::AAAA(), isc::dns::RRType::NS(),
                isc::dns::RRTTL(3600), ZoneFinder::DELEGATION, expected_rdatas_,
                expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // And when we ask direcly for the NS, we should still get delegation
     doFindTest(finder, isc::dns::Name("delegation.example.org."),
                isc::dns::RRType::NS(), isc::dns::RRType::NS(),
                isc::dns::RRTTL(3600), ZoneFinder::DELEGATION, expected_rdatas_,
                expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // Now test delegation. If it is below the delegation point, we should get
     // the DNAME (the one with data under DNAME is invalid zone, but we test
@@ -1047,17 +998,14 @@ TEST_F(DatabaseClientTest, findDelegation) {
                isc::dns::RRType::A(), isc::dns::RRType::DNAME(),
                isc::dns::RRTTL(3600), ZoneFinder::DNAME, expected_rdatas_,
                expected_sig_rdatas_, isc::dns::Name("dname.example.org."));
-    EXPECT_FALSE(current_database_->searchRunning());
     doFindTest(finder, isc::dns::Name("below.dname.example.org."),
                isc::dns::RRType::AAAA(), isc::dns::RRType::DNAME(),
                isc::dns::RRTTL(3600), ZoneFinder::DNAME, expected_rdatas_,
                expected_sig_rdatas_, isc::dns::Name("dname.example.org."));
-    EXPECT_FALSE(current_database_->searchRunning());
     doFindTest(finder, isc::dns::Name("really.deep.below.dname.example.org."),
                isc::dns::RRType::AAAA(), isc::dns::RRType::DNAME(),
                isc::dns::RRTTL(3600), ZoneFinder::DNAME, expected_rdatas_,
                expected_sig_rdatas_, isc::dns::Name("dname.example.org."));
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // Asking direcly for DNAME should give SUCCESS
     doFindTest(finder, isc::dns::Name("dname.example.org."),
@@ -1073,33 +1021,27 @@ TEST_F(DatabaseClientTest, findDelegation) {
                isc::dns::RRType::A(), isc::dns::RRType::A(),
                isc::dns::RRTTL(3600), ZoneFinder::SUCCESS, expected_rdatas_,
                expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
     expected_rdatas_.clear();
     doFindTest(finder, isc::dns::Name("dname.example.org."),
                isc::dns::RRType::AAAA(), isc::dns::RRType::AAAA(),
                isc::dns::RRTTL(3600), ZoneFinder::NXRRSET, expected_rdatas_,
                expected_sig_rdatas_);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // This is broken dname, it contains two targets
     EXPECT_THROW(finder->find(isc::dns::Name("below.baddname.example.org."),
                               isc::dns::RRType::A(), NULL,
                               ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
 
     // Broken NS - it lives together with something else
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("brokenns1.example.org."),
                               isc::dns::RRType::A(), NULL,
                               ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
     EXPECT_THROW(finder->find(isc::dns::Name("brokenns2.example.org."),
                               isc::dns::RRType::A(), NULL,
                               ZoneFinder::FIND_DEFAULT),
                  DataSourceError);
-    EXPECT_FALSE(current_database_->searchRunning());
 }
 
 // Glue-OK mode. Just go trough NS delegations down (but not trough
@@ -1154,13 +1096,11 @@ TEST_F(DatabaseClientTest, glueOK) {
                isc::dns::RRTTL(3600), ZoneFinder::DNAME, expected_rdatas_,
                expected_sig_rdatas_, isc::dns::Name("dname.example.org."),
                ZoneFinder::FIND_GLUE_OK);
-    EXPECT_FALSE(current_database_->searchRunning());
     doFindTest(finder, isc::dns::Name("below.dname.example.org."),
                isc::dns::RRType::AAAA(), isc::dns::RRType::DNAME(),
                isc::dns::RRTTL(3600), ZoneFinder::DNAME, expected_rdatas_,
                expected_sig_rdatas_, isc::dns::Name("dname.example.org."),
                ZoneFinder::FIND_GLUE_OK);
-    EXPECT_FALSE(current_database_->searchRunning());
 }
 
 TEST_F(DatabaseClientTest, getOrigin) {

+ 136 - 98
src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -35,6 +35,7 @@ std::string SQLITE_DBFILE_EXAMPLE_ROOT = TEST_DATA_DIR "/test-root.sqlite3";
 std::string SQLITE_DBNAME_EXAMPLE_ROOT = "sqlite3_test-root.sqlite3";
 std::string SQLITE_DBFILE_BROKENDB = TEST_DATA_DIR "/brokendb.sqlite3";
 std::string SQLITE_DBFILE_MEMORY = ":memory:";
+std::string SQLITE_DBFILE_EXAMPLE_ORG = TEST_DATA_DIR "/example.org.sqlite3";
 
 // The following file must be non existent and must be non"creatable";
 // the sqlite3 library will try to create a new DB file if it doesn't exist,
@@ -106,42 +107,92 @@ TEST_F(SQLite3Access, noClass) {
 // This tests the iterator context
 TEST_F(SQLite3Access, iterator) {
     // Our test zone is conveniently small, but not empty
-    initAccessor(SQLITE_DBFILE_EXAMPLE2, RRClass::IN());
+    initAccessor(SQLITE_DBFILE_EXAMPLE_ORG, RRClass::IN());
+
+    const std::pair<bool, int> zone_info(db->getZone(Name("example.org")));
+    ASSERT_TRUE(zone_info.first);
 
     // Get the iterator context
     DatabaseAccessor::IteratorContextPtr
-        context(db->getAllRecords(Name("example2.com"), 1));
+        context(db->getAllRecords(zone_info.second));
     ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
               context);
 
-    const size_t size(5);
-    std::string data[size];
+    std::string data[DatabaseAccessor::COLUMN_COUNT];
     // Get and check the first and only record
-    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[1]);
-    // Check there's no other
-    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());
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("DNAME", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("dname.example.info.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("dname.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("DNAME", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("dname2.example.info.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("dname2.foo.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("MX", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("10 mail.example.org.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("NS", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("ns1.example.org.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("NS", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("ns2.example.org.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("NS", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("ns3.example.org.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("SOA", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("ns1.example.org. admin.example.org. "
+              "1234 3600 1800 2419200 7200",
+              data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("A", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("192.0.2.10", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("mail.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("A", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("192.0.2.101", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("ns.sub.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("NS", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("ns.sub.example.org.", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("sub.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
+
+    EXPECT_TRUE(context->getNext(data));
+    EXPECT_EQ("A", data[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("192.0.2.1", data[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ("www.example.org.", data[DatabaseAccessor::NAME_COLUMN]);
 
-    // Get the iterator context
-    DatabaseAccessor::IteratorContextPtr
-        context(db->getAllRecords(Name("example2.com"), 1));
-    ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
-              context);
+    // Check there's no other
+    EXPECT_FALSE(context->getNext(data));
 
-    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));
+    // And make sure calling it again won't cause problems.
+    EXPECT_FALSE(context->getNext(data));
 }
 
 TEST(SQLite3Open, getDBNameExample2) {
@@ -164,11 +215,11 @@ checkRecordRow(const std::string columns[],
                const std::string& field3,
                const std::string& field4)
 {
-    EXPECT_EQ(field0, columns[0]);
-    EXPECT_EQ(field1, columns[1]);
-    EXPECT_EQ(field2, columns[2]);
-    EXPECT_EQ(field3, columns[3]);
-    EXPECT_EQ(field4, columns[4]);
+    EXPECT_EQ(field0, columns[DatabaseAccessor::TYPE_COLUMN]);
+    EXPECT_EQ(field1, columns[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ(field2, columns[DatabaseAccessor::SIGTYPE_COLUMN]);
+    EXPECT_EQ(field3, columns[DatabaseAccessor::RDATA_COLUMN]);
+    EXPECT_EQ(field4, columns[DatabaseAccessor::NAME_COLUMN]);
 }
 
 TEST_F(SQLite3Access, getRecords) {
@@ -178,95 +229,79 @@ TEST_F(SQLite3Access, getRecords) {
     const int zone_id = zone_info.second;
     ASSERT_EQ(1, zone_id);
 
-    const size_t column_count = DatabaseAccessor::COLUMN_COUNT;
-    std::string columns[column_count];
-
-    // without search, getNext() should return false
-    EXPECT_FALSE(db->getNextRecord(columns, column_count));
-    checkRecordRow(columns, "", "", "", "", "");
-
-    db->searchForRecords(zone_id, "foo.bar.");
-    EXPECT_FALSE(db->getNextRecord(columns, column_count));
-    checkRecordRow(columns, "", "", "", "", "");
+    std::string columns[DatabaseAccessor::COLUMN_COUNT];
 
-    db->searchForRecords(zone_id, "");
-    EXPECT_FALSE(db->getNextRecord(columns, column_count));
+    DatabaseAccessor::IteratorContextPtr
+        context(db->getRecords("foo.bar", 1));
+    ASSERT_NE(DatabaseAccessor::IteratorContextPtr(),
+              context);
+    EXPECT_FALSE(context->getNext(columns));
     checkRecordRow(columns, "", "", "", "", "");
 
-    // Should error on a bad number of columns
-    EXPECT_THROW(db->getNextRecord(columns, 4), DataSourceError);
-    EXPECT_THROW(db->getNextRecord(columns, 6), DataSourceError);
-
     // now try some real searches
-    db->searchForRecords(zone_id, "foo.example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+    context = db->getRecords("foo.example.com.", zone_id);
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "CNAME", "3600", "",
-                   "cnametest.example.org.", "foo.example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "cnametest.example.org.", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "CNAME",
                    "CNAME 5 3 3600 20100322084538 20100220084538 33495 "
-                   "example.com. FAKEFAKEFAKEFAKE", "foo.example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "example.com. FAKEFAKEFAKEFAKE", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "NSEC", "7200", "",
-                   "mail.example.com. CNAME RRSIG NSEC", "foo.example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "mail.example.com. CNAME RRSIG NSEC", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "7200", "NSEC",
                    "NSEC 5 3 7200 20100322084538 20100220084538 33495 "
-                   "example.com. FAKEFAKEFAKEFAKE", "foo.example.com.");
-    EXPECT_FALSE(db->getNextRecord(columns, column_count));
+                   "example.com. FAKEFAKEFAKEFAKE", "");
+    EXPECT_FALSE(context->getNext(columns));
     // with no more records, the array should not have been modified
     checkRecordRow(columns, "RRSIG", "7200", "NSEC",
                    "NSEC 5 3 7200 20100322084538 20100220084538 33495 "
-                   "example.com. FAKEFAKEFAKEFAKE", "foo.example.com.");
+                   "example.com. FAKEFAKEFAKEFAKE", "");
 
-    db->searchForRecords(zone_id, "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+    context = db->getRecords("example.com.", zone_id);
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "SOA", "3600", "",
                    "master.example.com. admin.example.com. "
-                   "1234 3600 1800 2419200 7200", "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "1234 3600 1800 2419200 7200", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "SOA",
                    "SOA 5 2 3600 20100322084538 20100220084538 "
-                   "33495 example.com. FAKEFAKEFAKEFAKE", "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
-    checkRecordRow(columns, "NS", "1200", "", "dns01.example.com.",
-                   "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
-    checkRecordRow(columns, "NS", "3600", "", "dns02.example.com.",
-                   "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
-    checkRecordRow(columns, "NS", "1800", "", "dns03.example.com.",
-                   "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "33495 example.com. FAKEFAKEFAKEFAKE", "");
+    ASSERT_TRUE(context->getNext(columns));
+    checkRecordRow(columns, "NS", "1200", "", "dns01.example.com.", "");
+    ASSERT_TRUE(context->getNext(columns));
+    checkRecordRow(columns, "NS", "3600", "", "dns02.example.com.", "");
+    ASSERT_TRUE(context->getNext(columns));
+    checkRecordRow(columns, "NS", "1800", "", "dns03.example.com.", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "NS",
                    "NS 5 2 3600 20100322084538 20100220084538 "
-                   "33495 example.com. FAKEFAKEFAKEFAKE",
-                   "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
-    checkRecordRow(columns, "MX", "3600", "", "10 mail.example.com.",
-                   "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "33495 example.com. FAKEFAKEFAKEFAKE", "");
+    ASSERT_TRUE(context->getNext(columns));
+    checkRecordRow(columns, "MX", "3600", "", "10 mail.example.com.", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "MX", "3600", "",
-                   "20 mail.subzone.example.com.", "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "20 mail.subzone.example.com.", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "MX",
                    "MX 5 2 3600 20100322084538 20100220084538 "
-                   "33495 example.com. FAKEFAKEFAKEFAKE", "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "33495 example.com. FAKEFAKEFAKEFAKE", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "NSEC", "7200", "",
-                   "cname-ext.example.com. NS SOA MX RRSIG NSEC DNSKEY",
-                   "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "cname-ext.example.com. NS SOA MX RRSIG NSEC DNSKEY", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "7200", "NSEC",
                    "NSEC 5 2 7200 20100322084538 20100220084538 "
-                   "33495 example.com. FAKEFAKEFAKEFAKE", "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "33495 example.com. FAKEFAKEFAKEFAKE", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "DNSKEY", "3600", "",
                    "256 3 5 AwEAAcOUBllYc1hf7ND9uDy+Yz1BF3sI0m4q NGV7W"
                    "cTD0WEiuV7IjXgHE36fCmS9QsUxSSOV o1I/FMxI2PJVqTYHkX"
                    "FBS7AzLGsQYMU7UjBZ SotBJ6Imt5pXMu+lEDNy8TOUzG3xm7g"
-                   "0qcbW YF6qCEfvZoBtAqi5Rk7Mlrqs8agxYyMx", "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "0qcbW YF6qCEfvZoBtAqi5Rk7Mlrqs8agxYyMx", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "DNSKEY", "3600", "",
                    "257 3 5 AwEAAe5WFbxdCPq2jZrZhlMj7oJdff3W7syJ tbvzg"
                    "62tRx0gkoCDoBI9DPjlOQG0UAbj+xUV 4HQZJStJaZ+fHU5AwV"
@@ -275,20 +310,23 @@ TEST_F(SQLite3Access, getRecords) {
                    "qiODyNZYQ+ZrLmF0KIJ2yPN3iO6Zq 23TaOrVTjB7d1a/h31OD"
                    "fiHAxFHrkY3t3D5J R9Nsl/7fdRmSznwtcSDgLXBoFEYmw6p86"
                    "Acv RyoYNcL1SXjaKVLG5jyU3UR+LcGZT5t/0xGf oIK/aKwEN"
-                   "rsjcKZZj660b1M=", "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "rsjcKZZj660b1M=", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "DNSKEY",
                    "DNSKEY 5 2 3600 20100322084538 20100220084538 "
-                   "4456 example.com. FAKEFAKEFAKEFAKE", "example.com.");
-    ASSERT_TRUE(db->getNextRecord(columns, column_count));
+                   "4456 example.com. FAKEFAKEFAKEFAKE", "");
+    ASSERT_TRUE(context->getNext(columns));
     checkRecordRow(columns, "RRSIG", "3600", "DNSKEY",
                    "DNSKEY 5 2 3600 20100322084538 20100220084538 "
-                   "33495 example.com. FAKEFAKEFAKEFAKE", "example.com.");
-    EXPECT_FALSE(db->getNextRecord(columns, column_count));
+                   "33495 example.com. FAKEFAKEFAKEFAKE", "");
+    EXPECT_FALSE(context->getNext(columns));
     // getnextrecord returning false should mean array is not altered
     checkRecordRow(columns, "RRSIG", "3600", "DNSKEY",
                    "DNSKEY 5 2 3600 20100322084538 20100220084538 "
-                   "33495 example.com. FAKEFAKEFAKEFAKE", "example.com.");
+                   "33495 example.com. FAKEFAKEFAKEFAKE", "");
+
+    // check that another getNext does not cause problems
+    EXPECT_FALSE(context->getNext(columns));
 }
 
 } // end anonymous namespace