Bug 1073358 - Sqlite.jsm should return some measure of what happened regardless of whether a row handler was used. r=mak
authorBlair McBride <bmcbride@mozilla.com>
Sat, 27 Sep 2014 01:17:00 +1200
changeset 207460 6351ff630f1a81e5d7d1c584b126ac192fb69111
parent 207459 c56275d516ecd4a865ca6fa0f5d9c21641d92af5
child 207461 8e4b92b32b1556b673211302f56288d7ab0c8a9e
push id27555
push userryanvm@gmail.com
push dateFri, 26 Sep 2014 20:30:28 +0000
treeherderautoland@4ff52be673f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1073358
milestone35.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 1073358 - Sqlite.jsm should return some measure of what happened regardless of whether a row handler was used. r=mak
toolkit/modules/Sqlite.jsm
toolkit/modules/tests/xpcshell/test_sqlite.js
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -574,16 +574,17 @@ ConnectionData.prototype = Object.freeze
     this._bindParameters(statement, params);
 
     let index = this._statementCounter++;
 
     let deferred = Promise.defer();
     let userCancelled = false;
     let errors = [];
     let rows = [];
+    let handledRow = false;
 
     // Don't incur overhead for serializing params unless the messages go
     // somewhere.
     if (this._log.level <= Log.Level.Trace) {
       let msg = "Stmt #" + index + " " + sql;
 
       if (params) {
         msg += " - " + JSON.stringify(params);
@@ -599,16 +600,18 @@ ConnectionData.prototype = Object.freeze
         // .cancel() may not be immediate and handleResult() could be called
         // after a .cancel().
         for (let row = resultSet.getNextRow(); row && !userCancelled; row = resultSet.getNextRow()) {
           if (!onRow) {
             rows.push(row);
             continue;
           }
 
+          handledRow = true;
+
           try {
             onRow(row);
           } catch (e if e instanceof StopIteration) {
             userCancelled = true;
             pending.cancel();
             break;
           } catch (ex) {
             self._log.warn("Exception when calling onRow callback: " +
@@ -624,26 +627,27 @@ ConnectionData.prototype = Object.freeze
       },
 
       handleCompletion: function (reason) {
         self._log.debug("Stmt #" + index + " finished.");
         self._pendingStatements.delete(index);
 
         switch (reason) {
           case Ci.mozIStorageStatementCallback.REASON_FINISHED:
-            // If there is an onRow handler, we always resolve to null.
-            let result = onRow ? null : rows;
+            // If there is an onRow handler, we always instead resolve to a
+            // boolean indicating whether the onRow handler was called or not.
+            let result = onRow ? handledRow : rows;
             deferred.resolve(result);
             break;
 
           case Ci.mozIStorageStatementCallback.REASON_CANCELED:
             // It is not an error if the user explicitly requested cancel via
             // the onRow handler.
             if (userCancelled) {
-              let result = onRow ? null : rows;
+              let result = onRow ? handledRow : rows;
               deferred.resolve(result);
             } else {
               deferred.reject(new Error("Statement was cancelled."));
             }
 
             break;
 
           case Ci.mozIStorageStatementCallback.REASON_ERROR:
--- a/toolkit/modules/tests/xpcshell/test_sqlite.js
+++ b/toolkit/modules/tests/xpcshell/test_sqlite.js
@@ -278,51 +278,67 @@ add_task(function test_on_row_exception_
   let c = yield getDummyDatabase("on_row_exception_ignored");
 
   let sql = "INSERT INTO dirs (path) VALUES (?)";
   for (let i = 0; i < 10; i++) {
     yield c.executeCached(sql, ["dir" + i]);
   }
 
   let i = 0;
-  yield c.execute("SELECT * FROM DIRS", null, function onRow(row) {
+  let hasResult = yield c.execute("SELECT * FROM DIRS", null, function onRow(row) {
     i++;
 
     throw new Error("Some silly error.");
   });
 
+  do_check_eq(hasResult, true);
   do_check_eq(i, 10);
 
   yield c.close();
 });
 
 // Ensure StopIteration during onRow causes processing to stop.
 add_task(function test_on_row_stop_iteration() {
   let c = yield getDummyDatabase("on_row_stop_iteration");
 
   let sql = "INSERT INTO dirs (path) VALUES (?)";
   for (let i = 0; i < 10; i++) {
     yield c.executeCached(sql, ["dir" + i]);
   }
 
   let i = 0;
-  let result = yield c.execute("SELECT * FROM dirs", null, function onRow(row) {
+  let hasResult = yield c.execute("SELECT * FROM dirs", null, function onRow(row) {
     i++;
 
     if (i == 5) {
       throw StopIteration;
     }
   });
 
-  do_check_null(result);
+  do_check_eq(hasResult, true);
   do_check_eq(i, 5);
 
   yield c.close();
 });
 
+// Ensure execute resolves to false when no rows are selected.
+add_task(function test_on_row_stop_iteration() {
+  let c = yield getDummyDatabase("no_on_row");
+
+  let i = 0;
+  let hasResult = yield c.execute(`SELECT * FROM dirs WHERE path="nonexistent"`, null, function onRow(row) {
+    i++;
+  });
+
+  do_check_eq(hasResult, false);
+  do_check_eq(i, 0);
+
+  yield c.close();
+});
+
 add_task(function test_invalid_transaction_type() {
   let c = yield getDummyDatabase("invalid_transaction_type");
 
   let errored = false;
   try {
     c.executeTransaction(function () {}, "foobar");
   } catch (ex) {
     errored = true;