Bug 1375223 - Remove Async.querySpinningly. r?kitcambridge draft
authorLuciano Italiani <lyretandrpg@gmail.com>
Sat, 05 Aug 2017 17:50:19 -0300
changeset 641139 4488f78b3bcc854bd43843adc1bd10acf162397a
parent 621399 4cfb674227051e22bab651e5759f3de503a50560
child 724733 83ea505e600c6e0c6d8276f5d848281262828e2e
push id72447
push userbmo:lyret@protonmail.com
push dateSat, 05 Aug 2017 20:50:55 +0000
reviewerskitcambridge
bugs1375223
milestone57.0a1
Bug 1375223 - Remove Async.querySpinningly. r?kitcambridge MozReview-Commit-ID: Fv8X3E3ByO0
services/common/async.js
services/common/tests/unit/test_async_querySpinningly.js
services/sync/tests/unit/test_history_store.js
services/sync/tests/unit/test_places_guid_downgrade.js
services/sync/tests/unit/test_telemetry.js
services/sync/tps/extensions/tps/resource/modules/history.jsm
--- a/services/common/async.js
+++ b/services/common/async.js
@@ -208,25 +208,16 @@ this.Async = {
       // the calling code probably still expects an array as a return value.
       if (this.names && !this.results) {
         this.results = [];
       }
       this.syncCb(this.results);
     }
   },
 
-  querySpinningly: function querySpinningly(query, names) {
-    // 'Synchronously' asyncExecute, fetching all results by name.
-    let storageCallback = Object.create(Async._storageCallbackPrototype);
-    storageCallback.names = names;
-    storageCallback.syncCb = Async.makeSyncCallback();
-    query.executeAsync(storageCallback);
-    return Async.waitForSyncCallback(storageCallback.syncCb);
-  },
-
   promiseSpinningly(promise) {
     let cb = Async.makeSpinningCallback();
     promise.then(result => {
       cb(null, result);
     }, err => {
       cb(err || new Error("Promise rejected without explicit error"));
     });
     return cb.wait();
--- a/services/common/tests/unit/test_async_querySpinningly.js
+++ b/services/common/tests/unit/test_async_querySpinningly.js
@@ -1,121 +1,18 @@
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://gre/modules/FormHistory.jsm");
-Cu.import("resource://services-common/async.js");
-Cu.import("resource://services-common/utils.js");
-
-_("Make sure querySpinningly will synchronously fetch rows for a query asyncly");
-
-const SQLITE_CONSTRAINT_VIOLATION = 19;  // http://www.sqlite.org/c3ref/c_abort.html
-
-// This test is a bit hacky - it was originally written to use the
-// formhistory.sqlite database using the nsIFormHistory2 sync APIs. However,
-// that's now been deprecated in favour of the async FormHistory.jsm.
-// Rather than re-write the test completely, we cheat - we use FormHistory.jsm
-// to initialize the database, then we just re-open it for these tests.
-
-// Init the forms database.
-FormHistory.schemaVersion;
-FormHistory.shutdown();
-
-// and open the database it just created.
-let dbFile = Services.dirsvc.get("ProfD", Ci.nsIFile).clone();
-dbFile.append("formhistory.sqlite");
-let dbConnection = Services.storage.openUnsharedDatabase(dbFile);
+/*
+if i remove this file i get:
 
-do_register_cleanup(() => {
-  let cb = Async.makeSpinningCallback();
-  dbConnection.asyncClose(cb);
-  return cb.wait();
-});
-
-function querySpinningly(query, names) {
-  let q = dbConnection.createStatement(query);
-  let r = Async.querySpinningly(q, names);
-  q.finalize();
-  return r;
-}
-
-function run_test() {
-  initTestLogging("Trace");
-
-  _("Make sure the call is async and allows other events to process");
-  let isAsync = false;
-  CommonUtils.nextTick(function() { isAsync = true; });
-  do_check_false(isAsync);
-
-  _("Empty out the formhistory table");
-  let r0 = querySpinningly("DELETE FROM moz_formhistory");
-  do_check_eq(r0, null);
-
-  _("Make sure there's nothing there");
-  let r1 = querySpinningly("SELECT 1 FROM moz_formhistory");
-  do_check_eq(r1, null);
-
-  _("Insert a row");
-  let r2 = querySpinningly("INSERT INTO moz_formhistory (fieldname, value) VALUES ('foo', 'bar')");
-  do_check_eq(r2, null);
+mozpack.errors.ErrorMessage: Symlink target path does not exist: /home/lucho/src/mozilla-central/services/common/tests/unit/test_async_querySpinningly.js
+Makefile:205: recipe for target 'install-test-files' failed
+make: *** [install-test-files] Error 1
+Error running mach:
 
-  _("Request a known value for the one row");
-  let r3 = querySpinningly("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 = querySpinningly("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");
+    ['xpcshell-test', 'services/sync/']
 
-  _("Get multiple columns with a different order");
-  let r5 = querySpinningly("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 = querySpinningly("INSERT INTO moz_formhistory (fieldname, value) SELECT 'foo', 'baz' UNION SELECT 'more', 'values'");
-  do_check_eq(r6, null);
-
-  _("Get multiple rows");
-  let r7 = querySpinningly("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");
+The error occurred in code that was called by the mach command. This is either
+a bug in the called code itself or in the way that mach is calling it.
 
-  _("Make sure updates work");
-  let r8 = querySpinningly("UPDATE moz_formhistory SET value = 'updated' WHERE fieldname = 'more'");
-  do_check_eq(r8, null);
-
-  _("Get the updated");
-  let r9 = querySpinningly("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 = querySpinningly("SELECT value, fieldname FROM moz_formhistory", ["fieldname"]);
-  do_check_eq(r10.length, 3);
+You should consider filing a bug for this issue.
 
-  _("Generate an execution error");
-  let query = "INSERT INTO moz_formhistory (fieldname, value) VALUES ('one', NULL)";
-  let stmt = dbConnection.createStatement(query);
-  let except;
-  try {
-    Async.querySpinningly(stmt);
-  } catch (e) {
-    except = e;
-  }
-  stmt.finalize()
-  do_check_true(!!except);
-  do_check_eq(except.result, SQLITE_CONSTRAINT_VIOLATION);
-
-  _("Cleaning up");
-  querySpinningly("DELETE FROM moz_formhistory");
-
-  _("Make sure the timeout got to run before this function ends");
-  do_check_true(isAsync);
-}
+If filing a bug, please include the full output of mach, including this error
+message.
+*/
--- a/services/sync/tests/unit/test_history_store.js
+++ b/services/sync/tests/unit/test_history_store.js
@@ -162,35 +162,33 @@ add_task(async function test_null_title(
   do_check_attribute_count((await store.getAllIDs()), 3);
   let queryres = queryHistoryVisits(resuri);
   do_check_eq(queryres.length, 1);
   do_check_eq(queryres[0].time, TIMESTAMP3);
 });
 
 add_task(async function test_invalid_records() {
   _("Make sure we handle invalid URLs in places databases gracefully.");
-  let connection = PlacesUtils.history
-                              .QueryInterface(Ci.nsPIPlacesDatabase)
-                              .DBConnection;
-  let stmt = connection.createAsyncStatement(
+  
+  await PlacesUtils.withConnectionWrapper("test_invalid_record", async function(db) {
+    await db.execute(
     "INSERT INTO moz_places "
-  + "(url, url_hash, title, rev_host, visit_count, last_visit_date) "
-  + "VALUES ('invalid-uri', hash('invalid-uri'), 'Invalid URI', '.', 1, " + TIMESTAMP3 + ")"
-  );
-  Async.querySpinningly(stmt);
-  stmt.finalize();
-  // Add the corresponding visit to retain database coherence.
-  stmt = connection.createAsyncStatement(
-    "INSERT INTO moz_historyvisits "
-  + "(place_id, visit_date, visit_type, session) "
-  + "VALUES ((SELECT id FROM moz_places WHERE url_hash = hash('invalid-uri') AND url = 'invalid-uri'), "
-  + TIMESTAMP3 + ", " + Ci.nsINavHistoryService.TRANSITION_TYPED + ", 1)"
-  );
-  Async.querySpinningly(stmt);
-  stmt.finalize();
+    + "(url, url_hash, title, rev_host, visit_count, last_visit_date) "
+    + "VALUES ('invalid-uri', hash('invalid-uri'), 'Invalid URI', '.', 1, " + TIMESTAMP3 + ")"
+    );
+  });
+  
+  await PlacesUtils.withConnectionWrapper("test_invalid_record", async function(db) {
+    await db.execute(
+      "INSERT INTO moz_historyvisits "
+    + "(place_id, visit_date, visit_type, session) "
+    + "VALUES ((SELECT id FROM moz_places WHERE url_hash = hash('invalid-uri') AND url = 'invalid-uri'), "
+    + TIMESTAMP3 + ", " + Ci.nsINavHistoryService.TRANSITION_TYPED + ", 1)"
+    );
+  });
   do_check_attribute_count((await store.getAllIDs()), 4);
 
   _("Make sure we report records with invalid URIs.");
   let invalid_uri_guid = Utils.makeGUID();
   let failed = await store.applyIncomingBatch([{
     id: invalid_uri_guid,
     histUri: ":::::::::::::::",
     title: "Doesn't have a valid URI",
--- a/services/sync/tests/unit/test_places_guid_downgrade.js
+++ b/services/sync/tests/unit/test_places_guid_downgrade.js
@@ -112,43 +112,41 @@ add_task(async function test_history_gui
 
   async function onVisitAdded() {
     let fxguid = await store.GUIDForUri(fxuri, true);
     let tbguid = await store.GUIDForUri(tburi, true);
     dump("fxguid: " + fxguid + "\n");
     dump("tbguid: " + tbguid + "\n");
 
     _("History: Verify GUIDs are added to the guid column.");
-    let connection = PlacesUtils.history
-                                .QueryInterface(Ci.nsPIPlacesDatabase)
-                                .DBConnection;
-    let stmt = connection.createAsyncStatement(
-      "SELECT id FROM moz_places WHERE guid = :guid");
-
-    stmt.params.guid = fxguid;
-    let result = Async.querySpinningly(stmt, ["id"]);
+    
+    let db = await PlacesUtils.promiseDBConnection();
+    let result = await db.execute(
+      "SELECT id FROM moz_places WHERE guid = :guid",
+      {guid: fxguid}
+    );
     do_check_eq(result.length, 1);
-
-    stmt.params.guid = tbguid;
-    result = Async.querySpinningly(stmt, ["id"]);
+    
+    result = await db.execute(
+      "SELECT id FROM moz_places WHERE guid = :guid",
+      {guid: tbguid}
+    );
     do_check_eq(result.length, 1);
-    stmt.finalize();
-
-    _("History: Verify GUIDs weren't added to annotations.");
-    stmt = connection.createAsyncStatement(
-      "SELECT a.content AS guid FROM moz_annos a WHERE guid = :guid");
-
-    stmt.params.guid = fxguid;
-    result = Async.querySpinningly(stmt, ["guid"]);
+    
+    result = await db.execute(
+      "SELECT a.content AS guid FROM moz_annos a WHERE guid = :guid",
+      {guid: fxguid}
+    );
     do_check_eq(result.length, 0);
-
-    stmt.params.guid = tbguid;
-    result = Async.querySpinningly(stmt, ["guid"]);
+    
+    result = await db.execute(
+      "SELECT a.content AS guid FROM moz_annos a WHERE guid = :guid",
+      {guid: tbguid}
+    );
     do_check_eq(result.length, 0);
-    stmt.finalize();
   }
 
   await new Promise((resolve, reject) => {
     PlacesUtils.asyncHistory.updatePlaces(places, {
       handleError: function handleError() {
         do_throw("Unexpected error in adding visit.");
       },
       handleResult: function handleResult() {},
@@ -171,44 +169,44 @@ add_task(async function test_bookmark_gu
     tburi,
     PlacesUtils.bookmarks.DEFAULT_INDEX,
     "Get Thunderbird!");
 
   let fxguid = await store.GUIDForId(fxid);
   let tbguid = await store.GUIDForId(tbid);
 
   _("Bookmarks: Verify GUIDs are added to the guid column.");
-  let connection = PlacesUtils.history
-                              .QueryInterface(Ci.nsPIPlacesDatabase)
-                              .DBConnection;
-  let stmt = connection.createAsyncStatement(
-    "SELECT id FROM moz_bookmarks WHERE guid = :guid");
-
-  stmt.params.guid = fxguid;
-  let result = Async.querySpinningly(stmt, ["id"]);
+  
+  let db = await PlacesUtils.promiseDBConnection();
+  let result = await db.execute(
+    "SELECT id FROM moz_bookmarks WHERE guid = :guid",
+    {guid: fxguid}
+  );
   do_check_eq(result.length, 1);
-  do_check_eq(result[0].id, fxid);
-
-  stmt.params.guid = tbguid;
-  result = Async.querySpinningly(stmt, ["id"]);
+  do_check_eq(result[0].getResultByName("id"), fxid);
+  
+  result = await db.execute(
+    "SELECT id FROM moz_bookmarks WHERE guid = :guid",
+    {guid: tbguid}
+  );
   do_check_eq(result.length, 1);
-  do_check_eq(result[0].id, tbid);
-  stmt.finalize();
-
+  do_check_eq(result[0].getResultByName("id"), tbid);
+  
   _("Bookmarks: Verify GUIDs weren't added to annotations.");
-  stmt = connection.createAsyncStatement(
-    "SELECT a.content AS guid FROM moz_items_annos a WHERE guid = :guid");
-
-  stmt.params.guid = fxguid;
-  result = Async.querySpinningly(stmt, ["guid"]);
+  
+  result = await db.execute(
+    "SELECT a.content AS guid FROM moz_items_annos a WHERE guid = :guid",
+    {guid: fxguid}
+  );
   do_check_eq(result.length, 0);
-
-  stmt.params.guid = tbguid;
-  result = Async.querySpinningly(stmt, ["guid"]);
+  
+  result = await db.execute(
+    "SELECT a.content AS guid FROM moz_items_annos a WHERE guid = :guid",
+    {guid: tbguid}
+  );
   do_check_eq(result.length, 0);
-  stmt.finalize();
 });
 
 function run_test() {
   setPlacesDatabase("places_v10_from_v11.sqlite");
 
   run_next_test();
 }
--- a/services/sync/tests/unit/test_telemetry.js
+++ b/services/sync/tests/unit/test_telemetry.js
@@ -561,20 +561,23 @@ add_task(async function test_no_foreign_
 add_task(async function test_sql_error() {
   enableValidationPrefs();
 
   await Service.engineManager.register(SteamEngine);
   let engine = Service.engineManager.get("steam");
   engine.enabled = true;
   let server = serverForFoo(engine);
   await SyncTestingInfrastructure(server);
-  engine._sync = function() {
+  engine._sync = async function() {
     // Just grab a DB connection and issue a bogus SQL statement synchronously.
-    let db = PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection;
-    Async.querySpinningly(db.createAsyncStatement("select bar from foo"));
+    /*
+    This not work but i have no clue how to handle.
+    */
+    let db = await PlacesUtils.promiseDBConnection();
+    await db.execute("select bar from foo");
   };
   try {
     _(`test_sql_error: Steam tracker contents: ${
       JSON.stringify(engine._tracker.changedIDs)}`);
     let ping = await sync_and_validate_telem(true);
     let enginePing = ping.engines.find(e => e.name === "steam");
     deepEqual(enginePing.failureReason, { name: "sqlerror", code: 1 });
   } finally {
--- a/services/sync/tps/extensions/tps/resource/modules/history.jsm
+++ b/services/sync/tps/extensions/tps/resource/modules/history.jsm
@@ -40,18 +40,18 @@ var DumpHistory = function TPS_History__
  * Contains methods for manipulating browser history entries.
  */
 var HistoryEntry = {
   /**
    * _db
    *
    * Returns the DBConnection object for the history service.
    */
-  get _db() {
-    return PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection;
+  get _db() {    
+    return PlacesUtils.promiseDBConnection();
   },
 
   /**
    * _visitStm
    *
    * Return the SQL statement for getting history visit information
    * from the moz_historyvisits table.  Borrowed from Weave's
    * history.js.
@@ -74,19 +74,28 @@ var HistoryEntry = {
    *
    * Gets history information about visits to a given uri.
    *
    * @param uri The uri to get visits for
    * @return an array of objects with 'date' and 'type' properties,
    * corresponding to the visits in the history database for the
    * given uri
    */
-  _getVisits: function HistStore__getVisits(uri) {
-    this._visitStm.params.url = uri;
-    return Async.querySpinningly(this._visitStm, ["date", "type"]);
+  _getVisits: async function HistStore__getVisits(uri) {
+    let db = await this._db();
+    return await db.execute(
+      "SELECT visit_type type, visit_date date " +
+      "FROM moz_historyvisits " +
+      "WHERE place_id = (" +
+        "SELECT id " +
+        "FROM moz_places " +
+        "WHERE url_hash = hash(:url) AND url = :url) " +
+      "ORDER BY date DESC LIMIT 20",
+      {url: uri}
+    );
   },
 
   /**
    * Add
    *
    * Adds visits for a uri to the history database.  Throws on error.
    *
    * @param item An object representing one or more visits to a specific uri