Bug 833965 - Sqlite.jsm should support an array of binding parameters. r=gps
authorRichard Newman <rnewman@mozilla.com>
Wed, 10 Apr 2013 17:57:42 -0700
changeset 139732 df617f125392eec740c7fed27ed9e1d2c6c849b1
parent 139731 e4fa648c30c76e1beab0c3c3db3807aa84754ff0
child 139733 6de778bb9c5103db26aa800c76ca2468afb23a27
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs833965
milestone23.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 833965 - Sqlite.jsm should support an array of binding parameters. r=gps
toolkit/modules/Sqlite.jsm
toolkit/modules/tests/xpcshell/test_sqlite.js
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -684,37 +684,70 @@ OpenedConnection.prototype = Object.free
       ++count;
       statement.finalize();
     }
     this._cachedStatements.clear();
     this._log.debug("Discarded " + count + " cached statements.");
     return count;
   },
 
+  /**
+   * Helper method to bind parameters of various kinds through
+   * reflection.
+   */
+  _bindParameters: function (statement, params) {
+    if (!params) {
+      return;
+    }
+
+    if (Array.isArray(params)) {
+      // It's an array of separate params.
+      if (params.length && (typeof(params[0]) == "object")) {
+        let paramsArray = statement.newBindingParamsArray();
+        for (let p of params) {
+          let bindings = paramsArray.newBindingParams();
+          for (let [key, value] of Iterator(p)) {
+            bindings.bindByName(key, value);
+          }
+          paramsArray.addParams(bindings);
+        }
+
+        statement.bindParameters(paramsArray);
+        return;
+      }
+
+      // Indexed params.
+      for (let i = 0; i < params.length; i++) {
+        statement.bindByIndex(i, params[i]);
+      }
+      return;
+    }
+
+    // Named params.
+    if (params && typeof(params) == "object") {
+      for (let k in params) {
+        statement.bindByName(k, params[k]);
+      }
+      return;
+    }
+
+    throw new Error("Invalid type for bound parameters. Expected Array or " +
+                    "object. Got: " + params);
+  },
+
   _executeStatement: function (sql, statement, params, onRow) {
     if (statement.state != statement.MOZ_STORAGE_STATEMENT_READY) {
       throw new Error("Statement is not ready for execution.");
     }
 
     if (onRow && typeof(onRow) != "function") {
       throw new Error("onRow must be a function. Got: " + onRow);
     }
 
-    if (Array.isArray(params)) {
-      for (let i = 0; i < params.length; i++) {
-        statement.bindByIndex(i, params[i]);
-      }
-    } else if (params && typeof(params) == "object") {
-      for (let k in params) {
-        statement.bindByName(k, params[k]);
-      }
-    } else if (params) {
-      throw new Error("Invalid type for bound parameters. Expected Array or " +
-                      "object. Got: " + params);
-    }
+    this._bindParameters(statement, params);
 
     let index = this._statementCounter++;
 
     let deferred = Promise.defer();
     let userCancelled = false;
     let errors = [];
     let rows = [];
 
--- a/toolkit/modules/tests/xpcshell/test_sqlite.js
+++ b/toolkit/modules/tests/xpcshell/test_sqlite.js
@@ -570,16 +570,128 @@ add_task(function test_discard_cached() 
   do_check_eq(2, c._cachedStatements.size);
 
   c.discardCachedStatements();
   do_check_eq(0, c._cachedStatements.size);
 
   yield c.close();
 });
 
+add_task(function test_programmatic_binding() {
+  let c = yield getDummyDatabase("programmatic_binding");
+
+  let bindings = [
+    {id: 1,    path: "foobar"},
+    {id: null, path: "baznoo"},
+    {id: 5,    path: "toofoo"},
+  ];
+
+  let sql = "INSERT INTO dirs VALUES (:id, :path)";
+  let result = yield c.execute(sql, bindings);
+  do_check_eq(result.length, 0);
+
+  let rows = yield c.executeCached("SELECT * from dirs");
+  do_check_eq(rows.length, 3);
+  yield c.close();
+});
+
+add_task(function test_programmatic_binding_transaction() {
+  let c = yield getDummyDatabase("programmatic_binding_transaction");
+
+  let bindings = [
+    {id: 1,    path: "foobar"},
+    {id: null, path: "baznoo"},
+    {id: 5,    path: "toofoo"},
+  ];
+
+  let sql = "INSERT INTO dirs VALUES (:id, :path)";
+  yield c.executeTransaction(function transaction() {
+    let result = yield c.execute(sql, bindings);
+    do_check_eq(result.length, 0);
+
+    let rows = yield c.executeCached("SELECT * from dirs");
+    do_check_eq(rows.length, 3);
+  });
+
+  // Transaction committed.
+  let rows = yield c.executeCached("SELECT * from dirs");
+  do_check_eq(rows.length, 3);
+  yield c.close();
+});
+
+add_task(function test_programmatic_binding_transaction_partial_rollback() {
+  let c = yield getDummyDatabase("programmatic_binding_transaction_partial_rollback");
+
+  let bindings = [
+    {id: 2, path: "foobar"},
+    {id: 3, path: "toofoo"},
+  ];
+
+  let sql = "INSERT INTO dirs VALUES (:id, :path)";
+
+  // Add some data in an implicit transaction before beginning the batch insert.
+  yield c.execute(sql, {id: 1, path: "works"});
+
+  let secondSucceeded = false;
+  try {
+    yield c.executeTransaction(function transaction() {
+      // Insert one row. This won't implicitly start a transaction.
+      let result = yield c.execute(sql, bindings[0]);
+
+      // Insert multiple rows. mozStorage will want to start a transaction.
+      // One of the inserts will fail, so the transaction should be rolled back.
+      let result = yield c.execute(sql, bindings);
+      secondSucceeded = true;
+    });
+  } catch (ex) {
+    print("Caught expected exception: " + ex);
+  }
+
+  // We did not get to the end of our in-transaction block.
+  do_check_false(secondSucceeded);
+
+  // Everything that happened in *our* transaction, not mozStorage's, got
+  // rolled back, but the first row still exists.
+  let rows = yield c.executeCached("SELECT * from dirs");
+  do_check_eq(rows.length, 1);
+  do_check_eq(rows[0].getResultByName("path"), "works");
+  yield c.close();
+});
+
+/**
+ * Just like the previous test, but relying on the implicit
+ * transaction established by mozStorage.
+ */
+add_task(function test_programmatic_binding_implicit_transaction() {
+  let c = yield getDummyDatabase("programmatic_binding_implicit_transaction");
+
+  let bindings = [
+    {id: 2, path: "foobar"},
+    {id: 1, path: "toofoo"},
+  ];
+
+  let sql = "INSERT INTO dirs VALUES (:id, :path)";
+  let secondSucceeded = false;
+  yield c.execute(sql, {id: 1, path: "works"});
+  try {
+    let result = yield c.execute(sql, bindings);
+    secondSucceeded = true;
+  } catch (ex) {
+    print("Caught expected exception: " + ex);
+  }
+
+  do_check_false(secondSucceeded);
+
+  // The entire batch failed.
+  let rows = yield c.executeCached("SELECT * from dirs");
+  do_check_eq(rows.length, 1);
+  do_check_eq(rows[0].getResultByName("path"), "works");
+  yield c.close();
+});
+
 /**
  * Test that direct binding of params and execution through mozStorage doesn't
  * error when we manually create a transaction. See Bug 856925.
  */
 add_task(function test_direct() {
   let file = FileUtils.getFile("TmpD", ["test_direct.sqlite"]);
   file.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
   print("Opening " + file.path);