Skip to content

Commit 1de1cfd

Browse files
nabramovitznorman-abramovitz
authored andcommitted
Destroy prior cloud-foundry list-config effects on re-entry
The cloud-foundry signal-config services (apps, orgs, spaces, users, routes, audit-events, buildpacks, cells, security-groups, feature-flags, org-quotas, space-quotas, revisions, stacks) are providedIn:'root' singletons, but their host pages call initialize()/startStatsPolling() per mount. Their effects (name-filter, the org/space cascade rule, the app-wall stats refresh, the orgs hasLoadedOnce latch) were created uncaptured, so each navigation stacked live effects on the root injector — multiplying reactive work and, for the app wall, app-stats HTTP traffic across a session. Capture each EffectRef and destroy the prior one on re-entry, matching the timer reset already guarding startStatsPolling and the resolverEffect pattern this service already used.
1 parent 7ea56ef commit 1de1cfd

15 files changed

Lines changed: 149 additions & 31 deletions

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/app/cf-apps-signal-config.service.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,42 @@ describe('CfAppsSignalConfigService', () => {
463463
expect(httpMock.get).toHaveBeenCalledTimes(2);
464464
});
465465

466+
it('startStatsPolling does not stack a reactive effect on re-entry', () => {
467+
// Regression: this service is a root singleton, and the app-wall calls
468+
// startStatsPolling() from ngOnInit on every mount. The reactive stats
469+
// effect used to be created uncaptured, so each navigation left a live
470+
// effect on the root injector — N visits meant N refreshes per page
471+
// change, multiplying app-stats traffic across a session. The fix
472+
// captures the EffectRef and destroys the prior one on re-entry.
473+
const http = makeHttp();
474+
const cf = makeStubCfService([{ guid: 'cnsi-1', name: 'Primary CF' }]);
475+
const svc = makeSvc(http, cf);
476+
svc.initialize(['cnsi-1']);
477+
seedApps(svc, [{
478+
guid: 'a', name: 'a-app', state: 'STARTED', cnsiGuid: 'cnsi-1',
479+
spaceGuid: 'sp-1', instances: 1, routes: [], createdAt: '', updatedAt: '',
480+
} as StApp]);
481+
TestBed.tick();
482+
483+
// Simulate two mounts of the app-wall (route re-entry).
484+
svc.startStatsPolling();
485+
svc.startStatsPolling();
486+
TestBed.tick();
487+
488+
// Count only refreshes driven by the reactive effect on a pagedItems
489+
// change — isolate from the synchronous runOnce()/interval legs.
490+
const refresh = vi.spyOn(svc as any, 'refreshStatsForKeys');
491+
seedApps(svc, [{
492+
guid: 'b', name: 'b-app', state: 'STARTED', cnsiGuid: 'cnsi-1',
493+
spaceGuid: 'sp-1', instances: 1, routes: [], createdAt: '', updatedAt: '',
494+
} as StApp]);
495+
TestBed.tick();
496+
497+
// Exactly one live effect → one reactive refresh. Without the fix the
498+
// two stacked effects would each fire, yielding 2.
499+
expect(refresh).toHaveBeenCalledTimes(1);
500+
});
501+
466502
it('cnsiOptions picks up connected CF endpoints from CloudFoundryService', () => {
467503
const http = makeHttp();
468504
const cf = makeStubCfService([

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/app/cf-apps-signal-config.service.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ export class CfAppsSignalConfigService {
7575
readonly appStats: Signal<Map<string, { running: number; total: number }>> =
7676
computed(() => this._appStats());
7777
private statsTimer?: ReturnType<typeof setInterval>;
78+
// Reactive stats refresh effect, captured so a re-entry (root singleton +
79+
// per-mount startStatsPolling) destroys the prior one instead of stacking.
80+
private statsEffect?: EffectRef;
7881
// Tracks `${cnsiGuid}:${appGuid}` keys with an in-flight stats request so
7982
// burst signal updates during initial render don't issue 4× duplicate
8083
// calls per app — the original symptom on multi-CNSI walls with slow CFs.
@@ -871,8 +874,14 @@ export class CfAppsSignalConfigService {
871874
this.statsTimer = setInterval(runOnce, intervalMs);
872875
// effect() requires an injection context; startStatsPolling is called
873876
// from the component's ngOnInit which isn't one. Wrap it explicitly.
877+
//
878+
// Tear down any previous mount's effect first. This service is a root
879+
// singleton, so a re-entry (app-wall remounted on navigation) would
880+
// otherwise stack one live effect per visit, multiplying stats fetches
881+
// across a session — same reasoning as the statsTimer reset above.
882+
this.statsEffect?.destroy();
874883
runInInjectionContext(this.injector, () => {
875-
effect(() => {
884+
this.statsEffect = effect(() => {
876885
const keys = this.view.pagedItems().map((a) => `${a.cnsiGuid}:${a.guid}`);
877886
this.refreshStatsForKeys(keys);
878887
});

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/cf-buildpacks/cf-buildpacks-signal-config.service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
1+
import { EffectRef, Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
22
import { HttpClient } from '@angular/common/http';
33

44
import { ListStateStore } from '@stratosui/core';
@@ -46,6 +46,10 @@ export class CfBuildpacksSignalConfigService {
4646

4747
view!: ViewPipeline<StBuildpack>;
4848

49+
// Captured so a re-entry (root singleton, but initialize() runs per mount)
50+
// destroys the prior filter effect instead of stacking one per navigation.
51+
private filterEffect?: EffectRef;
52+
4953
initialize(cnsiGuid: string): void {
5054
this.cnsiGuid = cnsiGuid;
5155
this.source = new CnsiBuildpacksSource(cnsiGuid, this.http);
@@ -58,8 +62,9 @@ export class CfBuildpacksSignalConfigService {
5862
this._sortExtractors.asReadonly(),
5963
);
6064

65+
this.filterEffect?.destroy();
6166
runInInjectionContext(this.injector, () => {
62-
effect(() => {
67+
this.filterEffect = effect(() => {
6368
const q = this.nameFilter().trim().toLowerCase();
6469
this.filter.set((b: StBuildpack) => {
6570
if (!q) return true;

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/cf-cells/cf-cells-signal-config.service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
1+
import { EffectRef, Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
22
import { HttpClient, HttpHeaders } from '@angular/common/http';
33
import { firstValueFrom } from 'rxjs';
44

@@ -105,6 +105,10 @@ export class CfCellsSignalConfigService {
105105

106106
view!: ViewPipeline<CfCellRow>;
107107

108+
// Captured so a re-entry (root singleton, but initialize() runs per mount)
109+
// destroys the prior filter effect instead of stacking one per navigation.
110+
private filterEffect?: EffectRef;
111+
108112
initialize(cnsiGuid: string): void {
109113
this.cnsiGuid = cnsiGuid;
110114
this.view = new ViewPipeline<CfCellRow>(
@@ -116,8 +120,9 @@ export class CfCellsSignalConfigService {
116120
this._sortExtractors.asReadonly(),
117121
);
118122

123+
this.filterEffect?.destroy();
119124
runInInjectionContext(this.injector, () => {
120-
effect(() => {
125+
this.filterEffect = effect(() => {
121126
const q = this.nameFilter().trim().toLowerCase();
122127
this.filter.set((c: CfCellRow) => {
123128
if (!q) return true;

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/cf-events/cf-audit-events-signal-config.service.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { DestroyRef, Injectable, Injector, Signal, WritableSignal, computed, effect, inject, runInInjectionContext, signal } from '@angular/core';
1+
import { DestroyRef, EffectRef, Injectable, Injector, Signal, WritableSignal, computed, effect, inject, runInInjectionContext, signal } from '@angular/core';
22
import { HttpClient } from '@angular/common/http';
33
import { firstValueFrom } from 'rxjs';
44

@@ -130,6 +130,11 @@ export class CfAuditEventsSignalConfigService {
130130

131131
view!: ViewPipeline<StAuditEvent>;
132132

133+
// Captured so a re-entry (root singleton, but initialize() runs per mount)
134+
// destroys the prior effects instead of stacking them per navigation.
135+
private filterEffect?: EffectRef;
136+
private cascadeEffect?: EffectRef;
137+
133138
initialize(cnsiGuid: string): void {
134139
// Swap CNSI: release the previous EDS handle if a re-initialize() in
135140
// the same singleton swapped foundations. Refcount-balanced —
@@ -156,8 +161,10 @@ export class CfAuditEventsSignalConfigService {
156161
this._sortExtractors.asReadonly(),
157162
);
158163

164+
this.filterEffect?.destroy();
165+
this.cascadeEffect?.destroy();
159166
runInInjectionContext(this.injector, () => {
160-
effect(() => {
167+
this.filterEffect = effect(() => {
161168
const q = this.nameFilter().trim().toLowerCase();
162169
const base = this.basePredicate();
163170
const org = this.selectedOrg();
@@ -180,7 +187,7 @@ export class CfAuditEventsSignalConfigService {
180187
// different specific org. When Org returns to All, the Space
181188
// dropdown shows every space (labelled "<space> - <org>"), so the
182189
// current selection remains valid and must be preserved.
183-
effect(() => {
190+
this.cascadeEffect = effect(() => {
184191
const org = this.selectedOrg();
185192
const space = this.selectedSpace();
186193
if (!space) return;

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/cf-feature-flags/cf-feature-flags-signal-config.service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
1+
import { EffectRef, Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
22
import { HttpClient } from '@angular/common/http';
33

44
import { ListStateStore } from '@stratosui/core';
@@ -44,6 +44,10 @@ export class CfFeatureFlagsSignalConfigService {
4444

4545
view!: ViewPipeline<StFeatureFlag>;
4646

47+
// Captured so a re-entry (root singleton, but initialize() runs per mount)
48+
// destroys the prior filter effect instead of stacking one per navigation.
49+
private filterEffect?: EffectRef;
50+
4751
initialize(cnsiGuid: string): void {
4852
this.cnsiGuid = cnsiGuid;
4953
this.source = new CnsiFeatureFlagsSource(cnsiGuid, this.http);
@@ -56,8 +60,9 @@ export class CfFeatureFlagsSignalConfigService {
5660
this._sortExtractors.asReadonly(),
5761
);
5862

63+
this.filterEffect?.destroy();
5964
runInInjectionContext(this.injector, () => {
60-
effect(() => {
65+
this.filterEffect = effect(() => {
6166
const q = this.nameFilter().trim().toLowerCase();
6267
this.filter.set((f: StFeatureFlag) => {
6368
if (!q) return true;

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/cf-quotas/cf-org-quotas-signal-config.service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
1+
import { EffectRef, Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
22
import { HttpClient } from '@angular/common/http';
33

44
import { ListStateStore } from '@stratosui/core';
@@ -54,6 +54,10 @@ export class CfOrgQuotasSignalConfigService {
5454

5555
view!: ViewPipeline<StOrgQuota>;
5656

57+
// Captured so a re-entry (root singleton, but initialize() runs per mount)
58+
// destroys the prior filter effect instead of stacking one per navigation.
59+
private filterEffect?: EffectRef;
60+
5761
initialize(cnsiGuid: string): void {
5862
this.cnsiGuid = cnsiGuid;
5963
this.source = new CnsiOrgQuotasSource(cnsiGuid, this.http);
@@ -66,8 +70,9 @@ export class CfOrgQuotasSignalConfigService {
6670
this._sortExtractors.asReadonly(),
6771
);
6872

73+
this.filterEffect?.destroy();
6974
runInInjectionContext(this.injector, () => {
70-
effect(() => {
75+
this.filterEffect = effect(() => {
7176
const q = this.nameFilter().trim().toLowerCase();
7277
this.filter.set((quota: StOrgQuota) => {
7378
if (!q) return true;

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/cf-security-groups/cf-security-groups-signal-config.service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
1+
import { EffectRef, Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
22
import { HttpClient } from '@angular/common/http';
33

44
import { ListStateStore } from '@stratosui/core';
@@ -45,6 +45,10 @@ export class CfSecurityGroupsSignalConfigService {
4545

4646
view!: ViewPipeline<StSecurityGroup>;
4747

48+
// Captured so a re-entry (root singleton, but initialize() runs per mount)
49+
// destroys the prior filter effect instead of stacking one per navigation.
50+
private filterEffect?: EffectRef;
51+
4852
initialize(cnsiGuid: string): void {
4953
this.cnsiGuid = cnsiGuid;
5054
this.source = new CnsiSecurityGroupsSource(cnsiGuid, this.http);
@@ -57,8 +61,9 @@ export class CfSecurityGroupsSignalConfigService {
5761
this._sortExtractors.asReadonly(),
5862
);
5963

64+
this.filterEffect?.destroy();
6065
runInInjectionContext(this.injector, () => {
61-
effect(() => {
66+
this.filterEffect = effect(() => {
6267
const q = this.nameFilter().trim().toLowerCase();
6368
this.filter.set((g: StSecurityGroup) => {
6469
if (!q) return true;

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/cf-space-quotas/cf-space-quotas-signal-config.service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
1+
import { EffectRef, Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
22
import { HttpClient } from '@angular/common/http';
33

44
import { ListStateStore } from '@stratosui/core';
@@ -57,6 +57,10 @@ export class CfSpaceQuotasSignalConfigService {
5757

5858
view!: ViewPipeline<StSpaceQuota>;
5959

60+
// Captured so a re-entry (root singleton, but initialize() runs per mount)
61+
// destroys the prior filter effect instead of stacking one per navigation.
62+
private filterEffect?: EffectRef;
63+
6064
initialize(cnsiGuid: string): void {
6165
this.cnsiGuid = cnsiGuid;
6266
this.source = new CnsiSpaceQuotasSource(cnsiGuid, this.http);
@@ -69,8 +73,9 @@ export class CfSpaceQuotasSignalConfigService {
6973
this._sortExtractors.asReadonly(),
7074
);
7175

76+
this.filterEffect?.destroy();
7277
runInInjectionContext(this.injector, () => {
73-
effect(() => {
78+
this.filterEffect = effect(() => {
7479
const q = this.nameFilter().trim().toLowerCase();
7580
const base = this.basePredicate();
7681
this.filter.set((quota: StSpaceQuota) => {

src/frontend/packages/cloud-foundry/src/shared/signal-list-configs/cf-stacks/cf-stacks-signal-config.service.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
1+
import { EffectRef, Injectable, Injector, Signal, WritableSignal, effect, inject, runInInjectionContext, signal } from '@angular/core';
22
import { HttpClient } from '@angular/common/http';
33

44
import { ListStateStore } from '@stratosui/core';
@@ -46,6 +46,10 @@ export class CfStacksSignalConfigService {
4646

4747
view!: ViewPipeline<StStack>;
4848

49+
// Captured so a re-entry (root singleton, but initialize() runs per mount)
50+
// destroys the prior filter effect instead of stacking one per navigation.
51+
private filterEffect?: EffectRef;
52+
4953
initialize(cnsiGuid: string): void {
5054
this.cnsiGuid = cnsiGuid;
5155
this.source = new CnsiStacksSource(cnsiGuid, this.http);
@@ -58,8 +62,9 @@ export class CfStacksSignalConfigService {
5862
this._sortExtractors.asReadonly(),
5963
);
6064

65+
this.filterEffect?.destroy();
6166
runInInjectionContext(this.injector, () => {
62-
effect(() => {
67+
this.filterEffect = effect(() => {
6368
const q = this.nameFilter().trim().toLowerCase();
6469
this.filter.set((s: StStack) => {
6570
if (!q) return true;

0 commit comments

Comments
 (0)