From cc1d936eb61303ffd4f8113c2b7df0a54b9619e7 Mon Sep 17 00:00:00 2001
From: SuZhou-Joe <suzhou@amazon.com>
Date: Sat, 23 Sep 2023 18:14:18 +0800
Subject: [PATCH] feat: optimize logic when checkConflict and bulkCreate (#189)

* feat: optimize logic when checkConflict and bulkCreate

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* feat: add options.workspace check

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* feat: throw error when workspace check error in repository create

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* feat: modify judgement

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* feat: always get objects from DB when create-with-override

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

---------

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
---
 .../saved_objects/service/lib/repository.ts   | 71 ++++++++++---------
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
index a49f356bbb81..e8b410350d9e 100644
--- a/src/core/server/saved_objects/service/lib/repository.ts
+++ b/src/core/server/saved_objects/service/lib/repository.ts
@@ -285,8 +285,14 @@ export class SavedObjectsRepository {
     if (id && overwrite && workspaces) {
       try {
         const currentItem = await this.get(type, id);
-        if (currentItem && currentItem.workspaces) {
-          // do not overwrite workspaces
+        if (
+          SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces(
+            workspaces,
+            currentItem.workspaces
+          ).length
+        ) {
+          throw SavedObjectsErrorHelpers.createConflictError(type, id);
+        } else {
           savedObjectWorkspaces = currentItem.workspaces;
         }
       } catch (e) {
@@ -419,8 +425,6 @@ export class SavedObjectsRepository {
         method,
       } = expectedBulkGetResult.value;
       let savedObjectWorkspaces: string[] | undefined;
-      let finalMethod = method;
-      let finalObjectId = object.id;
       if (opensearchRequestIndex !== undefined) {
         const indexFound = bulkGetResponse?.statusCode !== 404;
         const actualResult = indexFound
@@ -467,19 +471,9 @@ export class SavedObjectsRepository {
         versionProperties = getExpectedVersionProperties(version);
       }
 
-      if (expectedBulkGetResult.value.method === 'create') {
-        savedObjectWorkspaces = options.workspaces;
-      } else {
-        const changeToCreate = () => {
-          finalMethod = 'create';
-          finalObjectId = object.id;
-          savedObjectWorkspaces = options.workspaces;
-          versionProperties = {};
-        };
-        /**
-         * When overwrite, need to check if the object is workspace-specific
-         * if so, copy object to target workspace instead of refering it.
-         */
+      savedObjectWorkspaces = options.workspaces;
+
+      if (expectedBulkGetResult.value.method !== 'create') {
         const rawId = this._serializer.generateRawId(namespace, object.type, object.id);
         const findObject =
           bulkGetResponse?.statusCode !== 404
@@ -493,31 +487,31 @@ export class SavedObjectsRepository {
             options.workspaces,
             transformedObject.workspaces
           );
-          /**
-           * We need to create a new object when the object
-           * is about to import into workspaces it is not belong to
-           */
           if (filteredWorkspaces.length) {
-            /**
-             * Create a new object but only belong to the set of (target workspaces - original workspace)
-             */
-            changeToCreate();
-            finalObjectId = uuid.v1();
-            savedObjectWorkspaces = filteredWorkspaces;
+            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;
           }
-        } else {
-          savedObjectWorkspaces = options.workspaces;
         }
       }
 
       const expectedResult = {
         opensearchRequestIndex: bulkRequestIndexCounter++,
-        requestedId: finalObjectId,
+        requestedId: object.id,
         rawMigratedDoc: this._serializer.savedObjectToRaw(
           this._migrator.migrateDocument({
-            id: finalObjectId,
+            id: object.id,
             type: object.type,
             attributes: object.attributes,
             migrationVersion: object.migrationVersion,
@@ -533,7 +527,7 @@ export class SavedObjectsRepository {
 
       bulkCreateParams.push(
         {
-          [finalMethod]: {
+          [method]: {
             _id: expectedResult.rawMigratedDoc._id,
             _index: this.getIndexForType(object.type),
             ...(overwrite && versionProperties),
@@ -647,13 +641,24 @@ 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.filterWorkspacesAccordingToBaseWorkspaces(
+            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) && {
+            ...((!this.rawDocExistsInNamespace(doc!, namespace) || workspaceConflict) && {
               metadata: { isNotOverwritable: true },
             }),
           },