From 8e6f561b8a5b11e0a983b67eaf82ce72ff236ca5 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Wed, 28 Mar 2018 16:46:35 -0700 Subject: [PATCH 1/2] Add missing js class for System that extends AbstractJobPage --- ui/app/components/job-page/system.js | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ui/app/components/job-page/system.js diff --git a/ui/app/components/job-page/system.js b/ui/app/components/job-page/system.js new file mode 100644 index 000000000..559b3c8b8 --- /dev/null +++ b/ui/app/components/job-page/system.js @@ -0,0 +1,3 @@ +import AbstractJobPage from './abstract'; + +export default AbstractJobPage.extend(); From c06e1f80bd857576c97b8d971767e3f3271e1cd6 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 29 Mar 2018 11:44:26 -0700 Subject: [PATCH 2/2] Track multiple xhrs per URL rather than overriding It was possible for a url to be overridden then canceled, leaving the open connection open and forgotten about. --- ui/app/adapters/watchable.js | 41 +++++++++++++++++++----------- ui/tests/unit/adapters/job-test.js | 35 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/ui/app/adapters/watchable.js b/ui/app/adapters/watchable.js index 8e6ad273c..3c9147269 100644 --- a/ui/app/adapters/watchable.js +++ b/ui/app/adapters/watchable.js @@ -10,7 +10,27 @@ export default ApplicationAdapter.extend({ store: service(), xhrs: computed(function() { - return {}; + return { + list: {}, + track(key, xhr) { + if (this.list[key]) { + this.list[key].push(xhr); + } else { + this.list[key] = [xhr]; + } + }, + cancel(key) { + while (this.list[key] && this.list[key].length) { + this.remove(key, this.list[key][0]); + } + }, + remove(key, xhr) { + if (this.list[key]) { + xhr.abort(); + this.list[key].removeObject(xhr); + } + }, + }; }), ajaxOptions() { @@ -22,9 +42,9 @@ export default ApplicationAdapter.extend({ if (previousBeforeSend) { previousBeforeSend(...arguments); } - this.get('xhrs')[key] = jqXHR; + this.get('xhrs').track(key, jqXHR); jqXHR.always(() => { - delete this.get('xhrs')[key]; + this.get('xhrs').remove(key, jqXHR); }); }; @@ -127,10 +147,7 @@ export default ApplicationAdapter.extend({ return; } const url = this.urlForFindRecord(id, modelName); - const xhr = this.get('xhrs')[url]; - if (xhr) { - xhr.abort(); - } + this.get('xhrs').cancel(url); }, cancelFindAll(modelName) { @@ -142,10 +159,7 @@ export default ApplicationAdapter.extend({ if (params) { url = `${url}?${params}`; } - const xhr = this.get('xhrs')[url]; - if (xhr) { - xhr.abort(); - } + this.get('xhrs').cancel(url); }, cancelReloadRelationship(model, relationshipName) { @@ -159,10 +173,7 @@ export default ApplicationAdapter.extend({ ); } else { const url = model[relationship.kind](relationship.key).link(); - const xhr = this.get('xhrs')[url]; - if (xhr) { - xhr.abort(); - } + this.get('xhrs').cancel(url); } }, }); diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index 066b99198..b454cc8b1 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -257,6 +257,41 @@ test('relationship reloads can be canceled', function(assert) { }); }); +test('requests can be canceled even if multiple requests for the same URL were made', function(assert) { + const { pretender } = this.server; + const jobId = JSON.stringify(['job-1', 'default']); + + pretender.get('/v1/job/:id', () => [200, {}, '{}'], true); + + this.subject().findRecord(null, { modelName: 'job' }, jobId, { + reload: true, + adapterOptions: { watch: true }, + }); + + this.subject().findRecord(null, { modelName: 'job' }, jobId, { + reload: true, + adapterOptions: { watch: true }, + }); + + const { request: xhr } = pretender.requestReferences[0]; + assert.equal(xhr.status, 0, 'Request is still pending'); + assert.equal(pretender.requestReferences.length, 2, 'Two findRecord requests were made'); + assert.equal( + pretender.requestReferences.mapBy('url').uniq().length, + 1, + 'The two requests have the same URL' + ); + + // Schedule the cancelation before waiting + run.next(() => { + this.subject().cancelFindRecord('job', jobId); + }); + + return wait().then(() => { + assert.ok(xhr.aborted, 'Request was aborted'); + }); +}); + function makeMockModel(id, options) { return assign( {