From: JINMEI Tatuya <jinmei@isc.org>
Date: Tue, 17 Apr 2012 23:32:59 +0000 (-0700)
Subject: [1891] make sure nsec3 table is cleared when replacing a zone.
X-Git-Tag: trac2351_base~226^2~116^2~25^2~9
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=91cb18e244588b8a99ddf9b12fb99986b0258fdb;p=thirdparty%2Fkea.git

[1891] make sure nsec3 table is cleared when replacing a zone.

the test for this uncovered one old regression: accessor cannot be reused
for multiple updates.  it was fixed in this commit, too.
---

diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index e5a22f4ebe..a81a80e868 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -74,7 +74,8 @@ enum StatementID {
     NSEC3_PREVIOUS = 17,
     NSEC3_LAST = 18,
     ADD_NSEC3_RECORD = 19,
-    NUM_STATEMENTS = 20
+    DEL_ZONE_NSEC3_RECORDS = 20,
+    NUM_STATEMENTS = 21
 };
 
 const char* const text_statements[NUM_STATEMENTS] = {
@@ -151,7 +152,9 @@ const char* const text_statements[NUM_STATEMENTS] = {
         "ORDER BY hash DESC LIMIT 1",
     // ADD_NSEC3_RECORD: Add NSEC3-related (NSEC3 or NSEC3-covering RRSIG) RR
     "INSERT INTO nsec3 (zone_id, hash, owner, ttl, rdtype, rdata) "
-    "VALUES (?1, ?2, '', ?3, ?4, ?5)"
+    "VALUES (?1, ?2, '', ?3, ?4, ?5)",
+    // DEL_ZONE_NSEC3_RECORDS: delete all NSEC3-related records from the zone
+    "DELETE FROM nsec3 WHERE zone_id=?1"
 };
 
 struct SQLite3Parameters {
@@ -1025,19 +1028,29 @@ SQLite3Accessor::startUpdateZone(const string& zone_name, const bool replace) {
                        "start an SQLite3 update transaction").exec();
 
     if (replace) {
+        // First, clear all current data from tables.
+        typedef pair<StatementID, const char* const> StatementSpec;
+        const StatementSpec delzone_stmts[] =
+            { StatementSpec(DEL_ZONE_RECORDS, "delete zone records"),
+              StatementSpec(DEL_ZONE_NSEC3_RECORDS,
+                            "delete zone NSEC3 records") };
         try {
-            StatementProcessor delzone_exec(*dbparameters_, DEL_ZONE_RECORDS,
-                                            "delete zone records");
-
-            sqlite3_stmt* stmt = dbparameters_->getStatement(DEL_ZONE_RECORDS);
-            sqlite3_clear_bindings(stmt);
-            if (sqlite3_bind_int(stmt, 1, zone_info.second) != SQLITE_OK) {
-                isc_throw(DataSourceError,
-                          "failed to bind SQLite3 parameter: " <<
-                          sqlite3_errmsg(dbparameters_->db_));
+            for (size_t i = 0;
+                 i < sizeof(delzone_stmts) / sizeof(delzone_stmts[0]);
+                 ++i) {
+                StatementProcessor delzone_exec(*dbparameters_,
+                                                delzone_stmts[i].first,
+                                                delzone_stmts[i].second);
+                sqlite3_stmt* stmt =
+                    dbparameters_->getStatement(delzone_stmts[i].first);
+                sqlite3_clear_bindings(stmt);
+                if (sqlite3_bind_int(stmt, 1, zone_info.second) != SQLITE_OK) {
+                    isc_throw(DataSourceError,
+                              "failed to bind SQLite3 parameter: " <<
+                              sqlite3_errmsg(dbparameters_->db_));
+                }
+                delzone_exec.exec();
             }
-
-            delzone_exec.exec();
         } catch (const DataSourceError&) {
             // Once we start a transaction, if something unexpected happens
             // we need to rollback the transaction so that a subsequent update
@@ -1077,6 +1090,7 @@ SQLite3Accessor::commit() {
     StatementProcessor(*dbparameters_, COMMIT,
                        "commit an SQLite3 transaction").exec();
     dbparameters_->in_transaction = false;
+    dbparameters_->updating_zone = false;
     dbparameters_->updated_zone_id = -1;
 }
 
diff --git a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
index 12f7b395ff..c2e8418bd6 100644
--- a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
+++ b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
@@ -851,6 +851,26 @@ TEST_F(SQLite3Update, flushZone) {
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);
 }
 
+TEST_F(SQLite3Update, flushZoneWithNSEC3) {
+    // Similar to the previous case, but make sure the separate nsec3 table
+    // is also cleared.  We first need to add something to the table.
+    zone_id = accessor->startUpdateZone("example.com.", false).second;
+    copy(nsec3_data, nsec3_data + DatabaseAccessor::ADD_NSEC3_COLUMN_COUNT,
+         add_nsec3_columns);
+    accessor->addNSEC3RecordToZone(add_nsec3_columns);
+    accessor->commit();
+
+    // Confirm it surely exists.
+    expected_stored.clear();
+    expected_stored.push_back(nsec3_data);
+    checkNSEC3Records(*accessor, zone_id, apex_hash, expected_stored);
+
+    // Then starting zone replacement.  the NSEC3 record should have been
+    // removed.
+    zone_id = accessor->startUpdateZone("example.com.", true).second;
+    checkNSEC3Records(*accessor, zone_id, apex_hash, empty_stored);
+}
+
 TEST_F(SQLite3Update, readWhileUpdate) {
     zone_id = accessor->startUpdateZone("example.com.", true).second;
     checkRecords(*accessor, zone_id, "foo.bar.example.com.", empty_stored);