Bug 1435446 - Add a default transaction type for storage connections. r?mak draft
authorKit Cambridge <kit@yakshaving.ninja>
Wed, 28 Feb 2018 22:44:40 -0800
changeset 766323 94d62e36fe4df0a508643de97b3a287f6549c445
parent 766212 d6957f004e9cc3d7408ac3a8f2b49ff97556e27f
child 766324 93f2c5b7aaf1c182fb9115e7dc5a6f9caba1ef63
push id102282
push userbmo:kit@mozilla.com
push dateMon, 12 Mar 2018 17:42:48 +0000
reviewersmak
bugs1435446
milestone61.0a1
Bug 1435446 - Add a default transaction type for storage connections. r?mak This patch adds a `mozIStorageConnection::defaultTransactionType` attribute that controls the default transaction behavior for the connection. As before, `mozStorageTransaction` can override the default behavior for individual transactions. MozReview-Commit-ID: IRSlMesETWN
dom/cache/Connection.cpp
storage/mozIStorageAsyncConnection.idl
storage/mozIStorageConnection.idl
storage/mozStorageConnection.cpp
storage/mozStorageConnection.h
storage/mozStorageHelper.h
storage/test/unit/test_storage_connection.js
toolkit/modules/Sqlite.jsm
toolkit/modules/tests/xpcshell/test_sqlite.js
--- a/dom/cache/Connection.cpp
+++ b/dom/cache/Connection.cpp
@@ -241,28 +241,34 @@ Connection::IndexExists(const nsACString
 
 NS_IMETHODIMP
 Connection::GetTransactionInProgress(bool* aResultOut)
 {
   return mBase->GetTransactionInProgress(aResultOut);
 }
 
 NS_IMETHODIMP
+Connection::GetDefaultTransactionType(int32_t* aResultOut)
+{
+  return mBase->GetDefaultTransactionType(aResultOut);
+}
+
+NS_IMETHODIMP
+Connection::SetDefaultTransactionType(int32_t aType)
+{
+  return mBase->SetDefaultTransactionType(aType);
+}
+
+NS_IMETHODIMP
 Connection::BeginTransaction()
 {
   return mBase->BeginTransaction();
 }
 
 NS_IMETHODIMP
-Connection::BeginTransactionAs(int32_t aType)
-{
-  return mBase->BeginTransactionAs(aType);
-}
-
-NS_IMETHODIMP
 Connection::CommitTransaction()
 {
   return mBase->CommitTransaction();
 }
 
 NS_IMETHODIMP
 Connection::RollbackTransaction()
 {
--- a/storage/mozIStorageAsyncConnection.idl
+++ b/storage/mozIStorageAsyncConnection.idl
@@ -21,16 +21,31 @@ interface nsIFile;
  * connection attached to a specific file or to an in-memory data
  * storage.  It is the primary interface for interacting with a
  * database from the main thread, including creating prepared
  * statements, executing SQL, and examining database errors.
  */
 [scriptable, uuid(8bfd34d5-4ddf-4e4b-89dd-9b14f33534c6)]
 interface mozIStorageAsyncConnection : nsISupports {
   /**
+   * Transaction behavior constants.
+   */
+  const int32_t TRANSACTION_DEFAULT = -1;
+  const int32_t TRANSACTION_DEFERRED = 0;
+  const int32_t TRANSACTION_IMMEDIATE = 1;
+  const int32_t TRANSACTION_EXCLUSIVE = 2;
+
+  /**
+   * The default behavior for all transactions run on this connection. Defaults
+   * to `TRANSACTION_DEFERRED`, and can be overridden for individual
+   * transactions.
+   */
+  attribute int32_t defaultTransactionType;
+
+  /**
    * Close this database connection, allowing all pending statements
    * to complete first.
    *
    * @param aCallback [optional]
    *        A callback that will be notified when the close is completed,
    *        with the following arguments:
    *        - status: the status of the call
    *        - value: |null|
--- a/storage/mozIStorageConnection.idl
+++ b/storage/mozIStorageConnection.idl
@@ -175,30 +175,21 @@ interface mozIStorageConnection : mozISt
   //// Transactions
 
   /**
    * Returns true if a transaction is active on this connection.
    */
   readonly attribute boolean transactionInProgress;
 
   /**
-   * Begin a new transaction.  sqlite default transactions are deferred.
-   * If a transaction is active, throws an error.
+   * Begin a new transaction. If a transaction is active, throws an error.
    */
   void beginTransaction();
 
   /**
-   * Begins a new transaction with the given type.
-   */
-  const int32_t TRANSACTION_DEFERRED = 0;
-  const int32_t TRANSACTION_IMMEDIATE = 1;
-  const int32_t TRANSACTION_EXCLUSIVE = 2;
-  void beginTransactionAs(in int32_t transactionType);
-
-  /**
    * Commits the current transaction.  If no transaction is active,
    * @throws NS_ERROR_UNEXPECTED.
    * @throws NS_ERROR_NOT_INITIALIZED.
    */
   void commitTransaction();
 
   /**
    * Rolls back the current transaction.  If no transaction is active,
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -529,16 +529,17 @@ Connection::Connection(Service *aService
                        bool aAsyncOnly,
                        bool aIgnoreLockingMode)
 : sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex")
 , sharedDBMutex("Connection::sharedDBMutex")
 , threadOpenedOn(do_GetCurrentThread())
 , mDBConn(nullptr)
 , mAsyncExecutionThreadShuttingDown(false)
 , mConnectionClosed(false)
+, mDefaultTransactionType(mozIStorageConnection::TRANSACTION_DEFERRED)
 , mTransactionInProgress(false)
 , mDestroying(false)
 , mProgressHandler(nullptr)
 , mFlags(aFlags)
 , mIgnoreLockingMode(aIgnoreLockingMode)
 , mStorageService(aService)
 , mAsyncOnly(aAsyncOnly)
 {
@@ -1527,16 +1528,19 @@ Connection::initializeClone(Connection* 
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   auto guard = MakeScopeExit([&]() {
     aClone->initializeFailed();
   });
 
+  rv = aClone->SetDefaultTransactionType(mDefaultTransactionType);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // Re-attach on-disk databases that were attached to the original connection.
   {
     nsCOMPtr<mozIStorageStatement> stmt;
     rv = CreateStatement(NS_LITERAL_CSTRING("PRAGMA database_list"),
                          getter_AddRefs(stmt));
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     bool hasResult = false;
     while (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
@@ -1928,38 +1932,47 @@ Connection::GetTransactionInProgress(boo
   if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;
 
   SQLiteMutexAutoLock lockedScope(sharedDBMutex);
   *_inProgress = mTransactionInProgress;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-Connection::BeginTransaction()
+Connection::GetDefaultTransactionType(int32_t *_type)
 {
-  return BeginTransactionAs(mozIStorageConnection::TRANSACTION_DEFERRED);
+  *_type = mDefaultTransactionType;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
-Connection::BeginTransactionAs(int32_t aTransactionType)
+Connection::SetDefaultTransactionType(int32_t aType)
+{
+  NS_ENSURE_ARG_RANGE(aType, TRANSACTION_DEFERRED, TRANSACTION_EXCLUSIVE);
+  mDefaultTransactionType = aType;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+Connection::BeginTransaction()
 {
   if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;
 
-  return beginTransactionInternal(mDBConn, aTransactionType);
+  return beginTransactionInternal(mDBConn, mDefaultTransactionType);
 }
 
 nsresult
 Connection::beginTransactionInternal(sqlite3 *aNativeConnection,
                                      int32_t aTransactionType)
 {
   SQLiteMutexAutoLock lockedScope(sharedDBMutex);
   if (mTransactionInProgress)
     return NS_ERROR_FAILURE;
   nsresult rv;
-  switch(aTransactionType) {
+  switch (aTransactionType) {
     case TRANSACTION_DEFERRED:
       rv = convertResultCode(executeSql(aNativeConnection, "BEGIN DEFERRED"));
       break;
     case TRANSACTION_IMMEDIATE:
       rv = convertResultCode(executeSql(aNativeConnection, "BEGIN IMMEDIATE"));
       break;
     case TRANSACTION_EXCLUSIVE:
       rv = convertResultCode(executeSql(aNativeConnection, "BEGIN EXCLUSIVE"));
--- a/storage/mozStorageConnection.h
+++ b/storage/mozStorageConnection.h
@@ -371,16 +371,21 @@ private:
    * connection.
    *
    * This variable should be accessed while holding the
    * sharedAsyncExecutionMutex.
    */
   bool mConnectionClosed;
 
   /**
+   * Stores the default behavior for all transactions run on this connection.
+   */
+  mozilla::Atomic<int32_t> mDefaultTransactionType;
+
+  /**
    * Tracks if we have a transaction in progress or not.  Access protected by
    * sharedDBMutex.
    */
   bool mTransactionInProgress;
 
   /**
    * Used to trigger cleanup logic only the first time our refcount hits 1.  We
    * may trigger a failsafe Close() that invokes SpinningSynchronousClose()
--- a/storage/mozStorageHelper.h
+++ b/storage/mozStorageHelper.h
@@ -34,18 +34,18 @@
  * to rollback.
  *
  * @param aConnection
  *        The connection to create the transaction on.
  * @param aCommitOnComplete
  *        Controls whether the transaction is committed or rolled back when
  *        this object goes out of scope.
  * @param aType [optional]
- *        The transaction type, as defined in mozIStorageConnection.  Defaults
- *        to TRANSACTION_DEFERRED.
+ *        The transaction type, as defined in mozIStorageConnection. Uses the
+ *        default transaction behavior for the connection if unspecified.
  * @param aAsyncCommit [optional]
  *        Whether commit should be executed asynchronously on the helper thread.
  *        This is a special option introduced as an interim solution to reduce
  *        main-thread fsyncs in Places.  Can only be used on main-thread.
  *
  *        WARNING: YOU SHOULD _NOT_ WRITE NEW MAIN-THREAD CODE USING THIS!
  *
  *        Notice that async commit might cause synchronous statements to fail
@@ -58,27 +58,31 @@
  *        For all of the above reasons, this should only be used as an interim
  *        solution and avoided completely if possible.
  */
 class mozStorageTransaction
 {
 public:
   mozStorageTransaction(mozIStorageConnection* aConnection,
                         bool aCommitOnComplete,
-                        int32_t aType = mozIStorageConnection::TRANSACTION_DEFERRED,
+                        int32_t aType = mozIStorageConnection::TRANSACTION_DEFAULT,
                         bool aAsyncCommit = false)
     : mConnection(aConnection),
       mHasTransaction(false),
       mCommitOnComplete(aCommitOnComplete),
       mCompleted(false),
       mAsyncCommit(aAsyncCommit)
   {
     if (mConnection) {
       nsAutoCString query("BEGIN");
-      switch(aType) {
+      int32_t type = aType;
+      if (type == mozIStorageConnection::TRANSACTION_DEFAULT) {
+        MOZ_ALWAYS_SUCCEEDS(mConnection->GetDefaultTransactionType(&type));
+      }
+      switch (type) {
         case mozIStorageConnection::TRANSACTION_IMMEDIATE:
           query.AppendLiteral(" IMMEDIATE");
           break;
         case mozIStorageConnection::TRANSACTION_EXCLUSIVE:
           query.AppendLiteral(" EXCLUSIVE");
           break;
         case mozIStorageConnection::TRANSACTION_DEFERRED:
           query.AppendLiteral(" DEFERRED");
--- a/storage/test/unit/test_storage_connection.js
+++ b/storage/test/unit/test_storage_connection.js
@@ -909,16 +909,72 @@ add_task(async function test_sync_clone_
 
   Assert.equal(func.name, "kit");
 
   info("Clean up");
   db.close();
   clone.close();
 });
 
+add_task(async function test_defaultTransactionType() {
+  info("Open connection");
+  let db = Services.storage.openDatabase(getTestDB());
+  Assert.ok(db instanceof Ci.mozIStorageAsyncConnection);
+
+  info("Verify default transaction type");
+  Assert.equal(db.defaultTransactionType,
+    Ci.mozIStorageConnection.TRANSACTION_DEFERRED);
+
+  info("Test other transaction types");
+  for (let type of [Ci.mozIStorageConnection.TRANSACTION_IMMEDIATE,
+                    Ci.mozIStorageConnection.TRANSACTION_EXCLUSIVE]) {
+    db.defaultTransactionType = type;
+    Assert.equal(db.defaultTransactionType, type);
+  }
+
+  info("Should reject unknown transaction types");
+  Assert.throws(() => db.defaultTransactionType =
+                        Ci.mozIStorageConnection.TRANSACTION_DEFAULT,
+                /NS_ERROR_ILLEGAL_VALUE/);
+
+  db.defaultTransactionType =
+    Ci.mozIStorageConnection.TRANSACTION_IMMEDIATE;
+
+  info("Clone should inherit default transaction type");
+  let clone = await asyncClone(db, true);
+  Assert.ok(clone instanceof Ci.mozIStorageAsyncConnection);
+  Assert.equal(clone.defaultTransactionType,
+    Ci.mozIStorageConnection.TRANSACTION_IMMEDIATE);
+
+  info("Begin immediate transaction on main connection");
+  db.beginTransaction();
+
+  info("Queue immediate transaction on clone");
+  let stmts = [
+    clone.createAsyncStatement(`BEGIN IMMEDIATE TRANSACTION`),
+    clone.createAsyncStatement(`DELETE FROM test WHERE name = 'new'`),
+    clone.createAsyncStatement(`COMMIT`),
+  ];
+  let promiseStmtsRan = stmts.map(stmt => executeAsync(stmt));
+
+  info("Commit immediate transaction on main connection");
+  db.executeSimpleSQL(`INSERT INTO test(name) VALUES('new')`);
+  db.commitTransaction();
+
+  info("Wait for transaction to succeed on clone");
+  await Promise.all(promiseStmtsRan);
+
+  info("Clean up");
+  for (let stmt of stmts) {
+    stmt.finalize();
+  }
+  await asyncClose(clone);
+  await asyncClose(db);
+});
+
 add_task(async function test_getInterface() {
   let db = getOpenedDatabase();
   let target = db.QueryInterface(Ci.nsIInterfaceRequestor)
                  .getInterface(Ci.nsIEventTarget);
   // Just check that target is non-null.  Other tests will ensure that it has
   // the correct value.
   Assert.ok(target != null);
 
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -220,16 +220,22 @@ function ConnectionData(connection, iden
   this._pendingStatements = new Map();
 
   // Increments for each executed statement for the life of the connection.
   this._statementCounter = 0;
 
   // Increments whenever we request a unique operation id.
   this._operationsCounter = 0;
 
+  if ("defaultTransactionType" in options) {
+    this.defaultTransactionType = options.defaultTransactionType;
+  } else {
+    this.defaultTransactionType = convertStorageTransactionType(
+      this._dbConn.defaultTransactionType);
+  }
   this._hasInProgressTransaction = false;
   // Manages a chain of transactions promises, so that new transactions
   // always happen in queue to the previous ones.  It never rejects.
   this._transactionQueue = Promise.resolve();
 
   this._idleShrinkMS = options.shrinkMemoryOnConnectionIdleMS;
   if (this._idleShrinkMS) {
     this._idleShrinkTimer = Cc["@mozilla.org/timer;1"]
@@ -534,18 +540,20 @@ ConnectionData.prototype = Object.freeze
     });
   },
 
   get transactionInProgress() {
     return this._open && this._hasInProgressTransaction;
   },
 
   executeTransaction(func, type) {
-    if (typeof type == "undefined") {
-      throw new Error("Internal error: expected a type");
+    if (type == OpenedConnection.prototype.TRANSACTION_DEFAULT) {
+      type = this.defaultTransactionType;
+    } else if (!OpenedConnection.TRANSACTION_TYPES.includes(type)) {
+      throw new Error("Unknown transaction type: " + type);
     }
     this.ensureOpen();
 
     this._log.debug("Beginning transaction");
 
     let promise = this._transactionQueue.then(() => {
       if (this._closeRequested) {
         throw new Error("Transaction canceled due to a closed connection.");
@@ -913,16 +921,26 @@ function openConnection(options) {
       throw new Error("shrinkMemoryOnConnectionIdleMS must be an integer. " +
                       "Got: " + options.shrinkMemoryOnConnectionIdleMS);
     }
 
     openedOptions.shrinkMemoryOnConnectionIdleMS =
       options.shrinkMemoryOnConnectionIdleMS;
   }
 
+  if ("defaultTransactionType" in options) {
+    let defaultTransactionType = options.defaultTransactionType;
+    if (!OpenedConnection.TRANSACTION_TYPES.includes(defaultTransactionType)) {
+      throw new Error("Unknown default transaction type: " +
+                      defaultTransactionType);
+    }
+
+    openedOptions.defaultTransactionType = defaultTransactionType;
+  }
+
   let file = FileUtils.File(path);
   let identifier = getIdentifierByFileName(OS.Path.basename(path));
 
   log.info("Opening database: " + path + " (" + identifier + ")");
 
   return new Promise((resolve, reject) => {
     let dbOptions = Cc["@mozilla.org/hash-property-bag;1"].
                     createInstance(Ci.nsIWritablePropertyBag);
@@ -1151,23 +1169,33 @@ function OpenedConnection(connection, id
   // before its `forget` method has been called, an event with topic
   // "sqlite-finalization-witness" is broadcasted along with the
   // connection identifier string of the database.
   this._witness = FinalizationWitnessService.make(
     "sqlite-finalization-witness",
     this._connectionData._identifier);
 }
 
+OpenedConnection.TRANSACTION_TYPES = ["DEFERRED", "IMMEDIATE", "EXCLUSIVE"];
+
+// Converts a `mozIStorageAsyncConnection::TRANSACTION_*` constant into the
+// corresponding `OpenedConnection.TRANSACTION_TYPES` constant.
+function convertStorageTransactionType(type) {
+  if (!(type in OpenedConnection.TRANSACTION_TYPES)) {
+    throw new Error("Unknown storage transaction type: " + type);
+  }
+  return OpenedConnection.TRANSACTION_TYPES[type];
+}
+
 OpenedConnection.prototype = Object.freeze({
+  TRANSACTION_DEFAULT: "DEFAULT",
   TRANSACTION_DEFERRED: "DEFERRED",
   TRANSACTION_IMMEDIATE: "IMMEDIATE",
   TRANSACTION_EXCLUSIVE: "EXCLUSIVE",
 
-  TRANSACTION_TYPES: ["DEFERRED", "IMMEDIATE", "EXCLUSIVE"],
-
   /**
    * The integer schema version of the database.
    *
    * This is 0 if not schema version has been set.
    *
    * @return Promise<int>
    */
   getSchemaVersion(schemaName = "main") {
@@ -1324,16 +1352,23 @@ OpenedConnection.prototype = Object.free
   execute(sql, params = null, onRow = null) {
     if (isInvalidBoundLikeQuery(sql)) {
       throw new Error("Please enter a LIKE clause with bindings");
     }
     return this._connectionData.execute(sql, params, onRow);
   },
 
   /**
+   * The default behavior for transactions run on this connection.
+   */
+  get defaultTransactionType() {
+    return this._connectionData.defaultTransactionType;
+  },
+
+  /**
    * Whether a transaction is currently in progress.
    */
   get transactionInProgress() {
     return this._connectionData.transactionInProgress;
   },
 
   /**
    * Perform a transaction.
@@ -1368,21 +1403,17 @@ OpenedConnection.prototype = Object.free
    * be resolved to whatever value the supplied function resolves to. If
    * the transaction is rolled back, the promise is rejected.
    *
    * @param func
    *        (function) What to perform as part of the transaction.
    * @param type optional
    *        One of the TRANSACTION_* constants attached to this type.
    */
-  executeTransaction(func, type = this.TRANSACTION_DEFERRED) {
-    if (!this.TRANSACTION_TYPES.includes(type)) {
-      throw new Error("Unknown transaction type: " + type);
-    }
-
+  executeTransaction(func, type = this.TRANSACTION_DEFAULT) {
     return this._connectionData.executeTransaction(() => func(this), type);
   },
 
   /**
    * Whether a table exists in the database (both persistent and temporary tables).
    *
    * @param name
    *        (string) Name of the table.
--- a/toolkit/modules/tests/xpcshell/test_sqlite.js
+++ b/toolkit/modules/tests/xpcshell/test_sqlite.js
@@ -79,16 +79,25 @@ async function getDummyTempDatabase(name
 
 add_task(async function test_setup() {
   ChromeUtils.import("resource://testing-common/services/common/logging.js");
   initTestLogging("Trace");
 });
 
 add_task(async function test_open_normal() {
   let c = await Sqlite.openConnection({path: "test_open_normal.sqlite"});
+  Assert.equal(c.defaultTransactionType, "DEFERRED");
+  await c.close();
+});
+
+add_task(async function test_open_with_defaultTransactionType() {
+  let c = await getConnection("execute_transaction_types", {
+    defaultTransactionType: "IMMEDIATE",
+  });
+  Assert.equal(c.defaultTransactionType, "IMMEDIATE");
   await c.close();
 });
 
 add_task(async function test_open_normal_error() {
   let currentDir = await OS.File.getCurrentDirectory();
 
   let src = OS.Path.join(currentDir, "corrupt.sqlite");
   Assert.ok((await OS.File.exists(src)), "Database file found");
@@ -910,20 +919,25 @@ add_task(async function test_cloneStorag
         resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
       } else {
         reject(new Error(status));
       }
     });
   });
 
   let clone = await Sqlite.cloneStorageConnection({ connection: c, readOnly: true });
+  Assert.equal(clone.defaultTransactionType, "DEFERRED");
   // Just check that it works.
   await clone.execute("SELECT 1");
 
+  info("Set default transaction type on storage connection");
+  c.defaultTransactionType = Ci.mozIStorageConnection.TRANSACTION_IMMEDIATE;
+
   let clone2 = await Sqlite.cloneStorageConnection({ connection: c, readOnly: false });
+  Assert.equal(clone2.defaultTransactionType, "IMMEDIATE");
   // Just check that it works.
   await clone2.execute("CREATE TABLE test (id INTEGER PRIMARY KEY)");
 
   // Closing order should not matter.
   await c.asyncClose();
   await clone2.close();
   await clone.close();
 });
@@ -979,23 +993,35 @@ add_task(async function test_wrapStorage
         resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
       } else {
         reject(new Error(status));
       }
     });
   });
 
   let wrapper = await Sqlite.wrapStorageConnection({ connection: c });
+  Assert.equal(wrapper.defaultTransactionType, "DEFERRED");
   // Just check that it works.
   await wrapper.execute("SELECT 1");
   await wrapper.executeCached("SELECT 1");
 
   // Closing the wrapper should just finalize statements but not close the
   // database.
   await wrapper.close();
+
+  info("Set default transaction type on storage connection");
+  c.defaultTransactionType = Ci.mozIStorageConnection.TRANSACTION_EXCLUSIVE;
+
+  let wrapper2 = await Sqlite.wrapStorageConnection({ connection: c });
+  Assert.equal(wrapper2.defaultTransactionType, "EXCLUSIVE");
+  // Just check that it works.
+  await wrapper2.execute("SELECT 1");
+
+  await wrapper2.close();
+
   await c.asyncClose();
 });
 
 // Test finalization
 add_task(async function test_closed_by_witness() {
   failTestsOnAutoClose(false);
   let c = await getDummyDatabase("closed_by_witness");