[1068] added clone() method for DatabaseAccessor, and used it in the updater

factory.  this made the remaining database tests successful with sqlite3
accessor.

src/lib/datasrc/database.cc

@@ -319,15 +319,14 @@ ZoneUpdaterPtr
 DatabaseClient::startUpdateZone(const isc::dns::Name& name,
                                 bool replace) const
 {
-    // TODO: create a dedicated accessor
-
-    const std::pair<bool, int> zone(accessor_->startUpdateZone(name.toText(),
-                                                               replace));
+    shared_ptr<DatabaseAccessor> update_accessor(accessor_->clone());
+    const std::pair<bool, int> zone(update_accessor->startUpdateZone(
+                                        name.toText(), replace));
     if (!zone.first) {
         return (ZoneUpdaterPtr());
     }
 
-     return (ZoneUpdaterPtr(new Updater(accessor_, zone.second,
+     return (ZoneUpdaterPtr(new Updater(update_accessor, zone.second,
                                         name.toText(), rrclass_)));
 }
 

src/lib/datasrc/database.h

@@ -172,6 +172,9 @@ public:
     };
 
     /// TBD
+    virtual boost::shared_ptr<DatabaseAccessor> clone() = 0;
+
+    /// TBD
     virtual std::pair<bool, int> startUpdateZone(const std::string& zone_name,
                                                  bool replace) = 0;
 

src/lib/datasrc/sqlite3_accessor.cc

@@ -115,8 +115,9 @@ private:
 };
 
 SQLite3Accessor::SQLite3Accessor(const std::string& filename,
-                                     const isc::dns::RRClass& rrclass) :
+                                 const isc::dns::RRClass& rrclass) :
     dbparameters_(new SQLite3Parameters),
+    filename_(filename),
     class_(rrclass.toText()),
     database_name_("sqlite3_" +
                    isc::util::Filename(filename).nameAndExtension())
@@ -126,6 +127,25 @@ SQLite3Accessor::SQLite3Accessor(const std::string& filename,
     open(filename);
 }
 
+SQLite3Accessor::SQLite3Accessor(const std::string& filename,
+                                 const string& rrclass) :
+    dbparameters_(new SQLite3Parameters),
+    filename_(filename),
+    class_(rrclass),
+    database_name_("sqlite3_" +
+                   isc::util::Filename(filename).nameAndExtension())
+{
+    LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_SQLITE_NEWCONN);
+
+    open(filename);
+}
+
+boost::shared_ptr<DatabaseAccessor>
+SQLite3Accessor::clone() {
+    return (boost::shared_ptr<DatabaseAccessor>(new SQLite3Accessor(filename_,
+                                                                    class_)));
+}
+
 namespace {
 
 // This is a helper class to initialize a Sqlite3 DB safely.  An object of

src/lib/datasrc/sqlite3_accessor.h

@@ -68,12 +68,27 @@ public:
      */
     SQLite3Accessor(const std::string& filename,
                     const isc::dns::RRClass& rrclass);
+
+    /**
+     * \brief Constructor
+     *
+     * Same as the other version, but takes rrclass as a bare string.
+     * we should obsolete the other version and unify the constructor to
+     * this version; the SQLite3Accessor is expected to be "dumb" and
+     * shouldn't care about DNS specific information such as RRClass.
+     */
+    SQLite3Accessor(const std::string& filename, const std::string& rrclass);
+
     /**
      * \brief Destructor
      *
      * Closes the database.
      */
     ~SQLite3Accessor();
+
+    /// TBD
+    virtual boost::shared_ptr<DatabaseAccessor> clone();
+
     /**
      * \brief Look up a zone
      *
@@ -174,6 +189,8 @@ private:
 
     /// \brief Private database data
     SQLite3Parameters* dbparameters_;
+    /// \brief The filename of the DB (necessary for clone())
+    const std::string filename_;
     /// \brief The class for which the queries are done
     const std::string class_;
     /// \brief Opens the database

src/lib/datasrc/tests/database_unittest.cc

@@ -137,9 +137,21 @@ public:
     MockAccessor() : search_running_(false), rollbacked_(false),
                      database_name_("mock_database")
     {
+        readonly_records = &readonly_records_master;
+        update_records = &update_records_master;
+        empty_records = &empty_records_master;
         fillData();
     }
 
+    virtual shared_ptr<DatabaseAccessor> clone() {
+        shared_ptr<MockAccessor> cloned_accessor(new MockAccessor());
+        cloned_accessor->readonly_records = &readonly_records_master;
+        cloned_accessor->update_records = &update_records_master;
+        cloned_accessor->empty_records = &empty_records_master;
+        latest_clone_ = cloned_accessor;
+        return (cloned_accessor);
+    }
+
     virtual pair<bool, int> getZone(const Name& name) const {
         return (getZone(name.toText()));
     }
@@ -217,15 +229,15 @@ public:
         // we use an empty set; otherwise we use a writable copy of the
         // original.
         if (replace) {
-            update_records.clear();
+            update_records->clear();
         } else {
-            update_records = readonly_records;
+            *update_records = *readonly_records;
         }
 
         return (pair<bool, int>(true, WRITABLE_ZONE_ID));
     }
     virtual void commitUpdateZone() {
-        readonly_records = update_records;
+        *readonly_records = *update_records;
     }
     virtual void rollbackUpdateZone() {
         rollbacked_ = true;
@@ -233,13 +245,13 @@ public:
     virtual void addRecordToZone(const vector<string>& columns) {
         // Copy the current value to cur_name.  If it doesn't exist,
         // operator[] will create a new one.
-        cur_name = update_records[columns[DatabaseAccessor::ADD_NAME]];
+        cur_name = (*update_records)[columns[DatabaseAccessor::ADD_NAME]];
         addRecord(columns[DatabaseAccessor::ADD_TYPE],
                   columns[DatabaseAccessor::ADD_TTL],
                   columns[DatabaseAccessor::ADD_SIGTYPE],
                   columns[DatabaseAccessor::ADD_RDATA]);
         // copy back the added entry.
-        update_records[columns[DatabaseAccessor::ADD_NAME]] = cur_name;
+        (*update_records)[columns[DatabaseAccessor::ADD_NAME]] = cur_name;
 
         // remember this one so that test cases can check it.
         columns_lastadded = columns;
@@ -259,16 +271,16 @@ public:
 
     virtual void deleteRecordInZone(const vector<string>& params) {
         vector<vector<string> > records =
-            update_records[params[DatabaseAccessor::DEL_NAME]];
+            (*update_records)[params[DatabaseAccessor::DEL_NAME]];
         records.erase(remove_if(records.begin(), records.end(),
                                 deleteMatch(
                                     params[DatabaseAccessor::DEL_TYPE],
                                     params[DatabaseAccessor::DEL_RDATA])),
                       records.end());
         if (records.empty()) {
-            update_records.erase(params[DatabaseAccessor::DEL_NAME]);
+            (*update_records).erase(params[DatabaseAccessor::DEL_NAME]);
         } else {
-            update_records[params[DatabaseAccessor::DEL_NAME]] = records;
+            (*update_records)[params[DatabaseAccessor::DEL_NAME]] = records;
         }
     }
 
@@ -287,10 +299,18 @@ public:
     const vector<string>& getLastAdded() const {
         return (columns_lastadded);
     }
+
+    // This allows the test code to get the accessor used in an update context
+    shared_ptr<const DatabaseAccessor> getLatestClone() const {
+        return (latest_clone_);
+    }
 private:
-    RECORDS readonly_records;
-    RECORDS update_records;
-    RECORDS empty_records;
+    RECORDS readonly_records_master;
+    RECORDS* readonly_records;
+    RECORDS update_records_master;
+    RECORDS* update_records;
+    RECORDS empty_records_master;
+    RECORDS* empty_records;
 
     // used as internal index for getNextRecord()
     size_t cur_record;
@@ -316,13 +336,15 @@ private:
 
     const std::string database_name_;
 
+    boost::shared_ptr<MockAccessor> latest_clone_;
+
     const RECORDS& getRecords(int zone_id) const {
         if (zone_id == READONLY_ZONE_ID) {
-            return (readonly_records);
+            return (*readonly_records);
         } else if (zone_id == WRITABLE_ZONE_ID) {
-            return (update_records);
+            return (*update_records);
         }
-        return (empty_records);
+        return (*empty_records);
     }
 
     // Adds one record to the current name in the database
@@ -344,8 +366,8 @@ private:
     // to the actual fake database. This will clear cur_name,
     // so we can immediately start adding new records.
     void addCurName(const std::string& name) {
-        ASSERT_EQ(0, readonly_records.count(name));
-        readonly_records[name] = cur_name;
+        ASSERT_EQ(0, readonly_records->count(name));
+        (*readonly_records)[name] = cur_name;
         cur_name.clear();
     }
 
@@ -451,9 +473,9 @@ protected:
 
     bool isRollbacked(bool expected = false) const {
         if (is_mock_) {
-            const MockAccessor* mock_accessor =
-                dynamic_cast<const MockAccessor*>(current_accessor_);
-            return (mock_accessor->isRollbacked());
+            const MockAccessor& mock_accessor =
+                dynamic_cast<const MockAccessor&>(*update_accessor_);
+            return (mock_accessor.isRollbacked());
         } else {
             return (expected);
         }
@@ -471,6 +493,14 @@ protected:
         }
     }
 
+    void setUpdateAccessor() {
+        if (is_mock_) {
+            const MockAccessor* mock_accessor =
+                dynamic_cast<const MockAccessor*>(current_accessor_);
+            update_accessor_ = mock_accessor->getLatestClone();
+        }
+    }
+
     // Some tests only work for MockAccessor.  We remember whether our accessor
     // is of that type.
     bool is_mock_;
@@ -492,6 +522,7 @@ protected:
     RRsetPtr rrsigset;          // for adding/deleting an RRset
 
     ZoneUpdaterPtr updater;
+    shared_ptr<const DatabaseAccessor> update_accessor_;
     const std::vector<std::string> empty_rdatas; // for NXRRSET/NXDOMAIN
     std::vector<std::string> expected_rdatas;
     std::vector<std::string> expected_sig_rdatas;
@@ -921,6 +952,7 @@ TYPED_TEST(DatabaseClientTest, flushZone) {
     // start update in the replace mode.  the normal finder should still
     // be able to see the record, but the updater's finder shouldn't.
     this->updater = this->client_->startUpdateZone(this->zname, true);
+    this->setUpdateAccessor();
     EXPECT_EQ(ZoneFinder::SUCCESS,
               this->finder->find(this->qname, this->qtype).code);
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
@@ -944,6 +976,7 @@ TYPED_TEST(DatabaseClientTest, updateCancel) {
                                                       this->qtype).code);
 
     this->updater = this->client_->startUpdateZone(this->zname, true);
+    this->setUpdateAccessor();
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
               this->updater->getFinder().find(this->qname, this->qtype).code);
     // DB should not have been rolled back yet.

src/lib/datasrc/tests/sqlite3_accessor_unittest.cc

@@ -248,6 +248,33 @@ TEST_F(SQLite3AccessorTest, getRecords) {
                    "33495 example.com. FAKEFAKEFAKEFAKE");
 }
 
+TEST_F(SQLite3AccessorTest, clone) {
+    shared_ptr<DatabaseAccessor> cloned = accessor->clone();
+    EXPECT_EQ(accessor->getDBName(), cloned->getDBName());
+
+    // The cloned accessor should have a separate connection and search
+    // context, so it should be able to perform search in concurrent with
+    // the original accessor.
+    string columns1[DatabaseAccessor::COLUMN_COUNT];
+    string columns2[DatabaseAccessor::COLUMN_COUNT];
+
+    const std::pair<bool, int> zone_info1(
+        accessor->getZone(Name("example.com")));
+    const std::pair<bool, int> zone_info2(
+        accessor->getZone(Name("example.com")));
+
+    accessor->searchForRecords(zone_info1.second, "foo.example.com.");
+    ASSERT_TRUE(accessor->getNextRecord(columns1,
+                                        DatabaseAccessor::COLUMN_COUNT));
+    checkRecordRow(columns1, "CNAME", "3600", "", "cnametest.example.org.");
+
+
+    cloned->searchForRecords(zone_info2.second, "foo.example.com.");
+    ASSERT_TRUE(cloned->getNextRecord(columns2,
+                                      DatabaseAccessor::COLUMN_COUNT));
+    checkRecordRow(columns2, "CNAME", "3600", "", "cnametest.example.org.");
+}
+
 //
 // Commonly used data for update tests
 //
@@ -274,9 +301,8 @@ protected:
                             TEST_DATA_BUILDDIR "/test.sqlite3.copied"));
         initAccessor(TEST_DATA_BUILDDIR "/test.sqlite3.copied", RRClass::IN());
         zone_id = accessor->getZone(Name("example.com")).second;
-        another_accessor.reset(new SQLite3Accessor(
-                                   TEST_DATA_BUILDDIR "/test.sqlite3.copied",
-                                   RRClass::IN()));
+        another_accessor = boost::dynamic_pointer_cast<SQLite3Accessor>(
+            accessor->clone());
         expected_stored.push_back(common_expected_data);
     }
 
@@ -287,7 +313,8 @@ protected:
     vector<const char* const*> expected_stored; // placeholder for checkRecords
     vector<const char* const*> empty_stored; // indicate no corresponding data
 
-    // Another accessor, emulating one running on a different process/thread
+    // Another accessor, emulating one running on a different process/thread.
+    // It will also confirm clone() works as expected in terms of update.
     shared_ptr<SQLite3Accessor> another_accessor;
 };