From 669fb0f00546634517dcfb37cb5091497afde918 Mon Sep 17 00:00:00 2001 From: "opensearch-workspace-development[bot]" <144193788+opensearch-workspace-development[bot]@users.noreply.github.com> Date: Thu, 7 Sep 2023 11:54:31 +0800 Subject: [PATCH] refactor: derive currentWorkspace in workspaces_service (#126) (#136) (cherry picked from commit 56c897118a462ae75ba0ce3fbe21b323249f85ab) Signed-off-by: Yulong Ruan Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../collapsible_nav.test.tsx.snap | 60 ++++++++--------- .../header/__snapshots__/header.test.tsx.snap | 14 ++-- .../workspace/workspaces_service.mock.ts | 6 +- .../public/workspace/workspaces_service.ts | 47 +++++++++++-- src/plugins/workspace/public/plugin.ts | 4 +- .../workspace/public/workspace_client.ts | 66 ++----------------- 6 files changed, 90 insertions(+), 107 deletions(-) diff --git a/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav.test.tsx.snap index 40e51d52de3..27840e9fc62 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/collapsible_nav.test.tsx.snap @@ -341,7 +341,7 @@ exports[`CollapsibleNav renders links grouped by category 1`] = ` "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -511,7 +511,7 @@ exports[`CollapsibleNav renders links grouped by category 1`] = ` "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -653,9 +653,9 @@ exports[`CollapsibleNav renders links grouped by category 1`] = ` className="euiText euiText--medium" > - + OpenSearch Analytics - + @@ -2279,7 +2279,7 @@ exports[`CollapsibleNav renders the default nav 1`] = ` "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -2540,7 +2540,7 @@ exports[`CollapsibleNav renders the default nav 2`] = ` "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -2801,7 +2801,7 @@ exports[`CollapsibleNav renders the default nav 3`] = ` "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -2929,7 +2929,7 @@ exports[`CollapsibleNav renders the default nav 3`] = ` "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -3034,9 +3034,9 @@ exports[`CollapsibleNav renders the default nav 3`] = ` className="euiText euiText--medium" > - + OpenSearch Analytics - + @@ -3616,7 +3616,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 1`] = "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -3897,7 +3897,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 1`] = "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -4150,9 +4150,9 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 1`] = className="euiText euiText--medium" > - + OpenSearch Analytics - + @@ -5166,7 +5166,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 2`] = "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -5447,7 +5447,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 2`] = "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -5700,9 +5700,9 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 2`] = className="euiText euiText--medium" > - + OpenSearch Analytics - + @@ -6714,7 +6714,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 3`] = "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -6995,7 +6995,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 3`] = "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -7248,9 +7248,9 @@ exports[`CollapsibleNav renders the nav bar with custom logo in dark mode 3`] = className="euiText euiText--medium" > - + OpenSearch Analytics - + @@ -8265,7 +8265,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in default mode 1`] "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -8509,7 +8509,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in default mode 1`] "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -8725,9 +8725,9 @@ exports[`CollapsibleNav renders the nav bar with custom logo in default mode 1`] className="euiText euiText--medium" > - + OpenSearch Analytics - + @@ -9739,7 +9739,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in default mode 2`] "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -9983,7 +9983,7 @@ exports[`CollapsibleNav renders the nav bar with custom logo in default mode 2`] "observers": Array [], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -10199,9 +10199,9 @@ exports[`CollapsibleNav renders the nav bar with custom logo in default mode 2`] className="euiText euiText--medium" > - + OpenSearch Analytics - + diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap index 3acb1850919..067a1f427d4 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap @@ -1893,7 +1893,7 @@ exports[`Header handles visibility and lock changes 1`] = ` ], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -6038,7 +6038,7 @@ exports[`Header handles visibility and lock changes 1`] = ` ], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -6241,7 +6241,7 @@ exports[`Header handles visibility and lock changes 1`] = ` ], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -6383,9 +6383,9 @@ exports[`Header handles visibility and lock changes 1`] = ` className="euiText euiText--medium" > - + OpenSearch Dashboards - + @@ -8525,7 +8525,7 @@ exports[`Header renders condensed header 1`] = ` ], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, @@ -11715,7 +11715,7 @@ exports[`Header renders condensed header 1`] = ` ], "thrownError": null, }, - "hasFetchedWorkspaceList$": BehaviorSubject { + "initialized$": BehaviorSubject { "_isScalar": false, "_value": false, "closed": false, diff --git a/src/core/public/workspace/workspaces_service.mock.ts b/src/core/public/workspace/workspaces_service.mock.ts index ee813fffa8f..3c35315aa85 100644 --- a/src/core/public/workspace/workspaces_service.mock.ts +++ b/src/core/public/workspace/workspaces_service.mock.ts @@ -9,14 +9,14 @@ import { WorkspaceAttribute } from '..'; const currentWorkspaceId$ = new BehaviorSubject(''); const workspaceList$ = new BehaviorSubject([]); const currentWorkspace$ = new BehaviorSubject(null); -const hasFetchedWorkspaceList$ = new BehaviorSubject(false); +const initialized$ = new BehaviorSubject(false); const workspaceEnabled$ = new BehaviorSubject(false); const createWorkspacesSetupContractMock = () => ({ currentWorkspaceId$, workspaceList$, currentWorkspace$, - hasFetchedWorkspaceList$, + initialized$, workspaceEnabled$, registerWorkspaceMenuRender: jest.fn(), }); @@ -25,7 +25,7 @@ const createWorkspacesStartContractMock = () => ({ currentWorkspaceId$, workspaceList$, currentWorkspace$, - hasFetchedWorkspaceList$, + initialized$, workspaceEnabled$, renderWorkspaceMenu: jest.fn(), }); diff --git a/src/core/public/workspace/workspaces_service.ts b/src/core/public/workspace/workspaces_service.ts index 9b386599cea..b94bf0a17e2 100644 --- a/src/core/public/workspace/workspaces_service.ts +++ b/src/core/public/workspace/workspaces_service.ts @@ -3,7 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { BehaviorSubject } from 'rxjs'; +import { BehaviorSubject, combineLatest } from 'rxjs'; +import { isEqual } from 'lodash'; + import { CoreService, WorkspaceAttribute } from '../../types'; import { InternalApplicationStart } from '../application'; import { HttpSetup } from '../http'; @@ -23,7 +25,11 @@ export interface WorkspaceObservables { currentWorkspace$: BehaviorSubject; workspaceList$: BehaviorSubject; workspaceEnabled$: BehaviorSubject; - hasFetchedWorkspaceList$: BehaviorSubject; + initialized$: BehaviorSubject; +} + +enum WORKSPACE_ERROR_REASON_MAP { + WORKSPACE_STALED = 'WORKSPACE_STALED', } /** @@ -41,16 +47,45 @@ export class WorkspaceService implements CoreService(''); private workspaceList$ = new BehaviorSubject([]); private currentWorkspace$ = new BehaviorSubject(null); - private hasFetchedWorkspaceList$ = new BehaviorSubject(false); + private initialized$ = new BehaviorSubject(false); private workspaceEnabled$ = new BehaviorSubject(false); private _renderWorkspaceMenu: WorkspaceMenuRenderFn | null = null; + constructor() { + combineLatest([this.initialized$, this.workspaceList$, this.currentWorkspaceId$]).subscribe( + ([workspaceInitialized, workspaceList, currentWorkspaceId]) => { + if (workspaceInitialized) { + const currentWorkspace = workspaceList.find((w) => w && w.id === currentWorkspaceId); + + /** + * Do a simple idempotent verification here + */ + if (!isEqual(currentWorkspace, this.currentWorkspace$.getValue())) { + this.currentWorkspace$.next(currentWorkspace ?? null); + } + + if (currentWorkspaceId && !currentWorkspace?.id) { + /** + * Current workspace is staled + */ + this.currentWorkspaceId$.error({ + reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED, + }); + this.currentWorkspace$.error({ + reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED, + }); + } + } + } + ); + } + public setup(): WorkspaceSetup { return { currentWorkspaceId$: this.currentWorkspaceId$, currentWorkspace$: this.currentWorkspace$, workspaceList$: this.workspaceList$, - hasFetchedWorkspaceList$: this.hasFetchedWorkspaceList$, + initialized$: this.initialized$, workspaceEnabled$: this.workspaceEnabled$, registerWorkspaceMenuRender: (render: WorkspaceMenuRenderFn) => (this._renderWorkspaceMenu = render), @@ -68,7 +103,7 @@ export class WorkspaceService implements CoreService public async setup(core: CoreSetup, { savedObjectsManagement }: WorkspacePluginSetupDeps) { core.workspaces.workspaceEnabled$.next(true); core.workspaces.registerWorkspaceMenuRender(renderWorkspaceMenu); - + const workspaceClient = new WorkspaceClient(core.http, core.workspaces); - workspaceClient.init(); + await workspaceClient.init(); /** * Retrieve workspace id from url diff --git a/src/plugins/workspace/public/workspace_client.ts b/src/plugins/workspace/public/workspace_client.ts index a43624a5f07..60adc80a3d2 100644 --- a/src/plugins/workspace/public/workspace_client.ts +++ b/src/plugins/workspace/public/workspace_client.ts @@ -2,9 +2,6 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import { combineLatest } from 'rxjs'; -import { isEqual } from 'lodash'; - import { HttpFetchError, HttpFetchOptions, @@ -16,10 +13,6 @@ import { WorkspacePermissionMode } from '../../../core/public'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; -enum WORKSPACE_ERROR_REASON_MAP { - WORKSPACE_STALED = 'WORKSPACE_STALED', -} - const join = (...uriComponents: Array) => uriComponents .filter((comp): comp is string => Boolean(comp)) @@ -67,57 +60,14 @@ export class WorkspaceClient { constructor(http: HttpSetup, workspaces: WorkspaceSetup) { this.http = http; this.workspaces = workspaces; - - combineLatest([ - workspaces.hasFetchedWorkspaceList$, - workspaces.workspaceList$, - workspaces.currentWorkspaceId$, - ]).subscribe(([hasFetchedWorkspaceList, workspaceList, currentWorkspaceId]) => { - if (hasFetchedWorkspaceList) { - const currentWorkspace = this.findWorkspace([workspaceList, currentWorkspaceId]); - - /** - * Do a simple idempotent verification here - */ - if (!isEqual(currentWorkspace, workspaces.currentWorkspace$.getValue())) { - workspaces.currentWorkspace$.next(currentWorkspace); - } - - if (currentWorkspaceId && !currentWorkspace?.id) { - /** - * Current workspace is staled - */ - workspaces.currentWorkspaceId$.error({ - reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED, - }); - workspaces.currentWorkspace$.error({ - reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED, - }); - } - } - }); } /** * Initialize workspace list */ - public init() { - this.updateWorkspaceListAndNotify(); - } - - private findWorkspace(payload: [WorkspaceAttribute[], string]): WorkspaceAttribute | null { - const [workspaceList, currentWorkspaceId] = payload; - if (!currentWorkspaceId || !workspaceList || !workspaceList.length) { - return null; - } - - const findItem = workspaceList.find((item) => item?.id === currentWorkspaceId); - - if (!findItem) { - return null; - } - - return findItem; + public async init() { + await this.updateWorkspaceList(); + this.workspaces.initialized$.next(true); } /** @@ -155,7 +105,7 @@ export class WorkspaceClient { return [WORKSPACES_API_BASE_URL, join(...path)].filter((item) => item).join('/'); } - private async updateWorkspaceListAndNotify(): Promise { + private async updateWorkspaceList(): Promise { const result = await this.list({ perPage: 999, }); @@ -163,8 +113,6 @@ export class WorkspaceClient { if (result?.success) { this.workspaces.workspaceList$.next(result.result.workspaces); } - - this.workspaces.hasFetchedWorkspaceList$.next(true); } public async enterWorkspace(id: string): Promise> { @@ -234,7 +182,7 @@ export class WorkspaceClient { }); if (result.success) { - this.updateWorkspaceListAndNotify(); + this.updateWorkspaceList(); } return result; @@ -250,7 +198,7 @@ export class WorkspaceClient { const result = await this.safeFetch(this.getPath([id]), { method: 'DELETE' }); if (result.success) { - this.updateWorkspaceListAndNotify(); + this.updateWorkspaceList(); } return result; @@ -325,7 +273,7 @@ export class WorkspaceClient { }); if (result.success) { - this.updateWorkspaceListAndNotify(); + this.updateWorkspaceList(); } return result;