Merge pull request #4319 from hashicorp/b-ui-errant-acl-error

UI: XHR keys need to include the method as well
hashicorp · May 25, 2018 · 20bc6e4 · 20bc6e4
2 parents 32df9a6 + e8bbd0f
commit 20bc6e4
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 8 deletions.
diff --git 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 `${url}?namespace=${namespace}`;
+      return `${plainKey}?namespace=${namespace}`;
     }
-    return url;
+    return plainKey;
   },
 
   relationshipFallbackLinks: {

diff --git 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}`);
     }
   },
 });
diff --git 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(
     {