Skip to content

Commit

Permalink
fix(@aws-amplify/api): graphql API.cancel fix (aws-amplify#9578)
Browse files Browse the repository at this point in the history
  • Loading branch information
jamesaucode authored May 10, 2022
1 parent f72e3df commit a9ae27f
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 13 deletions.
26 changes: 26 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,22 @@ jobs:
sample_name: multi-part-copy-with-progress
spec: multi-part-copy-with-progress
browser: << parameters.browser >>
integ_react_graphql_api:
parameters:
browser:
type: string
executor: js-test-executor
<<: *test_env_vars
working_directory: ~/amplify-js-samples-staging/samples/react/api/graphql
steps:
- prepare_test_env
- integ_test_js:
test_name: 'React GraphQL API'
framework: react
category: api
sample_name: graphql
spec: graphql
browser: << parameters.browser >>
integ_react_storage_ui:
executor: js-test-executor
<<: *test_env_vars
Expand Down Expand Up @@ -1198,6 +1214,15 @@ workflows:
- build
filters:
<<: *releasable_branches
- integ_react_graphql_api:
requires:
- integ_setup
- build
filters:
<<: *releasable_branches
matrix:
parameters:
<<: *test_browsers
- integ_angular_auth:
requires:
- integ_setup
Expand Down Expand Up @@ -1569,6 +1594,7 @@ workflows:
- integ_datastore_auth_v2
- integ_duplicate_packages
- integ_auth_test_cypress_no_ui
- integ_react_graphql_api
- integ_react_geo
- post_release:
filters:
Expand Down
9 changes: 9 additions & 0 deletions packages/api-graphql/src/GraphQLAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,15 @@ export class GraphQLAPIClass {
return this._api.cancel(request, message);
}

/**
* Check if the request has a corresponding cancel token in the WeakMap.
* @params request - The request promise
* @return if the request has a corresponding cancel token.
*/
hasCancelToken(request: Promise<any>) {
return this._api.hasCancelToken(request);
}

private _graphqlSubscribe(
{
query,
Expand Down
25 changes: 19 additions & 6 deletions packages/api-rest/__tests__/RestClient-unit-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,11 @@ describe('RestClient test', () => {
});

describe('Cancel Token', () => {
afterEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
});

const apiOptions = {
headers: {},
endpoints: [
Expand All @@ -474,16 +479,24 @@ describe('RestClient test', () => {

test('request non existent', () => {
const restClient = new RestClient(apiOptions);
// if the request doesn't exist we can still say it is canceled successfully
expect(
restClient.cancel(
new Promise<any>((req, res) => {})
)
).toBeTruthy();
expect(restClient.cancel(new Promise<any>((req, res) => {}))).toBe(false);
});

test('request exist', () => {
const restClient = new RestClient(apiOptions);
const request = Promise.resolve();
restClient.updateRequestToBeCancellable(
request,
restClient.getCancellableToken()
);
expect(restClient.cancel(request)).toBe(true);
});

test('happy case', () => {
const restClient = new RestClient(apiOptions);
jest
.spyOn(RestClient.prototype, 'ajax')
.mockImplementationOnce(() => Promise.resolve());

const cancellableToken = restClient.getCancellableToken();
const request = restClient.ajax('url', 'method', { cancellableToken });
Expand Down
9 changes: 9 additions & 0 deletions packages/api-rest/src/RestAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,15 @@ export class RestAPIClass {
return this._api.cancel(request, message);
}

/**
* Check if the request has a corresponding cancel token in the WeakMap.
* @params request - The request promise
* @return if the request has a corresponding cancel token.
*/
hasCancelToken(request: Promise<any>) {
return this._api.hasCancelToken(request);
}

/**
* Getting endpoint for API
* @param {string} apiName - The name of the api
Expand Down
18 changes: 13 additions & 5 deletions packages/api-rest/src/RestClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,18 @@ export class RestClient {
const source = this._cancelTokenMap.get(request);
if (source) {
source.cancel(message);
return true;
}
return true;
return false;
}

/**
* Check if the request has a corresponding cancel token in the WeakMap.
* @params request - The request promise
* @return if the request has a corresponding cancel token.
*/
hasCancelToken(request: Promise<any>) {
return this._cancelTokenMap.has(request);
}

/**
Expand Down Expand Up @@ -366,10 +376,8 @@ export class RestClient {
/** private methods **/

private _signed(params, credentials, isAllResponse, { service, region }) {
const {
signerServiceInfo: signerServiceInfoParams,
...otherParams
} = params;
const { signerServiceInfo: signerServiceInfoParams, ...otherParams } =
params;

const endpoint_region: string =
region || this._region || this._options.region;
Expand Down
34 changes: 34 additions & 0 deletions packages/api/__tests__/API-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,38 @@ describe('API test', () => {
const api = new API(null);
expect(await api.graphql({ query: 'query' })).toBe('grapqhqlResponse');
});

describe('cancel', () => {
test('cancel RestAPI request', async () => {
jest
.spyOn(GraphQLAPIClass.prototype, 'hasCancelToken')
.mockImplementation(() => false);
const restAPICancelSpy = jest
.spyOn(RestAPIClass.prototype, 'cancel')
.mockImplementation(() => true);
jest
.spyOn(RestAPIClass.prototype, 'hasCancelToken')
.mockImplementation(() => true);
const api = new API(null);
const request = Promise.resolve();
expect(api.cancel(request)).toBe(true);
expect(restAPICancelSpy).toHaveBeenCalled();
});

test('cancel GraphQLAPI request', async () => {
jest
.spyOn(GraphQLAPIClass.prototype, 'hasCancelToken')
.mockImplementation(() => true);
const graphQLAPICancelSpy = jest
.spyOn(GraphQLAPIClass.prototype, 'cancel')
.mockImplementation(() => true);
jest
.spyOn(RestAPIClass.prototype, 'hasCancelToken')
.mockImplementation(() => false);
const api = new API(null);
const request = Promise.resolve();
expect(api.cancel(request)).toBe(true);
expect(graphQLAPICancelSpy).toHaveBeenCalled();
});
});
});
9 changes: 7 additions & 2 deletions packages/api/src/API.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,18 @@ export class APIClass {
return this._restApi.isCancel(error);
}
/**
* Cancels an inflight request
* Cancels an inflight request for either a GraphQL request or a Rest API request.
* @param request - request to cancel
* @param [message] - custom error message
* @return If the request was cancelled
*/
cancel(request: Promise<any>, message?: string): boolean {
return this._restApi.cancel(request, message);
if (this._restApi.hasCancelToken(request)) {
return this._restApi.cancel(request, message);
} else if (this._graphqlApi.hasCancelToken(request)) {
return this._graphqlApi.cancel(request, message);
}
return false;
}

/**
Expand Down

0 comments on commit a9ae27f

Please sign in to comment.