[sql] Clean up sql::Recovery unit test.

sql/recovery_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sql/recovery.h"
 
 #include <stddef.h>
 #include <string>
 #include <utility>
 
@@ skipping 49 matching lines @@
 // to create the table or index.  For certain automatic SQLite
 // structures with no sql, the name is used.
 std::string GetSchema(sql::Connection* db) {
   const char kSql[] =
       "SELECT COALESCE(sql, name) FROM sqlite_master ORDER BY 1";
   return ExecuteWithResults(db, kSql, "|", "\n");
 }
 
 using SQLRecoveryTest = sql::SQLTestBase;
 
+// Baseline sql::Recovery test covering the different ways to dispose of the
+// scoped pointer received from sql::Recovery::Begin().
 TEST_F(SQLRecoveryTest, RecoverBasic) {
   const char kCreateSql[] = "CREATE TABLE x (t TEXT)";
   const char kInsertSql[] = "INSERT INTO x VALUES ('This is a test')";
+  const char kAltInsertSql[] = "INSERT INTO x VALUES ('That was a test')";
   ASSERT_TRUE(db().Execute(kCreateSql));
   ASSERT_TRUE(db().Execute(kInsertSql));
   ASSERT_EQ("CREATE TABLE x (t TEXT)", GetSchema(&db()));
 
   // If the Recovery handle goes out of scope without being
   // Recovered(), the database is razed.
   {
     scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(&db(), db_path());
     ASSERT_TRUE(recovery.get());
   }
@@ skipping 29 matching lines @@
     recovery = sql::Recovery::Begin(&db(), db_path());
     ASSERT_FALSE(recovery.get());
   }
   ASSERT_TRUE(Reopen());
 
   // Recreate the database.
   ASSERT_TRUE(db().Execute(kCreateSql));
   ASSERT_TRUE(db().Execute(kInsertSql));
   ASSERT_EQ("CREATE TABLE x (t TEXT)", GetSchema(&db()));
 
+  // Unrecovered table to distinguish from recovered database.
+  ASSERT_TRUE(db().Execute("CREATE TABLE y (c INTEGER)"));
+  ASSERT_NE("CREATE TABLE x (t TEXT)", GetSchema(&db()));
+
   // Recovered() replaces the original with the "recovered" version.
   {
     scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(&db(), db_path());
     ASSERT_TRUE(recovery.get());
 
     // Create the new version of the table.
     ASSERT_TRUE(recovery->db()->Execute(kCreateSql));
 
     // Insert different data to distinguish from original database.
-    const char kAltInsertSql[] = "INSERT INTO x VALUES ('That was a test')";
     ASSERT_TRUE(recovery->db()->Execute(kAltInsertSql));
 
     // Successfully recovered.
     ASSERT_TRUE(sql::Recovery::Recovered(std::move(recovery)));
   }
   EXPECT_FALSE(db().is_open());
   ASSERT_TRUE(Reopen());
   EXPECT_TRUE(db().is_open());
   ASSERT_EQ("CREATE TABLE x (t TEXT)", GetSchema(&db()));
 
   const char* kXSql = "SELECT * FROM x ORDER BY 1";
   ASSERT_EQ("That was a test",
             ExecuteWithResults(&db(), kXSql, "|", "\n"));
+
+  // Reset the database contents.
+  ASSERT_TRUE(db().Execute("DELETE FROM x"));
+  ASSERT_TRUE(db().Execute(kInsertSql));
+
+  // Rollback() discards recovery progress and leaves the database as it was.
+  {
+    scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(&db(), db_path());
+    ASSERT_TRUE(recovery.get());
+
+    ASSERT_TRUE(recovery->db()->Execute(kCreateSql));
+    ASSERT_TRUE(recovery->db()->Execute(kAltInsertSql));
+
+    sql::Recovery::Rollback(std::move(recovery));
+  }
+  EXPECT_FALSE(db().is_open());
+  ASSERT_TRUE(Reopen());
+  EXPECT_TRUE(db().is_open());
+  ASSERT_EQ("CREATE TABLE x (t TEXT)", GetSchema(&db()));
+
+  ASSERT_EQ("This is a test",
+            ExecuteWithResults(&db(), kXSql, "|", "\n"));
 }
 
 // The recovery virtual table is only supported for Chromium's SQLite.
 #if !defined(USE_SYSTEM_SQLITE)

[Ryan Hamilton, 2016/02/04 20:24:19]

Can you remove this now since you're not using the virtual table?

[Scott Hess - ex-Googler, 2016/02/04 20:47:50]

AutoRecoverTable() is itself implemented in terms of the virtual table, which is
only present when we compile the version of SQLite included with Chromium.

My near-term goal is to remove the USE_SYSTEM_SQLITE ifdefs.  Rather than
elaborate here, I logged it in a bug http://crbug.com/584407 .  Right now I'm
just landing little cleanups which improve things without regard to that.

 
-// Run recovery through its paces on a valid database.
+// Test operation of the virtual table used by sql::Recovery.
 TEST_F(SQLRecoveryTest, VirtualTable) {
   const char kCreateSql[] = "CREATE TABLE x (t TEXT)";
   ASSERT_TRUE(db().Execute(kCreateSql));
   ASSERT_TRUE(db().Execute("INSERT INTO x VALUES ('This is a test')"));
   ASSERT_TRUE(db().Execute("INSERT INTO x VALUES ('That was a test')"));
 
   // Successfully recover the database.
   {
     scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(&db(), db_path());
 
@@ skipping 21 matching lines @@
   // data should be recovered.
   ASSERT_TRUE(Reopen());
   ASSERT_EQ("CREATE TABLE x (t TEXT)", GetSchema(&db()));
 
   const char* kXSql = "SELECT * FROM x ORDER BY 1";
   ASSERT_EQ("That was a test\nThis is a test",
             ExecuteWithResults(&db(), kXSql, "|", "\n"));
 }
 
 void RecoveryCallback(sql::Connection* db, const base::FilePath& db_path,
+                      const char* create_table, const char* create_index,
                       int* record_error, int error, sql::Statement* stmt) {
   *record_error = error;
 
   // Clear the error callback to prevent reentrancy.
   db->reset_error_callback();
 
   scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(db, db_path);
   ASSERT_TRUE(recovery.get());
 
-  const char kRecoveryCreateSql[] =
-      "CREATE VIRTUAL TABLE temp.recover_x using recover("
-      "  corrupt.x,"
-      "  id INTEGER STRICT,"
-      "  v INTEGER STRICT"
-      ")";
-  const char kCreateTable[] = "CREATE TABLE x (id INTEGER, v INTEGER)";
-  const char kCreateIndex[] = "CREATE UNIQUE INDEX x_id ON x (id)";
+  ASSERT_TRUE(recovery->db()->Execute(create_table));
+  ASSERT_TRUE(recovery->db()->Execute(create_index));
 
-  // Replicate data over.
-  const char kRecoveryCopySql[] =
-      "INSERT OR REPLACE INTO x SELECT id, v FROM recover_x";
-
-  ASSERT_TRUE(recovery->db()->Execute(kRecoveryCreateSql));
-  ASSERT_TRUE(recovery->db()->Execute(kCreateTable));
-  ASSERT_TRUE(recovery->db()->Execute(kCreateIndex));
-  ASSERT_TRUE(recovery->db()->Execute(kRecoveryCopySql));
+  size_t rows = 0;
+  ASSERT_TRUE(recovery->AutoRecoverTable("x", &rows));
 
   ASSERT_TRUE(sql::Recovery::Recovered(std::move(recovery)));
 }
 
 // Build a database, corrupt it by making an index reference to
 // deleted row, then recover when a query selects that row.
 TEST_F(SQLRecoveryTest, RecoverCorruptIndex) {
   const char kCreateTable[] = "CREATE TABLE x (id INTEGER, v INTEGER)";
   const char kCreateIndex[] = "CREATE UNIQUE INDEX x_id ON x (id)";
   ASSERT_TRUE(db().Execute(kCreateTable));
@@ skipping 18 matching lines @@
   db().Close();
 
   // Delete a row from the table, while leaving the index entry which
   // references it.
   const char kDeleteSql[] = "DELETE FROM x WHERE id = 0";
   ASSERT_TRUE(sql::test::CorruptTableOrIndex(db_path(), "x_id", kDeleteSql));
 
   ASSERT_TRUE(Reopen());
 
   int error = SQLITE_OK;
-  db().set_error_callback(base::Bind(&RecoveryCallback,
-                                     &db(), db_path(), &error));
+  db().set_error_callback(base::Bind(&RecoveryCallback, &db(), db_path(),
+                                     kCreateTable, kCreateIndex, &error));
 
   // This works before the callback is called.
   const char kTrivialSql[] = "SELECT COUNT(*) FROM sqlite_master";
   EXPECT_TRUE(db().IsSQLValid(kTrivialSql));
 
   // TODO(shess): Could this be delete?  Anything which fails should work.
   const char kSelectSql[] = "SELECT v FROM x WHERE id = 0";
   ASSERT_FALSE(db().Execute(kSelectSql));
   EXPECT_EQ(SQLITE_CORRUPT, error);
 
@@ skipping 34 matching lines @@
     }
 
     ASSERT_TRUE(db().CommitTransaction());
   }
   db().Close();
 
   // Delete a row from the index while leaving a table entry.
   const char kDeleteSql[] = "DELETE FROM x WHERE id = 0";
   ASSERT_TRUE(sql::test::CorruptTableOrIndex(db_path(), "x", kDeleteSql));
 
-  // TODO(shess): Figure out a query which causes SQLite to notice
-  // this organically.  Meanwhile, just handle it manually.
+  ASSERT_TRUE(Reopen());
 
-  ASSERT_TRUE(Reopen());
+  int error = SQLITE_OK;
+  db().set_error_callback(base::Bind(&RecoveryCallback, &db(), db_path(),
+                                     kCreateTable, kCreateIndex, &error));
 
   // Index shows one less than originally inserted.
   const char kCountSql[] = "SELECT COUNT (*) FROM x";
   EXPECT_EQ("9", ExecuteWithResults(&db(), kCountSql, "|", ","));
 
   // A full table scan shows all of the original data.  Using column [v] to
   // force use of the table rather than the index.
   const char kDistinctSql[] = "SELECT DISTINCT COUNT (v) FROM x";
   EXPECT_EQ("10", ExecuteWithResults(&db(), kDistinctSql, "|", ","));
 
   // Insert id 0 again.  Since it is not in the index, the insert
   // succeeds, but results in a duplicate value in the table.
   const char kInsertSql[] = "INSERT INTO x (id, v) VALUES (0, 100)";
   ASSERT_TRUE(db().Execute(kInsertSql));
 
   // Duplication is visible.
   EXPECT_EQ("10", ExecuteWithResults(&db(), kCountSql, "|", ","));
   EXPECT_EQ("11", ExecuteWithResults(&db(), kDistinctSql, "|", ","));
 
   // This works before the callback is called.
   const char kTrivialSql[] = "SELECT COUNT(*) FROM sqlite_master";
   EXPECT_TRUE(db().IsSQLValid(kTrivialSql));
 
-  // Call the recovery callback manually.
-  int error = SQLITE_OK;
-  RecoveryCallback(&db(), db_path(), &error, SQLITE_CORRUPT, NULL);
-  EXPECT_EQ(SQLITE_CORRUPT, error);
+  // TODO(shess): Figure out a statement which causes SQLite to notice the
+  // corruption.  SELECT doesn't see errors because missing index values aren't
+  // visible.  UPDATE or DELETE against v=0 don't see errors, even though the
+  // index item is missing.  I suspect SQLite only deletes the key in these
+  // cases, but doesn't verify that one or more keys were deleted.
+  ASSERT_FALSE(db().Execute("INSERT INTO x (id, v) VALUES (0, 101)"));
+  EXPECT_EQ(SQLITE_CONSTRAINT_UNIQUE, error);
 
   // Database handle has been poisoned.
   EXPECT_FALSE(db().IsSQLValid(kTrivialSql));
 
   ASSERT_TRUE(Reopen());
 
   // The recovered table has consistency between the index and the table.
   EXPECT_EQ("10", ExecuteWithResults(&db(), kCountSql, "|", ","));
   EXPECT_EQ("10", ExecuteWithResults(&db(), kDistinctSql, "|", ","));
 
@@ skipping 370 matching lines @@
   scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(&db(), db_path());
   ASSERT_TRUE(recovery.get());
 
   // In the current implementation, the PRAGMA successfully runs with no result
   // rows.  Running with a single result of |0| is also acceptable.
   sql::Statement s(recovery->db()->GetUniqueStatement("PRAGMA mmap_size"));
   EXPECT_TRUE(!s.Step() || !s.ColumnInt64(0));
 }
 
 }  // namespace