Implement sql::Connection::RazeAndClose().

sql/connection_unittest.cc

Index: sql/connection_unittest.cc
diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc
index 3ed2093cc4c1da75c045ed114db3a7ccfb442467..7a6fd88f58f7f14fd162eeebfaa6f87566bb6162 100644
--- a/sql/connection_unittest.cc
+++ b/sql/connection_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/logging.h"
 #include "sql/connection.h"
 #include "sql/meta_table.h"
 #include "sql/statement.h"
@@ -279,6 +280,107 @@ TEST_F(SQLConnectionTest, RazeLocked) {
   ASSERT_TRUE(db().Raze());
 }
 
+// Basic test of RazeAndClose() operation.
+TEST_F(SQLConnectionTest, RazeAndClose) {
+  const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
+  const char* kPopulateSql = "INSERT INTO foo (value) VALUES (12)";
+
+  // Test that RazeAndClose() closes the database, and that the
+  // database is empty when re-opened.
+  ASSERT_TRUE(db().Execute(kCreateSql));
+  ASSERT_TRUE(db().Execute(kPopulateSql));
+  ASSERT_TRUE(db().RazeAndClose());
+  ASSERT_FALSE(db().is_open());

[pkotwicz, 2013/02/06 17:58:57]

Nit: Should you call db().Close() here for the sake of clearing the "poisoned_"
flag?

[Scott Hess - ex-Googler, 2013/02/06 19:19:30]

Interesting.  This code really should break.  The later code all works due to
having a valid db_, so the poisoned_ flag is never checked.  This is a little
subtle.  For now, I've added a DLOG_IF(FATAL, !poisoned_) to OpenInternal(), so
that this case will not work.  But if anyone is doing if (!db().is_open())
db().Open(db_path); in production, it will still work.

I'm not sure what I would prefer, here.  Maybe this code should just access
poisoned_ directly, and is_open() should be changed to consider poisoned_.  Or
maybe the entire API should be refactored a bit so that handling this isn't a
hack.

Thanks for catching this!

+  ASSERT_TRUE(db().Open(db_path()));
+  {
+    sql::Statement s(db().GetUniqueStatement("SELECT * FROM sqlite_master"));
+    ASSERT_FALSE(s.Step());
+  }
+
+  // Test that RazeAndClose() can break transactions.
+  ASSERT_TRUE(db().Execute(kCreateSql));
+  ASSERT_TRUE(db().Execute(kPopulateSql));
+  ASSERT_TRUE(db().BeginTransaction());
+  ASSERT_TRUE(db().RazeAndClose());
+  ASSERT_FALSE(db().is_open());
+  ASSERT_FALSE(db().CommitTransaction());
+  db().Close();
+  ASSERT_TRUE(db().Open(db_path()));
+  {
+    sql::Statement s(db().GetUniqueStatement("SELECT * FROM sqlite_master"));
+    ASSERT_FALSE(s.Step());
+  }
+}
+
+// Test that various operations fail without crashing after
+// RazeAndClose().
+TEST_F(SQLConnectionTest, RazeAndCloseDiagnostics) {
+  const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
+  const char* kPopulateSql = "INSERT INTO foo (value) VALUES (12)";
+  const char* kSimpleSql = "SELECT 1";
+
+  ASSERT_TRUE(db().Execute(kCreateSql));
+  ASSERT_TRUE(db().Execute(kPopulateSql));
+
+  // Test baseline expectations.
+  db().Preload();
+  ASSERT_TRUE(db().DoesTableExist("foo"));
+  ASSERT_TRUE(db().IsSQLValid(kSimpleSql));
+  ASSERT_EQ(SQLITE_OK, db().ExecuteAndReturnErrorCode(kSimpleSql));
+  ASSERT_TRUE(db().Execute(kSimpleSql));
+  ASSERT_TRUE(db().is_open());
+  {
+    sql::Statement s(db().GetUniqueStatement(kSimpleSql));
+    ASSERT_TRUE(s.Step());
+  }
+  {
+    sql::Statement s(db().GetCachedStatement(SQL_FROM_HERE, kSimpleSql));
+    ASSERT_TRUE(s.Step());
+  }
+  ASSERT_TRUE(db().BeginTransaction());
+  ASSERT_TRUE(db().CommitTransaction());
+  ASSERT_TRUE(db().BeginTransaction());
+  db().RollbackTransaction();
+
+  ASSERT_TRUE(db().RazeAndClose());
+
+  // At this point, they should all fail, but not crash.
+  db().Preload();
+  ASSERT_FALSE(db().DoesTableExist("foo"));
+  ASSERT_FALSE(db().IsSQLValid(kSimpleSql));
+  ASSERT_EQ(SQLITE_ERROR, db().ExecuteAndReturnErrorCode(kSimpleSql));
+  ASSERT_FALSE(db().Execute(kSimpleSql));
+  ASSERT_FALSE(db().is_open());
+  {
+    sql::Statement s(db().GetUniqueStatement(kSimpleSql));
+    ASSERT_FALSE(s.Step());
+  }
+  {
+    sql::Statement s(db().GetCachedStatement(SQL_FROM_HERE, kSimpleSql));
+    ASSERT_FALSE(s.Step());
+  }
+  ASSERT_FALSE(db().BeginTransaction());
+  ASSERT_FALSE(db().CommitTransaction());
+  ASSERT_FALSE(db().BeginTransaction());
+  db().RollbackTransaction();
+
+  // Close normally to reset the poisoned flag.
+  db().Close();
+
+  // Once the real Close() has been called, various calls enforce API
+  // usage by becoming fatal in debug mode.  Since DEATH tests are
+  // expensive, just test one of them.
+  if (DLOG_IS_ON(FATAL)) {
+    ASSERT_DEATH({
+        db().IsSQLValid(kSimpleSql);
+      }, "Illegal use of connection without a db");
+  }
+}
+
+// TODO(shess): Spin up a background thread to hold other_db, to more
+// closely match real life.  That would also allow testing
+// RazeWithTimeout().
+
 #if defined(OS_ANDROID)
 TEST_F(SQLConnectionTest, SetTempDirForSQL) {
 
@@ -291,7 +393,3 @@ TEST_F(SQLConnectionTest, SetTempDirForSQL) {
   ASSERT_TRUE(meta_table.Init(&db(), 4, 4));
 }
 #endif
-
-// TODO(shess): Spin up a background thread to hold other_db, to more
-// closely match real life.  That would also allow testing
-// RazeWithTimeout().