From 5bfd1d0ddd08377a34dfd8ce5047174edfedeecb Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 29 Mar 2019 16:09:14 -0700 Subject: [PATCH] Reattach resources to stats trackers in the event they were destroyed --- ui/app/services/stats-trackers-registry.js | 15 ++++-- .../services/stats-trackers-registry-test.js | 47 +++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/ui/app/services/stats-trackers-registry.js b/ui/app/services/stats-trackers-registry.js index 0233c74d3..31f5c383b 100644 --- a/ui/app/services/stats-trackers-registry.js +++ b/ui/app/services/stats-trackers-registry.js @@ -11,6 +11,9 @@ import AllocationStatsTracker from 'nomad-ui/utils/classes/allocation-stats-trac const MAX_STAT_TRACKERS = 10; let registry; +const exists = (tracker, prop) => + tracker.get(prop) && !tracker.get(prop).isDestroyed && !tracker.get(prop).isDestroying; + export default Service.extend({ token: service(), @@ -30,13 +33,17 @@ export default Service.extend({ const type = resource && resource.constructor.modelName; const key = `${type}:${resource.get('id')}`; - - const cachedTracker = registry.get(key); - if (cachedTracker) return cachedTracker; - const Constructor = type === 'node' ? NodeStatsTracker : AllocationStatsTracker; const resourceProp = type === 'node' ? 'node' : 'allocation'; + const cachedTracker = registry.get(key); + if (cachedTracker) { + // It's possible for the resource on a cachedTracker to have been + // deleted. Rebind it if that's the case. + if (!exists(cachedTracker, resourceProp)) cachedTracker.set(resourceProp, resource); + return cachedTracker; + } + const tracker = Constructor.create({ fetch: url => this.get('token').authorizedRequest(url), [resourceProp]: resource, diff --git a/ui/tests/unit/services/stats-trackers-registry-test.js b/ui/tests/unit/services/stats-trackers-registry-test.js index 9916fafcf..559590410 100644 --- a/ui/tests/unit/services/stats-trackers-registry-test.js +++ b/ui/tests/unit/services/stats-trackers-registry-test.js @@ -1,6 +1,7 @@ import EmberObject from '@ember/object'; import { getOwner } from '@ember/application'; import Service from '@ember/service'; +import { run } from '@ember/runloop'; import wait from 'ember-test-helpers/wait'; import { moduleFor, test } from 'ember-qunit'; import Pretender from 'pretender'; @@ -104,6 +105,52 @@ test('Has a max size', function(assert) { assert.ok(ref.limit < Infinity, `A limit (${ref.limit}) is set`); }); +test('Registry re-attaches deleted resources to cached trackers', function(assert) { + const registry = this.subject(); + const id = 'some-id'; + + const node1 = mockNode.create({ id }); + let tracker = registry.getTracker(node1); + + assert.ok(tracker.get('node'), 'The tracker has a node'); + + tracker.set('node', null); + assert.notOk(tracker.get('node'), 'The tracker does not have a node'); + + tracker = registry.getTracker(node1); + assert.equal( + tracker.get('node'), + node1, + 'The node was re-attached to the tracker after calling getTracker again' + ); +}); + +test('Registry re-attaches destroyed resources to cached trackers', function(assert) { + const registry = this.subject(); + const id = 'some-id'; + + const node1 = mockNode.create({ id }); + let tracker = registry.getTracker(node1); + + assert.ok(tracker.get('node'), 'The tracker has a node'); + + run(() => { + node1.destroy(); + }); + + return wait().then(() => { + assert.ok(tracker.get('node').isDestroyed, 'The tracker node is destroyed'); + + const node2 = mockNode.create({ id }); + tracker = registry.getTracker(node2); + assert.equal( + tracker.get('node'), + node2, + 'Since node1 was destroyed but it matches the tracker of node2, node2 is attached to the tracker' + ); + }); +}); + test('Removes least recently used when something needs to be removed', function(assert) { const registry = this.subject(); const activeNode = mockNode.create({ id: 'active' });