Bug 570635 - Use async queries for fetching form data [r=mconnor]

Create a Utils.queryAsync to wrap executeAsync that fetches all rows and columns by name. Update form and history engines to use it.

services/sync/modules/engines/forms.js

@@ -49,34 +49,21 @@ Cu.import("resource://weave/type_records/forms.js");
 
 let FormWrapper = {
   getAllEntries: function getAllEntries() {
-    let entries = [];
     // Sort by (lastUsed - minLast) / (maxLast - minLast) * timesUsed / maxTimes
     let query = this.createStatement(
-      "SELECT fieldname, value FROM moz_formhistory " +
+      "SELECT fieldname name, value FROM moz_formhistory " +
       "ORDER BY 1.0 * (lastUsed - (SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed ASC LIMIT 1)) / " +
         "((SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed DESC LIMIT 1) - (SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed ASC LIMIT 1)) * " +
         "timesUsed / (SELECT timesUsed FROM moz_formhistory ORDER BY timesUsed DESC LIMIT 1) DESC " +
       "LIMIT 500");
-    while (query.executeStep()) {
-      entries.push({
-        name: query.row.fieldname,
-        value: query.row.value
-      });
-    }
-    return entries;
+    return Utils.queryAsync(query, ["name", "value"]);
   },
 
   getEntry: function getEntry(guid) {
     let query = this.createStatement(
-      "SELECT fieldname, value FROM moz_formhistory WHERE guid = :guid");
+      "SELECT fieldname name, value FROM moz_formhistory WHERE guid = :guid");
     query.params.guid = guid;
-    if (!query.executeStep())
-      return;
-
-    return {
-      name: query.row.fieldname,
-      value: query.row.value
-    };
+    return Utils.queryAsync(query, ["name", "value"])[0];
   },
 
   getGUID: function getGUID(name, value) {
@@ -86,11 +73,11 @@ let FormWrapper = {
       "WHERE fieldname = :name AND value = :value");
     getQuery.params.name = name;
     getQuery.params.value = value;
-    getQuery.executeStep();
 
     // Give the guid if we found one
-    if (getQuery.row.guid != null)
-      return getQuery.row.guid;
+    let item = Utils.queryAsync(getQuery, "guid")[0];
+    if (item != null)
+      return item.guid;
 
     // We need to create a guid for this entry
     let setQuery = this.createStatement(
@@ -100,7 +87,7 @@ let FormWrapper = {
     setQuery.params.guid = guid;
     setQuery.params.name = name;
     setQuery.params.value = value;
-    setQuery.execute();
+    Utils.queryAsync(setQuery);
 
     return guid;
   },
@@ -109,7 +96,7 @@ let FormWrapper = {
     let query = this.createStatement(
       "SELECT 1 FROM moz_formhistory WHERE guid = :guid");
     query.params.guid = guid;
-    return query.executeStep();
+    return Utils.queryAsync(query).length == 1;
   },
 
   replaceGUID: function replaceGUID(oldGUID, newGUID) {
@@ -117,7 +104,7 @@ let FormWrapper = {
       "UPDATE moz_formhistory SET guid = :newGUID WHERE guid = :oldGUID");
     query.params.oldGUID = oldGUID;
     query.params.newGUID = newGUID;
-    query.execute();
+    Utils.queryAsync(query);
   },
 
   createStatement: function createStatement(query) {

services/sync/modules/engines/history.js

@@ -43,7 +43,6 @@ const Cu = Components.utils;
 const GUID_ANNO = "weave/guid";
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://weave/ext/Sync.js");
 Cu.import("resource://weave/util.js");
 Cu.import("resource://weave/engines.js");
 Cu.import("resource://weave/stores.js");
@@ -150,48 +149,13 @@ HistoryStore.prototype = {
   // See bug 320831 for why we use SQL here
   _getVisits: function HistStore__getVisits(uri) {
     this._visitStm.params.url = uri;
-    let [exec, execCb] = Sync.withCb(this._visitStm.executeAsync, this._visitStm);
-    return exec({
-      visits: [],
-      handleResult: function handleResult(results) {
-        let row;
-        while ((row = results.getNextRow()) != null) {
-          this.visits.push({
-            date: row.getResultByName("date"),
-            type: row.getResultByName("type")
-          });
-        }
-      },
-      handleError: function handleError(error) {
-        execCb.throw(error);
-      },
-      handleCompletion: function handleCompletion(reason) {
-        execCb(this.visits);
-      }
-    });
+    return Utils.queryAsync(this._visitStm, ["date", "type"]);
   },
 
   // See bug 468732 for why we use SQL here
   _findURLByGUID: function HistStore__findURLByGUID(guid) {
     this._urlStm.params.guid = guid;
-    let [exec, execCb] = Sync.withCb(this._urlStm.executeAsync, this._urlStm);
-    return exec({
-      handleResult: function(results) {
-        // Save the one result and its columns
-        let row = results.getNextRow();
-        this.urlInfo = {
-          url: row.getResultByName("url"),
-          title: row.getResultByName("title"),
-          frecency: row.getResultByName("frecency"),
-        };
-      },
-      handleError: function(error) {
-        execCb.throw(error);
-      },
-      handleCompletion: function(reason) {
-        execCb(this.urlInfo);
-      }
-    });
+    return Utils.queryAsync(this._urlStm, ["url", "title", "frecency"])[0];
   },
 
   changeItemID: function HStore_changeItemID(oldID, newID) {
@@ -204,23 +168,11 @@ HistoryStore.prototype = {
     this._allUrlStm.params.cutoff_date = (Date.now() - 2592000000) * 1000;
     this._allUrlStm.params.max_results = 5000;
 
-    let [exec, execCb] = Sync.withCb(this._allUrlStm.executeAsync, this._allUrlStm);
-    return exec({
-      ids: {},
-      handleResult: function handleResult(results) {
-        let row;
-        while ((row = results.getNextRow()) != null) {
-          let url = row.getResultByName("url");
-          this.ids[GUIDForUri(url, true)] = url;
-        }
-      },
-      handleError: function handleError(error) {
-        execCb.throw(error);
-      },
-      handleCompletion: function handleCompletion(reason) {
-        execCb(this.ids);
-      }
-    });
+    let urls = Utils.queryAsync(this._allUrlStm, "url");
+    return urls.reduce(function(ids, item) {
+      ids[GUIDForUri(item.url, true)] = item.url;
+      return ids;
+    }, {});
   },
 
   create: function HistStore_create(record) {

services/sync/modules/util.js

@@ -44,6 +44,7 @@ const Cu = Components.utils;
 Cu.import("resource://weave/ext/Preferences.js");
 Cu.import("resource://weave/ext/Observers.js");
 Cu.import("resource://weave/ext/StringBundle.js");
+Cu.import("resource://weave/ext/Sync.js");
 Cu.import("resource://weave/constants.js");
 Cu.import("resource://weave/log4moz.js");
 
@@ -146,6 +147,33 @@ let Utils = {
     };
   },
 
+  queryAsync: function(query, names) {
+    // Allow array of names, single name, and no name
+    if (!Utils.isArray(names))
+      names = names == null ? [] : [names];
+
+    // Synchronously asyncExecute fetching all results by name
+    let [exec, execCb] = Sync.withCb(query.executeAsync, query);
+    return exec({
+      items: [],
+      handleResult: function handleResult(results) {
+        let row;
+        while ((row = results.getNextRow()) != null) {
+          this.items.push(names.reduce(function(item, name) {
+            item[name] = row.getResultByName(name);
+            return item;
+          }, {}));
+        }
+      },
+      handleError: function handleError(error) {
+        execCb.throw(error);
+      },
+      handleCompletion: function handleCompletion(reason) {
+        execCb(this.items);
+      }
+    });
+  },
+
   // Generates a brand-new globally unique identifier (GUID).
   makeGUID: function makeGUID() {
     // 70 characters that are not-escaped URL-friendly

services/sync/tests/unit/test_utils_queryAsync.js

@@ -0,0 +1,69 @@
+_("Make sure queryAsync will synchronously fetch rows for a query asyncly");
+Cu.import("resource://weave/util.js");
+
+function run_test() {
+  _("Using the form service to test queries");
+  function c(query) Svc.Form.DBConnection.createStatement(query);
+
+  _("Make sure the call is async and allows other events to process");
+  let isAsync = false;
+  setTimeout(function() isAsync = true, 0);
+  do_check_false(isAsync);
+
+  _("Empty out the formhistory table");
+  let r0 = Utils.queryAsync(c("DELETE FROM moz_formhistory"));
+  do_check_true(isAsync);
+  do_check_eq(r0.length, 0);
+
+  _("Make sure there's nothing there");
+  let r1 = Utils.queryAsync(c("SELECT 1 FROM moz_formhistory"));
+  do_check_eq(r1.length, 0);
+
+  _("Insert a row");
+  let r2 = Utils.queryAsync(c("INSERT INTO moz_formhistory (fieldname, value) VALUES ('foo', 'bar')"));
+  do_check_eq(r2.length, 0);
+
+  _("Request a known value for the one row");
+  let r3 = Utils.queryAsync(c("SELECT 42 num FROM moz_formhistory"), "num");
+  do_check_eq(r3.length, 1);
+  do_check_eq(r3[0].num, 42);
+
+  _("Get multiple columns");
+  let r4 = Utils.queryAsync(c("SELECT fieldname, value FROM moz_formhistory"), ["fieldname", "value"]);
+  do_check_eq(r4.length, 1);
+  do_check_eq(r4[0].fieldname, "foo");
+  do_check_eq(r4[0].value, "bar");
+
+  _("Get multiple columns with a different order");
+  let r5 = Utils.queryAsync(c("SELECT fieldname, value FROM moz_formhistory"), ["value", "fieldname"]);
+  do_check_eq(r5.length, 1);
+  do_check_eq(r5[0].fieldname, "foo");
+  do_check_eq(r5[0].value, "bar");
+
+  _("Add multiple entries (sqlite doesn't support multiple VALUES)");
+  let r6 = Utils.queryAsync(c("INSERT INTO moz_formhistory (fieldname, value) SELECT 'foo', 'baz' UNION SELECT 'more', 'values'"));
+  do_check_eq(r6.length, 0);
+
+  _("Get multiple rows");
+  let r7 = Utils.queryAsync(c("SELECT fieldname, value FROM moz_formhistory WHERE fieldname = 'foo'"), ["fieldname", "value"]);
+  do_check_eq(r7.length, 2);
+  do_check_eq(r7[0].fieldname, "foo");
+  do_check_eq(r7[1].fieldname, "foo");
+
+  _("Make sure updates work");
+  let r8 = Utils.queryAsync(c("UPDATE moz_formhistory SET value = 'updated' WHERE fieldname = 'more'"));
+  do_check_eq(r8.length, 0);
+
+  _("Get the updated");
+  let r9 = Utils.queryAsync(c("SELECT value, fieldname FROM moz_formhistory WHERE fieldname = 'more'"), ["fieldname", "value"]);
+  do_check_eq(r9.length, 1);
+  do_check_eq(r9[0].fieldname, "more");
+  do_check_eq(r9[0].value, "updated");
+
+  _("Grabbing fewer fields than queried is fine");
+  let r10 = Utils.queryAsync(c("SELECT value, fieldname FROM moz_formhistory"), "fieldname");
+  do_check_eq(r10.length, 3);
+
+  _("Cleaning up");
+  Utils.queryAsync(c("DELETE FROM moz_formhistory"));
+}