From 4ceb732dd3589c761a4110a780254de8087bca96 Mon Sep 17 00:00:00 2001 From: suzhou Date: Thu, 15 Jun 2023 15:44:29 +0800 Subject: [PATCH] feat: use BehaviorObject and optimize code (#14) Signed-off-by: SuZhoue-Joe --- src/core/public/workspace/consts.ts | 4 + .../public/workspace/workspaces_client.ts | 83 +++++++------------ src/core/server/workspaces/routes/index.ts | 10 +-- src/plugins/workspace/public/plugin.ts | 18 ++-- 4 files changed, 48 insertions(+), 67 deletions(-) diff --git a/src/core/public/workspace/consts.ts b/src/core/public/workspace/consts.ts index 77f9144a27a3..662baeaa5d19 100644 --- a/src/core/public/workspace/consts.ts +++ b/src/core/public/workspace/consts.ts @@ -6,3 +6,7 @@ export const WORKSPACES_API_BASE_URL = '/api/workspaces'; export const WORKSPACE_ID_QUERYSTRING_NAME = '_workspace_id_'; + +export enum WORKSPACE_ERROR_REASON_MAP { + WORKSPACE_STALED = 'WORKSPACE_STALED', +} diff --git a/src/core/public/workspace/workspaces_client.ts b/src/core/public/workspace/workspaces_client.ts index 8508921cbb0d..91d83dd1639f 100644 --- a/src/core/public/workspace/workspaces_client.ts +++ b/src/core/public/workspace/workspaces_client.ts @@ -2,12 +2,11 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import { resolve as resolveUrl } from 'url'; import type { PublicContract } from '@osd/utility-types'; -import { Subject } from 'rxjs'; +import { BehaviorSubject } from 'rxjs'; import { HttpFetchError, HttpFetchOptions, HttpSetup } from '../http'; import { WorkspaceAttribute, WorkspaceFindOptions } from '.'; -import { WORKSPACES_API_BASE_URL } from './consts'; +import { WORKSPACES_API_BASE_URL, WORKSPACE_ERROR_REASON_MAP } from './consts'; /** * WorkspacesClientContract as implemented by the {@link WorkspacesClient} @@ -40,27 +39,25 @@ type IResponse = */ export class WorkspacesClient { private http: HttpSetup; - private currentWorkspaceId = ''; - public currentWorkspaceId$ = new Subject(); - public workspaceList$ = new Subject(); + public currentWorkspaceId$ = new BehaviorSubject(''); + public workspaceList$ = new BehaviorSubject([]); constructor(http: HttpSetup) { this.http = http; - this.currentWorkspaceId$.subscribe( - (currentWorkspaceId) => (this.currentWorkspaceId = currentWorkspaceId) - ); /** * Add logic to check if current workspace id is still valid * If not, remove the current workspace id and notify other subscribers */ this.workspaceList$.subscribe(async (workspaceList) => { - const currentWorkspaceId = this.currentWorkspaceId; + const currentWorkspaceId = this.currentWorkspaceId$.getValue(); if (currentWorkspaceId) { const findItem = workspaceList.find((item) => item.id === currentWorkspaceId); if (!findItem) { /** * Current workspace is staled */ - this.currentWorkspaceId$.next(''); + this.currentWorkspaceId$.error({ + reason: WORKSPACE_ERROR_REASON_MAP.WORKSPACE_STALED, + }); } } }); @@ -71,29 +68,39 @@ export class WorkspacesClient { this.updateWorkspaceListAndNotify(); } - private catchedFetch = async >( + /** + * Add a non-throw-error fetch method for internal use. + */ + private safeFetch = async ( path: string, options: HttpFetchOptions - ) => { + ): Promise> => { try { - return await this.http.fetch(path, options); + return await this.http.fetch>(path, options); } catch (error: unknown) { - if (error instanceof HttpFetchError || error instanceof Error) { + if (error instanceof HttpFetchError) { + return { + success: false, + error: error.body?.message || error.body?.error || error.message, + }; + } + + if (error instanceof Error) { return { success: false, error: error.message, - } as T; + }; } return { success: false, error: 'Unknown error', - } as T; + }; } }; private getPath(path: Array): string { - return resolveUrl(`${WORKSPACES_API_BASE_URL}/`, join(...path)); + return [WORKSPACES_API_BASE_URL, join(...path)].filter((item) => item).join('/'); } private async updateWorkspaceListAndNotify(): Promise { @@ -128,7 +135,7 @@ export class WorkspacesClient { } public async getCurrentWorkspaceId(): Promise> { - const currentWorkspaceId = this.currentWorkspaceId; + const currentWorkspaceId = this.currentWorkspaceId$.getValue(); if (!currentWorkspaceId) { return { success: false, @@ -161,16 +168,9 @@ export class WorkspacesClient { public async create( attributes: Omit ): Promise> { - if (!attributes) { - return { - success: false, - error: 'Workspace attributes is required', - }; - } - const path = this.getPath([]); - const result = await this.catchedFetch>(path, { + const result = await this.safeFetch(path, { method: 'POST', body: JSON.stringify({ attributes, @@ -191,14 +191,7 @@ export class WorkspacesClient { * @returns */ public async delete(id: string): Promise> { - if (!id) { - return { - success: false, - error: 'Id is required.', - }; - } - - const result = await this.catchedFetch(this.getPath([id]), { method: 'DELETE' }); + const result = await this.safeFetch(this.getPath([id]), { method: 'DELETE' }); if (result.success) { this.updateWorkspaceListAndNotify(); @@ -230,7 +223,7 @@ export class WorkspacesClient { }> > => { const path = this.getPath(['_list']); - return this.catchedFetch(path, { + return this.safeFetch(path, { method: 'POST', body: JSON.stringify(options || {}), }); @@ -243,15 +236,8 @@ export class WorkspacesClient { * @returns The workspace for the given id. */ public async get(id: string): Promise> { - if (!id) { - return { - success: false, - error: 'Id is required.', - }; - } - const path = this.getPath([id]); - return this.catchedFetch(path, { + return this.safeFetch(path, { method: 'GET', }); } @@ -267,19 +253,12 @@ export class WorkspacesClient { id: string, attributes: Partial ): Promise> { - if (!id || !attributes) { - return { - success: false, - error: 'Id and attributes are required.', - }; - } - const path = this.getPath([id]); const body = { attributes, }; - const result = await this.catchedFetch(path, { + const result = await this.safeFetch(path, { method: 'PUT', body: JSON.stringify(body), }); diff --git a/src/core/server/workspaces/routes/index.ts b/src/core/server/workspaces/routes/index.ts index 24345b6a34d9..980364103ba8 100644 --- a/src/core/server/workspaces/routes/index.ts +++ b/src/core/server/workspaces/routes/index.ts @@ -7,9 +7,7 @@ import { InternalHttpServiceSetup } from '../../http'; import { Logger } from '../../logging'; import { IWorkspaceDBImpl } from '../types'; -export const WORKSPACES_API_BASE_URL = '/api/workspaces'; - -export const WORKSPACE_ID_QUERYSTRING_NAME = '_workspace_id_'; +const WORKSPACES_API_BASE_URL = '/api/workspaces'; export function registerRoutes({ client, @@ -71,7 +69,7 @@ export function registerRoutes({ ); router.post( { - path: '/', + path: '', validate: { body: schema.object({ attributes: schema.object({ @@ -98,7 +96,7 @@ export function registerRoutes({ ); router.put( { - path: '/{id}', + path: '/{id?}', validate: { params: schema.object({ id: schema.string(), @@ -130,7 +128,7 @@ export function registerRoutes({ ); router.delete( { - path: '/{id}', + path: '/{id?}', validate: { params: schema.object({ id: schema.string(), diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index 911c9a651137..39069ca09bb8 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -4,7 +4,6 @@ */ import { i18n } from '@osd/i18n'; -import { parse } from 'querystring'; import { CoreSetup, CoreStart, @@ -28,9 +27,9 @@ export class WorkspacesPlugin implements Plugin<{}, {}> { } }); } - private getWorkpsaceIdFromQueryString(): string { - const querystringObject = parse(window.location.search.replace(/^\??/, '')); - return querystringObject[WORKSPACE_ID_QUERYSTRING_NAME] as string; + private getWorkpsaceIdFromQueryString(): string | null { + const searchParams = new URLSearchParams(window.location.search); + return searchParams.get(WORKSPACE_ID_QUERYSTRING_NAME); } private getWorkpsaceIdFromSessionStorage(): string { try { @@ -53,11 +52,6 @@ export class WorkspacesPlugin implements Plugin<{}, {}> { } public async setup(core: CoreSetup) { this.core = core; - /** - * register a listener - */ - this.addWorkspaceListener(); - /** * Retrive workspace id from url or sessionstorage * url > sessionstorage @@ -77,6 +71,12 @@ export class WorkspacesPlugin implements Plugin<{}, {}> { ); } } + + /** + * register a listener + */ + this.addWorkspaceListener(); + core.application.register({ id: WORKSPACE_APP_ID, title: i18n.translate('workspace.settings.title', {