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

--HG--
extra : rebase_source : fc63af108bb246bc096cb9ef7c13b41fabba5563

dom/cache/Connection.cpp

@@ -246,15 +246,21 @@ Connection::GetTransactionInProgress(bool* aResultOut)
 }
 
 NS_IMETHODIMP
-Connection::BeginTransaction()
+Connection::GetDefaultTransactionType(int32_t* aResultOut)
 {
-  return mBase->BeginTransaction();
+  return mBase->GetDefaultTransactionType(aResultOut);
 }
 
 NS_IMETHODIMP
-Connection::BeginTransactionAs(int32_t aType)
+Connection::SetDefaultTransactionType(int32_t aType)
 {
-  return mBase->BeginTransactionAs(aType);
+  return mBase->SetDefaultTransactionType(aType);
+}
+
+NS_IMETHODIMP
+Connection::BeginTransaction()
+{
+  return mBase->BeginTransaction();
 }
 
 NS_IMETHODIMP

storage/mozIStorageAsyncConnection.idl

@@ -25,6 +25,21 @@ interface nsIFile;
  */
 [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.

storage/mozIStorageConnection.idl

@@ -180,19 +180,10 @@ interface mozIStorageConnection : mozIStorageAsyncConnection {
   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.

storage/mozStorageConnection.cpp

@@ -534,6 +534,7 @@ Connection::Connection(Service *aService,
 , mDBConn(nullptr)
 , mAsyncExecutionThreadShuttingDown(false)
 , mConnectionClosed(false)
+, mDefaultTransactionType(mozIStorageConnection::TRANSACTION_DEFERRED)
 , mTransactionInProgress(false)
 , mDestroying(false)
 , mProgressHandler(nullptr)
@@ -1532,6 +1533,9 @@ Connection::initializeClone(Connection* aClone, bool aReadOnly)
     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;
@@ -1933,17 +1937,26 @@ Connection::GetTransactionInProgress(bool *_inProgress)
 }
 
 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
@@ -1954,7 +1967,7 @@ Connection::beginTransactionInternal(sqlite3 *aNativeConnection,
   if (mTransactionInProgress)
     return NS_ERROR_FAILURE;
   nsresult rv;
-  switch(aTransactionType) {
+  switch (aTransactionType) {
     case TRANSACTION_DEFERRED:
       rv = convertResultCode(executeSql(aNativeConnection, "BEGIN DEFERRED"));
       break;

storage/mozStorageConnection.h

@@ -375,6 +375,11 @@ private:
    */
   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.

storage/mozStorageHelper.h

@@ -39,8 +39,8 @@
  *        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
@@ -63,7 +63,7 @@ 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),
@@ -73,7 +73,11 @@ public:
   {
     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;

storage/test/unit/test_storage_connection.js

@@ -914,6 +914,62 @@ add_task(async function test_sync_clone_with_function() {
   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)

toolkit/modules/Sqlite.jsm

@@ -225,6 +225,12 @@ function ConnectionData(connection, identifier, options = {}) {
   // 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.
@@ -539,8 +545,10 @@ ConnectionData.prototype = Object.freeze({
   },
 
   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();
 
@@ -918,6 +926,16 @@ function openConnection(options) {
       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));
 
@@ -1156,13 +1174,23 @@ function OpenedConnection(connection, identifier, options = {}) {
     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.
    *
@@ -1328,6 +1356,13 @@ OpenedConnection.prototype = Object.freeze({
     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.
    */
@@ -1373,11 +1408,7 @@ OpenedConnection.prototype = Object.freeze({
    * @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);
   },
 

toolkit/modules/tests/xpcshell/test_sqlite.js

@@ -84,6 +84,15 @@ add_task(async function test_setup() {
 
 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();
 });
 
@@ -915,10 +924,15 @@ add_task(async function test_cloneStorageConnection() {
   });
 
   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)");
 
@@ -984,6 +998,7 @@ add_task(async function test_wrapStorageConnection() {
   });
 
   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");
@@ -991,6 +1006,17 @@ add_task(async function test_wrapStorageConnection() {
   // 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();
 });