692347 - Helpers for asynchronous queries

Status: RESOLVED WONTFIX

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

I noticed some code duplication between my favicons sync work and our repositories/async rewrite.

Here's a little chunk of code to make asynchronous querying easier.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Part 1: Add AsyncQueryCallback, Async.queryForRows helper. v1

Comment 2

[Richard Newman [:rnewman]]

Attachment: Part 2: Tests for Async.queryForRows. v1

Tests are fairly sparse, because of Part 3…

Comment 3

[Richard Newman [:rnewman]]

Attachment: Part 3: Rewrite querySpinningly in terms of queryForRows. v1

Comment 4

[Philipp von Weitershausen [:philikon]]

Comment on attachment 565111 [details] [diff] [review]
Part 1: Add AsyncQueryCallback, Async.queryForRows helper. v1

>+/**
>+ * Create a mozIStorageStatementCallback object for use in asynchronous
>+ * queries.
>+ *
>+ * Usage:
>+ *
>+ *   function getGUID(row) {
>+ *     return row.guid;
>+ *   }
>+ *   let stmt = helper.getStatement("SELECT guid ...");
>+ *   let cb = new QueryCallback(stmt, getGUID, function (err, guids) {
>+ *     _("Got GUIDs! " + JSON.stringify(guids[0]));
>+ *   };
>+ *   cb.run();

Summarizing IRL discussion:

This is a bit more generalization than we need for the async stuff, at the expense of lots of function calls and potential inner closures. What do we need for the async APIs? We need:

(a) a way to get an array of just one column's values (the GUIDs), e.g.:

  guidsSince: function guidsSince(timestamp, guidsCallback) {
    queryForRows(this.guidsStmt, "guid", guidsCallback);
  }

(b) a way to get rows and create objects from them, passed to a fetchCallback. This one has a particular signature, expecting the DONE sentinel value at handleCompletion. Your transform function would work well here:

  fetchSince: function fetchSince(timestamp, fetchCallback) {
    let stmt = this.recordsSinceStmt;
    stmt.timestamp = timestamp;
    new FetchQueryCallback(stmt, this.dbRowToRecord, fetchCallback);
  },

  dbRowToRecord: function dbRowToRecord(row) {
    return {id:    row.getResultByName("guids"),
            title: row.getResultByName("title"),
            ttl:   GENERIC_COLLECTION_TTL,
            etc.};
  },

As for the favicon use case, it seems you only ever need 1 result anyway, which means you can optimize the mozIStorageCallback implementation for that. Yes, this would mean we would have a bunch of different mozIStorageCallback implementations in our code, but they're all serving a slight different purpose.

Comment 5

[Richard Newman [:rnewman]]

Will work in `repositories`, and handle the favicons case separately.