refact: move workspace specific logic to savedObjectWrapper

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
SuZhou-Joe · Oct 18, 2023 · 292931a · 292931a
1 parent ddecbcb
commit 292931a
Show file tree

Hide file tree

Showing 6 changed files with 297 additions and 86 deletions.
diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
@@ -280,29 +280,6 @@ export class SavedObjectsRepository {
       }
     }
 
-    let savedObjectWorkspaces = workspaces;
-
-    if (id && overwrite && workspaces) {
-      let currentItem;
-      try {
-        currentItem = await this.get(type, id);
-      } catch (e) {
-        // this.get will throw an error when no items can be found
-      }
-      if (currentItem) {
-        if (
-          SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces(
-            workspaces,
-            currentItem.workspaces
-          ).length
-        ) {
-          throw SavedObjectsErrorHelpers.createConflictError(type, id);
-        } else {
-          savedObjectWorkspaces = currentItem.workspaces;
-        }
-      }
-    }
-
     const migrated = this._migrator.migrateDocument({
       id,
       type,
@@ -313,7 +290,7 @@ export class SavedObjectsRepository {
       migrationVersion,
       updated_at: time,
       ...(Array.isArray(references) && { references }),
-      ...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }),
+      ...(Array.isArray(workspaces) && { workspaces }),
     });
 
     const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc);
@@ -380,16 +357,12 @@ export class SavedObjectsRepository {
 
       const method = object.id && overwrite ? 'index' : 'create';
       const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type);
-      /**
-       * It requires a check when overwriting objects to target workspaces
-       */
-      const requiresWorkspaceCheck = !!(object.id && options.workspaces);
 
       if (object.id == null) object.id = uuid.v1();
 
       let opensearchRequestIndexPayload = {};
 
-      if (requiresNamespacesCheck || requiresWorkspaceCheck) {
+      if (requiresNamespacesCheck) {
         opensearchRequestIndexPayload = {
           opensearchRequestIndex: bulkGetRequestIndexCounter,
         };
@@ -412,7 +385,7 @@ export class SavedObjectsRepository {
       .map(({ value: { object: { type, id } } }) => ({
         _id: this._serializer.generateRawId(namespace, type, id),
         _index: this.getIndexForType(type),
-        _source: ['type', 'namespaces', 'workspaces'],
+        _source: ['type', 'namespaces'],
       }));
     const bulkGetResponse = bulkGetDocs.length
       ? await this.client.mget(
@@ -479,45 +452,7 @@ export class SavedObjectsRepository {
       let savedObjectWorkspaces: string[] | undefined = options.workspaces;
 
       if (expectedBulkGetResult.value.method !== 'create') {
-        const rawId = this._serializer.generateRawId(namespace, object.type, object.id);
-        const findObject =
-          bulkGetResponse?.statusCode !== 404
-            ? bulkGetResponse?.body.docs?.find((item) => item._id === rawId)
-            : null;
-        /**
-         * When it is about to overwrite a object into options.workspace.
-         * We need to check if the options.workspaces is the subset of object.workspaces,
-         * Or it will be treated as a conflict
-         */
-        if (findObject && findObject.found) {
-          const transformedObject = this._serializer.rawToSavedObject(
-            findObject as SavedObjectsRawDoc
-          ) as SavedObject;
-          const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces(
-            options.workspaces,
-            transformedObject.workspaces
-          );
-          if (filteredWorkspaces.length) {
-            /**
-             * options.workspaces is not a subset of object.workspaces,
-             * return a conflict error.
-             */
-            const { id, type } = object;
-            return {
-              tag: 'Left' as 'Left',
-              error: {
-                id,
-                type,
-                error: {
-                  ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)),
-                  metadata: { isNotOverwritable: true },
-                },
-              },
-            };
-          } else {
-            savedObjectWorkspaces = transformedObject.workspaces;
-          }
-        }
+        savedObjectWorkspaces = object.workspaces;
       }
 
       const expectedResult = {
@@ -534,7 +469,7 @@ export class SavedObjectsRepository {
             updated_at: time,
             references: object.references || [],
             originId: object.originId,
-            workspaces: savedObjectWorkspaces,
+            ...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }),
           }) as SavedObjectSanitizedDoc
         ),
       };
@@ -632,7 +567,7 @@ export class SavedObjectsRepository {
     const bulkGetDocs = expectedBulkGetResults.filter(isRight).map(({ value: { type, id } }) => ({
       _id: this._serializer.generateRawId(namespace, type, id),
       _index: this.getIndexForType(type),
-      _source: ['type', 'namespaces', 'workspaces'],
+      _source: ['type', 'namespaces'],
     }));
     const bulkGetResponse = bulkGetDocs.length
       ? await this.client.mget(
@@ -655,24 +590,13 @@ export class SavedObjectsRepository {
       const { type, id, opensearchRequestIndex } = expectedResult.value;
       const doc = bulkGetResponse?.body.docs[opensearchRequestIndex];
       if (doc?.found) {
-        let workspaceConflict = false;
-        if (options.workspaces) {
-          const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc);
-          const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces(
-            options.workspaces,
-            transformedObject.workspaces
-          );
-          if (filteredWorkspaces.length) {
-            workspaceConflict = true;
-          }
-        }
         errors.push({
           id,
           type,
           error: {
             ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)),
             // @ts-expect-error MultiGetHit._source is optional
-            ...((!this.rawDocExistsInNamespace(doc!, namespace) || workspaceConflict) && {
+            ...(!this.rawDocExistsInNamespace(doc!, namespace) && {
               metadata: { isNotOverwritable: true },
             }),
           },

diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts
@@ -68,6 +68,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions {
    * Note: this can only be used for multi-namespace object types.
    */
   initialNamespaces?: string[];
+  /**
+   * workspaces the new created objects belong to
+   */
+  workspaces?: string[];
 }
 
 /**
@@ -91,6 +95,10 @@ export interface SavedObjectsBulkCreateObject<T = unknown> {
    * Note: this can only be used for multi-namespace object types.
    */
   initialNamespaces?: string[];
+  /**
+   * workspaces the objects belong to, will only be used when overwrite is enabled.
+   */
+  workspaces?: string[];
 }
 
 /**

diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts
@@ -4,3 +4,5 @@
  */
 
 export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace';
+export const WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID =
+  'workspace_conflict_control';
diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts
@@ -13,10 +13,13 @@ import {
 import { IWorkspaceClientImpl } from './types';
 import { WorkspaceClient } from './workspace_client';
 import { registerRoutes } from './routes';
+import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict';
+import { WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants';
 
 export class WorkspacePlugin implements Plugin<{}, {}> {
   private readonly logger: Logger;
   private client?: IWorkspaceClientImpl;
+  private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper;
 
   constructor(initializerContext: PluginInitializerContext) {
     this.logger = initializerContext.logger.get('plugins', 'workspace');
@@ -29,6 +32,14 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
 
     await this.client.setup(core);
 
+    this.workspaceConflictControl = new WorkspaceConflictSavedObjectsClientWrapper();
+
+    core.savedObjects.addClientWrapper(
+      -1,
+      WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
+      this.workspaceConflictControl.wrapperFactory
+    );
+
     registerRoutes({
       http: core.http,
       logger: this.logger,
@@ -43,6 +54,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
   public start(core: CoreStart) {
     this.logger.debug('Starting Workspace service');
     this.client?.setSavedObjects(core.savedObjects);
+    this.workspaceConflictControl?.setSerializer(core.savedObjects.createSerializer());
 
     return {
       client: this.client as IWorkspaceClientImpl,

diff --git a/.../lib/integration_tests/repository.test.ts → ...pper_for_check_workspace_conflict.test.ts b/.../lib/integration_tests/repository.test.ts → ...pper_for_check_workspace_conflict.test.ts
@@ -2,23 +2,31 @@
  * Copyright OpenSearch Contributors
  * SPDX-License-Identifier: Apache-2.0
  */
+
 import { SavedObject } from 'src/core/types';
 import { isEqual } from 'lodash';
-import * as osdTestServer from '../../../../../test_helpers/osd_server';
 import { Readable } from 'stream';
+import * as osdTestServer from '../../../../../core/test_helpers/osd_server';
 
 const dashboard: Omit<SavedObject, 'id'> = {
   type: 'dashboard',
   attributes: {},
   references: [],
 };
 
-describe('repository integration test', () => {
+describe('saved_objects_wrapper_for_check_workspace_conflict integration test', () => {
   let root: ReturnType<typeof osdTestServer.createRoot>;
   let opensearchServer: osdTestServer.TestOpenSearchUtils;
   beforeAll(async () => {
     const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({
       adjustTimeout: (t: number) => jest.setTimeout(t),
+      settings: {
+        osd: {
+          workspace: {
+            enabled: true,
+          },
+        },
+      },
     });
     opensearchServer = await startOpenSearch();
     const startOSDResp = await startOpenSearchDashboards();
@@ -35,7 +43,7 @@ describe('repository integration test', () => {
         (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`))
           .statusCode
       )
-    );
+    ).toEqual(true);
   };
 
   const getItem = async (object: Pick<SavedObject, 'id' | 'type'>) => {