diff --git a/ui/app/adapters/watchable.js b/ui/app/adapters/watchable.js index c4fbc5ad8228..bb2903ab447d 100644 --- a/ui/app/adapters/watchable.js +++ b/ui/app/adapters/watchable.js @@ -1,4 +1,4 @@ -import { get, computed } from '@ember/object'; +import { get } from '@ember/object'; import { assign } from '@ember/polyfills'; import { inject as service } from '@ember/service'; import queryString from 'query-string'; @@ -9,52 +9,24 @@ export default ApplicationAdapter.extend({ watchList: service(), store: service(), - xhrs: computed(function() { - 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() { + ajaxOptions(url, type, options) { const ajaxOptions = this._super(...arguments); - const key = this.xhrKey(...arguments); - - const previousBeforeSend = ajaxOptions.beforeSend; - ajaxOptions.beforeSend = function(jqXHR) { - if (previousBeforeSend) { - previousBeforeSend(...arguments); - } - this.xhrs.track(key, jqXHR); - jqXHR.always(() => { - this.xhrs.remove(key, jqXHR); - }); - }; + const abortToken = (options || {}).abortToken; + if (abortToken) { + delete options.abortToken; + + const previousBeforeSend = ajaxOptions.beforeSend; + ajaxOptions.beforeSend = function(jqXHR) { + abortToken.capture(jqXHR); + if (previousBeforeSend) { + previousBeforeSend(...arguments); + } + }; + } return ajaxOptions; }, - xhrKey(url, method /* options */) { - return `${method} ${url}`; - }, - findAll(store, type, sinceToken, snapshotRecordArray, additionalParams = {}) { const params = assign(this.buildQuery(), additionalParams); const url = this.urlForFindAll(type.modelName); @@ -63,7 +35,9 @@ export default ApplicationAdapter.extend({ params.index = this.watchList.getIndexFor(url); } + const abortToken = get(snapshotRecordArray || {}, 'adapterOptions.abortToken'); return this.ajax(url, 'GET', { + abortToken, data: params, }); }, @@ -76,7 +50,9 @@ export default ApplicationAdapter.extend({ params.index = this.watchList.getIndexFor(url); } + const abortToken = get(snapshot || {}, 'adapterOptions.abortToken'); return this.ajax(url, 'GET', { + abortToken, data: params, }).catch(error => { if (error instanceof AbortError) { @@ -86,7 +62,8 @@ export default ApplicationAdapter.extend({ }); }, - reloadRelationship(model, relationshipName, watch = false) { + reloadRelationship(model, relationshipName, options = { watch: false, abortToken: null }) { + const { watch, abortToken } = options; const relationship = model.relationshipFor(relationshipName); if (relationship.kind !== 'belongsTo' && relationship.kind !== 'hasMany') { throw new Error( @@ -110,6 +87,7 @@ export default ApplicationAdapter.extend({ } return this.ajax(url, 'GET', { + abortToken, data: params, }).then( json => { @@ -143,39 +121,4 @@ export default ApplicationAdapter.extend({ return this._super(...arguments); }, - - cancelFindRecord(modelName, id) { - if (!modelName || id == null) { - return; - } - const url = this.urlForFindRecord(id, modelName); - this.xhrs.cancel(`GET ${url}`); - }, - - cancelFindAll(modelName) { - if (!modelName) { - return; - } - let url = this.urlForFindAll(modelName); - const params = queryString.stringify(this.buildQuery()); - if (params) { - url = `${url}?${params}`; - } - this.xhrs.cancel(`GET ${url}`); - }, - - cancelReloadRelationship(model, relationshipName) { - if (!model || !relationshipName) { - return; - } - const relationship = model.relationshipFor(relationshipName); - if (relationship.kind !== 'belongsTo' && relationship.kind !== 'hasMany') { - throw new Error( - `${relationship.key} must be a belongsTo or hasMany, instead it was ${relationship.kind}` - ); - } else { - const url = model[relationship.kind](relationship.key).link(); - this.xhrs.cancel(`GET ${url}`); - } - }, }); diff --git a/ui/app/utils/classes/xhr-token.js b/ui/app/utils/classes/xhr-token.js new file mode 100644 index 000000000000..20b6386b3e95 --- /dev/null +++ b/ui/app/utils/classes/xhr-token.js @@ -0,0 +1,11 @@ +export default class XHRToken { + capture(xhr) { + this._xhr = xhr; + } + + abort() { + if (this._xhr) { + this._xhr.abort(); + } + } +} diff --git a/ui/app/utils/properties/watch.js b/ui/app/utils/properties/watch.js index e5f9464fec29..ee6f5f9ec3cf 100644 --- a/ui/app/utils/properties/watch.js +++ b/ui/app/utils/properties/watch.js @@ -3,12 +3,14 @@ import { get } from '@ember/object'; import RSVP from 'rsvp'; import { task } from 'ember-concurrency'; import wait from 'nomad-ui/utils/wait'; +import XHRToken from 'nomad-ui/utils/classes/xhr-token'; import config from 'nomad-ui/config/environment'; const isEnabled = config.APP.blockingQueries !== false; export function watchRecord(modelName) { return task(function*(id, throttle = 2000) { + const token = new XHRToken(); if (typeof id === 'object') { id = get(id, 'id'); } @@ -17,7 +19,7 @@ export function watchRecord(modelName) { yield RSVP.all([ this.store.findRecord(modelName, id, { reload: true, - adapterOptions: { watch: true }, + adapterOptions: { watch: true, abortToken: token }, }), wait(throttle), ]); @@ -25,9 +27,7 @@ export function watchRecord(modelName) { yield e; break; } finally { - this.store - .adapterFor(modelName) - .cancelFindRecord(modelName, id); + token.abort(); } } }).drop(); @@ -35,21 +35,20 @@ export function watchRecord(modelName) { export function watchRelationship(relationshipName) { return task(function*(model, throttle = 2000) { + const token = new XHRToken(); while (isEnabled && !Ember.testing) { try { yield RSVP.all([ this.store .adapterFor(model.constructor.modelName) - .reloadRelationship(model, relationshipName, true), + .reloadRelationship(model, relationshipName, { watch: true, abortToken: token }), wait(throttle), ]); } catch (e) { yield e; break; } finally { - this.store - .adapterFor(model.constructor.modelName) - .cancelReloadRelationship(model, relationshipName); + token.abort(); } } }).drop(); @@ -57,19 +56,21 @@ export function watchRelationship(relationshipName) { export function watchAll(modelName) { return task(function*(throttle = 2000) { + const token = new XHRToken(); while (isEnabled && !Ember.testing) { try { yield RSVP.all([ - this.store.findAll(modelName, { reload: true, adapterOptions: { watch: true } }), + this.store.findAll(modelName, { + reload: true, + adapterOptions: { watch: true, abortToken: token }, + }), wait(throttle), ]); } catch (e) { yield e; break; } finally { - this.store - .adapterFor(modelName) - .cancelFindAll(modelName); + token.abort(); } } }).drop(); diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index 34cf5fc9a8e3..6c7c94503504 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -1,10 +1,10 @@ -import EmberObject from '@ember/object'; import { run } from '@ember/runloop'; import { assign } from '@ember/polyfills'; import { settled } from '@ember/test-helpers'; import { setupTest } from 'ember-qunit'; import { module, test } from 'qunit'; import { startMirage } from 'nomad-ui/initializers/ember-cli-mirage'; +import XHRToken from 'nomad-ui/utils/classes/xhr-token'; module('Unit | Adapter | Job', function(hooks) { setupTest(hooks); @@ -233,7 +233,7 @@ module('Unit | Adapter | Job', function(hooks) { const plainId = 'job-1'; const mockModel = makeMockModel(plainId); - this.subject().reloadRelationship(mockModel, 'summary', true); + this.subject().reloadRelationship(mockModel, 'summary', { watch: true }); assert.equal( pretender.handledRequests[0].url, '/v1/job/job-1/summary?index=1', @@ -241,7 +241,7 @@ module('Unit | Adapter | Job', function(hooks) { ); await settled(); - this.subject().reloadRelationship(mockModel, 'summary', true); + this.subject().reloadRelationship(mockModel, 'summary', { watch: true }); assert.equal( pretender.handledRequests[1].url, '/v1/job/job-1/summary?index=2', @@ -253,12 +253,14 @@ module('Unit | Adapter | Job', function(hooks) { await this.initializeUI(); const { pretender } = this.server; + const token = new XHRToken(); + pretender.get('/v1/jobs', () => [200, {}, '[]'], true); this.subject() .findAll(null, { modelName: 'job' }, null, { reload: true, - adapterOptions: { watch: true }, + adapterOptions: { watch: true, abortToken: token }, }) .catch(() => {}); @@ -267,7 +269,7 @@ module('Unit | Adapter | Job', function(hooks) { // Schedule the cancelation before waiting run.next(() => { - this.subject().cancelFindAll('job'); + token.abort(); }); await settled(); @@ -279,12 +281,13 @@ module('Unit | Adapter | Job', function(hooks) { const { pretender } = this.server; const jobId = JSON.stringify(['job-1', 'default']); + const token = new XHRToken(); pretender.get('/v1/job/:id', () => [200, {}, '{}'], true); this.subject().findRecord(null, { modelName: 'job' }, jobId, { reload: true, - adapterOptions: { watch: true }, + adapterOptions: { watch: true, abortToken: token }, }); const { request: xhr } = pretender.requestReferences[0]; @@ -292,7 +295,7 @@ module('Unit | Adapter | Job', function(hooks) { // Schedule the cancelation before waiting run.next(() => { - this.subject().cancelFindRecord('job', jobId); + token.abort(); }); await settled(); @@ -304,17 +307,18 @@ module('Unit | Adapter | Job', function(hooks) { const { pretender } = this.server; const plainId = 'job-1'; + const token = new XHRToken(); const mockModel = makeMockModel(plainId); pretender.get('/v1/job/:id/summary', () => [200, {}, '{}'], true); - this.subject().reloadRelationship(mockModel, 'summary', true); + this.subject().reloadRelationship(mockModel, 'summary', { watch: true, abortToken: token }); const { request: xhr } = pretender.requestReferences[0]; assert.equal(xhr.status, 0, 'Request is still pending'); // Schedule the cancelation before waiting run.next(() => { - this.subject().cancelReloadRelationship(mockModel, 'summary'); + token.abort(); }); await settled(); @@ -326,20 +330,23 @@ module('Unit | Adapter | Job', function(hooks) { const { pretender } = this.server; const jobId = JSON.stringify(['job-1', 'default']); + const token1 = new XHRToken(); + const token2 = new XHRToken(); pretender.get('/v1/job/:id', () => [200, {}, '{}'], true); this.subject().findRecord(null, { modelName: 'job' }, jobId, { reload: true, - adapterOptions: { watch: true }, + adapterOptions: { watch: true, abortToken: token1 }, }); this.subject().findRecord(null, { modelName: 'job' }, jobId, { reload: true, - adapterOptions: { watch: true }, + adapterOptions: { watch: true, abortToken: token2 }, }); const { request: xhr } = pretender.requestReferences[0]; + const { request: xhr2 } = pretender.requestReferences[1]; assert.equal(xhr.status, 0, 'Request is still pending'); assert.equal(pretender.requestReferences.length, 2, 'Two findRecord requests were made'); assert.equal( @@ -348,44 +355,15 @@ module('Unit | Adapter | Job', function(hooks) { 'The two requests have the same URL' ); - // Schedule the cancelation before waiting - run.next(() => { - this.subject().cancelFindRecord('job', jobId); - }); - - await settled(); - assert.ok(xhr.aborted, 'Request was aborted'); - }); - - test('canceling a find record request will never cancel a request with the same url but different method', async function(assert) { - await this.initializeUI(); - - 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 + // Schedule the cancelation and resolution before waiting run.next(() => { - this.subject().cancelFindRecord('job', jobId); + token1.abort(); + pretender.resolve(xhr2); }); await settled(); - assert.ok(getXHR.aborted, 'Get request was aborted'); - assert.notOk(deleteXHR.aborted, 'Delete request was aborted'); + assert.ok(xhr.aborted, 'Request one was aborted'); + assert.notOk(xhr2.aborted, 'Request two was not aborted'); }); test('when there is no region set, requests are made without the region query param', async function(assert) {