Factor out pagination through search results into a separate module

Extract the logic for paging through search results from
the API into a separate module.

h/static/scripts/search-client.js

+'use strict';
+
+var EventEmitter = require('tiny-emitter');
+var inherits = require('inherits');
+
+/**
+ * Client for the Hypothesis search API.
+ *
+ * SearchClient handles paging through results, canceling search etc.
+ *
+ * @param {Object} resource - ngResource class instance for the /search API
+ * @param {Object} opts - Search options
+ * @constructor
+ */
+function SearchClient(resource, opts) {
+  opts = opts || {};
+
+  var DEFAULT_CHUNK_SIZE = 200;
+  this._resource = resource;
+  this._chunkSize = opts.chunkSize || DEFAULT_CHUNK_SIZE;
+  if (typeof opts.incremental !== 'undefined') {
+    this._incremental = opts.incremental;
+  } else {
+    this._incremental = true;
+  }
+  this._canceled = false;
+}
+inherits(SearchClient, EventEmitter);
+
+SearchClient.prototype._getBatch = function (query, offset) {
+  var searchQuery = Object.assign({
+    limit: this._chunkSize,
+    offset: offset,
+    sort: 'created',
+    order: 'asc',
+    _separate_replies: true,
+  }, query);
+
+  var self = this;
+  this._resource.get(searchQuery).$promise.then(function (results) {
+    if (self._canceled) {
+      return;
+    }
+
+    var chunk = results.rows.concat(results.replies || []);
+    if (self._incremental) {
+      self.emit('results', chunk);
+    } else {
+      self._results = self._results.concat(chunk);
+    }
+
+    var nextOffset = offset + results.rows.length;
+    if (results.total > nextOffset) {
+      self._getBatch(query, nextOffset);
+    } else {
+      if (!self._incremental) {
+        self.emit('results', self._results);
+      }
+      self.emit('end');
+    }
+  }).catch(function (err) {
+    if (self._canceled) {
+      return;
+    }
+    self.emit('error', err);
+  }).then(function () {
+    if (self._canceled) {
+      return;
+    }
+    self.emit('end');
+  });
+};
+
+/**
+ * Perform a search against the Hypothesis API.
+ *
+ * Emits a 'results' event with an array of annotations as they become
+ * available (in incremental mode) or when all annotations are available
+ * (in non-incremental mode).
+ *
+ * Emits an 'error' event if the search fails.
+ * Emits an 'end' event once the search completes.
+ */
+SearchClient.prototype.get = function (query) {
+  this._results = [];
+  this._getBatch(query, 0);
+};
+
+/**
+ * Cancel the current search and emit the 'end' event.
+ * No further events will be emitted after this.
+ */
+SearchClient.prototype.cancel = function () {
+  this._canceled = true;
+  this.emit('end');
+};
+
+module.exports = SearchClient;

h/static/scripts/test/search-client-test.js

+'use strict';
+
+var SearchClient = require('../search-client');
+
+function await(emitter, event) {
+  return new Promise(function (resolve) {
+    emitter.on(event, resolve);
+  });
+}
+
+describe('SearchClient', function () {
+  var RESULTS = [
+    {id: 'one'},
+    {id: 'two'},
+    {id: 'three'},
+    {id: 'four'},
+  ];
+
+  var fakeResource;
+
+  beforeEach(function () {
+    fakeResource = {
+      get: sinon.spy(function (params) {
+        return {
+          $promise: Promise.resolve({
+            rows: RESULTS.slice(params.offset,
+              params.offset + params.limit),
+            total: RESULTS.length,
+          }),
+        };
+      }),
+    };
+  });
+
+  it('emits "results"', function () {
+    var client = new SearchClient(fakeResource);
+    var onResults = sinon.stub();
+    client.on('results', onResults);
+    client.get({uri: 'http://example.com'});
+    return await(client, 'end').then(function () {
+      assert.calledWith(onResults, RESULTS);
+    });
+  });
+
+  it('emits "results" with chunks in incremental mode', function () {
+    var client = new SearchClient(fakeResource, {chunkSize: 2});
+    var onResults = sinon.stub();
+    client.on('results', onResults);
+    client.get({uri: 'http://example.com'});
+    return await(client, 'end').then(function () {
+      assert.calledWith(onResults, RESULTS.slice(0,2));
+      assert.calledWith(onResults, RESULTS.slice(2,4));
+    });
+  });
+
+  it('emits "results" once in non-incremental mode', function () {
+    var client = new SearchClient(fakeResource,
+      {chunkSize: 2, incremental: false});
+    var onResults = sinon.stub();
+    client.on('results', onResults);
+    client.get({uri: 'http://example.com'});
+    return await(client, 'end').then(function () {
+      assert.calledOnce(onResults);
+      assert.calledWith(onResults, RESULTS);
+    });
+  });
+
+  it('does not emit "results" if canceled', function () {
+    var client = new SearchClient(fakeResource);
+    var onResults = sinon.stub();
+    var onEnd = sinon.stub();
+    client.on('results', onResults);
+    client.on('end', onEnd);
+    client.get({uri: 'http://example.com'});
+    client.cancel();
+    return Promise.resolve().then(function () {
+      assert.notCalled(onResults);
+      assert.called(onEnd);
+    });
+  });
+
+  it('emits "error" event if search fails', function () {
+    var err = new Error('search failed');
+    fakeResource.get = function () {
+      return {
+        $promise: Promise.reject(err),
+      };
+    };
+    var client = new SearchClient(fakeResource);
+    var onError = sinon.stub();
+    client.on('error', onError);
+    client.get({uri: 'http://example.com'});
+    return await(client, 'end').then(function () {
+      assert.calledWith(onError, err);
+    });
+  });
+});

h/static/scripts/test/widget-controller-test.js

 'use strict';
 
 var angular = require('angular');
+var inherits = require('inherits');
+var proxyquire = require('proxyquire');
+var EventEmitter = require('tiny-emitter');
 
 var events = require('../events');
 
+function noCallThru(stub) {
+  return Object.assign(stub, {'@noCallThru':true});
+}
+
+var searchClients;
+function FakeSearchClient(resource) {
+  assert.ok(resource);
+  searchClients.push(this);
+  this.cancel = sinon.stub();
+
+  this.get = function (query) {
+    assert.ok(query.uri);
+    this.emit('results', [{id: query.uri + '123', group: '__world__'}]);
+    this.emit('results', [{id: query.uri + '456', group: 'private-group'}]);
+    this.emit('end');
+  };
+}
+inherits(FakeSearchClient, EventEmitter);
+
 describe('WidgetController', function () {
   var $scope = null;
   var $rootScope = null;
   var fakeAnnotationMapper = null;
   var fakeAnnotationUI = null;
-  var fakeAuth = null;
   var fakeCrossFrame = null;
   var fakeDrafts = null;
   var fakeStore = null;
@@ -22,12 +43,16 @@ describe('WidgetController', function () {
 
   before(function () {
     angular.module('h', [])
-      .controller('WidgetController', require('../widget-controller'));
+      .controller('WidgetController', proxyquire('../widget-controller', {
+        angular: noCallThru(angular),
+        './search-client': noCallThru(FakeSearchClient),
+      }));
   });
 
   beforeEach(angular.mock.module('h'));
 
   beforeEach(angular.mock.module(function ($provide) {
+    searchClients = [];
     sandbox = sinon.sandbox.create();
 
     fakeAnnotationMapper = {
@@ -36,45 +61,17 @@ describe('WidgetController', function () {
     };
 
     fakeAnnotationUI = {
-      tool: 'comment',
-      clearSelectedAnnotations: sandbox.spy()
+      clearSelectedAnnotations: sandbox.spy(),
+      selectedAnnotationMap: {},
+      hasSelectedAnnotations: function () {
+        return Object.keys(this.selectedAnnotationMap).length > 0;
+      },
     };
-    fakeAuth = {user: null};
     fakeCrossFrame = {frames: []};
     fakeDrafts = {
       unsaved: sandbox.stub()
     };
 
-    fakeStore = {
-      SearchResource: {
-        get: function (query, callback) {
-          var offset = query.offset || 0;
-          var limit = query.limit || 20;
-          var result =
-            {
-              total: 100,
-              rows: ((function () {
-                var result1 = [];
-                var end = offset + limit - 1;
-                var i = offset;
-                if (offset <= end) {
-                  while (i <= end) {
-                    result1.push(i++);
-                  }
-                } else {
-                  while (i >= end) {
-                    result1.push(i--);
-                  }
-                }
-                return result1;
-              })()),
-              replies: []
-            };
-          return callback(result);
-        }
-      },
-    };
-
     fakeStreamer = {
       setConfig: sandbox.spy()
     };
@@ -87,11 +84,19 @@ describe('WidgetController', function () {
 
     fakeThreading = {
       root: {},
-      thread: sandbox.stub()
+      thread: sandbox.stub(),
+      annotationList: function () {
+        return [{id: '123'}];
+      },
     };
 
     fakeGroups = {
-      focused: function () { return {id: 'foo'}; }
+      focused: function () { return {id: 'foo'}; },
+      focus: sinon.stub(),
+    };
+
+    fakeStore = {
+      SearchResource: {},
     };
 
     $provide.value('annotationMapper', fakeAnnotationMapper);
@@ -103,7 +108,6 @@ describe('WidgetController', function () {
     $provide.value('streamFilter', fakeStreamFilter);
     $provide.value('threading', fakeThreading);
     $provide.value('groups', fakeGroups);
-    return;
   }));
 
   beforeEach(angular.mock.inject(function ($controller, _$rootScope_) {
@@ -118,60 +122,40 @@ describe('WidgetController', function () {
 
   describe('loadAnnotations', function () {
     it('loads all annotations for a frame', function () {
-      $scope.chunkSize = 20;
-      fakeCrossFrame.frames.push({uri: 'http://example.com'});
+      var uri = 'http://example.com';
+      fakeCrossFrame.frames.push({uri: uri});
       $scope.$digest();
       var loadSpy = fakeAnnotationMapper.loadAnnotations;
-      assert.callCount(loadSpy, 5);
-      assert.calledWith(loadSpy, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19]);
-      assert.calledWith(loadSpy, [20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39]);
-      assert.calledWith(loadSpy, [40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59]);
-      assert.calledWith(loadSpy, [60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79]);
-      assert.calledWith(loadSpy, [80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99]);
+      assert.calledWith(loadSpy, [sinon.match({id: uri + '123'})]);
+      assert.calledWith(loadSpy, [sinon.match({id: uri + '456'})]);
     });
 
-    it('passes _separate_replies: true to the search API', function () {
-      fakeStore.SearchResource.get = sandbox.stub();
-      fakeCrossFrame.frames.push({uri: 'http://example.com'});
-
-      $scope.$digest();
-
-      assert.equal(
-        fakeStore.SearchResource.get.firstCall.args[0]._separate_replies, true);
+    it('loads all annotations for all frames', function () {
+      var uris = ['http://example.com', 'http://foobar.com'];
+      fakeCrossFrame.frames = uris.map(function (uri) {
+        return {uri: uri};
       });
-
-    return it('passes annotations and replies from search to loadAnnotations()', function () {
-      fakeStore.SearchResource.get = function (query, callback) {
-        return callback({
-          rows: ['annotation_1', 'annotation_2'],
-          replies: ['reply_1', 'reply_2', 'reply_3']
-        });
-      };
-      fakeCrossFrame.frames.push({uri: 'http://example.com'});
       $scope.$digest();
-
-      assert(fakeAnnotationMapper.loadAnnotations.calledOnce);
-      assert(fakeAnnotationMapper.loadAnnotations.calledWith(
-        ['annotation_1', 'annotation_2'], ['reply_1', 'reply_2', 'reply_3']
-      ));
+      var loadSpy = fakeAnnotationMapper.loadAnnotations;
+      assert.calledWith(loadSpy, [sinon.match({id: uris[0] + '123'})]);
+      assert.calledWith(loadSpy, [sinon.match({id: uris[0] + '456'})]);
+      assert.calledWith(loadSpy, [sinon.match({id: uris[1] + '123'})]);
+      assert.calledWith(loadSpy, [sinon.match({id: uris[1] + '456'})]);
     });
   });
 
   describe('when the focused group changes', function () {
-    return it('should load annotations for the new group', function () {
-      fakeThreading.annotationList = sandbox.stub().returns([{id: '1'}]);
-      fakeCrossFrame.frames.push({uri: 'http://example.com'});
-      var searchResult = {total: 10, rows: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10], replies: []};
-      fakeStore.SearchResource.get = function (query, callback) {
-        return callback(searchResult);
-      };
+    it('should load annotations for the new group', function () {
+      var uri = 'http://example.com';
+      fakeCrossFrame.frames.push({uri: uri});
+      var loadSpy = fakeAnnotationMapper.loadAnnotations;
 
       $scope.$broadcast(events.GROUP_FOCUSED);
-
-      assert.calledWith(fakeAnnotationMapper.unloadAnnotations, [{id: '1'}]);
-      $scope.$digest();
-      assert.calledWith(fakeAnnotationMapper.loadAnnotations, searchResult.rows);
+      assert.calledWith(fakeAnnotationMapper.unloadAnnotations, [{id: '123'}]);
       assert.calledWith(fakeThreading.thread, fakeDrafts.unsaved());
+      $scope.$digest();
+      assert.calledWith(loadSpy, [sinon.match({id: uri + '123'})]);
+      assert.calledWith(loadSpy, [sinon.match({id: uri + '456'})]);
     });
   });
 

h/static/scripts/widget-controller.js

@@ -3,6 +3,7 @@
 var angular = require('angular');
 
 var events = require('./events');
+var SearchClient = require('./search-client');
 
 // @ngInject
 module.exports = function WidgetController(
@@ -12,9 +13,6 @@ module.exports = function WidgetController(
   $scope.threadRoot = threading.root;
   $scope.sortOptions = ['Newest', 'Oldest', 'Location'];
 
-  var DEFAULT_CHUNK_SIZE = 200;
-  var loaded = [];
-
   var _resetAnnotations = function () {
     // Unload all the annotations
     annotationMapper.unloadAnnotations(threading.annotationList());
@@ -22,51 +20,59 @@ module.exports = function WidgetController(
     threading.thread(drafts.unsaved());
   };
 
-  var _loadAnnotationsFrom = function (query, offset) {
-    var queryCore = {
-      limit: $scope.chunkSize || DEFAULT_CHUNK_SIZE,
-      offset: offset,
-      sort: 'created',
-      order: 'asc',
-      group: groups.focused().id
-    };
-    var q = angular.extend(queryCore, query);
-    q._separate_replies = true;
-
-    store.SearchResource.get(q, function (results) {
-      var total = results.total;
-      offset += results.rows.length;
-      if (offset < total) {
-        _loadAnnotationsFrom(query, offset);
-      }
+  var searchClients = [];
 
-      annotationMapper.loadAnnotations(results.rows, results.replies);
+  function _loadAnnotationsFor(uri, group) {
+    var searchClient = new SearchClient(store.SearchResource);
+    searchClients.push(searchClient);
+    searchClient.on('results', function (results) {
+      annotationMapper.loadAnnotations(results);
     });
-  };
+    searchClient.on('end', function () {
+      searchClients.splice(searchClients.indexOf(searchClient), 1);
+    });
+    searchClient.get({
+      uri: uri,
+      group: group,
+    });
+  }
 
+  /**
+   * Load annotations for all URLs associated with @p frames.
+   *
+   * @param {Array<{uri:string}>} frames - Hypothesis client frames
+   *        to load annotations for.
+   */
   var loadAnnotations = function (frames) {
-    for (var i = 0, f; i < frames.length; i++) {
-      f = frames[i];
-      var ref;
-      if (ref = f.uri, loaded.indexOf(ref) >= 0) {
-        continue;
+    searchClients.forEach(function (client) {
+      client.cancel();
+    });
+
+    var urls = frames.reduce(function (urls, frame) {
+      if (urls.indexOf(frame.uri) !== -1) {
+        return urls;
+      } else {
+        return urls.concat(frame.uri);
       }
-      loaded.push(f.uri);
-      _loadAnnotationsFrom({uri: f.uri}, 0);
+    }, []);
+
+    for (var i=0; i < urls.length; i++) {
+      _loadAnnotationsFor(urls[i], groups.focused().id);
     }
 
-    if (loaded.length > 0) {
-      streamFilter.resetFilter().addClause('/uri', 'one_of', loaded);
+    if (urls.length > 0) {
+      streamFilter.resetFilter().addClause('/uri', 'one_of', urls);
       streamer.setConfig('filter', {filter: streamFilter.getFilter()});
     }
   };
 
   $scope.$on(events.GROUP_FOCUSED, function () {
+    annotationUI.clearSelectedAnnotations();
     _resetAnnotations(annotationMapper, drafts, threading);
-    loaded = [];
     return loadAnnotations(crossframe.frames);
   });
 
+  // Watch anything that may require us to reload annotations.
   $scope.$watchCollection(function () {
     return crossframe.frames;
   }, loadAnnotations);