Skip to content

Commit

Permalink
[Bug #491] add option to abort AsyncAction
Browse files Browse the repository at this point in the history
  • Loading branch information
lukaskabc committed Jul 29, 2024
1 parent 3ed61db commit 7dfdeb1
Show file tree
Hide file tree
Showing 10 changed files with 186 additions and 125 deletions.
155 changes: 53 additions & 102 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/action/ActionType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
export interface AsyncAction extends Action {
status: AsyncActionStatus;
ignoreLoading?: boolean; // Allows to prevent loading spinner display on async action
// allows to abort the action (needs to be explicitly implemented)
abortController?: AbortController;
}

export interface FailureAction extends Action {
Expand Down
30 changes: 29 additions & 1 deletion src/action/AsyncActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import UserRole, { UserRoleData } from "../model/UserRole";
import { loadTermCount } from "./AsyncVocabularyActions";
import { getApiPrefix } from "./ActionUtils";
import { getShortLocale } from "../util/IntlUtil";
import AsyncActionStatus from "./AsyncActionStatus";

/*
* Asynchronous actions involve requests to the backend server REST API. As per recommendations in the Redux docs, this consists
Expand All @@ -86,8 +87,35 @@ import { getShortLocale } from "../util/IntlUtil";
* TODO Consider splitting this file into multiple, it is becoming too long
*/

/**
* @returns true if there is a pending action that has not been aborted
*/
export function isActionRequestPending(state: TermItState, action: Action) {
return state.pendingActions[action.type] !== undefined;
let isPending = state.pendingActions[action.type] !== undefined;
let isAborted = isActionStatusAborted(state.pendingActions[action.type]);

return isPending && !isAborted;
}

/**
* @returns True if the status is AbortController with aborted signal, false otherwise
*/
export function isActionStatusAborted(
status: AsyncActionStatus | AbortController
) {
return status && status["signal"] && status["signal"].aborted;
}

/**
* Checks if a pending action exists in the TermItState,
* and if it does and the AbortController is available in the state, the pending action is aborted.
*/
export function abortPendingActionRequest(state: TermItState, action: Action) {
const pendingAction = state.pendingActions[action.type];
if (pendingAction && typeof pendingAction["abort"] === "function") {
const controller = pendingAction as AbortController;
controller.abort();
}
}

export function createVocabulary(vocabulary: Vocabulary) {
Expand Down
16 changes: 13 additions & 3 deletions src/action/AsyncAnnotatorActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,25 @@ import JsonLdUtils from "../util/JsonLdUtils";
import Term, { CONTEXT as TERM_CONTEXT, TermData } from "../model/Term";
import { ErrorData } from "../model/ErrorInfo";
import {
abortPendingActionRequest,
isActionRequestPending,
loadTermByIri as loadTermByIriBase,
} from "./AsyncActions";
import TermOccurrence from "../model/TermOccurrence";

export function loadAllTerms(
vocabularyIri: IRI,
includeImported: boolean = false
includeImported: boolean = false,
abortPrevious: boolean = false
) {
const action = {
type: ActionType.ANNOTATOR_LOAD_TERMS,
abortController: new AbortController(),
};
return (dispatch: ThunkDispatch, getState: GetStoreState) => {
if (isActionRequestPending(getState(), action)) {
if (abortPrevious) {
abortPendingActionRequest(getState(), action);
} else if (isActionRequestPending(getState(), action)) {
return Promise.resolve({});
}
dispatch(asyncActionRequest(action, true));
Expand All @@ -38,7 +43,7 @@ export function loadAllTerms(
includeImported,
namespace: vocabularyIri.namespace,
})
)
).signal(action.abortController)
)
.then((data: object[]) =>
data.length !== 0
Expand All @@ -51,6 +56,11 @@ export function loadAllTerms(
.then((data: TermData[]) => {
const terms = {};
data.forEach((d) => (terms[d.iri!] = new Term(d)));

if (action.abortController.signal.aborted) {
return; // if we somehow got here even though the request was aborted, we don't want to propagate the result.
}

return dispatch(asyncActionSuccessWithPayload(action, terms));
})
.catch((error: ErrorData) => dispatch(asyncActionFailure(action, error)));
Expand Down
42 changes: 42 additions & 0 deletions src/action/__tests__/AsyncAnnotatorActions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,48 @@ describe("AsyncAnnotatorActions", () => {
expect(Ajax.get).not.toHaveBeenCalled();
});
});

it("does invoke Ajax when a pending request was aborted", () => {
const controller = new AbortController();
controller.abort();

store.getState().pendingActions[ActionType.ANNOTATOR_LOAD_TERMS] =
controller;
const terms = require("../../rest-mock/terms");
Ajax.get = jest.fn().mockResolvedValue(terms);

expect(controller.signal.aborted).toBeTruthy();
return Promise.resolve(
(store.dispatch as ThunkDispatch)(
loadAllTerms({ fragment: vocabularyName }, true)
)
).then(() => {
expect(Ajax.get).toHaveBeenCalled();
});
});

it("does abort previous pending request when pending request exists", () => {
const controller = new AbortController();

store.getState().pendingActions[ActionType.ANNOTATOR_LOAD_TERMS] =
controller;
const terms = require("../../rest-mock/terms");
Ajax.get = jest.fn().mockResolvedValue(terms);

expect(controller.signal.aborted).toBeFalsy();
return Promise.resolve(
(store.dispatch as ThunkDispatch)(
loadAllTerms({ fragment: vocabularyName }, true, true)
)
).finally(() => {
expect(controller.signal.aborted).toBeTruthy();
expect(Ajax.get).toHaveBeenCalled();
expect(store.getActions().length).toBeGreaterThanOrEqual(2);
expect(Object.values(store.getActions()[1].payload)).toHaveLength(
terms.length
);
});
});
});

describe("loadTermByIri", () => {
Expand Down
33 changes: 19 additions & 14 deletions src/component/file/FileContentDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ interface FileDetailOwnProps extends HasI18n {
saveFileContent: (fileIri: IRI, fileContent: string) => Promise<any>;
clearFileContent: () => void;
loadVocabulary: (vocabularyIri: IRI) => void;
fetchTerms: (vocabularyIri: IRI) => Promise<any>;
fetchTerms: (vocabularyIri: IRI, abortPrevious: boolean) => Promise<any>;
}

type FileDetailProps = FileDetailOwnProps & FileDetailProvidedProps;
Expand All @@ -64,23 +64,15 @@ export class FileContentDetail extends React.Component<
});
};

private initializeTermFetching = (): void => {
trackPromise(this.props.fetchTerms(this.props.vocabularyIri), "annotator");
};

public componentDidMount(): void {
this.loadFileContentData();
this.initializeTermFetching();
this.props.loadVocabulary(this.props.vocabularyIri);
}

public componentDidUpdate(prevProps: FileDetailProps): void {
if (
isDifferent(this.props.iri, prevProps.iri) ||
isDifferent(this.props.vocabularyIri, prevProps.vocabularyIri)
) {
this.loadFileContentData();
this.initializeTermFetching();
// abortPrevious: we received new iri, so we want to cancel any pending action
// and request new data with current iri
this.initializeTermFetching(true);
this.props.loadVocabulary(this.props.vocabularyIri);
}

Expand All @@ -98,6 +90,19 @@ export class FileContentDetail extends React.Component<
}
}

public componentDidMount(): void {
this.loadFileContentData();
this.initializeTermFetching();
this.props.loadVocabulary(this.props.vocabularyIri);
}

private initializeTermFetching = (abortPrevious = false): void => {
trackPromise(
this.props.fetchTerms(this.props.vocabularyIri, abortPrevious),
"annotator"
);
};

public componentWillUnmount(): void {
this.props.clearFileContent();
}
Expand Down Expand Up @@ -158,8 +163,8 @@ export default connect(
clearFileContent: () => dispatch(clearFileContent()),
loadVocabulary: (vocabularyIri: IRI) =>
dispatch(loadVocabulary(vocabularyIri, false)),
fetchTerms: (vocabularyIri: IRI) =>
dispatch(loadAllTerms(vocabularyIri, true)),
fetchTerms: (vocabularyIri: IRI, abortPrevious = false) =>
dispatch(loadAllTerms(vocabularyIri, true, abortPrevious)),
consumeNotification: (notification: AppNotification) =>
dispatch(consumeNotification(notification)),
};
Expand Down
7 changes: 5 additions & 2 deletions src/component/file/__tests__/FileContentDetail.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe("FileDetail", () => {
saveFileContent: (fileIri: IRI, fileContent: string) => Promise<any>;
clearFileContent: () => void;
loadVocabulary: (vocabularyIri: IRI) => void;
fetchTerms: (vocabularyIri: IRI) => Promise<Term[]>;
fetchTerms: (vocabularyIri: IRI, abortPrevious: boolean) => Promise<Term[]>;
};
let mockDataProps: {
defaultTerms: Term[];
Expand Down Expand Up @@ -106,7 +106,10 @@ describe("FileDetail", () => {
/>
);

expect(mockedFunctionLikeProps.fetchTerms).toBeCalledWith(vocabularyIri);
expect(mockedFunctionLikeProps.fetchTerms).toBeCalledWith(
vocabularyIri,
expect.any(Boolean)
);
});

it("resets file content before unmounting", () => {
Expand Down
2 changes: 1 addition & 1 deletion src/model/TermItState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default class TermItState {
// Represents a queue of inter-component notifications
public notifications: AppNotification[];
// Pending asynchronous actions. Can be used to prevent repeated requests when some are already pending
public pendingActions: { [key: string]: AsyncActionStatus };
public pendingActions: { [key: string]: AsyncActionStatus | AbortController };
public errors: ErrorLogItem[];
public lastModified: { [key: string]: string };
public sidebarExpanded: boolean;
Expand Down
8 changes: 6 additions & 2 deletions src/reducer/TermItReducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ function notifications(
}

function pendingActions(
state: { [key: string]: AsyncActionStatus } = {},
state: { [key: string]: AsyncActionStatus | AbortController } = {},
action: AsyncAction
) {
switch (action.status) {
Expand All @@ -450,7 +450,11 @@ function pendingActions(
return state;
}
const toAdd = {};
toAdd[action.type] = action.status;
if (action.abortController) {
toAdd[action.type] = action.abortController;
} else {
toAdd[action.type] = action.status;
}
return Object.assign({}, state, toAdd);
case AsyncActionStatus.SUCCESS:
case AsyncActionStatus.FAILURE:
Expand Down
16 changes: 16 additions & 0 deletions src/util/Ajax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ class RequestConfigBuilder {
private mFormData?: {};
private mResponseType?: ResponseType;
private mHeaders: {};
private mAbortSignal?: AbortSignal;
private preserveAcceptHeader: boolean;

constructor() {
this.mHeaders = {};
this.mHeaders[Constants.Headers.CONTENT_TYPE] = Constants.JSON_LD_MIME_TYPE;
this.mHeaders[Constants.Headers.ACCEPT] = Constants.JSON_LD_MIME_TYPE;
this.mResponseType = undefined;
this.mAbortSignal = undefined;
this.preserveAcceptHeader = false;
}

Expand Down Expand Up @@ -124,6 +126,15 @@ class RequestConfigBuilder {
public getResponseType() {
return this.mResponseType;
}

public getAbortSignal() {
return this.mAbortSignal;
}

public signal(controller: AbortController) {
this.mAbortSignal = controller.signal;
return this;
}
}

export function content(value: any): RequestConfigBuilder {
Expand Down Expand Up @@ -277,6 +288,7 @@ export class Ajax {
) {
const conf = {
params: config.getParams(),
signal: config.getAbortSignal(),
paramsSerializer,
};
return this.axiosInstance.head(path, conf);
Expand All @@ -303,6 +315,7 @@ export class Ajax {
params: config.getParams(),
headers: config.getHeaders(),
responseType: config.getResponseType(),
signal: config.getAbortSignal(),
validateStatus: Ajax.validateGetStatus,
paramsSerializer,
};
Expand All @@ -326,6 +339,7 @@ export class Ajax {
) {
const conf = {
headers: config.getHeaders(),
signal: config.getAbortSignal(),
paramsSerializer,
};
if (!config.shouldPreserveAcceptHeader()) {
Expand Down Expand Up @@ -354,6 +368,7 @@ export class Ajax {
const conf = {
params: config.getParams(),
headers: config.getHeaders(),
signal: config.getAbortSignal(),
paramsSerializer,
};
if (
Expand All @@ -370,6 +385,7 @@ export class Ajax {
if (config) {
conf = {
params: config.getParams(),
signal: config.getAbortSignal(),
paramsSerializer,
};
if (config.getContent()) {
Expand Down

0 comments on commit 7dfdeb1

Please sign in to comment.