optimize code according to comments

Signed-off-by: yubonluo <yubonluo@amazon.com>
yubonluo · Apr 3, 2024 · dc77999 · dc77999
1 parent 518cd75
commit dc77999
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 11 deletions.
diff --git a/src/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts
@@ -11,9 +11,11 @@ describe('updateWorkspaceState', () => {
     const requestMock = httpServerMock.createOpenSearchDashboardsRequest();
     updateWorkspaceState(requestMock, {
       id: 'foo',
+      isDashboardAdmin: true,
     });
     expect(getWorkspaceState(requestMock)).toEqual({
       id: 'foo',
+      isDashboardAdmin: true,
     });
   });
 });
diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts
@@ -11,7 +11,6 @@ import {
   Plugin,
   Logger,
   CoreStart,
-  OpenSearchDashboardsRequest,
 } from '../../../core/server';
 import {
   WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
@@ -32,7 +31,7 @@ import {
   SavedObjectsPermissionControlContract,
 } from './permission_control/client';
 import { WorkspacePluginConfigType } from '../config';
-import { isRequestByDashboardAdmin } from './utils';
+import { updateDashboardAdminStateForRequest } from './utils';
 
 export class WorkspacePlugin implements Plugin<{}, {}> {
   private readonly logger: Logger;
@@ -63,7 +62,6 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
 
   private async setupPermission(
     core: CoreSetup,
-    config: WorkspacePluginConfigType,
     { applicationConfig }: AppPluginSetupDependencies
   ) {
     this.permissionControl = new SavedObjectsPermissionControl(this.logger);
@@ -100,7 +98,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
             .catch(() => undefined),
         ]);
 
-        isRequestByDashboardAdmin(
+        updateDashboardAdminStateForRequest(
           request,
           groups,
           users,
@@ -110,9 +108,10 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
         return toolkit.next();
       }
 
+      const config: WorkspacePluginConfigType = await this.config$.pipe(first()).toPromise();
       const configGroups = config.dashboardAdmin.groups || [];
       const configUsers = config.dashboardAdmin.users || [];
-      isRequestByDashboardAdmin(request, groups, users, configGroups, configUsers);
+      updateDashboardAdminStateForRequest(request, groups, users, configGroups, configUsers);
       return toolkit.next();
     });
 
@@ -152,7 +151,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
     );
 
     this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled);
-    if (isPermissionControlEnabled) this.setupPermission(core, config, { applicationConfig });
+    if (isPermissionControlEnabled) await this.setupPermission(core, { applicationConfig });
 
     registerRoutes({
       http: core.http,

diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts
@@ -5,7 +5,11 @@
 
 import { AuthStatus } from '../../../core/server';
 import { httpServerMock, httpServiceMock } from '../../../core/server/mocks';
-import { generateRandomId, getPrincipalsFromRequest, isRequestByDashboardAdmin } from './utils';
+import {
+  generateRandomId,
+  getPrincipalsFromRequest,
+  updateDashboardAdminStateForRequest,
+} from './utils';
 import { getWorkspaceState } from '../../../core/server/utils';
 
 describe('workspace utils', () => {
@@ -81,7 +85,7 @@ describe('workspace utils', () => {
     const users: string[] = [];
     const configGroups: string[] = ['dashboard_admin'];
     const configUsers: string[] = [];
-    isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers);
+    updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers);
     expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(true);
   });
 
@@ -91,7 +95,7 @@ describe('workspace utils', () => {
     const users: string[] = ['dashboard_admin'];
     const configGroups: string[] = [];
     const configUsers: string[] = ['dashboard_admin'];
-    isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers);
+    updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers);
     expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(true);
   });
 
@@ -101,7 +105,7 @@ describe('workspace utils', () => {
     const users: string[] = [];
     const configGroups: string[] = [];
     const configUsers: string[] = ['dashboard_admin'];
-    isRequestByDashboardAdmin(mockRequest, groups, users, configGroups, configUsers);
+    updateDashboardAdminStateForRequest(mockRequest, groups, users, configGroups, configUsers);
     expect(getWorkspaceState(mockRequest)?.isDashboardAdmin).toBe(false);
   });
 });
diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts
@@ -52,7 +52,7 @@ export const getPrincipalsFromRequest = (
   throw new Error('UNEXPECTED_AUTHORIZATION_STATUS');
 };
 
-export const isRequestByDashboardAdmin = (
+export const updateDashboardAdminStateForRequest = (
   request: OpenSearchDashboardsRequest,
   groups: string[],
   users: string[],