From 15bc387ce2c6f9fdeecdba55e176cc76d5fa48f4 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Tue, 22 May 2018 14:48:27 -0700 Subject: [PATCH 1/3] XHR keys need to include the method as well The URL alone doesn't guarantee uniqueness --- ui/app/adapters/job.js | 4 ++-- ui/app/adapters/watchable.js | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index 984564572..0e16fdcb4 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -49,9 +49,9 @@ export default Watchable.extend({ xhrKey(url, method, options = {}) { const namespace = options.data && options.data.namespace; if (namespace) { - return `${url}?namespace=${namespace}`; + return `${method} ${url}?namespace=${namespace}`; } - return url; + return `${method} ${url}`; }, relationshipFallbackLinks: { diff --git a/ui/app/adapters/watchable.js b/ui/app/adapters/watchable.js index 2f3257efd..9f7c072cc 100644 --- a/ui/app/adapters/watchable.js +++ b/ui/app/adapters/watchable.js @@ -51,8 +51,8 @@ export default ApplicationAdapter.extend({ return ajaxOptions; }, - xhrKey(url /* method, options */) { - return url; + xhrKey(url, method /* options */) { + return `${method} ${url}`; }, findAll(store, type, sinceToken, snapshotRecordArray, additionalParams = {}) { @@ -149,7 +149,7 @@ export default ApplicationAdapter.extend({ return; } const url = this.urlForFindRecord(id, modelName); - this.get('xhrs').cancel(url); + this.get('xhrs').cancel(`GET ${url}`); }, cancelFindAll(modelName) { @@ -161,7 +161,7 @@ export default ApplicationAdapter.extend({ if (params) { url = `${url}?${params}`; } - this.get('xhrs').cancel(url); + this.get('xhrs').cancel(`GET ${url}`); }, cancelReloadRelationship(model, relationshipName) { @@ -175,7 +175,7 @@ export default ApplicationAdapter.extend({ ); } else { const url = model[relationship.kind](relationship.key).link(); - this.get('xhrs').cancel(url); + this.get('xhrs').cancel(`GET ${url}`); } }, }); From 9c6b060de98e8465dabdd94672c6dfa6052ab8aa Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 25 May 2018 09:13:18 -0700 Subject: [PATCH 2/3] Add a test to assert that canceling GETs can't instead cancel DELETEs --- ui/tests/unit/adapters/job-test.js | 37 +++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index b454cc8b1..aa6827d5a 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -1,3 +1,4 @@ +import EmberObject from '@ember/object'; import { run } from '@ember/runloop'; import { assign } from '@ember/polyfills'; import { test } from 'ember-qunit'; @@ -10,8 +11,12 @@ moduleForAdapter('job', 'Unit | Adapter | Job', { 'adapter:job', 'service:token', 'service:system', - 'model:namespace', + 'model:allocation', + 'model:deployment', + 'model:evaluation', 'model:job-summary', + 'model:job-version', + 'model:namespace', 'adapter:application', 'service:watchList', ], @@ -292,6 +297,36 @@ test('requests can be canceled even if multiple requests for the same URL were m }); }); +test('canceling a find record request will never cancel a request with the same url but different method', function(assert) { + const { pretender } = this.server; + const jobId = JSON.stringify(['job-1', 'default']); + + pretender.get('/v1/job/:id', () => [200, {}, '{}'], true); + pretender.delete('/v1/job/:id', () => [204, {}, ''], 200); + + this.subject().findRecord(null, { modelName: 'job' }, jobId, { + reload: true, + adapterOptions: { watch: true }, + }); + + this.subject().stop(EmberObject.create({ id: jobId })); + + const { request: getXHR } = pretender.requestReferences[0]; + const { request: deleteXHR } = pretender.requestReferences[1]; + assert.equal(getXHR.status, 0, 'Get request is still pending'); + assert.equal(deleteXHR.status, 0, 'Delete request is still pending'); + + // Schedule the cancelation before waiting + run.next(() => { + this.subject().cancelFindRecord('job', jobId); + }); + + return wait().then(() => { + assert.ok(getXHR.aborted, 'Get request was aborted'); + assert.notOk(deleteXHR.aborted, 'Delete request was aborted'); + }); +}); + function makeMockModel(id, options) { return assign( { From 1078a99948ed857531d5365d6d16f9a29edfece2 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 25 May 2018 09:15:45 -0700 Subject: [PATCH 3/3] Refactor the job xhrKey to use super --- ui/app/adapters/job.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ui/app/adapters/job.js b/ui/app/adapters/job.js index 0e16fdcb4..eeba06522 100644 --- a/ui/app/adapters/job.js +++ b/ui/app/adapters/job.js @@ -47,11 +47,12 @@ export default Watchable.extend({ }, xhrKey(url, method, options = {}) { + const plainKey = this._super(...arguments); const namespace = options.data && options.data.namespace; if (namespace) { - return `${method} ${url}?namespace=${namespace}`; + return `${plainKey}?namespace=${namespace}`; } - return `${method} ${url}`; + return plainKey; }, relationshipFallbackLinks: {