Move workspace specific logic to wrapper

CHANGELOG.md

@@ -41,6 +41,11 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851))
 - [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827))
 - [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895))
+- [Multiple Datasource] Concatenate data source name with index pattern name and change delimiter to double colon ([#5907](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5907))
+- [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881))
+- [Multiple Datasource] Improved error handling for the search API when a null value is passed for the dataSourceId ([#5882](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882))
+- [Multiple Datasource] Hide/Show authentication method in multi data source plugin based on configuration ([#5916](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5916))
+- [Workspace] Optional workspaces params in repository ([#5949](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5949))
 
 ### 🐛 Bug Fixes
 

src/core/server/saved_objects/export/get_sorted_objects_for_export.ts

@@ -125,7 +125,7 @@ async function fetchObjectsToExport({
       search,
       perPage: exportSizeLimit,
       namespaces: namespace ? [namespace] : undefined,
-      workspaces,
+      ...(workspaces ? { workspaces } : {}),
     });
     if (findResponse.total > exportSizeLimit) {
       throw Boom.badRequest(`Can't export more than ${exportSizeLimit} objects`);

src/core/server/saved_objects/import/import_saved_objects.test.ts

@@ -42,7 +42,7 @@ import { typeRegistryMock } from '../saved_objects_type_registry.mock';
 import { importSavedObjectsFromStream } from './import_saved_objects';
 
 import { collectSavedObjects } from './collect_saved_objects';
-import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids';
+import { regenerateIds } from './regenerate_ids';
 import { validateReferences } from './validate_references';
 import { checkConflicts } from './check_conflicts';
 import { checkOriginConflicts } from './check_origin_conflicts';
@@ -70,7 +70,6 @@ describe('#importSavedObjectsFromStream', () => {
       importIdMap: new Map(),
     });
     getMockFn(regenerateIds).mockReturnValue(new Map());
-    getMockFn(regenerateIdsWithReference).mockReturnValue(Promise.resolve(new Map()));
     getMockFn(validateReferences).mockResolvedValue([]);
     getMockFn(checkConflicts).mockResolvedValue({
       errors: [],
@@ -279,15 +278,6 @@ describe('#importSavedObjectsFromStream', () => {
           ]),
         });
         getMockFn(validateReferences).mockResolvedValue([errors[1]]);
-        getMockFn(regenerateIdsWithReference).mockResolvedValue(
-          Promise.resolve(
-            new Map([
-              ['foo', {}],
-              ['bar', {}],
-              ['baz', {}],
-            ])
-          )
-        );
         getMockFn(checkConflicts).mockResolvedValue({
           errors: [errors[2]],
           filteredObjects,

src/core/server/saved_objects/import/import_saved_objects.ts

@@ -38,7 +38,7 @@ import { validateReferences } from './validate_references';
 import { checkOriginConflicts } from './check_origin_conflicts';
 import { createSavedObjects } from './create_saved_objects';
 import { checkConflicts } from './check_conflicts';
-import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids';
+import { regenerateIds } from './regenerate_ids';
 import { checkConflictsForDataSource } from './check_conflict_for_data_source';
 
 /**
@@ -86,13 +86,6 @@ export async function importSavedObjectsFromStream({
     // randomly generated id
     importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects, dataSourceId);
   } else {
-    importIdMap = await regenerateIdsWithReference({
-      savedObjects: collectSavedObjectsResult.collectedObjects,
-      savedObjectsClient,
-      workspaces,
-      objectLimit,
-      importIdMap,
-    });
     // in check conclict and override mode
     // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces
     const checkConflictsParams = {
@@ -152,6 +145,7 @@ export async function importSavedObjectsFromStream({
     ...(workspaces ? { workspaces } : {}),
     dataSourceId,
     dataSourceTitle,
+    ...(workspaces ? { workspaces } : {}),
   };
   const createSavedObjectsResult = await createSavedObjects(createSavedObjectsParams);
   errorAccumulator = [...errorAccumulator, ...createSavedObjectsResult.errors];

src/core/server/saved_objects/import/regenerate_ids.ts

@@ -29,8 +29,7 @@
  */
 
 import { v4 as uuidv4 } from 'uuid';
-import { SavedObject, SavedObjectsClientContract } from '../types';
-import { SavedObjectsUtils } from '../service';
+import { SavedObject } from '../types';
 
 /**
  * Takes an array of saved objects and returns an importIdMap of randomly-generated new IDs.
@@ -48,40 +47,3 @@ export const regenerateIds = (objects: SavedObject[], dataSourceId: string | und
   }, new Map<string, { id: string; omitOriginId?: boolean }>());
   return importIdMap;
 };
-
-export const regenerateIdsWithReference = async (props: {
-  savedObjects: SavedObject[];
-  savedObjectsClient: SavedObjectsClientContract;
-  workspaces?: string[];
-  objectLimit: number;
-  importIdMap: Map<string, { id?: string; omitOriginId?: boolean }>;
-}): Promise<Map<string, { id?: string; omitOriginId?: boolean }>> => {
-  const { savedObjects, savedObjectsClient, workspaces, importIdMap } = props;
-  if (!workspaces || !workspaces.length) {
-    return savedObjects.reduce((acc, object) => {
-      return acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false });
-    }, importIdMap);
-  }
-
-  const bulkGetResult = await savedObjectsClient.bulkGet(
-    savedObjects.map((item) => ({ type: item.type, id: item.id }))
-  );
-
-  return bulkGetResult.saved_objects.reduce((acc, object) => {
-    if (object.error?.statusCode === 404) {
-      acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: true });
-      return acc;
-    }
-
-    const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces(
-      workspaces,
-      object.workspaces
-    );
-    if (filteredWorkspaces.length) {
-      acc.set(`${object.type}:${object.id}`, { id: uuidv4(), omitOriginId: true });
-    } else {
-      acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false });
-    }
-    return acc;
-  }, importIdMap);
-};

src/core/server/saved_objects/import/types.ts

@@ -187,7 +187,7 @@ export interface SavedObjectsImportOptions {
   namespace?: string;
   /** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */
   createNewCopies: boolean;
-  /** if specified, will import in given workspaces, else will import as global object */
+  /** if specified, will import in given workspaces */
   workspaces?: string[];
   dataSourceId?: string;
   dataSourceTitle?: string;
@@ -212,7 +212,7 @@ export interface SavedObjectsResolveImportErrorsOptions {
   namespace?: string;
   /** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */
   createNewCopies: boolean;
-  /** if specified, will import in given workspaces, else will import as global object */
+  /** if specified, will import in given workspaces */
   workspaces?: string[];
   dataSourceId?: string;
   dataSourceTitle?: string;

src/core/server/saved_objects/import/validate_references.ts

@@ -73,7 +73,10 @@ export async function getNonExistingReferenceAsKeys(
   }
 
   // Fetch references to see if they exist
-  const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ ...obj, fields: ['id'] }));
+  const bulkGetOpts = Array.from(collector.values()).map((obj) => ({
+    ...obj,
+    fields: ['id'],
+  }));
   const bulkGetResponse = await savedObjectsClient.bulkGet(bulkGetOpts, { namespace });
 
   // Error handling

src/core/server/saved_objects/service/lib/repository.test.js

@@ -629,6 +629,7 @@ describe('SavedObjectsRepository', () => {
       references: [{ name: 'ref_0', type: 'test', id: '2' }],
     };
     const namespace = 'foo-namespace';
+    const workspace = 'foo-workspace';
 
     const getMockBulkCreateResponse = (objects, namespace) => {
       return {
@@ -922,6 +923,16 @@ describe('SavedObjectsRepository', () => {
         await bulkCreateSuccess(objects, { namespace });
         expectClientCallArgsAction(objects, { method: 'create', getId });
       });
+
+      it(`adds workspaces to request body for any types`, async () => {
+        await bulkCreateSuccess([obj1, obj2], { workspaces: [workspace] });
+        const expected = expect.objectContaining({ workspaces: [workspace] });
+        const body = [expect.any(Object), expected, expect.any(Object), expected];
+        expect(client.bulk).toHaveBeenCalledWith(
+          expect.objectContaining({ body }),
+          expect.anything()
+        );
+      });
     });
 
     describe('errors', () => {

src/core/server/saved_objects/service/lib/repository.ts

@@ -287,26 +287,6 @@ export class SavedObjectsRepository {
       }
     }
 
-    let savedObjectWorkspaces = workspaces;
-
-    if (id && overwrite) {
-      try {
-        const currentItem = await this.get(type, id);
-        if (
-          SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces(
-            workspaces,
-            currentItem.workspaces
-          ).length
-        ) {
-          throw SavedObjectsErrorHelpers.createConflictError(type, id);
-        } else {
-          savedObjectWorkspaces = currentItem.workspaces;
-        }
-      } catch (e) {
-        // this.get will throw an error when no items can be found
-      }
-    }
-
     const migrated = this._migrator.migrateDocument({
       id,
       type,
@@ -317,7 +297,7 @@ export class SavedObjectsRepository {
       migrationVersion,
       updated_at: time,
       ...(Array.isArray(references) && { references }),
-      ...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }),
+      ...(Array.isArray(workspaces) && { workspaces }),
       ...(permissions && { permissions }),
     });
 
@@ -445,7 +425,6 @@ export class SavedObjectsRepository {
         object: { initialNamespaces, version, ...object },
         method,
       } = expectedBulkGetResult.value;
-      let savedObjectWorkspaces: string[] | undefined;
       if (opensearchRequestIndex !== undefined) {
         const indexFound = bulkGetResponse?.statusCode !== 404;
         const actualResult = indexFound
@@ -492,39 +471,10 @@ export class SavedObjectsRepository {
         versionProperties = getExpectedVersionProperties(version);
       }
 
-      savedObjectWorkspaces = options.workspaces;
+      let savedObjectWorkspaces = 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;
-        if (findObject && findObject.found) {
-          const transformedObject = this._serializer.rawToSavedObject(
-            findObject as SavedObjectsRawDoc
-          ) as SavedObject;
-          const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces(
-            options.workspaces,
-            transformedObject.workspaces
-          );
-          if (filteredWorkspaces.length) {
-            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 = {
@@ -541,7 +491,7 @@ export class SavedObjectsRepository {
             updated_at: time,
             references: object.references || [],
             originId: object.originId,
-            workspaces: savedObjectWorkspaces,
+            ...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }),
           }) as SavedObjectSanitizedDoc
         ),
       };
@@ -639,7 +589,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(
@@ -662,24 +612,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.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) || workspaceConflict) && {
+            ...(!this.rawDocExistsInNamespace(doc!, namespace) && {
               metadata: { isNotOverwritable: true },
             }),
           },

src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts

@@ -644,6 +644,27 @@ describe('#getQueryParams', () => {
         ]);
       });
     });
+
+    describe('when using workspace search', () => {
+      it('using normal workspaces', () => {
+        const result: Result = getQueryParams({
+          registry,
+          workspaces: ['foo'],
+        });
+        expect(result.query.bool.filter[1]).toEqual({
+          bool: {
+            should: [
+              {
+                bool: {
+                  must: [{ term: { workspaces: 'foo' } }],
+                },
+              },
+            ],
+            minimum_should_match: 1,
+          },
+        });
+      });
+    });
   });
 
   describe('namespaces property', () => {

src/core/server/saved_objects/service/lib/utils.ts

@@ -81,7 +81,7 @@ export class SavedObjectsUtils {
     saved_objects: [],
   });
 
-  public static filterWorkspacesAccordingToBaseWorkspaces(
+  public static filterWorkspacesAccordingToSourceWorkspaces(
     targetWorkspaces?: string[],
     baseWorkspaces?: string[]
   ): string[] {

src/core/server/saved_objects/service/saved_objects_client.ts

@@ -71,6 +71,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions {
   initialNamespaces?: string[];
   /** permission control describe by ACL object */
   permissions?: Permissions;
+  /**
+   * workspaces the new created objects belong to
+   */
+  workspaces?: string[];
 }
 
 /**
@@ -94,6 +98,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[];
 }
 
 /**

src/core/server/saved_objects/types.ts

@@ -113,6 +113,7 @@ export interface SavedObjectsFindOptions {
   typeToNamespacesMap?: Map<string, string[] | undefined>;
   /** An optional OpenSearch preference value to be used for the query **/
   preference?: string;
+  /** If specified, will only retrieve objects that are in the workspaces */
   workspaces?: string[];
   /**
    * The params here will be combined with bool clause and is used for filtering with ACL structure.
@@ -131,6 +132,7 @@ export interface SavedObjectsFindOptions {
 export interface SavedObjectsBaseOptions {
   /** Specify the namespace for this operation */
   namespace?: string;
+  /** Specify the workspaces for this operation */
   workspaces?: string[];
 }