diff --git a/ui/app/components/primary-metric/node.js b/ui/app/components/primary-metric/node.js index d0fc56e66da..0499fcc48f0 100644 --- a/ui/app/components/primary-metric/node.js +++ b/ui/app/components/primary-metric/node.js @@ -74,7 +74,11 @@ export default class NodePrimaryMetric extends Component { @task(function* () { do { - this.tracker.poll.perform(); + const tracker = this.tracker; + const nodeId = tracker && get(tracker, 'node.id'); + if (tracker && nodeId) { + tracker.poll.perform(); + } yield timeout(100); } while (!Ember.testing); }) @@ -88,6 +92,8 @@ export default class NodePrimaryMetric extends Component { willDestroy() { super.willDestroy(...arguments); this.poller.cancelAll(); - this.tracker.signalPause.perform(); + if (this.tracker) { + this.tracker.signalPause.perform(); + } } } diff --git a/ui/app/services/stats-trackers-registry.js b/ui/app/services/stats-trackers-registry.js index 1d4f810ecfa..607315d4803 100644 --- a/ui/app/services/stats-trackers-registry.js +++ b/ui/app/services/stats-trackers-registry.js @@ -44,18 +44,49 @@ export default class StatsTrackersRegistryService extends Service { if (!resource) return; const type = resource && resource.constructor.modelName; - const key = `${type}:${resource.get('id')}`; + const resourceId = resource.get('id'); + + if (!resourceId) { + return; + } + + const key = `${type}:${resourceId}`; 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 cachedResource = cachedTracker.get(resourceProp); + const shouldReuse = + exists(cachedTracker, resourceProp) && cachedResource === resource; + + if (shouldReuse) { + return cachedTracker; + } + + // Replace stale/mismatched trackers instead of mutating an already-used + // tracker during render. + if ( + cachedTracker.poll && + typeof cachedTracker.poll.cancelAll === 'function' + ) { + cachedTracker.poll.cancelAll(); + } + if ( + cachedTracker.signalPause && + typeof cachedTracker.signalPause.cancelAll === 'function' + ) { + cachedTracker.signalPause.cancelAll(); + } + + const replacementTracker = Constructor.create({ + fetch: (url) => this.token.authorizedRequest(url), + [resourceProp]: resource, + }); + + registry.set(key, replacementTracker); + return replacementTracker; } const tracker = Constructor.create({ diff --git a/ui/tests/integration/components/primary-metric/primary-metric.js b/ui/tests/integration/components/primary-metric/primary-metric.js index 1ca49220e77..0e6bafef45a 100644 --- a/ui/tests/integration/components/primary-metric/primary-metric.js +++ b/ui/tests/integration/components/primary-metric/primary-metric.js @@ -17,6 +17,9 @@ export function setupPrimaryMetricMocks(hooks, tasks = []) { const trackerSignalPauseSpy = (this.trackerSignalPauseSpy = sinon.spy()); const MockTracker = EmberObject.extend({ + node: computed(function () { + return { id: 'test-node-id' }; + }), poll: task(function* () { yield trackerPollSpy(); }), diff --git a/ui/tests/unit/services/stats-trackers-registry-test.js b/ui/tests/unit/services/stats-trackers-registry-test.js index fe964b27c74..0007c226097 100644 --- a/ui/tests/unit/services/stats-trackers-registry-test.js +++ b/ui/tests/unit/services/stats-trackers-registry-test.js @@ -107,7 +107,7 @@ module('Unit | Service | Stats Trackers Registry', function (hooks) { ); }); - test('Registry does not depend on persistent object references', function (assert) { + test('Registry replaces cached tracker when resource reference changes for same id', function (assert) { const registry = this.subject(); const id = 'some-id'; @@ -122,10 +122,18 @@ module('Unit | Service | Stats Trackers Registry', function (hooks) { 'And the same className' ); + const tracker1 = registry.getTracker(node1); + const tracker2 = registry.getTracker(node2); + + assert.notEqual( + tracker1, + tracker2, + 'Returns a replacement tracker for a different resource reference with same id' + ); assert.equal( - registry.getTracker(node1), - registry.getTracker(node2), - 'Return the same tracker' + tracker2.get('node'), + node2, + 'The replacement tracker is bound to the latest resource reference' ); assert.equal( registry.get('registryRef').size,