Commit b55a7dc2 authored by Lyza Danger Gardner's avatar Lyza Danger Gardner Committed by Lyza Gardner

Make `SearchClient` take a `maxResults` option

Add a pressure valve to keep from loading too many annotations in groups
that have a large number of annotations.
parent f3da63d9
...@@ -21,16 +21,28 @@ export default class SearchClient extends TinyEmitter { ...@@ -21,16 +21,28 @@ export default class SearchClient extends TinyEmitter {
* replies. * replies.
* @param {boolean} [options.incremental] - Emit `results` events incrementally * @param {boolean} [options.incremental] - Emit `results` events incrementally
* as batches of annotations are available * as batches of annotations are available
* @param {number|null} [options.maxResults] - Safety valve for protection when
* loading all annotations in a group in the NotebookView. If the Notebook
* is opened while focused on a group that contains many thousands of
* annotations, it could cause rendering and network misery in the browser.
* When present, do not load annotations if the result set size exceeds
* this value.
*/ */
constructor( constructor(
searchFn, searchFn,
{ chunkSize = 200, separateReplies = true, incremental = true } = {} {
chunkSize = 200,
separateReplies = true,
incremental = true,
maxResults = null,
} = {}
) { ) {
super(); super();
this._searchFn = searchFn; this._searchFn = searchFn;
this._chunkSize = chunkSize; this._chunkSize = chunkSize;
this._separateReplies = separateReplies; this._separateReplies = separateReplies;
this._incremental = incremental; this._incremental = incremental;
this._maxResults = maxResults;
this._canceled = false; this._canceled = false;
/** @type {Annotation[]} */ /** @type {Annotation[]} */
...@@ -56,6 +68,27 @@ export default class SearchClient extends TinyEmitter { ...@@ -56,6 +68,27 @@ export default class SearchClient extends TinyEmitter {
return; return;
} }
// For now, abort loading of annotations if `maxResults` is set and the
// number of annotations in the results set exceeds that value.
//
// NB: We can’t currently, reliably load a subset of a group’s
// annotations, as replies are mixed in with top-level annotations—when
// `separateReplies` is false, which it is in most or all cases—so we’d
// end up with partially-loaded threads.
//
// This change has no effect on loading annotations in the SidebarView,
// where the `maxResults` option is not used.
//
// TODO: Implement pagination
if (self._maxResults && results.total > self._maxResults) {
self.emit(
'error',
new Error('Results size exceeds maximum allowed annotations')
);
self.emit('end');
return;
}
const chunk = results.rows.concat(results.replies || []); const chunk = results.rows.concat(results.replies || []);
if (self._incremental) { if (self._incremental) {
self.emit('results', chunk); self.emit('results', chunk);
......
...@@ -2,6 +2,14 @@ ...@@ -2,6 +2,14 @@
* A service for fetching annotations, filtered by document URIs and group. * A service for fetching annotations, filtered by document URIs and group.
*/ */
/**
* @typedef LoadAnnotationOptions
* @prop {string} groupId
* @prop {string[]} [uris]
* @prop {number} [maxResults] - If number of annotations in search results
* exceeds this value, do not load annotations (see: `SearchClient`)
*/
import SearchClient from '../search-client'; import SearchClient from '../search-client';
import { isReply } from '../util/annotation-metadata'; import { isReply } from '../util/annotation-metadata';
...@@ -16,14 +24,12 @@ export default function loadAnnotationsService( ...@@ -16,14 +24,12 @@ export default function loadAnnotationsService(
let searchClient = null; let searchClient = null;
/** /**
* Load all annotations that match `options` criteria * Load annotations
* *
* @param {Object} options * @param {LoadAnnotationOptions} options
* @param {string} options.groupId
* @param {string[]} [options.uris]
*/ */
function load(options) { function load(options) {
const { uris } = options; const { groupId, uris } = options;
store.removeAnnotations(store.savedAnnotations()); store.removeAnnotations(store.savedAnnotations());
// Cancel previously running search client. // Cancel previously running search client.
...@@ -35,21 +41,14 @@ export default function loadAnnotationsService( ...@@ -35,21 +41,14 @@ export default function loadAnnotationsService(
streamFilter.resetFilter().addClause('/uri', 'one_of', uris); streamFilter.resetFilter().addClause('/uri', 'one_of', uris);
streamer.setConfig('filter', { filter: streamFilter.getFilter() }); streamer.setConfig('filter', { filter: streamFilter.getFilter() });
} }
searchAndLoad(options);
}
/** const searchOptions = {
* @param {Object} options
* @param {string[]} [options.uris]
* @param {string} options.groupId
*/
function searchAndLoad(options) {
const { groupId, uris } = options;
searchClient = new SearchClient(api.search, {
incremental: true, incremental: true,
maxResults: options.maxResults ?? null,
separateReplies: false, separateReplies: false,
}); };
searchClient = new SearchClient(api.search, searchOptions);
searchClient.on('results', results => { searchClient.on('results', results => {
if (results.length) { if (results.length) {
......
...@@ -122,4 +122,34 @@ describe('SearchClient', () => { ...@@ -122,4 +122,34 @@ describe('SearchClient', () => {
assert.calledWith(onError, err); assert.calledWith(onError, err);
}); });
}); });
context('`maxResults` option present', () => {
it('emits error if results size exceeds `maxResults`', () => {
const client = new SearchClient(fakeSearchFn, { maxResults: 2 });
const onError = sinon.stub();
client.on('error', onError);
client.get({ uri: 'http://example.com' });
return awaitEvent(client, 'end').then(() => {
assert.calledOnce(onError);
assert.equal(
onError.getCall(0).args[0].message,
'Results size exceeds maximum allowed annotations'
);
});
});
it('does not emit an error if results size is <= `maxResults`', () => {
const client = new SearchClient(fakeSearchFn, { maxResults: 20 });
const onError = sinon.stub();
client.on('error', onError);
client.get({ uri: 'http://example.com' });
return awaitEvent(client, 'end').then(() => {
assert.notCalled(onError);
});
});
});
}); });
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment