Use $q rather than native Promise in `store`

Testing code that mixes native promises with $q promises (eg. as
returned by $http methods) is a PITA in tests because in that
environment:

 1. Native promises behave as usual - resolving on the next tick

 2. $q promises do not resolve until a digest is triggered explicitly.

As a result, causing a promise pipeline involving both types of promises
to fully execute is tricky.

Sticking to `$q` promises for the moment means that
`$httpBackend.flush()` will synchronously flush any resolved promises
and HTTP requests.

In future we can replace this crap with plain fetch() as part of
extracting the Hypothesis API client as a separate entity from the
client.

src/sidebar/store.js

@@ -78,15 +78,19 @@ function serializeParams(params) {
  * Creates a function that will make an API call to a named route.
  *
  * @param $http - The Angular HTTP service
+ * @param $q - The Angular Promises ($q) service.
  * @param links - Object or promise for an object mapping named API routes to
  *                URL templates and methods
  * @param route - The dotted path of the named API route (eg. `annotation.create`)
  * @param {Function} tokenGetter - Function which returns a Promise for an
  *                   access token for the API.
  */
-function createAPICall($http, links, route, tokenGetter) {
+function createAPICall($http, $q, links, route, tokenGetter) {
   return function (params, data) {
-    return Promise.all([links, tokenGetter()]).then(function (linksAndToken) {
+    // `$q.all` is used here rather than `Promise.all` because testing code that
+    // mixes native Promises with the `$q` promises returned by `$http`
+    // functions gets awkward in tests.
+    return $q.all([links, tokenGetter()]).then(function (linksAndToken) {
       var links = linksAndToken[0];
       var token = linksAndToken[1];
 
@@ -120,22 +124,24 @@ function createAPICall($http, links, route, tokenGetter) {
  * the Hypothesis API (see http://h.readthedocs.io/en/latest/api/).
  */
 // @ngInject
-function store($http, auth, settings) {
+function store($http, $q, auth, settings) {
   var links = retryUtil.retryPromiseOperation(function () {
     return $http.get(settings.apiUrl);
   }).then(function (response) {
     return response.data.links;
   });
 
-  var tokenGetter = auth.tokenGetter;
+  function apiCall(route) {
+    return createAPICall($http, $q, links, route, auth.tokenGetter);
+  }
 
   return {
-    search: createAPICall($http, links, 'search', tokenGetter),
+    search: apiCall('search'),
     annotation: {
-      create: createAPICall($http, links, 'annotation.create', tokenGetter),
-      delete: createAPICall($http, links, 'annotation.delete', tokenGetter),
-      get: createAPICall($http, links, 'annotation.read', tokenGetter),
-      update: createAPICall($http, links, 'annotation.update', tokenGetter),
+      create: apiCall('annotation.create'),
+      delete: apiCall('annotation.delete'),
+      get: apiCall('annotation.read'),
+      update: apiCall('annotation.update'),
     },
   };
 }

src/sidebar/test/store-test.js

@@ -25,41 +25,20 @@ describe('store', function () {
   beforeEach(function () {
     sandbox = sinon.sandbox.create();
 
-    var fakeTokenGetter = function () {
-      return Promise.resolve('faketoken');
-    };
+    var fakeAuth = {};
 
     angular.mock.module('h', {
-      auth: {
-        tokenGetter: fakeTokenGetter,
-      },
+      auth: fakeAuth,
       settings: {apiUrl: 'http://example.com/api'},
     });
-  });
 
-  /**
-   * Wrapper around $httpBackend.flush() which allows for methods under test
-   * needing to fetch data using Promise-returning functions before they
-   * actually make an HTTP request.
-   *
-   * @param {number} [maxTicks] - Maximum number of event loop turns to wait.
-   */
-  function flushHttpRequests(maxTicks) {
-    try {
-      $httpBackend.flush();
-      return null;
-    } catch (err) {
-      if (maxTicks === 0) {
-        throw new Error('No HTTP request was made');
-      }
-      // No HTTP request was made yet. Try again on the next tick of the event
-      // loop.
-      maxTicks = maxTicks || 5;
-      return Promise.resolve().then(function () {
-        flushHttpRequests(maxTicks - 1);
-      });
-    }
-  }
+    angular.mock.inject(function (_$q_) {
+      var $q = _$q_;
+      fakeAuth.tokenGetter = function () {
+        return $q.resolve('faketoken');
+      };
+    });
+  });
 
   afterEach(function () {
     $httpBackend.verifyNoOutstandingExpectation();
@@ -97,70 +76,69 @@ describe('store', function () {
     $httpBackend.flush();
   }));
 
-  it('saves a new annotation', function () {
-    var done = store.annotation.create({}, {}).then(function (saved) {
+  it('saves a new annotation', function (done) {
+    store.annotation.create({}, {}).then(function (saved) {
       assert.isNotNull(saved.id);
+      done();
     });
 
     $httpBackend.expectPOST('http://example.com/api/annotations')
       .respond(function () {
         return [201, {id: 'new-id'}, {}];
       });
-    flushHttpRequests();
-
-    return done;
+    $httpBackend.flush();
   });
 
-  it('updates an annotation', function () {
-    var done = store.annotation.update({id: 'an-id'}, {text: 'updated'});
+  it('updates an annotation', function (done) {
+    store.annotation.update({id: 'an-id'}, {text: 'updated'}).then(function () {
+      done();
+    });
 
     $httpBackend.expectPUT('http://example.com/api/annotations/an-id')
       .respond(function () {
         return [200, {}, {}];
       });
-    flushHttpRequests();
-
-    return done;
+    $httpBackend.flush();
   });
 
-  it('deletes an annotation', function () {
-    var done = store.annotation.delete({id: 'an-id'}, {});
+  it('deletes an annotation', function (done) {
+    store.annotation.delete({id: 'an-id'}, {}).then(function () {
+      done();
+    });
 
     $httpBackend.expectDELETE('http://example.com/api/annotations/an-id')
       .respond(function () {
         return [200, {}, {}];
       });
-    flushHttpRequests();
-
-    return done;
+    $httpBackend.flush();
   });
 
-  it('removes internal properties before sending data to the server', function () {
+  it('removes internal properties before sending data to the server', function (done) {
     var annotation = {
       $highlight: true,
       $notme: 'nooooo!',
       allowed: 123,
     };
-    var done = store.annotation.create({}, annotation);
+    store.annotation.create({}, annotation).then(function () {
+      done();
+    });
 
     $httpBackend.expectPOST('http://example.com/api/annotations', {
       allowed: 123,
     })
       .respond(function () { return [200, {id: 'test'}, {}]; });
-    flushHttpRequests();
-
-    return done;
+    $httpBackend.flush();
   });
 
   // Our backend service interprets semicolons as query param delimiters, so we
   // must ensure to encode them in the query string.
-  it('encodes semicolons in query parameters', function () {
-    var done = store.search({'uri': 'http://example.com/?foo=bar;baz=qux'});
+  it('encodes semicolons in query parameters', function (done) {
+    store.search({'uri': 'http://example.com/?foo=bar;baz=qux'}).then(function () {
+      done();
+    });
 
     $httpBackend.expectGET('http://example.com/api/search?uri=http%3A%2F%2Fexample.com%2F%3Ffoo%3Dbar%3Bbaz%3Dqux')
       .respond(function () { return [200, {}, {}]; });
-    flushHttpRequests();
-
-    return done;
+    $httpBackend.flush();
   });
 });