Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - Fetch annotations for each frame separately #4193

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/sidebar/components/NotebookView.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ function NotebookView({ loadAnnotationsService, streamer }) {
store.setSortKey('Newest');
if (groupId) {
loadAnnotationsService.load({
frameId: null,
groupId,
// Load annotations in reverse-chronological order because that is how
// threads are sorted in the notebook view. By aligning the fetch
Expand Down
17 changes: 10 additions & 7 deletions src/sidebar/components/SidebarView.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function SidebarView({
? tabForAnnotation(linkedAnnotation)
: 'annotation';

const searchUris = store.searchUris();
const searchURIMap = store.searchURIMap();
const sidebarHasOpened = store.hasSidebarOpened();
const userId = store.profile().userid;

Expand Down Expand Up @@ -105,13 +105,16 @@ function SidebarView({
}
prevGroupId.current = focusedGroupId;
}
if (focusedGroupId && searchUris.length) {
loadAnnotationsService.load({
groupId: focusedGroupId,
uris: searchUris,
});
if (focusedGroupId && searchURIMap.size) {
for (let [frameId, uris] of searchURIMap) {
loadAnnotationsService.load({
frameId,
groupId: focusedGroupId,
uris,
});
}
}
}, [store, loadAnnotationsService, focusedGroupId, userId, searchUris]);
}, [store, loadAnnotationsService, focusedGroupId, userId, searchURIMap]);

// When a `linkedAnnotationAnchorTag` becomes available, scroll to it
// and focus it
Expand Down
5 changes: 4 additions & 1 deletion src/sidebar/components/StreamView.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ function StreamView({ api, toastMessenger }) {
try {
store.annotationFetchStarted();
const results = await api.search(queryParams);
store.addAnnotations([...results.rows, ...results.replies]);
store.addAnnotations(null /* frameId */, [
...results.rows,
...results.replies,
]);
} finally {
store.annotationFetchFinished();
}
Expand Down
20 changes: 12 additions & 8 deletions src/sidebar/services/annotations.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ export class AnnotationsService {
/**
* Extend new annotation objects with defaults and permissions.
*
* @param {string|null} frameId
* @param {Omit<AnnotationData, '$tag'>} annotationData
* @param {Date} now
* @return {Annotation}
*/
_initialize(annotationData, now = new Date()) {
_initialize(frameId, annotationData, now = new Date()) {
const defaultPrivacy = this._store.getDefault('annotationPrivacy');
const groupid = this._store.focusedGroupId();
const profile = this._store.profile();
Expand All @@ -76,6 +77,9 @@ export class AnnotationsService {
/** @type {Annotation} */
const annotation = Object.assign(
{
$frameId: frameId,
$tag,

created: now.toISOString(),
group: groupid,
permissions: defaultPermissions(userid, groupid, defaultPrivacy),
Expand All @@ -84,7 +88,6 @@ export class AnnotationsService {
updated: now.toISOString(),
user: userid,
user_info: userInfo,
$tag,
hidden: false,
links: {},
document: { title: '' },
Expand All @@ -104,13 +107,14 @@ export class AnnotationsService {
* Create a draft for it unless it's a highlight and clear other empty
* drafts out of the way.
*
* @param {string|null} frameId
* @param {Omit<AnnotationData, '$tag'>} annotationData
* @param {Date} now
*/
create(annotationData, now = new Date()) {
const annotation = this._initialize(annotationData, now);
create(frameId, annotationData, now = new Date()) {
const annotation = this._initialize(frameId, annotationData, now);

this._store.addAnnotations([annotation]);
this._store.addAnnotations(frameId, [annotation]);

// Remove other drafts that are in the way, and their annotations (if new)
this._store.deleteNewAndEmptyDrafts();
Expand Down Expand Up @@ -159,7 +163,7 @@ export class AnnotationsService {
target: [],
uri: topLevelFrame.uri,
};
this.create(pageNoteAnnotation);
this.create(topLevelFrame.id, pageNoteAnnotation);
}

/**
Expand Down Expand Up @@ -196,7 +200,7 @@ export class AnnotationsService {
target: [{ source: annotation.target[0].source }],
uri: annotation.uri,
};
this.create(replyAnnotation);
this.create(annotation.$frameId, replyAnnotation);
}

/**
Expand Down Expand Up @@ -238,7 +242,7 @@ export class AnnotationsService {
this._store.removeDraft(annotation);

// Add (or, in effect, update) the annotation to the store's collection
this._store.addAnnotations([savedAnnotation]);
this._store.addAnnotations(annotation.$frameId, [savedAnnotation]);
return savedAnnotation;
}
}
2 changes: 1 addition & 1 deletion src/sidebar/services/frame-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ export class FrameSyncService {
this._hostRPC.call('showHighlights');

// Create the new annotation in the sidebar.
this._annotationsService.create(annot);
this._annotationsService.create(frameIdentifier, annot);
}
);

Expand Down
4 changes: 2 additions & 2 deletions src/sidebar/services/groups.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,8 @@ export class GroupsService {
.filter(ann => !isReply(ann))
.map(ann => ({ ...ann, group: newGroupId }));

if (updatedAnnotations.length) {
this._store.addAnnotations(updatedAnnotations);
for (let ann of updatedAnnotations) {
this._store.addAnnotations(ann.$frameId, [ann]);
}

// Persist this group as the last focused group default
Expand Down
39 changes: 25 additions & 14 deletions src/sidebar/services/load-annotations.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { SearchClient } from '../search-client';

/**
* @typedef LoadAnnotationOptions
* @prop {string|null} frameId
* @prop {string} groupId
* @prop {string[]} [uris]
* @prop {number} [maxResults] - If number of annotations in search results
Expand Down Expand Up @@ -47,8 +48,8 @@ export class LoadAnnotationsService {
this._streamer = streamer;
this._streamFilter = streamFilter;

/** @type {SearchClient|null} */
this._searchClient = null;
/** @type {Map<string|null, SearchClient>} */
this._searchClients = new Map();
}

/**
Expand All @@ -60,6 +61,7 @@ export class LoadAnnotationsService {
* @param {LoadAnnotationOptions} options
*/
load({
frameId,
groupId,
uris,
onError,
Expand All @@ -68,18 +70,24 @@ export class LoadAnnotationsService {
sortOrder,
streamFilterBy = 'uri',
}) {
this._store.removeAnnotations(this._store.savedAnnotations());
const currentAnnotations = this._store
.savedAnnotations()
.filter(a => a.$frameId === frameId);
this._store.removeAnnotations(currentAnnotations);

// Cancel previously running search client.
//
// This will emit the "end" event for the existing client and trigger cleanup
// associated with that client (eg. resetting the count of in-flight
// annotation fetches).
if (this._searchClient) {
this._searchClient.cancel();
const prevSearchClient = this._searchClients.get(frameId);
if (prevSearchClient) {
prevSearchClient.cancel();
}

// Set the filter for the websocket stream
//
// TODO: How will multiple frames be handled here? Multiple WebSocket clients?
switch (streamFilterBy) {
case 'group':
this._streamFilter
Expand Down Expand Up @@ -120,33 +128,36 @@ export class LoadAnnotationsService {
sortOrder,
};

this._searchClient = new SearchClient(this._api.search, searchOptions);
const searchClient = new SearchClient(this._api.search, searchOptions);
this._searchClients.set(frameId, searchClient);

this._searchClient.on('resultCount', resultCount => {
searchClient.on('resultCount', resultCount => {
// TODO - Pass frame ID here.
this._store.setAnnotationResultCount(resultCount);
});

this._searchClient.on('results', results => {
searchClient.on('results', results => {
if (results.length) {
this._store.addAnnotations(results);
this._store.addAnnotations(frameId, results);
}
});

this._searchClient.on('error', error => {
searchClient.on('error', error => {
if (typeof onError === 'function') {
onError(error);
} else {
console.error(error);
}
});

this._searchClient.on('end', () => {
searchClient.on('end', () => {
// Remove client as it's no longer active.
this._searchClient = null;
this._searchClients.delete(frameId);

if (uris && uris.length > 0) {
this._store.frames().forEach(frame => {
if (uris.indexOf(frame.uri) >= 0) {
// TODO - Replace `frame.uri` with frame ID here?
this._store.updateFrameAnnotationFetchStatus(frame.uri, true);
}
});
Expand All @@ -155,7 +166,7 @@ export class LoadAnnotationsService {
});

this._store.annotationFetchStarted();
this._searchClient.get({ group: groupId, uri: uris });
searchClient.get({ group: groupId, uri: uris });
}

/**
Expand Down Expand Up @@ -192,7 +203,7 @@ export class LoadAnnotationsService {
}
const threadAnnotations = [annotation, ...replySearchResult.rows];

this._store.addAnnotations(threadAnnotations);
this._store.addAnnotations(null /* frameId */, threadAnnotations);

// If we've been successful in retrieving a thread, with a top-level annotation,
// configure the connection to the real-time update service to send us
Expand Down
4 changes: 4 additions & 0 deletions src/sidebar/services/streamer.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export class StreamerService {
applyPendingUpdates() {
const updates = Object.values(this._store.pendingUpdates());
if (updates.length) {
// TODO - Handle multiple frames
this._store.addAnnotations(updates);
}

Expand Down Expand Up @@ -169,6 +170,9 @@ export class StreamerService {
* @param {object} configMessage
*/
setConfig(key, configMessage) {
// TODO - Figure out how to handle multiple frames. Do we create a separate
// connection for each frame or make changes on the backend so we can figure
// out which annotation goes with which frame?
this._configMessages[key] = configMessage;
if (this._socket?.isConnected()) {
this._socket.send(configMessage);
Expand Down
28 changes: 15 additions & 13 deletions src/sidebar/store/modules/annotations.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,31 +70,31 @@ function findByTag(annotations, tag) {
}

/**
* Set custom private fields on an annotation object about to be added to the
* store's collection of `annotations`.
* Initialize the local ("$"-prefixed) fields of an annotation.
*
* `annotation` may either be new (unsaved) or a persisted annotation retrieved
* from the service.
*
* @param {Omit<Annotation, '$anchorTimeout'>} annotation
* @param {Annotation} annotation - New (unsaved) annotation or one retrieved from
* the backend. This annotation may not have all local fields set.
* @param {string} tag - The `$tag` value that should be used for this
* if it doesn't have a `$tag` already
* @return {Annotation} - annotation with local (`$*`) fields set
* @param {string} frameId
* @return {Annotation} - Annotation with all local fields set
*/
function initializeAnnotation(annotation, tag) {
function initializeAnnotation(annotation, tag, frameId) {
let orphan = annotation.$orphan;

if (!annotation.id) {
// New annotations must be anchored
orphan = false;
}

return Object.assign({}, annotation, {
// Flag indicating whether waiting for the annotation to anchor timed out.
return {
...annotation,

$anchorTimeout: false,
$frameId: frameId,
$tag: annotation.$tag || tag,
$orphan: orphan,
});
};
}

const initialState = {
Expand Down Expand Up @@ -155,7 +155,7 @@ const reducers = {
updatedTags[existing.$tag] = true;
}
} else {
added.push(initializeAnnotation(annot, 't' + nextTag));
added.push(initializeAnnotation(annot, 't' + nextTag, action.frameId));
++nextTag;
}
});
Expand Down Expand Up @@ -267,9 +267,10 @@ const actions = util.actionTypes(reducers);
/**
* Add these `annotations` to the current collection of annotations in the store.
*
* @param {string|null} frameId
* @param {Annotation[]} annotations - Array of annotation objects to add.
*/
function addAnnotations(annotations) {
function addAnnotations(frameId, annotations) {
return function (dispatch, getState) {
const added = annotations.filter(annot => {
return (
Expand All @@ -279,6 +280,7 @@ function addAnnotations(annotations) {

dispatch({
type: actions.ADD_ANNOTATIONS,
frameId,
annotations,
currentAnnotationCount: getState().annotations.annotations.length,
});
Expand Down
Loading