From a9ae27f65e1a782321c0be87556f92d2ee432352 Mon Sep 17 00:00:00 2001 From: James Au <40404256+jamesaucode@users.noreply.github.com> Date: Tue, 10 May 2022 10:37:51 -0700 Subject: [PATCH] fix(@aws-amplify/api): graphql API.cancel fix (#9578) --- .circleci/config.yml | 26 ++++++++++++++ packages/api-graphql/src/GraphQLAPI.ts | 9 +++++ .../__tests__/RestClient-unit-test.ts | 25 ++++++++++---- packages/api-rest/src/RestAPI.ts | 9 +++++ packages/api-rest/src/RestClient.ts | 18 +++++++--- packages/api/__tests__/API-test.ts | 34 +++++++++++++++++++ packages/api/src/API.ts | 9 +++-- 7 files changed, 117 insertions(+), 13 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 15d698f5379..712566fd8bd 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -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 @@ -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 @@ -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: diff --git a/packages/api-graphql/src/GraphQLAPI.ts b/packages/api-graphql/src/GraphQLAPI.ts index ce6f0fdcd23..286e4ea0394 100644 --- a/packages/api-graphql/src/GraphQLAPI.ts +++ b/packages/api-graphql/src/GraphQLAPI.ts @@ -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) { + return this._api.hasCancelToken(request); + } + private _graphqlSubscribe( { query, diff --git a/packages/api-rest/__tests__/RestClient-unit-test.ts b/packages/api-rest/__tests__/RestClient-unit-test.ts index 38331035d7d..aa4941ea9ce 100644 --- a/packages/api-rest/__tests__/RestClient-unit-test.ts +++ b/packages/api-rest/__tests__/RestClient-unit-test.ts @@ -452,6 +452,11 @@ describe('RestClient test', () => { }); describe('Cancel Token', () => { + afterEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + }); + const apiOptions = { headers: {}, endpoints: [ @@ -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((req, res) => {}) - ) - ).toBeTruthy(); + expect(restClient.cancel(new Promise((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 }); diff --git a/packages/api-rest/src/RestAPI.ts b/packages/api-rest/src/RestAPI.ts index 6828e37653d..0ed75253e00 100644 --- a/packages/api-rest/src/RestAPI.ts +++ b/packages/api-rest/src/RestAPI.ts @@ -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) { + return this._api.hasCancelToken(request); + } + /** * Getting endpoint for API * @param {string} apiName - The name of the api diff --git a/packages/api-rest/src/RestClient.ts b/packages/api-rest/src/RestClient.ts index 3d643a1574f..a9337d9c55b 100644 --- a/packages/api-rest/src/RestClient.ts +++ b/packages/api-rest/src/RestClient.ts @@ -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) { + return this._cancelTokenMap.has(request); } /** @@ -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; diff --git a/packages/api/__tests__/API-test.ts b/packages/api/__tests__/API-test.ts index ecc84897591..b0a923d3dc5 100644 --- a/packages/api/__tests__/API-test.ts +++ b/packages/api/__tests__/API-test.ts @@ -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(); + }); + }); }); diff --git a/packages/api/src/API.ts b/packages/api/src/API.ts index 94d48c1c13b..e0477e4f24a 100644 --- a/packages/api/src/API.ts +++ b/packages/api/src/API.ts @@ -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, 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; } /**