[Workspace][Bug] Unable to copy assets to a workspace without assigni…

…ng related data source (#7690) * fix copy assets issue Signed-off-by: yubonluo <[email protected]> * refactor copy result summary flyout Signed-off-by: yubonluo <[email protected]> * optimize the code Signed-off-by: yubonluo <[email protected]> * optimize the code Signed-off-by: yubonluo <[email protected]> * delete useless code Signed-off-by: yubonluo <[email protected]> * Changeset file for PR #7690 created/updated * optimize the code Signed-off-by: yubonluo <[email protected]> * add data source page link at the missing data source callout Signed-off-by: yubonluo <[email protected]> * fix test error Signed-off-by: yubonluo <[email protected]> * delete useless code Signed-off-by: yubonluo <[email protected]> * optimize code Signed-off-by: yubonluo <[email protected]> * add 403 error filter Signed-off-by: yubonluo <[email protected]> * optimzize the code Signed-off-by: yubonluo <[email protected]> * delete useless code Signed-off-by: yubonluo <[email protected]> * optimize the code Signed-off-by: yubonluo <[email protected]> * fix error Signed-off-by: yubonluo <[email protected]> * delete useless code Signed-off-by: yubonluo <[email protected]> --------- Signed-off-by: yubonluo <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-project · Jan 2, 2025 · a2fa2d1 · a2fa2d1
1 parent 7fb119d
commit a2fa2d1
Show file tree

Hide file tree

Showing 25 changed files with 947 additions and 785 deletions.
diff --git a/changelogs/fragments/7690.yml b/changelogs/fragments/7690.yml
@@ -0,0 +1,2 @@
+fix:
+- [Workspace] Unable to copy assets to a workspace without assigning related data source ([#7690](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7690))
diff --git a/src/core/public/workspace/types.ts b/src/core/public/workspace/types.ts
@@ -9,6 +9,7 @@ import {
   WorkspaceAttributeWithPermission,
   WorkspaceFindOptions,
 } from '../../types';
+import { SavedObjectsImportResponse } from '../saved_objects';
 
 export type WorkspaceObject = WorkspaceAttribute & { readonly?: boolean; owner?: boolean };
 
@@ -38,9 +39,13 @@ export interface IWorkspaceClient {
    * @param {Array<{ id: string; type: string }>} objects
    * @param {string} targetWorkspace
    * @param {boolean} includeReferencesDeep
-   * @returns {Promise<IResponse<any>>} result for this operation
+   * @returns {Promise<SavedObjectsImportResponse>} result for this operation
    */
-  copy(objects: any[], targetWorkspace: string, includeReferencesDeep?: boolean): Promise<any>;
+  copy(
+    objects: Array<{ id: string; type: string }>,
+    targetWorkspace: string,
+    includeReferencesDeep?: boolean
+  ): Promise<SavedObjectsImportResponse>;
 
   /**
    * Associates a list of objects with the given workspace ID.

diff --git a/src/core/server/saved_objects/import/create_saved_objects.test.ts b/src/core/server/saved_objects/import/create_saved_objects.test.ts
@@ -395,6 +395,11 @@ describe('#createSavedObjects', () => {
         id: 'baz-id',
         error: { type: 'missing_references' },
       } as SavedObjectsImportError,
+      {
+        type: 'baz',
+        id: 'baz-id',
+        error: { type: 'missing_data_source' },
+      } as SavedObjectsImportError,
     ];
     const unresolvableErrors: SavedObjectsImportError[] = [
       { type: 'qux', id: 'qux-id', error: { type: 'unsupported_type' } } as SavedObjectsImportError,

diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts
@@ -229,7 +229,12 @@ export const createSavedObjects = async <T>({
       return { ...object, ...(references && { references }) };
     })
   );
-  const resolvableErrors = ['conflict', 'ambiguous_conflict', 'missing_references'];
+  const resolvableErrors = [
+    'conflict',
+    'ambiguous_conflict',
+    'missing_references',
+    'missing_data_source',
+  ];
   let expectedResults = objectsToCreate;
   if (!accumulatedErrors.some(({ error: { type } }) => resolvableErrors.includes(type))) {
     const bulkCreateResponse = await savedObjectsClient.bulkCreate(objectsToCreate, {

diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts
@@ -35,6 +35,7 @@ import {
   SavedObjectsType,
   SavedObject,
   SavedObjectsImportError,
+  SavedObjectsBaseOptions,
 } from '../types';
 import { savedObjectsClientMock } from '../../mocks';
 import { SavedObjectsImportOptions, ISavedObjectTypeRegistry } from '..';
@@ -48,14 +49,17 @@ import { checkConflicts } from './check_conflicts';
 import { checkOriginConflicts } from './check_origin_conflicts';
 import { createSavedObjects } from './create_saved_objects';
 import { checkConflictsForDataSource } from './check_conflict_for_data_source';
+import { validateDataSources } from './validate_data_sources';
 
 jest.mock('./collect_saved_objects');
 jest.mock('./regenerate_ids');
 jest.mock('./validate_references');
+jest.mock('./validate_data_sources');
 jest.mock('./check_conflicts');
 jest.mock('./check_origin_conflicts');
 jest.mock('./create_saved_objects');
 jest.mock('./check_conflict_for_data_source');
+jest.mock('./utils');
 
 const getMockFn = <T extends (...args: any[]) => any, U>(fn: (...args: Parameters<T>) => U) =>
   fn as jest.MockedFunction<(...args: Parameters<T>) => U>;
@@ -71,6 +75,7 @@ describe('#importSavedObjectsFromStream', () => {
     });
     getMockFn(regenerateIds).mockReturnValue(new Map());
     getMockFn(validateReferences).mockResolvedValue([]);
+    getMockFn(validateDataSources).mockResolvedValue([]);
     getMockFn(checkConflicts).mockResolvedValue({
       errors: [],
       filteredObjects: [],
@@ -101,7 +106,9 @@ describe('#importSavedObjectsFromStream', () => {
   const setupOptions = (
     createNewCopies: boolean = false,
     dataSourceId: string | undefined = undefined,
-    dataSourceEnabled: boolean | undefined = false
+    dataSourceEnabled: boolean | undefined = false,
+    workspaces: SavedObjectsBaseOptions['workspaces'] = undefined,
+    isCopy: boolean = false
   ): SavedObjectsImportOptions => {
     readStream = new Readable();
     savedObjectsClient = savedObjectsClientMock.create();
@@ -122,6 +129,9 @@ describe('#importSavedObjectsFromStream', () => {
       namespace,
       createNewCopies,
       dataSourceId,
+      workspaces,
+      isCopy,
+      dataSourceEnabled,
     };
   };
   const createObject = (
@@ -195,6 +205,25 @@ describe('#importSavedObjectsFromStream', () => {
       );
     });
 
+    test('validates data sources', async () => {
+      const options = setupOptions(true, undefined, true, ['workspace-1'], true);
+      const collectedObjects = [createObject()];
+      const errorAccumulator: SavedObjectsImportError[] = [];
+      getMockFn(collectSavedObjects).mockResolvedValue({
+        errors: [],
+        collectedObjects,
+        importIdMap: new Map(),
+      });
+
+      await importSavedObjectsFromStream(options);
+      expect(validateDataSources).toHaveBeenCalledWith(
+        collectedObjects,
+        savedObjectsClient,
+        errorAccumulator,
+        ['workspace-1']
+      );
+    });
+
     describe('with createNewCopies disabled', () => {
       test('does not regenerate object IDs', async () => {
         const options = setupOptions();
@@ -602,6 +631,37 @@ describe('#importSavedObjectsFromStream', () => {
       expect(result).toEqual({ success: false, successCount: 0, errors: expectedErrors });
     });
 
+    test('performs a copy operation accumulates multiple errors', async () => {
+      const options = setupOptions(true, undefined, true, ['workspace-1'], true);
+      const errors = [createError(), createError(), createError(), createError()];
+      getMockFn(collectSavedObjects).mockResolvedValue({
+        errors: [errors[0]],
+        collectedObjects: [],
+        importIdMap: new Map(), // doesn't matter
+      });
+      getMockFn(validateReferences).mockResolvedValue([errors[1]]);
+      getMockFn(validateDataSources).mockResolvedValue([errors[2]]);
+      getMockFn(createSavedObjects).mockResolvedValue({ errors: [errors[3]], createdObjects: [] });
+
+      // it will not accumulate
+      getMockFn(checkConflicts).mockResolvedValue({
+        errors: [errors[2]],
+        filteredObjects: [],
+        importIdMap: new Map(), // doesn't matter
+        pendingOverwrites: new Set(),
+      });
+      // it will not accumulate
+      getMockFn(checkOriginConflicts).mockResolvedValue({
+        errors: [errors[3]],
+        importIdMap: new Map(), // doesn't matter
+        pendingOverwrites: new Set(),
+      });
+
+      const result = await importSavedObjectsFromStream(options);
+      const expectedErrors = errors.map(({ type, id }) => expect.objectContaining({ type, id }));
+      expect(result).toEqual({ success: false, successCount: 0, errors: expectedErrors });
+    });
+
     test('early return if import data source objects to non-MDS cluster', async () => {
       const options = setupOptions(false, testDataSourceId, false);
       const dsObj = createDataSourceObject();

diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts
@@ -42,6 +42,7 @@ import { checkConflicts } from './check_conflicts';
 import { regenerateIds } from './regenerate_ids';
 import { checkConflictsForDataSource } from './check_conflict_for_data_source';
 import { isSavedObjectWithDataSource } from './validate_object_id';
+import { validateDataSources } from './validate_data_sources';
 
 /**
  * Import saved objects from given stream. See the {@link SavedObjectsImportOptions | options} for more
@@ -61,6 +62,7 @@ export async function importSavedObjectsFromStream({
   dataSourceTitle,
   workspaces,
   dataSourceEnabled,
+  isCopy,
 }: SavedObjectsImportOptions): Promise<SavedObjectsImportResponse> {
   let errorAccumulator: SavedObjectsImportError[] = [];
   const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name);
@@ -108,6 +110,20 @@ export async function importSavedObjectsFromStream({
   );
   errorAccumulator = [...errorAccumulator, ...validateReferencesResult];
 
+  if (isCopy) {
+    // Data sources can only be assigned to workspaces and can not be copied between workspaces.
+    collectSavedObjectsResult.collectedObjects = collectSavedObjectsResult.collectedObjects.filter(
+      (obj) => obj.type !== 'data-source'
+    );
+    const validateDataSourcesResult = await validateDataSources(
+      collectSavedObjectsResult.collectedObjects,
+      savedObjectsClient,
+      errorAccumulator,
+      workspaces
+    );
+    errorAccumulator = [...errorAccumulator, ...validateDataSourcesResult];
+  }
+
   if (createNewCopies) {
     // randomly generated id
     importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects, dataSourceId);

diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts
@@ -105,6 +105,15 @@ export interface SavedObjectsImportMissingReferencesError {
   references: Array<{ type: string; id: string }>;
 }
 
+/**
+ * Represents an error that occurs when an import fails due to a missing data source in the target workspace.
+ * @public
+ */
+export interface SavedObjectsImportMissingDataSourceError {
+  type: 'missing_data_source';
+  dataSource: string;
+}
+
 /**
  * Represents a failure to import.
  * @public
@@ -126,7 +135,8 @@ export interface SavedObjectsImportError {
     | SavedObjectsImportAmbiguousConflictError
     | SavedObjectsImportUnsupportedTypeError
     | SavedObjectsImportMissingReferencesError
-    | SavedObjectsImportUnknownError;
+    | SavedObjectsImportUnknownError
+    | SavedObjectsImportMissingDataSourceError;
 }
 
 /**
@@ -191,6 +201,7 @@ export interface SavedObjectsImportOptions {
   dataSourceTitle?: string;
   dataSourceEnabled?: boolean;
   workspaces?: SavedObjectsBaseOptions['workspaces'];
+  isCopy?: boolean;
 }
 
 /**

diff --git a/src/core/server/saved_objects/import/utils.test.ts b/src/core/server/saved_objects/import/utils.test.ts
@@ -10,6 +10,7 @@ import {
   getUpdatedTSVBVisState,
   updateDataSourceNameInVegaSpec,
   updateDataSourceNameInTimeline,
+  findReferenceDataSourceForObject,
 } from './utils';
 import { parse } from 'hjson';
 import { isEqual } from 'lodash';
@@ -342,3 +343,118 @@ describe('getUpdatedTSVBVisState', () => {
     }
   );
 });
+
+describe('findReferenceDataSourceForObject', () => {
+  const savedObjects: Array<SavedObject<{ title?: string }>> = [
+    {
+      id: '1',
+      references: [{ id: '5', type: 'data-source', name: '5' }],
+      type: 'index-pattern',
+      attributes: {},
+    },
+    {
+      id: '2',
+      references: [{ id: '3', type: 'non-data-source', name: '3' }],
+      type: 'index-pattern',
+      attributes: {},
+    },
+    {
+      id: '3',
+      references: [],
+      type: 'non-data-source',
+      attributes: {},
+    },
+    {
+      id: '4',
+      references: [{ id: '1', type: 'index-pattern', name: '1' }],
+      type: 'non-data-source',
+      attributes: {},
+    },
+    {
+      id: '6',
+      references: [{ id: '7', type: 'index-pattern', name: '7' }],
+      type: 'non-data-source',
+      attributes: {},
+    },
+    {
+      id: '8',
+      references: [
+        { id: '1', type: 'index-pattern', name: '1' },
+        { id: '6', type: 'data-source', name: '6' },
+      ],
+      type: 'dashboards',
+      attributes: {},
+    },
+  ];
+
+  const ObjectsMap = new Map(savedObjects.map((so) => [so.id, so]));
+
+  test('returns the data-source reference if it exists in the references', () => {
+    const result = findReferenceDataSourceForObject(savedObjects[0], ObjectsMap);
+    expect(result).toEqual(new Set('5'));
+  });
+
+  test('returns empty Set if there is no data-source reference and no nested references', () => {
+    expect(findReferenceDataSourceForObject(savedObjects[1], ObjectsMap)).toEqual(new Set());
+  });
+
+  test('returns empty Set if no references exist', () => {
+    expect(findReferenceDataSourceForObject(savedObjects[2], ObjectsMap)).toEqual(new Set());
+  });
+
+  test('returns nested data-source reference if found', () => {
+    const result = findReferenceDataSourceForObject(savedObjects[3], ObjectsMap);
+    expect(result).toEqual(new Set('5'));
+  });
+
+  test('returns empty Set if the nested references have no data-source reference', () => {
+    expect(findReferenceDataSourceForObject(savedObjects[4], ObjectsMap)).toEqual(new Set());
+  });
+
+  test('returns empty Set if circular reference', () => {
+    const circularAssets: Array<SavedObject<{ title?: string }>> = [
+      {
+        id: '1',
+        references: [{ id: '2', type: 'non-data-source', name: '2' }],
+        type: 'non-data-source',
+        attributes: {},
+      },
+      {
+        id: '2',
+        references: [{ id: '3', type: 'non-data-source', name: '3' }],
+        type: 'non-data-source',
+        attributes: {},
+      },
+      {
+        id: '3',
+        references: [{ id: '1', type: 'non-data-source', name: '1' }],
+        type: 'non-data-source',
+        attributes: {},
+      },
+    ];
+    const circularAssetsMap = new Map(circularAssets.map((so) => [so.id, so]));
+
+    const result = findReferenceDataSourceForObject(circularAssets[0], circularAssetsMap);
+    expect(result).toEqual(new Set());
+  });
+
+  test('returns empty Set if circular reference is itself', () => {
+    const circularAssets: Array<SavedObject<{ title?: string }>> = [
+      {
+        id: '1',
+        references: [{ id: '1', type: 'non-data-source', name: '1' }],
+        type: 'non-data-source',
+        attributes: {},
+      },
+    ];
+    const circularAssetsMap = new Map(circularAssets.map((so) => [so.id, so]));
+
+    const result = findReferenceDataSourceForObject(circularAssets[0], circularAssetsMap);
+    expect(result).toEqual(new Set());
+  });
+
+  test('returns multiple data sources if they exist in the references', () => {
+    const result = findReferenceDataSourceForObject(savedObjects[5], ObjectsMap);
+    expect(result).toEqual(new Set(['5', '6']));
+  });
+});
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		fix:
		- [Workspace] Unable to copy assets to a workspace without assigning related data source ([#7690](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7690))