From 323baa9ddb0ae7f3dfa489422d3e4296a2c8bcc6 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 31 Jan 2025 17:36:57 -0500 Subject: [PATCH 1/5] fix(services): perform lazy initialization --- src/app/AppLayout/AppLayout.tsx | 2 + src/app/Shared/Services/Api.service.tsx | 2 +- src/app/Shared/Services/Login.service.tsx | 61 +++++++++++---------- src/app/Shared/Services/Services.tsx | 2 +- src/app/Shared/Services/Targets.service.tsx | 3 - 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/app/AppLayout/AppLayout.tsx b/src/app/AppLayout/AppLayout.tsx index 0d3470af8..9e7627895 100644 --- a/src/app/AppLayout/AppLayout.tsx +++ b/src/app/AppLayout/AppLayout.tsx @@ -130,8 +130,10 @@ export const AppLayout: React.FC = ({ children }) => { }, [theme]); React.useEffect(() => { + serviceContext.login.checkAuth(); serviceContext.api.testBaseServer(); serviceContext.notificationChannel.connect(); + serviceContext.targets.queryForTargets().subscribe(); }, [serviceContext.api, serviceContext.notificationChannel]); React.useEffect(() => { diff --git a/src/app/Shared/Services/Api.service.tsx b/src/app/Shared/Services/Api.service.tsx index cc9f67c70..1daff45e5 100644 --- a/src/app/Shared/Services/Api.service.tsx +++ b/src/app/Shared/Services/Api.service.tsx @@ -1522,7 +1522,7 @@ export class ApiService { return out; } - private sendRequest( + sendRequest( apiVersion: ApiVersion, path: string, config?: RequestInit, diff --git a/src/app/Shared/Services/Login.service.tsx b/src/app/Shared/Services/Login.service.tsx index 059e93f6c..3095e0cb2 100644 --- a/src/app/Shared/Services/Login.service.tsx +++ b/src/app/Shared/Services/Login.service.tsx @@ -18,6 +18,7 @@ import { fromFetch } from 'rxjs/fetch'; import { catchError, concatMap, debounceTime, distinctUntilChanged, finalize, map, tap } from 'rxjs/operators'; import { SessionState } from './service.types'; import type { SettingsService } from './Settings.service'; +import { ApiService } from './Api.service'; export class LoginService { private readonly logout = new ReplaySubject(1); @@ -25,21 +26,22 @@ export class LoginService { private readonly sessionState = new ReplaySubject(1); constructor( - private readonly authority: (path: string) => Observable, + private readonly api: ApiService, private readonly settings: SettingsService, ) { this.sessionState.next(SessionState.CREATING_USER_SESSION); + } - authority('/api/v4/auth') + checkAuth(): void { + this.api + .sendRequest('v4', 'auth', { + credentials: 'include', + mode: 'cors', + method: 'POST', + body: null, + }) .pipe( - concatMap((u) => - fromFetch(u, { - credentials: 'include', - mode: 'cors', - method: 'POST', - body: null, - }), - ), + concatMap((parts) => fromFetch(parts[0], {})), concatMap((response) => { let gapAuth = response?.headers?.get('Gap-Auth'); if (gapAuth) { @@ -69,27 +71,26 @@ export class LoginService { } setLoggedOut(): Observable { - return this.authority('/api/v4/logout').pipe( - concatMap((u) => - fromFetch(u, { - credentials: 'include', - mode: 'cors', - method: 'POST', - body: null, + return this.api + .sendRequest('v4', 'logout', { + credentials: 'include', + mode: 'cors', + method: 'POST', + body: null, + }) + .pipe( + concatMap((response) => { + return of(response).pipe( + map((response) => response.ok), + tap(() => this.resetSessionState()), + ); + }), + catchError((e: Error): ObservableInput => { + window.console.error(JSON.stringify(e, Object.getOwnPropertyNames(e))); + return of(false); }), - ), - concatMap((response) => { - return of(response).pipe( - map((response) => response.ok), - tap(() => this.resetSessionState()), - ); - }), - catchError((e: Error): ObservableInput => { - window.console.error(JSON.stringify(e, Object.getOwnPropertyNames(e))); - return of(false); - }), - finalize(() => this.navigateToLoginPage()), - ); + finalize(() => this.navigateToLoginPage()), + ); } setSessionState(state: SessionState): void { diff --git a/src/app/Shared/Services/Services.tsx b/src/app/Shared/Services/Services.tsx index c7ce969c1..59c1ee174 100644 --- a/src/app/Shared/Services/Services.tsx +++ b/src/app/Shared/Services/Services.tsx @@ -47,7 +47,7 @@ export const defaultContext: CryostatContext = { const target = new TargetService(); const settings = new SettingsService(); -const login = new LoginService(defaultContext.url, settings); +const login = new LoginService(defaultContext, settings); const api = new ApiService(defaultContext, target, NotificationsInstance); const notificationChannel = new NotificationChannel(defaultContext, NotificationsInstance, login); const reports = new ReportService(defaultContext, NotificationsInstance, notificationChannel); diff --git a/src/app/Shared/Services/Targets.service.tsx b/src/app/Shared/Services/Targets.service.tsx index e61966be3..9932fb486 100644 --- a/src/app/Shared/Services/Targets.service.tsx +++ b/src/app/Shared/Services/Targets.service.tsx @@ -30,9 +30,6 @@ export class TargetsService { private readonly notifications: NotificationService, notificationChannel: NotificationChannel, ) { - // just trigger a startup query - this.queryForTargets().subscribe(); - notificationChannel.messages(NotificationCategory.TargetJvmDiscovery).subscribe((v) => { const evt: TargetDiscoveryEvent = v.message.event; switch (evt.kind) { From be4d538bf383f0969287788d5c82cbaa41e2ab0b Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 31 Jan 2025 17:39:42 -0500 Subject: [PATCH 2/5] lint --- src/app/AppLayout/AppLayout.tsx | 2 +- src/app/Shared/Services/Login.service.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/AppLayout/AppLayout.tsx b/src/app/AppLayout/AppLayout.tsx index 9e7627895..e485b642f 100644 --- a/src/app/AppLayout/AppLayout.tsx +++ b/src/app/AppLayout/AppLayout.tsx @@ -134,7 +134,7 @@ export const AppLayout: React.FC = ({ children }) => { serviceContext.api.testBaseServer(); serviceContext.notificationChannel.connect(); serviceContext.targets.queryForTargets().subscribe(); - }, [serviceContext.api, serviceContext.notificationChannel]); + }, [serviceContext.login, serviceContext.api, serviceContext.notificationChannel, serviceContext.targets]); React.useEffect(() => { addSubscription( diff --git a/src/app/Shared/Services/Login.service.tsx b/src/app/Shared/Services/Login.service.tsx index 3095e0cb2..bdb06dd98 100644 --- a/src/app/Shared/Services/Login.service.tsx +++ b/src/app/Shared/Services/Login.service.tsx @@ -16,9 +16,9 @@ import { Observable, ObservableInput, of, ReplaySubject } from 'rxjs'; import { fromFetch } from 'rxjs/fetch'; import { catchError, concatMap, debounceTime, distinctUntilChanged, finalize, map, tap } from 'rxjs/operators'; +import { ApiService } from './Api.service'; import { SessionState } from './service.types'; import type { SettingsService } from './Settings.service'; -import { ApiService } from './Api.service'; export class LoginService { private readonly logout = new ReplaySubject(1); From fce835f5c0beeb07f62194fc76afd9191ac70e4b Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 31 Jan 2025 18:05:17 -0500 Subject: [PATCH 3/5] accidental fromFetch --- src/app/Shared/Services/Login.service.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/app/Shared/Services/Login.service.tsx b/src/app/Shared/Services/Login.service.tsx index bdb06dd98..ad62a7c06 100644 --- a/src/app/Shared/Services/Login.service.tsx +++ b/src/app/Shared/Services/Login.service.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ import { Observable, ObservableInput, of, ReplaySubject } from 'rxjs'; -import { fromFetch } from 'rxjs/fetch'; import { catchError, concatMap, debounceTime, distinctUntilChanged, finalize, map, tap } from 'rxjs/operators'; import { ApiService } from './Api.service'; import { SessionState } from './service.types'; @@ -41,7 +40,6 @@ export class LoginService { body: null, }) .pipe( - concatMap((parts) => fromFetch(parts[0], {})), concatMap((response) => { let gapAuth = response?.headers?.get('Gap-Auth'); if (gapAuth) { From 2e5b4c265d311a5c420841e57efe69981a2ff7ac Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 31 Jan 2025 18:05:24 -0500 Subject: [PATCH 4/5] test --- .../Shared/Services/Login.service.test.tsx | 64 ++++++++++++++----- 1 file changed, 47 insertions(+), 17 deletions(-) diff --git a/src/test/Shared/Services/Login.service.test.tsx b/src/test/Shared/Services/Login.service.test.tsx index bb93a559b..b1466e312 100644 --- a/src/test/Shared/Services/Login.service.test.tsx +++ b/src/test/Shared/Services/Login.service.test.tsx @@ -14,25 +14,22 @@ * limitations under the License. */ +import { ApiService } from '@app/Shared/Services/Api.service'; import { LoginService } from '@app/Shared/Services/Login.service'; +import { NotificationService } from '@app/Shared/Services/Notifications.service'; import { SessionState } from '@app/Shared/Services/service.types'; import { SettingsService } from '@app/Shared/Services/Settings.service'; +import { TargetService } from '@app/Shared/Services/Target.service'; import { firstValueFrom, of, timeout } from 'rxjs'; -import { fromFetch } from 'rxjs/fetch'; - -jest.mock('rxjs/fetch', () => { - return { - fromFetch: jest.fn((_url: unknown, _opts: unknown): unknown => of()), - }; -}); jest.unmock('@app/Shared/Services/Login.service'); +jest.mock('@app/Shared/Services/Api.service'); describe('Login.service', () => { - const mockFromFetch = fromFetch as jest.Mock; let svc: LoginService; describe('setLoggedOut', () => { + let apiSvc: ApiService; let settingsSvc: SettingsService; let saveLocation: Location; @@ -58,6 +55,14 @@ describe('Login.service', () => { }); beforeEach(() => { + apiSvc = new ApiService( + { + headers: () => of(new Headers()), + url: (p) => of(`./${p}`), + }, + {} as TargetService, + {} as NotificationService, + ); settingsSvc = new SettingsService(); (settingsSvc.webSocketDebounceMs as jest.Mock).mockReturnValue(0); }); @@ -90,31 +95,56 @@ describe('Login.service', () => { }, }); const logoutResp = createResponse(200, true); - mockFromFetch - .mockReturnValueOnce(of(initAuthResp)) - .mockReturnValueOnce(of(authResp)) - .mockReturnValueOnce(of(logoutResp)); + jest + .spyOn(apiSvc, 'sendRequest') + .mockReturnValue( + of({ + ok: true, + json: new Promise((resolve) => resolve(initAuthResp)), + } as unknown as Response), + ) + .mockReturnValue( + of({ + ok: true, + json: new Promise((resolve) => resolve(authResp)), + } as unknown as Response), + ) + .mockReturnValue( + of({ + ok: true, + json: new Promise((resolve) => resolve(logoutResp)), + } as unknown as Response), + ); window.location.href = 'https://example.com/'; location.href = window.location.href; - svc = new LoginService((p) => of(`.${p}`), settingsSvc); + svc = new LoginService(apiSvc, settingsSvc); }); it('should emit true', async () => { + const logoutResp = createResponse(200, true); + jest.spyOn(apiSvc, 'sendRequest').mockReturnValue( + of({ + ok: true, + json: new Promise((resolve) => resolve(logoutResp)), + } as unknown as Response), + ); const result = await firstValueFrom(svc.setLoggedOut()); expect(result).toBeTruthy(); }); it('should make expected API calls', async () => { - expect(mockFromFetch).toHaveBeenCalledTimes(1); - expect(mockFromFetch).toHaveBeenNthCalledWith(1, `./api/v4/auth`, { + expect(apiSvc.sendRequest).toHaveBeenCalledTimes(0); + svc.checkAuth(); + expect(apiSvc.sendRequest).toHaveBeenCalledTimes(1); + expect(apiSvc.sendRequest).toHaveBeenNthCalledWith(1, 'v4', 'auth', { credentials: 'include', mode: 'cors', method: 'POST', body: null, }); await firstValueFrom(svc.setLoggedOut()); - expect(mockFromFetch).toHaveBeenCalledTimes(2); - expect(mockFromFetch).toHaveBeenNthCalledWith(2, `./api/v4/logout`, { + expect(apiSvc.sendRequest).toHaveBeenCalledTimes(2); + expect(apiSvc.sendRequest).toHaveBeenNthCalledWith(2, 'v4', 'logout', { credentials: 'include', mode: 'cors', method: 'POST', From 753fad5dd5de4073954b8cc7e80466ee7512adb4 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 31 Jan 2025 18:06:51 -0500 Subject: [PATCH 5/5] fixup --- src/app/Shared/Services/Services.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/app/Shared/Services/Services.tsx b/src/app/Shared/Services/Services.tsx index 59c1ee174..8dc77f4d6 100644 --- a/src/app/Shared/Services/Services.tsx +++ b/src/app/Shared/Services/Services.tsx @@ -47,20 +47,20 @@ export const defaultContext: CryostatContext = { const target = new TargetService(); const settings = new SettingsService(); -const login = new LoginService(defaultContext, settings); const api = new ApiService(defaultContext, target, NotificationsInstance); +const login = new LoginService(api, settings); const notificationChannel = new NotificationChannel(defaultContext, NotificationsInstance, login); const reports = new ReportService(defaultContext, NotificationsInstance, notificationChannel); const targets = new TargetsService(api, NotificationsInstance, notificationChannel); const defaultServices: Services = { target, - targets, - reports, - api, - notificationChannel, settings, + api, login, + notificationChannel, + reports, + targets, }; const ServiceContext: React.Context = React.createContext(defaultServices);