Load annotations using the annotations service

- Load annotations using the new annotations service.
- Use the new store isFetchingAnnotations rather than the previous
isLoading method to determine whether an annotation fetch is currently
in progress.

src/sidebar/components/sidebar-content.js

 'use strict';
 
-const SearchClient = require('../search-client');
 const events = require('../events');
 const isThirdPartyService = require('../util/is-third-party-service');
 const tabs = require('../tabs');
 
-/**
- * Returns the group ID of the first annotation in `results` whose
- * ID is `annId`.
- */
-function getGroupID(annId, results) {
-  const annot = results.find(function(annot) {
-    return annot.id === annId;
-  });
-  if (!annot) {
-    return null;
-  }
-  return annot.group;
-}
-
 // @ngInject
 function SidebarContentController(
   $scope,
   analytics,
+  annotations,
   store,
-  annotationMapper,
-  api,
-  features,
   frameSync,
-  groups,
   rootThread,
   settings,
-  streamer,
-  streamFilter
+  streamer
 ) {
   const self = this;
 
@@ -90,61 +71,7 @@ function SidebarContentController(
     }
   }
 
-  const searchClients = [];
-
-  function _resetAnnotations() {
-    annotationMapper.unloadAnnotations(store.savedAnnotations());
-  }
-
-  function _loadAnnotationsFor(uris, group) {
-    const searchClient = new SearchClient(api.search, {
-      // If no group is specified, we are fetching annotations from
-      // all groups in order to find out which group contains the selected
-      // annotation, therefore we need to load all chunks before processing
-      // the results
-      incremental: !!group,
-    });
-    searchClients.push(searchClient);
-    searchClient.on('results', function(results) {
-      if (store.hasSelectedAnnotations()) {
-        // Focus the group containing the selected annotation and filter
-        // annotations to those from this group
-        let groupID = getGroupID(store.getFirstSelectedAnnotationId(), results);
-        if (!groupID) {
-          // If the selected annotation is not available, fall back to
-          // loading annotations for the currently focused group
-          groupID = groups.focused().id;
-        }
-        results = results.filter(function(result) {
-          return result.group === groupID;
-        });
-        groups.focus(groupID);
-      }
-
-      if (results.length) {
-        annotationMapper.loadAnnotations(results);
-      }
-    });
-    searchClient.on('end', function() {
-      // Remove client from list of active search clients.
-      //
-      // $evalAsync is required here because search results are emitted
-      // asynchronously. A better solution would be that the loading state is
-      // tracked as part of the app state.
-      $scope.$evalAsync(function() {
-        searchClients.splice(searchClients.indexOf(searchClient), 1);
-      });
-
-      store.frames().forEach(function(frame) {
-        if (0 <= uris.indexOf(frame.uri)) {
-          store.updateFrameAnnotationFetchStatus(frame.uri, true);
-        }
-      });
-    });
-    searchClient.get({ uri: uris, group: group });
-  }
-
-  this.isLoading = function() {
+  this.isLoading = () => {
     if (
       !store.frames().some(function(frame) {
         return frame.uri;
@@ -153,47 +80,9 @@ function SidebarContentController(
       // The document's URL isn't known so the document must still be loading.
       return true;
     }
-
-    if (searchClients.length > 0) {
-      // We're still waiting for annotation search results from the API.
-      return true;
-    }
-
-    return false;
+    return store.isFetchingAnnotations();
   };
 
-  /**
-   * Load annotations for all URLs associated with `frames`.
-   */
-  function loadAnnotations() {
-    _resetAnnotations();
-
-    searchClients.forEach(function(client) {
-      client.cancel();
-    });
-
-    // If there is no selection, load annotations only for the focused group.
-    //
-    // If there is a selection, we load annotations for all groups, find out
-    // which group the first selected annotation is in and then filter the
-    // results on the client by that group.
-    //
-    // In the common case where the total number of annotations on
-    // a page that are visible to the user is not greater than
-    // the batch size, this saves an extra roundtrip to the server
-    // to fetch the selected annotation in order to determine which group
-    // it is in before fetching the remaining annotations.
-    const group = store.hasSelectedAnnotations() ? null : groups.focused().id;
-
-    const searchUris = store.searchUris();
-    if (searchUris.length > 0) {
-      _loadAnnotationsFor(searchUris, group);
-
-      streamFilter.resetFilter().addClause('/uri', 'one_of', searchUris);
-      streamer.setConfig('filter', { filter: streamFilter.getFilter() });
-    }
-  }
-
   $scope.$on('sidebarOpened', function() {
     analytics.track(analytics.events.SIDEBAR_OPENED);
 
@@ -247,17 +136,11 @@ function SidebarContentController(
       }
 
       if (!prevGroupId || currentGroupId !== prevGroupId) {
-        // The focused group may be changed during loading annotations as a result
-        // of switching to the group containing a direct-linked annotation.
-        //
-        // In that case, we don't want to trigger reloading annotations again.
-        if (this.isLoading()) {
-          return;
-        }
         store.clearSelectedAnnotations();
       }
 
-      loadAnnotations();
+      const searchUris = store.searchUris();
+      annotations.load(searchUris, currentGroupId);
     },
     true
   );
@@ -281,7 +164,7 @@ function SidebarContentController(
   this.scrollTo = scrollToAnnotation;
 
   this.selectedGroupUnavailable = function() {
-    return !this.isLoading() && store.getState().directLinkedGroupFetchFailed;
+    return store.getState().directLinkedGroupFetchFailed;
   };
 
   this.selectedAnnotationUnavailable = function() {
@@ -314,7 +197,9 @@ function SidebarContentController(
     // selection is available to the user, show the CTA.
     const selectedID = store.getFirstSelectedAnnotationId();
     return (
-      !this.isLoading() && !!selectedID && store.annotationExists(selectedID)
+      !store.isFetchingAnnotations() &&
+      !!selectedID &&
+      store.annotationExists(selectedID)
     );
   };
 }

src/sidebar/components/test/sidebar-content-test.js

@@ -5,32 +5,6 @@ const EventEmitter = require('tiny-emitter');
 
 const events = require('../../events');
 const sidebarContent = require('../sidebar-content');
-const util = require('../../directive/test/util');
-
-let searchClients;
-
-class FakeSearchClient extends EventEmitter {
-  constructor(searchFn, opts) {
-    super();
-
-    assert.ok(searchFn);
-    searchClients.push(this);
-    this.cancel = sinon.stub();
-    this.incremental = !!opts.incremental;
-
-    this.get = sinon.spy(function(query) {
-      assert.ok(query.uri);
-
-      for (let i = 0; i < query.uri.length; i++) {
-        const uri = query.uri[i];
-        this.emit('results', [{ id: uri + '123', group: '__world__' }]);
-        this.emit('results', [{ id: uri + '456', group: 'private-group' }]);
-      }
-
-      this.emit('end');
-    });
-  }
-}
 
 class FakeRootThread extends EventEmitter {
   constructor() {
@@ -47,16 +21,11 @@ describe('sidebar.components.sidebar-content', function() {
   let store;
   let ctrl;
   let fakeAnalytics;
-  let fakeAnnotationMapper;
-  let fakeDrafts;
-  let fakeFeatures;
+  let fakeAnnotations;
   let fakeFrameSync;
-  let fakeGroups;
   let fakeRootThread;
   let fakeSettings;
-  let fakeApi;
   let fakeStreamer;
-  let fakeStreamFilter;
   let sandbox;
 
   before(function() {
@@ -70,7 +39,6 @@ describe('sidebar.components.sidebar-content', function() {
 
   beforeEach(() => {
     angular.mock.module(function($provide) {
-      searchClients = [];
       sandbox = sinon.sandbox.create();
 
       fakeAnalytics = {
@@ -78,65 +46,32 @@ describe('sidebar.components.sidebar-content', function() {
         events: {},
       };
 
-      fakeAnnotationMapper = {
-        loadAnnotations: sandbox.stub(),
-        unloadAnnotations: sandbox.stub(),
-      };
-
       fakeFrameSync = {
         focusAnnotations: sinon.stub(),
         scrollToAnnotation: sinon.stub(),
       };
 
-      fakeDrafts = {
-        unsaved: sandbox.stub().returns([]),
-      };
-
-      fakeFeatures = {
-        flagEnabled: sandbox.stub().returns(true),
-      };
-
       fakeStreamer = {
         setConfig: sandbox.stub(),
         connect: sandbox.stub(),
         reconnect: sandbox.stub(),
       };
 
-      fakeStreamFilter = {
-        resetFilter: sandbox.stub().returnsThis(),
-        addClause: sandbox.stub().returnsThis(),
-        getFilter: sandbox.stub().returns({}),
-      };
-
-      fakeGroups = {
-        focused: sinon.stub().returns({ id: 'foo' }),
-        focus: sinon.stub(),
+      fakeAnnotations = {
+        load: sinon.stub(),
       };
 
       fakeRootThread = new FakeRootThread();
 
       fakeSettings = {};
 
-      fakeApi = {
-        search: sinon.stub(),
-      };
-
       $provide.value('analytics', fakeAnalytics);
-      $provide.value('annotationMapper', fakeAnnotationMapper);
-      $provide.value('api', fakeApi);
-      $provide.value('drafts', fakeDrafts);
-      $provide.value('features', fakeFeatures);
       $provide.value('frameSync', fakeFrameSync);
       $provide.value('rootThread', fakeRootThread);
       $provide.value('streamer', fakeStreamer);
-      $provide.value('streamFilter', fakeStreamFilter);
-      $provide.value('groups', fakeGroups);
+      $provide.value('annotations', fakeAnnotations);
       $provide.value('settings', fakeSettings);
     });
-
-    sidebarContent.$imports.$mock({
-      '../search-client': FakeSearchClient,
-    });
   });
 
   afterEach(() => {
@@ -149,23 +84,11 @@ describe('sidebar.components.sidebar-content', function() {
     });
   }
 
-  function createSidebarContent(
-    { userid } = { userid: 'acct:person@example.com' }
-  ) {
-    return util.createDirective(document, 'sidebarContent', {
-      auth: {
-        status: userid ? 'logged-in' : 'logged-out',
-        userid: userid,
-      },
-      search: sinon.stub().returns({ query: sinon.stub() }),
-      onLogin: sinon.stub(),
-    });
-  }
-
   const makeSidebarContentController = () => {
     angular.mock.inject(function($componentController, _store_, _$rootScope_) {
       $rootScope = _$rootScope_;
       $scope = $rootScope.$new();
+
       store = _store_;
       store.updateFrameAnnotationFetchStatus = sinon.stub();
       store.clearGroups();
@@ -190,6 +113,23 @@ describe('sidebar.components.sidebar-content', function() {
     return sandbox.restore();
   });
 
+  describe('isLoading', () => {
+    it("returns true if the document's url isn't known", () => {
+      assert.isTrue(ctrl.isLoading());
+    });
+
+    it('returns true if annotations are still being fetched', () => {
+      setFrames([{ uri: 'http://www.example.com' }]);
+      store.annotationFetchStarted('tag:foo');
+      assert.isTrue(ctrl.isLoading());
+    });
+
+    it('returns false if annotations have been fetched', () => {
+      setFrames([{ uri: 'http://www.example.com' }]);
+      assert.isFalse(ctrl.isLoading());
+    });
+  });
+
   describe('showSelectedTabs', () => {
     beforeEach(() => {
       setFrames([{ uri: 'http://www.example.com' }]);
@@ -218,211 +158,10 @@ describe('sidebar.components.sidebar-content', function() {
     });
   });
 
-  describe('#loadAnnotations', function() {
-    it('unloads any existing annotations', function() {
-      // When new clients connect, all existing annotations should be unloaded
-      // before reloading annotations for each currently-connected client
-      store.addAnnotations([{ id: '123' }]);
-      const uri1 = 'http://example.com/page-a';
-      let frames = [{ uri: uri1 }];
-      setFrames(frames);
-      $scope.$digest();
-      fakeAnnotationMapper.unloadAnnotations = sandbox.spy();
-      const uri2 = 'http://example.com/page-b';
-      frames = frames.concat({ uri: uri2 });
-      setFrames(frames);
-      $scope.$digest();
-      assert.calledWith(
-        fakeAnnotationMapper.unloadAnnotations,
-        store.getState().annotations
-      );
-    });
-
-    it('loads all annotations for a frame', function() {
-      const uri = 'http://example.com';
-      setFrames([{ uri: uri }]);
-      $scope.$digest();
-      const loadSpy = fakeAnnotationMapper.loadAnnotations;
-      assert.calledWith(loadSpy, [sinon.match({ id: uri + '123' })]);
-      assert.calledWith(loadSpy, [sinon.match({ id: uri + '456' })]);
-    });
-
-    it('loads all annotations for a frame with multiple urls', function() {
-      const uri = 'http://example.com/test.pdf';
-      const fingerprint = 'urn:x-pdf:fingerprint';
-      setFrames([
-        {
-          uri: uri,
-          metadata: {
-            documentFingerprint: 'fingerprint',
-            link: [
-              {
-                href: fingerprint,
-              },
-              {
-                href: uri,
-              },
-            ],
-          },
-        },
-      ]);
-      $scope.$digest();
-      const loadSpy = fakeAnnotationMapper.loadAnnotations;
-
-      assert.calledWith(loadSpy, [sinon.match({ id: uri + '123' })]);
-      assert.calledWith(loadSpy, [sinon.match({ id: fingerprint + '123' })]);
-      assert.calledWith(loadSpy, [sinon.match({ id: uri + '456' })]);
-      assert.calledWith(loadSpy, [sinon.match({ id: fingerprint + '456' })]);
-    });
-
-    it('loads all annotations for all frames', function() {
-      const uris = ['http://example.com', 'http://foobar.com'];
-      setFrames(
-        uris.map(function(uri) {
-          return { uri: uri };
-        })
-      );
-      $scope.$digest();
-      const 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' })]);
-    });
-
-    it('updates annotation fetch status for all frames', function() {
-      const frameUris = ['http://example.com', 'http://foobar.com'];
-      setFrames(
-        frameUris.map(function(frameUri) {
-          return { uri: frameUri };
-        })
-      );
-      $scope.$digest();
-      const updateSpy = store.updateFrameAnnotationFetchStatus;
-      assert.isTrue(updateSpy.calledWith(frameUris[0], true));
-      assert.isTrue(updateSpy.calledWith(frameUris[1], true));
-    });
-
-    context('when there is a direct-linked group error', () => {
-      beforeEach(() => {
-        setFrames([{ uri: 'http://www.example.com' }]);
-        fakeSettings.group = 'group-id';
-        store.setDirectLinkedGroupFetchFailed();
-        $scope.$digest();
-      });
-
-      [null, 'acct:person@example.com'].forEach(userid => {
-        it('displays same group error message regardless of login state', () => {
-          const element = createSidebarContent({ userid });
-
-          const sidebarContentError = element.find('.sidebar-content-error');
-          const errorMessage = sidebarContentError.attr(
-            'logged-in-error-message'
-          );
-
-          assert.equal(errorMessage, "'This group is not available.'");
-        });
-      });
-
-      it('selectedGroupUnavailable returns true', () => {
-        assert.isTrue(ctrl.selectedGroupUnavailable());
-      });
-    });
-
-    context('when there is a direct-linked group selection', () => {
-      beforeEach(() => {
-        setFrames([{ uri: 'http://www.example.com' }]);
-        fakeSettings.group = 'group-id';
-        store.loadGroups([{ id: fakeSettings.group }]);
-        store.focusGroup(fakeSettings.group);
-        fakeGroups.focused.returns({ id: fakeSettings.group });
-        $scope.$digest();
-      });
-
-      it('selectedGroupUnavailable returns false', () => {
-        assert.isFalse(ctrl.selectedGroupUnavailable());
-      });
-
-      it('fetches annotations for the direct-linked group', () => {
-        assert.calledWith(searchClients[0].get, {
-          uri: ['http://www.example.com'],
-          group: 'group-id',
-        });
-      });
-    });
-
-    context('when there is a direct-linked annotation selection', function() {
-      const uri = 'http://example.com';
-      const id = uri + '123';
-
-      beforeEach(function() {
-        setFrames([{ uri: uri }]);
-        store.selectAnnotations([id]);
-        $scope.$digest();
-      });
-
-      it("switches to the selected annotation's group", function() {
-        assert.calledWith(fakeGroups.focus, '__world__');
-        assert.calledOnce(fakeAnnotationMapper.loadAnnotations);
-        assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
-          { id: uri + '123', group: '__world__' },
-        ]);
-      });
-
-      it('fetches annotations for all groups', function() {
-        assert.calledWith(searchClients[0].get, { uri: [uri], group: null });
-      });
-
-      it('loads annotations in one batch', function() {
-        assert.notOk(searchClients[0].incremental);
-      });
-    });
-
-    context('when there is no selection', function() {
-      const uri = 'http://example.com';
-
-      beforeEach(function() {
-        setFrames([{ uri: uri }]);
-        fakeGroups.focused.returns({ id: 'a-group' });
-        $scope.$digest();
-      });
-
-      it('fetches annotations for the current group', function() {
-        assert.calledWith(searchClients[0].get, {
-          uri: [uri],
-          group: 'a-group',
-        });
-      });
-
-      it('loads annotations in batches', function() {
-        assert.ok(searchClients[0].incremental);
-      });
-    });
-
-    context('when the selected annotation is not available', function() {
-      const uri = 'http://example.com';
-      const id = uri + 'does-not-exist';
-
-      beforeEach(function() {
-        setFrames([{ uri: uri }]);
-        store.selectAnnotations([id]);
-        fakeGroups.focused.returns({ id: 'private-group' });
-        $scope.$digest();
-      });
-
-      it('loads annotations from the focused group instead', function() {
-        assert.calledWith(fakeGroups.focus, 'private-group');
-        assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
-          { group: 'private-group', id: 'http://example.com456' },
-        ]);
-      });
-    });
-  });
-
   function connectFrameAndPerformInitialFetch() {
     setFrames([{ uri: 'https://a-page.com' }]);
     $scope.$digest();
-    fakeAnnotationMapper.loadAnnotations.reset();
+    fakeAnnotations.load.reset();
   }
 
   context('when the search URIs of connected frames change', () => {
@@ -433,7 +172,11 @@ describe('sidebar.components.sidebar-content', function() {
 
       $scope.$digest();
 
-      assert.called(fakeAnnotationMapper.loadAnnotations);
+      assert.calledWith(
+        fakeAnnotations.load,
+        ['https://a-page.com', 'https://new-frame.com'],
+        'group-id'
+      );
     });
   });
 
@@ -448,7 +191,11 @@ describe('sidebar.components.sidebar-content', function() {
       store.updateSession(newProfile);
       $scope.$digest();
 
-      assert.called(fakeAnnotationMapper.loadAnnotations);
+      assert.calledWith(
+        fakeAnnotations.load,
+        ['https://a-page.com'],
+        'group-id'
+      );
     });
 
     it('does not reload annotations if the user ID is the same', () => {
@@ -461,7 +208,7 @@ describe('sidebar.components.sidebar-content', function() {
       store.updateSession(newProfile);
       $scope.$digest();
 
-      assert.notCalled(fakeAnnotationMapper.loadAnnotations);
+      assert.notCalled(fakeAnnotations.load);
     });
   });
 
@@ -490,24 +237,22 @@ describe('sidebar.components.sidebar-content', function() {
       // annotations loaded.
       store.addAnnotations([{ id: '123' }]);
       store.addAnnotations = sinon.stub();
-      fakeDrafts.unsaved.returns([{ id: uri + '123' }, { id: uri + '456' }]);
       setFrames([{ uri: uri }]);
       $scope.$digest();
+      fakeAnnotations.load = sinon.stub();
     });
 
     it('should load annotations for the new group', () => {
-      const loadSpy = fakeAnnotationMapper.loadAnnotations;
       store.loadGroups([{ id: 'different-group' }]);
       store.focusGroup('different-group');
 
       $scope.$digest();
 
-      assert.calledWith(fakeAnnotationMapper.unloadAnnotations, [
-        sinon.match({ id: '123' }),
-      ]);
-      $scope.$digest();
-      assert.calledWith(loadSpy, [sinon.match({ id: uri + '123' })]);
-      assert.calledWith(loadSpy, [sinon.match({ id: uri + '456' })]);
+      assert.calledWith(
+        fakeAnnotations.load,
+        ['http://example.com'],
+        'different-group'
+      );
     });
 
     it('should clear the selection', () => {
@@ -565,10 +310,9 @@ describe('sidebar.components.sidebar-content', function() {
     });
 
     it("doesn't show a message if the document isn't loaded yet", function() {
-      // No search requests have been sent yet.
-      searchClients = [];
       // There is a selection but the selected annotation isn't available.
       store.selectAnnotations(['missing']);
+      store.annotationFetchStarted();
       $scope.$digest();
 
       assert.isFalse(ctrl.selectedAnnotationUnavailable());