fix: leaks, hangs and visible bugs from the idiom audit

changelog.d/0013-audit-leaks-hangs-rendering.md

@@ -0,0 +1,41 @@
+[BugFixes]
+- Fixed pages that waited forever:
+  - The UAA setup wizard checks the credentials in step 1 and shows an
+    error instead of an endless "Saving configuration" overlay. The local
+    account wizard gets the same overlay fix.
+  - Kubernetes config import no longer stalls at the first Connect, and
+    moves on past a cluster that fails to register or connect.
+  - The role picker in Add User and Manage Roles no longer shows
+    "Loading organizations..." forever; a failure shows Retry.
+  - The navigation progress bar no longer stays at 95% after a page
+    fails to load.
+  - After a failed session check near expiry, navigation no longer stops
+    working until a reload.
+- Fixed views that did not show or showed out-of-date content:
+  - SSH sessions show their progress and server errors.
+  - Deploy keeps its completion notice after you leave the page and keeps
+    every log line. A second deploy from the same wizard is labelled
+    "Deploy again".
+  - The application Autoscale tab is back. It shows the reason when the
+    autoscaler refuses a request, and Create Policy only when there is no
+    policy. Its scaling history and metric charts load again; every
+    request for them had failed with an HTTP 400 error.
+  - The git tab no longer stays on "Retrieving Git Details" after a
+    failed lookup.
+  - Service broker cards show the Space row, and the broker URL as the
+    link text.
+  - Home page application cards follow start, stop and delete.
+  - Helm release pages no longer show earlier data after a failed reload,
+    and the Helm catalog sidebar shows.
+  - The Kubernetes resource viewer shows its header, and Delete works.
+  - Metrics charts with a time range selector render.
+  - An active user is no longer warned and logged out as idle after a
+    short session check failure.
+- Stopped background work that kept running after you left a page:
+  - the application stats poll (one request per Cloud Foundry every 30
+    seconds, until a reload);
+  - the metrics polls;
+  - watches on endpoints, permissions and quotas.
+- Disconnecting a Kubernetes endpoint clears its cached data and errors.
+- Removed the `users/remove` page, which nothing linked to any more. The
+  users tabs remove users directly.

src/frontend/packages/cf-autoscaler/src/cf-autoscaler-package.module.spec.ts

@@ -0,0 +1,13 @@
+import { describe, it, expect } from 'vitest';
+
+import { getTabsFromExtensions, StratosTabType } from '../../core/src/core/extension/extension-service';
+import { CfAutoscalerPackageModule } from './cf-autoscaler-package.module';
+
+describe('CfAutoscalerPackageModule', () => {
+  // The app imports only this module from the package, so it must load the
+  // tab component for its decorator to register the Autoscale tab
+  it('registers the Autoscale application tab', () => {
+    expect(CfAutoscalerPackageModule).toBeDefined();
+    expect(getTabsFromExtensions(StratosTabType.Application).map(t => t.link)).toContain('autoscale');
+  });
+});

src/frontend/packages/cf-autoscaler/src/cf-autoscaler-package.module.ts

@@ -1,6 +1,7 @@
 import { Injector, NgModule, inject } from '@angular/core';
 
 import { EntityCatalogModule } from '../../store/src/entity-catalog.module';
+import { CfAutoscalerModule } from './cf-autoscaler.module';
 import { setAutoscalerHelperInjector } from './core/autoscaler-helpers/autoscaler-available';
 import { generateASEntities } from './store/autoscaler-entity-generator';
 
@@ -17,6 +18,8 @@ import { generateASEntities } from './store/autoscaler-entity-generator';
 @NgModule({
   imports: [
     EntityCatalogModule.forFeature(generateASEntities),
+    // Loads the tab component, whose decorator registers the Autoscale tab
+    CfAutoscalerModule,
   ],
 })
 export class CfAutoscalerPackageModule {

src/frontend/packages/cf-autoscaler/src/cf-autoscaler-routing.module.ts

@@ -3,7 +3,7 @@ import { RouterModule, Routes } from '@angular/router';
 
 // Things in this path include add/edit policy stepper, policy metrics page and policy events page
 // They're include like this to ensure those parts are lazy loaded.
-// The core application policy tab is included via CfAutoscalerModule, which is imported in the Application Module
+// The core application policy tab is included via CfAutoscalerModule, which CfAutoscalerPackageModule imports
 const customRoutes: Routes = [
   {
     path: 'autoscaler',

src/frontend/packages/cf-autoscaler/src/core/autoscaler-helpers/autoscaler-available.spec.ts

@@ -0,0 +1,93 @@
+import { computed, Injector, provideZonelessChangeDetection, signal } from '@angular/core';
+import { TestBed } from '@angular/core/testing';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { AutoscalerInfoDataService } from '../../services/domain-data/autoscaler-info-data.service';
+import { fetchAutoscalerInfo, isAutoscalerEnabled, setAutoscalerHelperInjector } from './autoscaler-available';
+
+// The helpers run off the root injector captured by the package module, so
+// whatever watches the data service signals must end with the subscription,
+// not with the (permanent) root injector.
+describe('autoscaler-available helpers', () => {
+  // Each data service signal is read through a computed that counts its
+  // evaluations, so a still-running watcher shows up as extra reads after a
+  // write to the backing signal.
+  let available: ReturnType<typeof signal<boolean>>;
+  let info: ReturnType<typeof signal<unknown>>;
+  let reads: number;
+
+  beforeEach(() => {
+    available = signal(false);
+    info = signal<unknown>(null);
+    reads = 0;
+    const counted = <T>(read: () => T) => computed(() => {
+      reads++;
+      return read();
+    });
+    TestBed.configureTestingModule({
+      providers: [
+        provideZonelessChangeDetection(),
+        {
+          provide: AutoscalerInfoDataService,
+          useValue: {
+            load: vi.fn().mockResolvedValue(undefined),
+            isAvailable: () => counted(() => available()),
+            info: () => counted(() => info()),
+            error: () => computed(() => null),
+            loading: () => computed(() => false),
+          },
+        },
+      ],
+    });
+    setAutoscalerHelperInjector(TestBed.inject(Injector));
+  });
+
+  describe('isAutoscalerEnabled', () => {
+    it('does not read the availability signal until subscribed', () => {
+      isAutoscalerEnabled('ep-1');
+      TestBed.tick();
+      available.set(true);
+      TestBed.tick();
+
+      expect(reads).toBe(0);
+    });
+
+    it('stops reading the availability signal after unsubscribe', () => {
+      const values: boolean[] = [];
+      const sub = isAutoscalerEnabled('ep-1').subscribe(v => values.push(v));
+      TestBed.tick();
+      available.set(true);
+      TestBed.tick();
+      // startWith(false) sits after distinctUntilChanged, so the seed and the
+      // first signal value both come through.
+      expect(values).toEqual([false, false, true]);
+      const readsWhileSubscribed = reads;
+
+      sub.unsubscribe();
+      available.set(false);
+      TestBed.tick();
+
+      expect(reads).toBe(readsWhileSubscribed);
+      expect(values).toEqual([false, false, true]);
+    });
+  });
+
+  describe('fetchAutoscalerInfo', () => {
+    it('stops reading the info signal after unsubscribe', () => {
+      const builds: unknown[] = [];
+      const sub = fetchAutoscalerInfo('ep-1').subscribe(v => builds.push(v.entity?.entity));
+      TestBed.tick();
+      info.set({ build: '1' });
+      TestBed.tick();
+      expect(builds).toEqual([undefined, { build: '1' }]);
+      const readsWhileSubscribed = reads;
+
+      sub.unsubscribe();
+      info.set({ build: '2' });
+      TestBed.tick();
+
+      expect(reads).toBe(readsWhileSubscribed);
+      expect(builds).toEqual([undefined, { build: '1' }]);
+    });
+  });
+});

src/frontend/packages/cf-autoscaler/src/core/autoscaler-helpers/autoscaler-available.ts

@@ -1,8 +1,8 @@
-import { Injector, computed, runInInjectionContext } from '@angular/core';
-import { toObservable } from '@angular/core/rxjs-interop';
+import { Injector, computed } from '@angular/core';
 import { Observable, of } from 'rxjs';
 import { catchError, distinctUntilChanged, map, startWith } from 'rxjs/operators';
 
+import { observeSignal } from '@stratosui/store';
 import { APIResource, EntityInfo } from '../../../../store/src/types/api.types';
 import { AutoscalerInfoDataService } from '../../services/domain-data/autoscaler-info-data.service';
 import { AutoscalerInfo } from '../../store/app-autoscaler.types';
@@ -18,6 +18,11 @@ import { AutoscalerInfo } from '../../store/app-autoscaler.types';
 // args) and the cloud-foundry summary card. The `esf` arg is retained
 // for backwards compat but is unused — the data service is fetched via
 // the module-level injector captured by CfAutoscalerPackageModule.
+//
+// That injector is the root one, which is never destroyed, so the signals
+// are watched with observeSignal: the watcher starts on subscribe and stops
+// on unsubscribe. toObservable would start one at call time that runs for
+// the rest of the session.
 
 let helperInjector: Injector | null = null;
 
@@ -67,7 +72,7 @@ function asEntityInfoObservable(
         : null,
     } as EntityInfo<APIResource<AutoscalerInfo>>;
   });
-  return runInInjectionContext(requireInjector(), () => toObservable(envelope));
+  return observeSignal(envelope, requireInjector());
 }
 
 export const fetchAutoscalerInfo = (
@@ -90,9 +95,7 @@ export const isAutoscalerEnabled = (
 ): Observable<boolean> => {
   const svc = getDataService();
   void svc.load(endpointGuid);
-  return runInInjectionContext(requireInjector(), () =>
-    toObservable(svc.isAvailable(endpointGuid)),
-  ).pipe(
+  return observeSignal(svc.isAvailable(endpointGuid), requireInjector()).pipe(
     distinctUntilChanged(),
     map(value => !!value),
     catchError(() => of(false)),

src/frontend/packages/cf-autoscaler/src/core/autoscaler-helpers/autoscaler-util.spec.ts

@@ -1,6 +1,7 @@
+import { HttpErrorResponse } from '@angular/common/http';
 import { describe, it, expect } from 'vitest';
 import {
-  isEqual, buildLegendData, shiftArray,
+  autoscalerErrorMessage, isEqual, buildLegendData, shiftArray,
 } from './autoscaler-util';
 
 describe('Autoscaler Util Helper', () => {
@@ -71,4 +72,10 @@ describe('Autoscaler Util Helper', () => {
     expect(shiftArray([0, 2, 3], 1)).toEqual([1, 3, 4]);
     expect(shiftArray([1, 6, 9], -1)).toEqual([0, 5, 8]);
   });
+  it('autoscalerErrorMessage', () => {
+    const refusal = new HttpErrorResponse({ status: 403, error: { error: 'The application is not bound to Auto-Scaling service' } });
+    expect(autoscalerErrorMessage(refusal)).toBe('The application is not bound to Auto-Scaling service');
+    const other = new HttpErrorResponse({ status: 500, error: 'boom', url: '/x' });
+    expect(autoscalerErrorMessage(other)).toBe(other.message);
+  });
 });

src/frontend/packages/cf-autoscaler/src/core/autoscaler-helpers/autoscaler-util.ts

@@ -1,3 +1,4 @@
+import { HttpErrorResponse } from '@angular/common/http';
 import { addDays, format, setHours, setMinutes } from 'date-fns';
 
 import {
@@ -142,6 +143,13 @@ export const PolicyAlert = {
   alertInvalidPolicyTriggerScheduleEmpty: 'At least one Scaling Rule or Schedule should be defined.',
 };
 
+// The autoscaler API gives its reason as {"error": "..."}; the request's own
+// message only names the URL and the status
+export function autoscalerErrorMessage(err: unknown): string {
+  const reason = (err as HttpErrorResponse)?.error?.error;
+  return typeof reason === 'string' ? reason : (err as HttpErrorResponse)?.message ?? String(err);
+}
+
 export function isEqual(a: any, b: any): boolean {
   if (typeof a !== typeof b) {
     return false;

src/frontend/packages/cf-autoscaler/src/features/autoscaler-base.component.spec.ts

@@ -57,4 +57,11 @@ describe('AutoscalerBaseComponent', () => {
   it('should be created', () => {
     expect(component).toBeTruthy();
   });
+
+  // The policy editor and the other autoscaler pages get ApplicationService
+  // from this component. It needs the app-detail services the app page
+  // provides; without them every autoscaler page failed with NG0201.
+  it('provides an ApplicationService its pages can use', () => {
+    expect(() => fixture.debugElement.injector.get(ApplicationService)).not.toThrow();
+  });
 });

src/frontend/packages/cf-autoscaler/src/features/autoscaler-base.component.ts

@@ -1,26 +1,15 @@
 import { ChangeDetectionStrategy, Component } from '@angular/core';
-import { ActivatedRoute, RouterOutlet } from '@angular/router';
+import { RouterOutlet } from '@angular/router';
 
-import { ApplicationService, getGuids } from '@stratosui/cloud-foundry';
-import { APP_GUID, CF_GUID } from '@stratosui/core';
+import { APP_DETAIL_PROVIDERS } from '@stratosui/cloud-foundry';
 
 @Component({
   selector: 'app-autoscaler-base',
   templateUrl: './autoscaler-base.component.html',
   changeDetection: ChangeDetectionStrategy.OnPush,
-  providers: [
-    ApplicationService,
-    {
-      provide: CF_GUID,
-      useFactory: getGuids('cf'),
-      deps: [ActivatedRoute]
-    },
-    {
-      provide: APP_GUID,
-      useFactory: getGuids(),
-      deps: [ActivatedRoute]
-    },
-  ],
+  // The same app-detail providers as the app page: ApplicationService
+  // needs AppDetailDataService and the services it depends on.
+  providers: APP_DETAIL_PROVIDERS,
   standalone: true,
   imports: [RouterOutlet]
 })