From 229a13417ce8e1a48be7a76ba2f5fcfbbfd5e8dd Mon Sep 17 00:00:00 2001 From: yubonluo Date: Fri, 1 Nov 2024 16:00:58 +0800 Subject: [PATCH 01/13] fix copy assets issue Signed-off-by: yubonluo --- changelogs/fragments/7690.yml | 2 + .../import/import_saved_objects.test.ts | 88 ++++++++++++++- .../import/import_saved_objects.ts | 39 +++++++ src/core/server/saved_objects/import/types.ts | 12 +- .../server/saved_objects/import/utils.test.ts | 106 ++++++++++++++++++ src/core/server/saved_objects/import/utils.ts | 51 +++++++++ .../duplicate_result_flyout.test.tsx.snap | 52 ++++++++- .../duplicate_result_flyout.test.tsx | 24 +++- .../components/duplicate_result_flyout.tsx | 2 + .../workspace/opensearch_dashboards.json | 2 +- src/plugins/workspace/server/plugin.ts | 9 +- .../workspace/server/routes/duplicate.ts | 5 +- src/plugins/workspace/server/routes/index.ts | 4 +- 13 files changed, 385 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/7690.yml diff --git a/changelogs/fragments/7690.yml b/changelogs/fragments/7690.yml new file mode 100644 index 000000000000..18332d1b737d --- /dev/null +++ b/changelogs/fragments/7690.yml @@ -0,0 +1,2 @@ +fix: +- [Workspace] Show error for so unassigned data source ([#7690](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7690)) \ No newline at end of file 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 index ea4ac66b1a0b..545cea5c8e6e 100644 --- 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,6 +49,7 @@ 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 { findDataSourceForObject } from './utils'; jest.mock('./collect_saved_objects'); jest.mock('./regenerate_ids'); @@ -56,6 +58,7 @@ 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 = any, U>(fn: (...args: Parameters) => U) => fn as jest.MockedFunction<(...args: Parameters) => U>; @@ -101,7 +104,8 @@ describe('#importSavedObjectsFromStream', () => { const setupOptions = ( createNewCopies: boolean = false, dataSourceId: string | undefined = undefined, - dataSourceEnabled: boolean | undefined = false + dataSourceEnabled: boolean | undefined = false, + workspaces: SavedObjectsBaseOptions['workspaces'] = undefined ): SavedObjectsImportOptions => { readStream = new Readable(); savedObjectsClient = savedObjectsClientMock.create(); @@ -122,6 +126,7 @@ describe('#importSavedObjectsFromStream', () => { namespace, createNewCopies, dataSourceId, + workspaces, }; }; const createObject = ( @@ -157,6 +162,20 @@ describe('#importSavedObjectsFromStream', () => { error: { type: 'conflict' }, }; }; + const findObjectsMock = { + saved_objects: [ + { + id: 'data-source-1', + score: 0, + type: 'data-source', + attributes: {}, + references: [], + }, + ], + total: 1, + page: 1, + per_page: 1, + }; /** * These tests use minimal mocks which don't look realistic, but are sufficient to exercise the code paths correctly. For example, for an @@ -397,6 +416,73 @@ describe('#importSavedObjectsFromStream', () => { expect(result).toEqual({ success: false, successCount: 0, errors: [expect.any(Object)] }); }); + test('validates workspace with assigned data source', async () => { + const options = setupOptions(false, undefined, true, ['workspace-1']); + const collectedObjects = [createObject()]; + getMockFn(collectSavedObjects).mockResolvedValue({ + errors: [], + collectedObjects, + importIdMap: new Map(), + }); + getMockFn(findDataSourceForObject).mockResolvedValue('data-source-1'); + const clientMock = savedObjectsClientMock.create(); + clientMock.find.mockResolvedValueOnce(findObjectsMock); + + const result = await importSavedObjectsFromStream({ + ...options, + savedObjectsClient: clientMock, + }); + expect(result).toEqual({ success: true, successCount: 0 }); + }); + + test('validates workspace with unassigned data source', async () => { + const options = setupOptions(false, undefined, true, ['workspace-1']); + const collectedObjects = [createObject()]; + getMockFn(collectSavedObjects).mockResolvedValue({ + errors: [], + collectedObjects, + importIdMap: new Map(), + }); + getMockFn(findDataSourceForObject).mockResolvedValue('data-source-2'); + const clientMock = savedObjectsClientMock.create(); + clientMock.find.mockResolvedValueOnce(findObjectsMock); + + const result = await importSavedObjectsFromStream({ + ...options, + savedObjectsClient: clientMock, + }); + expect(result).toEqual({ success: false, successCount: 0, errors: [expect.any(Object)] }); + expect(result.errors?.[0].error).toEqual({ + message: + 'The object hasn’t be copied to the selected workspace. The data source (data-source-2) is not available in the selected workspace.', + type: 'missing_target_workspace_assigned_data_source', + }); + }); + + test('validates workspace with catch error', async () => { + const options = setupOptions(false, undefined, true, ['workspace-1']); + const collectedObjects = [createObject()]; + getMockFn(collectSavedObjects).mockResolvedValue({ + errors: [], + collectedObjects, + importIdMap: new Map(), + }); + getMockFn(findDataSourceForObject).mockRejectedValue(null); + const clientMock = savedObjectsClientMock.create(); + clientMock.find.mockResolvedValueOnce(findObjectsMock); + + const result = await importSavedObjectsFromStream({ + ...options, + savedObjectsClient: clientMock, + }); + expect(result).toEqual({ success: false, successCount: 0, errors: [expect.any(Object)] }); + expect(result.errors?.[0].error).toEqual({ + message: null, + statusCode: 500, + type: 'unknown', + }); + }); + describe('handles a mix of successes and errors and injects metadata', () => { const obj1 = createObject(); const tmp = createObject(); diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index 1289af145c58..7997d4af6e8d 100644 --- 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 { findDataSourceForObject } from './utils'; /** * Import saved objects from given stream. See the {@link SavedObjectsImportOptions | options} for more @@ -95,6 +96,44 @@ export async function importSavedObjectsFromStream({ } } + // If target workspace is exists, it will check whether the assigned data sources in the target workspace + // include the data sources of the imported saved objects. + if (workspaces && workspaces.length > 0) { + const assignedDataSourcesInTargetWorkspace = await savedObjectsClient + .find({ + type: 'data-source', + fields: ['id'], + perPage: 10000, + workspaces, + }) + .then((response) => { + return response?.saved_objects?.map((source) => source.id) ?? []; + }); + for (const object of collectSavedObjectsResult.collectedObjects) { + try { + const referenceDSId = await findDataSourceForObject(object, savedObjectsClient); + if (referenceDSId && !assignedDataSourcesInTargetWorkspace.includes(referenceDSId)) { + collectSavedObjectsResult.errors.push({ + type: object.type, + id: object.id, + error: { + type: 'missing_target_workspace_assigned_data_source', + message: `The object hasn’t be copied to the selected workspace. The data source (${referenceDSId}) is not available in the selected workspace.`, + }, + meta: { title: object.attributes?.title }, + }); + } + } catch (err) { + collectSavedObjectsResult.errors.push({ + type: object.type, + id: object.id, + error: { type: 'unknown', message: err, statusCode: 500 }, + meta: { title: object.attributes?.title }, + }); + } + } + } + errorAccumulator = [...errorAccumulator, ...collectSavedObjectsResult.errors]; /** Map of all IDs for objects that we are attempting to import; each value is empty by default */ let importIdMap = collectSavedObjectsResult.importIdMap; diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 8612870fcf56..a6f1af19b519 100644 --- 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 a failure to import due to missing target workspace assigned data source. + * @public + */ +export interface SavedObjectsImportMissingTargetWorkspaceAssignedDataSourceError { + type: 'missing_target_workspace_assigned_data_source'; + message: string; +} + /** * Represents a failure to import. * @public @@ -126,7 +135,8 @@ export interface SavedObjectsImportError { | SavedObjectsImportAmbiguousConflictError | SavedObjectsImportUnsupportedTypeError | SavedObjectsImportMissingReferencesError - | SavedObjectsImportUnknownError; + | SavedObjectsImportUnknownError + | SavedObjectsImportMissingTargetWorkspaceAssignedDataSourceError; } /** diff --git a/src/core/server/saved_objects/import/utils.test.ts b/src/core/server/saved_objects/import/utils.test.ts index 0203a574c049..5d68e7f905ae 100644 --- a/src/core/server/saved_objects/import/utils.test.ts +++ b/src/core/server/saved_objects/import/utils.test.ts @@ -10,11 +10,13 @@ import { getUpdatedTSVBVisState, updateDataSourceNameInVegaSpec, updateDataSourceNameInTimeline, + findDataSourceForObject, } from './utils'; import { parse } from 'hjson'; import { isEqual } from 'lodash'; import { join } from 'path'; import { SavedObject, SavedObjectsClientContract } from '../types'; +import { savedObjectsClientMock } from '../../mocks'; describe('updateDataSourceNameInVegaSpec()', () => { const loadHJSONStringFromFile = (filepath: string) => { @@ -342,3 +344,107 @@ describe('getUpdatedTSVBVisState', () => { } ); }); + +describe('findDataSourceForObject', () => { + let savedObjectsClient: jest.Mocked; + const indexPatternObject: SavedObject = { + id: 'indexPattern', + type: 'index-pattern', + references: [{ type: 'data-source', id: 'dataSource', name: 'dataSource' }], + attributes: {}, + }; + + beforeEach(() => { + savedObjectsClient = savedObjectsClientMock.create(); + }); + + it('should return the data source id for an index-pattern object', async () => { + const dataSourceId = await findDataSourceForObject(indexPatternObject, savedObjectsClient); + expect(dataSourceId).toBe('dataSource'); + }); + + it('should use bulkGet to resolve multiple references and return data source', async () => { + const savedObject: SavedObject = { + type: 'dashboard', + id: 'dashboard', + references: [{ type: 'visualization', id: 'visualization', name: 'visualization' }], + attributes: {}, + }; + + const visualizationObject: SavedObject = { + type: 'visualization', + id: 'visualization', + references: [{ type: 'index-pattern', id: 'indexPattern', name: 'indexPattern' }], + attributes: {}, + }; + + savedObjectsClient.bulkGet.mockResolvedValueOnce({ + saved_objects: [visualizationObject], + }); + savedObjectsClient.bulkGet.mockResolvedValueOnce({ + saved_objects: [indexPatternObject], + }); + + const dataSourceId = await findDataSourceForObject(savedObject, savedObjectsClient); + expect(dataSourceId).toBe('dataSource'); + }); + + it('should use bulkGet to resolve multiple references and return the first found data source', async () => { + const savedObject: SavedObject = { + id: 'visualization', + type: 'visualization', + references: [ + // { type: 'index-pattern', id: 'indexPattern', name: 'index-pattern' }, + { type: 'dashboard', id: 'dashboard', name: 'dashboard' }, + ], + attributes: {}, + }; + + savedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: [indexPatternObject], + }); + + const dataSourceId = await findDataSourceForObject(savedObject, savedObjectsClient); + expect(dataSourceId).toBe('dataSource'); + expect(savedObjectsClient.bulkGet).toHaveBeenCalledWith([ + { type: 'dashboard', id: 'dashboard' }, + ]); + }); + + it('should return null if no data source is found', async () => { + const savedObject: SavedObject = { + id: 'visualization', + type: 'visualization', + references: [], + attributes: {}, + }; + + const dataSourceId = await findDataSourceForObject(savedObject, savedObjectsClient); + expect(dataSourceId).toBeNull(); + }); + + it('should return null if there is an error in bulkGet', async () => { + const savedObject: SavedObject = { + id: 'visualization', + type: 'visualization', + references: [{ type: 'index-pattern', id: 'indexPattern', name: 'indexPattern' }], + attributes: {}, + }; + + savedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: [ + { + id: 'indexPattern', + type: 'index-pattern', + error: { error: '', statusCode: 400, message: '' }, + attributes: undefined, + references: [], + }, + ], + }); + + await expect(findDataSourceForObject(savedObject, savedObjectsClient)).rejects.toThrow( + 'Bad Request' + ); + }); +}); diff --git a/src/core/server/saved_objects/import/utils.ts b/src/core/server/saved_objects/import/utils.ts index 367824b8db41..9566993f7b9a 100644 --- a/src/core/server/saved_objects/import/utils.ts +++ b/src/core/server/saved_objects/import/utils.ts @@ -4,6 +4,7 @@ */ import { parse, stringify } from 'hjson'; +import Boom from '@hapi/boom'; import { SavedObject, SavedObjectReference, SavedObjectsClientContract } from '../types'; import { VisualizationObject } from './types'; @@ -183,3 +184,53 @@ const parseJSONSpec = (spec: string) => { return undefined; }; + +export async function findDataSourceForObject( + savedObject: SavedObject, + savedObjectsClient: SavedObjectsClientContract, + visitedObjects: Set = new Set() +): Promise { + const references = savedObject.references; + if (!references || references.length === 0) { + return null; + } + + const objectKey = `${savedObject.type}:${savedObject.id}`; + if (visitedObjects.has(objectKey)) { + return null; + } + visitedObjects.add(objectKey); + + const dataSourceReference = references.find((ref) => ref.type === 'data-source'); + if (dataSourceReference) { + return dataSourceReference.id; + } + + const bulkGetResponse = await savedObjectsClient.bulkGet( + references.map((reference) => ({ + type: reference.type, + id: reference.id, + })) + ); + + const referencedObjects = bulkGetResponse.saved_objects; + const erroredObjects = referencedObjects.filter( + (obj) => obj.error && obj.error.statusCode !== 404 + ); + + if (erroredObjects.length > 0) { + const err = Boom.badRequest(); + err.output.payload.attributes = { + objects: erroredObjects, + }; + throw err; + } + + for (const referencedObject of referencedObjects) { + const dataSource = await findDataSourceForObject(referencedObject, savedObjectsClient); + if (dataSource) { + return dataSource; + } + } + return null; +} diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/duplicate_result_flyout.test.tsx.snap b/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/duplicate_result_flyout.test.tsx.snap index 9501523a3755..f86aa5155d62 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/duplicate_result_flyout.test.tsx.snap +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/duplicate_result_flyout.test.tsx.snap @@ -369,7 +369,7 @@ HTMLCollection [ data-test-subj="copySavedObjectsSuccess" > - 4 saved objects copied + 5 saved objects copied
- 2 Error copying file + 3 Error copying file
@@ -500,6 +500,54 @@ HTMLCollection [ +
+
+ + + +
+
+
+

+ data-source [id=3] +

+
+
+
+
+ + + +
+
+
diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.test.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.test.tsx index 154a4c24e3eb..15aa79007853 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.test.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.test.tsx @@ -23,6 +23,15 @@ describe('DuplicateResultFlyout', () => { meta: {}, error: { type: 'unsupported_type' }, }, + { + type: 'data-source', + id: '3', + meta: {}, + error: { + type: 'missing_target_workspace_assigned_data_source', + message: 'The object hasn’t be copied to the selected workspace.', + }, + }, ]; const successfulCopies: SavedObjectsImportSuccess[] = [ { @@ -45,6 +54,14 @@ describe('DuplicateResultFlyout', () => { onClose: onCloseMock, }; + const DuplicateResultFlyoutComponent = (props: DuplicateResultFlyoutProps) => { + return ( + + + + ); + }; + afterEach(() => { jest.clearAllMocks(); }); @@ -65,9 +82,9 @@ describe('DuplicateResultFlyout', () => { expect(screen.getByText('Copy saved objects to targetWorkspace')).toBeInTheDocument(); // Check result counts - expect(screen.getByText('4 saved objects copied')).toBeInTheDocument(); + expect(screen.getByText('5 saved objects copied')).toBeInTheDocument(); expect(screen.getByText('2 Successful')).toBeInTheDocument(); - expect(screen.getByText('2 Error copying file')).toBeInTheDocument(); + expect(screen.getByText('3 Error copying file')).toBeInTheDocument(); // Check successful copy icon and message expect(screen.getByLabelText('visualization')).toBeInTheDocument(); @@ -82,6 +99,9 @@ describe('DuplicateResultFlyout', () => { expect(screen.getByLabelText('config')).toBeInTheDocument(); expect(screen.getByText('Failed Config Title')).toBeInTheDocument(); + + expect(screen.getByLabelText('data-source')).toBeInTheDocument(); + expect(screen.getByText('data-source [id=3]')).toBeInTheDocument(); }); it('calls onClose when the close button is clicked', () => { diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx index 0698a55b6d52..e7e42175605e 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx @@ -65,6 +65,8 @@ const getErrorMessage = ({ error }: SavedObjectsImportError) => { return error.message; } else if (error.type === 'unsupported_type') { return unsupportedTypeErrorMessage; + } else if (error.type === 'missing_target_workspace_assigned_data_source') { + return error.message; } }; diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 2cbdab7f012c..4e93cf456503 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -8,6 +8,6 @@ "opensearchDashboardsReact", "navigation" ], - "optionalPlugins": ["savedObjectsManagement","management","dataSourceManagement","contentManagement"], + "optionalPlugins": ["savedObjectsManagement","management","dataSourceManagement","contentManagement", "dataSource"], "requiredBundles": ["opensearchDashboardsReact","dataSource", "dataSourceManagement","contentManagement"] } diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 8aa8de48c8c7..82e84e714dbd 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -54,6 +54,11 @@ import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consume import { WorkspaceUiSettingsClientWrapper } from './saved_objects/workspace_ui_settings_client_wrapper'; import { uiSettings } from './ui_settings'; import { RepositoryWrapper } from './saved_objects/repository_wrapper'; +import { DataSourcePluginSetup } from '../../data_source/server'; + +export interface WorkspacePluginDependencies { + dataSource: DataSourcePluginSetup; +} export class WorkspacePlugin implements Plugin { private readonly logger: Logger; @@ -212,10 +217,11 @@ export class WorkspacePlugin implements Plugin ({ diff --git a/src/plugins/workspace/server/routes/duplicate.ts b/src/plugins/workspace/server/routes/duplicate.ts index 8b92376ecc12..079951954ae1 100644 --- a/src/plugins/workspace/server/routes/duplicate.ts +++ b/src/plugins/workspace/server/routes/duplicate.ts @@ -17,7 +17,8 @@ export const registerDuplicateRoute = ( router: IRouter, logger: Logger, client: IWorkspaceClientImpl, - maxImportExportSize: number + maxImportExportSize: number, + isDataSourceEnabled: boolean ) => { router.post( { @@ -88,7 +89,7 @@ export const registerDuplicateRoute = ( overwrite: false, createNewCopies: true, workspaces: [targetWorkspace], - dataSourceEnabled: true, + dataSourceEnabled: isDataSourceEnabled, }); return res.ok({ body: result }); diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index 8a599812f0dc..21611f4d8a03 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -114,6 +114,7 @@ export function registerRoutes({ maxImportExportSize, permissionControlClient, isPermissionControlEnabled, + isDataSourceEnabled, }: { client: IWorkspaceClientImpl; logger: Logger; @@ -121,6 +122,7 @@ export function registerRoutes({ maxImportExportSize: number; permissionControlClient?: SavedObjectsPermissionControlContract; isPermissionControlEnabled: boolean; + isDataSourceEnabled: boolean; }) { router.post( { @@ -336,5 +338,5 @@ export function registerRoutes({ ); // duplicate saved objects among workspaces - registerDuplicateRoute(router, logger, client, maxImportExportSize); + registerDuplicateRoute(router, logger, client, maxImportExportSize, isDataSourceEnabled); } From 0db2dd8991675c170957619ca52b855f780db0cb Mon Sep 17 00:00:00 2001 From: yubonluo Date: Fri, 8 Nov 2024 13:18:50 +0800 Subject: [PATCH 02/13] refactor copy result summary flyout Signed-off-by: yubonluo --- .../import/create_saved_objects.test.ts | 5 + .../import/create_saved_objects.ts | 7 +- .../import/import_saved_objects.test.ts | 140 +++++------ .../import/import_saved_objects.ts | 55 ++--- src/core/server/saved_objects/import/types.ts | 9 +- .../server/saved_objects/import/utils.test.ts | 164 ++++++------- src/core/server/saved_objects/import/utils.ts | 60 ++--- .../import/validate_data_sources.test.ts | 87 +++++++ .../import/validate_data_sources.ts | 61 +++++ .../components/duplicate_result_flyout.tsx | 227 +++++++++++++++--- .../objects_table/saved_objects_table.tsx | 64 +++++ .../workspace/server/routes/duplicate.ts | 1 + 12 files changed, 595 insertions(+), 285 deletions(-) create mode 100644 src/core/server/saved_objects/import/validate_data_sources.test.ts create mode 100644 src/core/server/saved_objects/import/validate_data_sources.ts 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 index 2237017f3400..db7abdae104f 100644 --- 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 index a90ad802edaa..492ff68e0d09 100644 --- 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 ({ 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 index 545cea5c8e6e..a42786c968d8 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -49,11 +49,12 @@ 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 { findDataSourceForObject } from './utils'; +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'); @@ -74,6 +75,7 @@ describe('#importSavedObjectsFromStream', () => { }); getMockFn(regenerateIds).mockReturnValue(new Map()); getMockFn(validateReferences).mockResolvedValue([]); + getMockFn(validateDataSources).mockResolvedValue([]); getMockFn(checkConflicts).mockResolvedValue({ errors: [], filteredObjects: [], @@ -105,7 +107,8 @@ describe('#importSavedObjectsFromStream', () => { createNewCopies: boolean = false, dataSourceId: string | undefined = undefined, dataSourceEnabled: boolean | undefined = false, - workspaces: SavedObjectsBaseOptions['workspaces'] = undefined + workspaces: SavedObjectsBaseOptions['workspaces'] = undefined, + isCopy: boolean = false ): SavedObjectsImportOptions => { readStream = new Readable(); savedObjectsClient = savedObjectsClientMock.create(); @@ -127,6 +130,8 @@ describe('#importSavedObjectsFromStream', () => { createNewCopies, dataSourceId, workspaces, + isCopy, + dataSourceEnabled, }; }; const createObject = ( @@ -162,20 +167,6 @@ describe('#importSavedObjectsFromStream', () => { error: { type: 'conflict' }, }; }; - const findObjectsMock = { - saved_objects: [ - { - id: 'data-source-1', - score: 0, - type: 'data-source', - attributes: {}, - references: [], - }, - ], - total: 1, - page: 1, - per_page: 1, - }; /** * These tests use minimal mocks which don't look realistic, but are sufficient to exercise the code paths correctly. For example, for an @@ -214,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(); @@ -416,73 +426,6 @@ describe('#importSavedObjectsFromStream', () => { expect(result).toEqual({ success: false, successCount: 0, errors: [expect.any(Object)] }); }); - test('validates workspace with assigned data source', async () => { - const options = setupOptions(false, undefined, true, ['workspace-1']); - const collectedObjects = [createObject()]; - getMockFn(collectSavedObjects).mockResolvedValue({ - errors: [], - collectedObjects, - importIdMap: new Map(), - }); - getMockFn(findDataSourceForObject).mockResolvedValue('data-source-1'); - const clientMock = savedObjectsClientMock.create(); - clientMock.find.mockResolvedValueOnce(findObjectsMock); - - const result = await importSavedObjectsFromStream({ - ...options, - savedObjectsClient: clientMock, - }); - expect(result).toEqual({ success: true, successCount: 0 }); - }); - - test('validates workspace with unassigned data source', async () => { - const options = setupOptions(false, undefined, true, ['workspace-1']); - const collectedObjects = [createObject()]; - getMockFn(collectSavedObjects).mockResolvedValue({ - errors: [], - collectedObjects, - importIdMap: new Map(), - }); - getMockFn(findDataSourceForObject).mockResolvedValue('data-source-2'); - const clientMock = savedObjectsClientMock.create(); - clientMock.find.mockResolvedValueOnce(findObjectsMock); - - const result = await importSavedObjectsFromStream({ - ...options, - savedObjectsClient: clientMock, - }); - expect(result).toEqual({ success: false, successCount: 0, errors: [expect.any(Object)] }); - expect(result.errors?.[0].error).toEqual({ - message: - 'The object hasn’t be copied to the selected workspace. The data source (data-source-2) is not available in the selected workspace.', - type: 'missing_target_workspace_assigned_data_source', - }); - }); - - test('validates workspace with catch error', async () => { - const options = setupOptions(false, undefined, true, ['workspace-1']); - const collectedObjects = [createObject()]; - getMockFn(collectSavedObjects).mockResolvedValue({ - errors: [], - collectedObjects, - importIdMap: new Map(), - }); - getMockFn(findDataSourceForObject).mockRejectedValue(null); - const clientMock = savedObjectsClientMock.create(); - clientMock.find.mockResolvedValueOnce(findObjectsMock); - - const result = await importSavedObjectsFromStream({ - ...options, - savedObjectsClient: clientMock, - }); - expect(result).toEqual({ success: false, successCount: 0, errors: [expect.any(Object)] }); - expect(result.errors?.[0].error).toEqual({ - message: null, - statusCode: 500, - type: 'unknown', - }); - }); - describe('handles a mix of successes and errors and injects metadata', () => { const obj1 = createObject(); const tmp = createObject(); @@ -688,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 index 7997d4af6e8d..2737610a1af9 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -42,7 +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 { findDataSourceForObject } from './utils'; +import { validateDataSources } from './validate_data_sources'; /** * Import saved objects from given stream. See the {@link SavedObjectsImportOptions | options} for more @@ -62,6 +62,7 @@ export async function importSavedObjectsFromStream({ dataSourceTitle, workspaces, dataSourceEnabled, + isCopy, }: SavedObjectsImportOptions): Promise { let errorAccumulator: SavedObjectsImportError[] = []; const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); @@ -96,44 +97,6 @@ export async function importSavedObjectsFromStream({ } } - // If target workspace is exists, it will check whether the assigned data sources in the target workspace - // include the data sources of the imported saved objects. - if (workspaces && workspaces.length > 0) { - const assignedDataSourcesInTargetWorkspace = await savedObjectsClient - .find({ - type: 'data-source', - fields: ['id'], - perPage: 10000, - workspaces, - }) - .then((response) => { - return response?.saved_objects?.map((source) => source.id) ?? []; - }); - for (const object of collectSavedObjectsResult.collectedObjects) { - try { - const referenceDSId = await findDataSourceForObject(object, savedObjectsClient); - if (referenceDSId && !assignedDataSourcesInTargetWorkspace.includes(referenceDSId)) { - collectSavedObjectsResult.errors.push({ - type: object.type, - id: object.id, - error: { - type: 'missing_target_workspace_assigned_data_source', - message: `The object hasn’t be copied to the selected workspace. The data source (${referenceDSId}) is not available in the selected workspace.`, - }, - meta: { title: object.attributes?.title }, - }); - } - } catch (err) { - collectSavedObjectsResult.errors.push({ - type: object.type, - id: object.id, - error: { type: 'unknown', message: err, statusCode: 500 }, - meta: { title: object.attributes?.title }, - }); - } - } - } - errorAccumulator = [...errorAccumulator, ...collectSavedObjectsResult.errors]; /** Map of all IDs for objects that we are attempting to import; each value is empty by default */ let importIdMap = collectSavedObjectsResult.importIdMap; @@ -147,6 +110,20 @@ export async function importSavedObjectsFromStream({ ); errorAccumulator = [...errorAccumulator, ...validateReferencesResult]; + if (isCopy) { + // Unable to copy data source among 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 index a6f1af19b519..55d4d8d7e83e 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -109,9 +109,9 @@ export interface SavedObjectsImportMissingReferencesError { * Represents a failure to import due to missing target workspace assigned data source. * @public */ -export interface SavedObjectsImportMissingTargetWorkspaceAssignedDataSourceError { - type: 'missing_target_workspace_assigned_data_source'; - message: string; +export interface SavedObjectsImportMissingDataSourceError { + type: 'missing_data_source'; + dataSourceName: string; } /** @@ -136,7 +136,7 @@ export interface SavedObjectsImportError { | SavedObjectsImportUnsupportedTypeError | SavedObjectsImportMissingReferencesError | SavedObjectsImportUnknownError - | SavedObjectsImportMissingTargetWorkspaceAssignedDataSourceError; + | SavedObjectsImportMissingDataSourceError; } /** @@ -201,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 index 5d68e7f905ae..e39fa228ed06 100644 --- a/src/core/server/saved_objects/import/utils.test.ts +++ b/src/core/server/saved_objects/import/utils.test.ts @@ -10,13 +10,12 @@ import { getUpdatedTSVBVisState, updateDataSourceNameInVegaSpec, updateDataSourceNameInTimeline, - findDataSourceForObject, + findReferenceDataSourceForObject, } from './utils'; import { parse } from 'hjson'; import { isEqual } from 'lodash'; import { join } from 'path'; import { SavedObject, SavedObjectsClientContract } from '../types'; -import { savedObjectsClientMock } from '../../mocks'; describe('updateDataSourceNameInVegaSpec()', () => { const loadHJSONStringFromFile = (filepath: string) => { @@ -345,106 +344,93 @@ describe('getUpdatedTSVBVisState', () => { ); }); -describe('findDataSourceForObject', () => { - let savedObjectsClient: jest.Mocked; - const indexPatternObject: SavedObject = { - id: 'indexPattern', - type: 'index-pattern', - references: [{ type: 'data-source', id: 'dataSource', name: 'dataSource' }], - attributes: {}, - }; - - beforeEach(() => { - savedObjectsClient = savedObjectsClientMock.create(); - }); - - it('should return the data source id for an index-pattern object', async () => { - const dataSourceId = await findDataSourceForObject(indexPatternObject, savedObjectsClient); - expect(dataSourceId).toBe('dataSource'); - }); - - it('should use bulkGet to resolve multiple references and return data source', async () => { - const savedObject: SavedObject = { - type: 'dashboard', - id: 'dashboard', - references: [{ type: 'visualization', id: 'visualization', name: 'visualization' }], +describe('findReferenceDataSourceForObject', () => { + const savedObjects: Array> = [ + { + id: '1', + references: [{ id: '5', type: 'data-source', name: '5' }], + type: 'index-pattern', attributes: {}, - }; - - const visualizationObject: SavedObject = { - type: 'visualization', - id: 'visualization', - references: [{ type: 'index-pattern', id: 'indexPattern', name: 'indexPattern' }], + }, + { + 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: {}, + }, + ]; - savedObjectsClient.bulkGet.mockResolvedValueOnce({ - saved_objects: [visualizationObject], - }); - savedObjectsClient.bulkGet.mockResolvedValueOnce({ - saved_objects: [indexPatternObject], - }); + const ObjectsMap = new Map(savedObjects.map((so) => [so.id, so])); - const dataSourceId = await findDataSourceForObject(savedObject, savedObjectsClient); - expect(dataSourceId).toBe('dataSource'); + test('returns null if no references exist', () => { + expect(findReferenceDataSourceForObject(savedObjects[2], ObjectsMap)).toBeNull(); }); - it('should use bulkGet to resolve multiple references and return the first found data source', async () => { - const savedObject: SavedObject = { - id: 'visualization', - type: 'visualization', - references: [ - // { type: 'index-pattern', id: 'indexPattern', name: 'index-pattern' }, - { type: 'dashboard', id: 'dashboard', name: 'dashboard' }, - ], - attributes: {}, - }; + test('returns the data-source reference if it exists in the references', () => { + const result = findReferenceDataSourceForObject(savedObjects[0], ObjectsMap); + expect(result).toEqual({ id: '5', type: 'data-source', name: '5' }); + }); - savedObjectsClient.bulkGet.mockResolvedValue({ - saved_objects: [indexPatternObject], - }); + test('returns null if there is no data-source reference and no nested references', () => { + expect(findReferenceDataSourceForObject(savedObjects[1], ObjectsMap)).toBeNull(); + }); - const dataSourceId = await findDataSourceForObject(savedObject, savedObjectsClient); - expect(dataSourceId).toBe('dataSource'); - expect(savedObjectsClient.bulkGet).toHaveBeenCalledWith([ - { type: 'dashboard', id: 'dashboard' }, - ]); + test('returns nested data-source reference if found', () => { + const result = findReferenceDataSourceForObject(savedObjects[3], ObjectsMap); + expect(result).toEqual({ id: '5', type: 'data-source', name: '5' }); }); - it('should return null if no data source is found', async () => { - const savedObject: SavedObject = { - id: 'visualization', - type: 'visualization', - references: [], - attributes: {}, - }; + test('returns null if circular reference', () => { + const circularAssets: Array> = [ + { + 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 dataSourceId = await findDataSourceForObject(savedObject, savedObjectsClient); - expect(dataSourceId).toBeNull(); + const result = findReferenceDataSourceForObject(circularAssets[0], circularAssetsMap); + expect(result).toBeNull(); }); - it('should return null if there is an error in bulkGet', async () => { - const savedObject: SavedObject = { - id: 'visualization', - type: 'visualization', - references: [{ type: 'index-pattern', id: 'indexPattern', name: 'indexPattern' }], - attributes: {}, - }; - - savedObjectsClient.bulkGet.mockResolvedValue({ - saved_objects: [ - { - id: 'indexPattern', - type: 'index-pattern', - error: { error: '', statusCode: 400, message: '' }, - attributes: undefined, - references: [], - }, - ], - }); + test('returns null if circular reference is itself', () => { + const circularAssets: Array> = [ + { + 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])); - await expect(findDataSourceForObject(savedObject, savedObjectsClient)).rejects.toThrow( - 'Bad Request' - ); + const result = findReferenceDataSourceForObject(circularAssets[0], circularAssetsMap); + expect(result).toBeNull(); }); }); diff --git a/src/core/server/saved_objects/import/utils.ts b/src/core/server/saved_objects/import/utils.ts index 9566993f7b9a..8a637d0db179 100644 --- a/src/core/server/saved_objects/import/utils.ts +++ b/src/core/server/saved_objects/import/utils.ts @@ -185,52 +185,42 @@ const parseJSONSpec = (spec: string) => { return undefined; }; -export async function findDataSourceForObject( +export function findReferenceDataSourceForObject( savedObject: SavedObject, - savedObjectsClient: SavedObjectsClientContract, - visitedObjects: Set = new Set() -): Promise { - const references = savedObject.references; + savedObjects: Map< + string, + SavedObject<{ + title?: string | undefined; + }> + >, + visited = new Set() +): SavedObjectReference | null { + const { references } = savedObject; if (!references || references.length === 0) { return null; } - const objectKey = `${savedObject.type}:${savedObject.id}`; - if (visitedObjects.has(objectKey)) { - return null; - } - visitedObjects.add(objectKey); - const dataSourceReference = references.find((ref) => ref.type === 'data-source'); if (dataSourceReference) { - return dataSourceReference.id; + return dataSourceReference; } - const bulkGetResponse = await savedObjectsClient.bulkGet( - references.map((reference) => ({ - type: reference.type, - id: reference.id, - })) - ); - - const referencedObjects = bulkGetResponse.saved_objects; - const erroredObjects = referencedObjects.filter( - (obj) => obj.error && obj.error.statusCode !== 404 - ); - - if (erroredObjects.length > 0) { - const err = Boom.badRequest(); - err.output.payload.attributes = { - objects: erroredObjects, - }; - throw err; - } + visited.add(savedObject.id); - for (const referencedObject of referencedObjects) { - const dataSource = await findDataSourceForObject(referencedObject, savedObjectsClient); - if (dataSource) { - return dataSource; + for (const { id } of references) { + if (visited.has(id)) { + continue; + } + const referenceObject = savedObjects.get(id); + if (referenceObject) { + const dataSource = findReferenceDataSourceForObject(referenceObject, savedObjects, visited); + if (dataSource) { + return dataSource; + } } } + + visited.delete(savedObject.id); + return null; } diff --git a/src/core/server/saved_objects/import/validate_data_sources.test.ts b/src/core/server/saved_objects/import/validate_data_sources.test.ts new file mode 100644 index 000000000000..a99f70f6363a --- /dev/null +++ b/src/core/server/saved_objects/import/validate_data_sources.test.ts @@ -0,0 +1,87 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObjectsImportError } from '../types'; +import { validateDataSources } from './validate_data_sources'; +import { savedObjectsClientMock } from '../service/saved_objects_client.mock'; +import * as utilsExports from './utils'; + +describe('validateDataSources', () => { + const savedObjectsClient = savedObjectsClientMock.create(); + const errorAccumulator: SavedObjectsImportError[] = [ + { + id: '1', + type: 'dashboards', + meta: {}, + error: { type: 'missing_references', references: [] }, + }, + ]; + const workspaces = ['workspace-1']; + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('returns empty array if no valid objects', async () => { + const savedObjects = [{ id: '1', type: 'dashboards', attributes: {}, references: [] }]; + const result = await validateDataSources( + savedObjects, + savedObjectsClient, + errorAccumulator, + workspaces + ); + expect(result).toEqual([]); + }); + + it('validates data sources within the target workspace', async () => { + savedObjectsClient.find.mockResolvedValueOnce({ + saved_objects: [{ id: 'data-source-1' }], + }); + + jest.spyOn(utilsExports, 'findReferenceDataSourceForObject').mockImplementation(() => ({ + id: 'data-source-1', + type: 'data-source', + name: 'DataSource 1', + })); + + const savedObjects = [{ id: '1', type: 'dashboards', attributes: {}, references: [] }]; + + const result = await validateDataSources(savedObjects, savedObjectsClient, [], workspaces); + expect(result).toEqual([]); + expect(savedObjectsClient.find).toHaveBeenCalled(); + }); + + it('accumulates missing data source errors', async () => { + savedObjectsClient.find.mockResolvedValueOnce({ + saved_objects: [{ id: 'data-source-1' }], + }); + + jest.spyOn(utilsExports, 'findReferenceDataSourceForObject').mockImplementation(() => ({ + id: 'data-source-2', + type: 'data-source', + name: 'DataSource 2', + })); + + const savedObjects = [ + { id: '1', type: 'dashboards', attributes: { title: 'dashboards' }, references: [] }, + ]; + const result = await validateDataSources(savedObjects, savedObjectsClient, [], workspaces); + + expect(result).toEqual([ + { + error: { + dataSourceName: 'DataSource 2', + type: 'missing_data_source', + }, + id: '1', + meta: { + title: 'dashboards', + }, + title: 'dashboards', + type: 'dashboards', + }, + ]); + }); +}); diff --git a/src/core/server/saved_objects/import/validate_data_sources.ts b/src/core/server/saved_objects/import/validate_data_sources.ts new file mode 100644 index 000000000000..a0cb2dbbf80c --- /dev/null +++ b/src/core/server/saved_objects/import/validate_data_sources.ts @@ -0,0 +1,61 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObject, SavedObjectsBaseOptions, SavedObjectsClientContract } from '../types'; +import { SavedObjectsImportError } from './types'; +import { findReferenceDataSourceForObject } from './utils'; + +// Check whether the target workspace includes the data source from the references of coping assets. +export async function validateDataSources( + savedObjects: Array>, + savedObjectsClient: SavedObjectsClientContract, + errorAccumulator: SavedObjectsImportError[], + workspaces: SavedObjectsBaseOptions['workspaces'] +) { + // Filter out any objects that resulted in errors + const errorSet = errorAccumulator.reduce( + (acc, { type, id }) => acc.add(`${type}:${id}`), + new Set() + ); + + const filteredObjects = savedObjects.filter(({ type, id }) => !errorSet.has(`${type}:${id}`)); + + if (filteredObjects?.length === 0) { + return []; + } + + const errorMap: { [key: string]: SavedObjectsImportError } = {}; + const assignedDataSourcesInTargetWorkspace = await savedObjectsClient + .find({ + type: 'data-source', + fields: ['id'], + perPage: 999, + workspaces, + }) + .then((response) => { + return response?.saved_objects?.map((ds) => ds.id) ?? []; + }); + + const filteredObjectsMap = new Map(savedObjects.map((so) => [so.id, so])); + + for (const object of filteredObjects) { + const { id, type, attributes } = object; + const referenceDS = findReferenceDataSourceForObject(object, filteredObjectsMap); + if (referenceDS && !assignedDataSourcesInTargetWorkspace.includes(referenceDS.id)) { + errorMap[`${type}:${id}`] = { + id, + type, + title: attributes?.title, + meta: { title: attributes?.title }, + error: { + type: 'missing_data_source', + dataSourceName: referenceDS.name, + }, + }; + } + } + + return Object.values(errorMap); +} diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx index e7e42175605e..e3249cf1f608 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx @@ -5,6 +5,7 @@ import './import_summary.scss'; import { + EuiCallOut, EuiFlexGroup, EuiFlexItem, EuiFlyout, @@ -15,6 +16,7 @@ import { EuiIcon, EuiIconTip, EuiSmallButton, + EuiSmallButtonEmpty, EuiSpacer, EuiText, EuiTitle, @@ -27,6 +29,7 @@ import { SavedObjectsImportError, SavedObjectsImportSuccess } from 'opensearch-d import { FormattedMessage } from '@osd/i18n/react'; import { getSavedObjectLabel } from '../../..'; import { getDefaultTitle } from '../../../lib'; +import { DuplicateObject } from '../../types'; interface CopyItem { type: string; @@ -47,10 +50,20 @@ export interface DuplicateResultFlyoutProps { failedCopies: SavedObjectsImportError[]; successfulCopies: SavedObjectsImportSuccess[]; onClose: () => void; + onCopy: ( + savedObjects: DuplicateObject[], + includeReferencesDeep: boolean, + targetWorkspace: string, + targetWorkspaceName: string, + isSolveErrorsOperation: boolean + ) => Promise; + targetWorkspace: string; + useUpdatedUX: boolean; } interface State { isLoading: boolean; + showRemainingButton: boolean; } const DEFAULT_ICON = 'apps'; @@ -65,17 +78,83 @@ const getErrorMessage = ({ error }: SavedObjectsImportError) => { return error.message; } else if (error.type === 'unsupported_type') { return unsupportedTypeErrorMessage; - } else if (error.type === 'missing_target_workspace_assigned_data_source') { - return error.message; + } else if (error.type === 'missing_data_source') { + return i18n.translate('savedObjectsManagement.objectsTable.copyResult.missingDataSourceError', { + defaultMessage: 'Missing data source: {dsName}.', + values: { dsName: error.dataSourceName }, + }); + } else if (error.type === 'missing_references') { + return i18n.translate('savedObjectsManagement.objectsTable.copyResult.missingReferencesError', { + defaultMessage: 'Missing references.', + }); } }; export class DuplicateResultFlyout extends React.Component { constructor(props: DuplicateResultFlyoutProps) { super(props); - this.state = { isLoading: false }; + this.state = { + isLoading: false, + showRemainingButton: + props.successfulCopies.length > 0 && + !!props.failedCopies.find( + ({ error }) => error.type === 'missing_references' || error.type === 'missing_data_source' + ), + }; } + indexPatternConflictsWarning = ( + + } + color="warning" + iconType="help" + > +

+ +

+ + ); + + missingDataSourceWarning = ( + + } + color="warning" + iconType="help" + > +

+ +

+ + ); + getCountIndicators(copyItems: CopyItem[]) { if (!copyItems.length) { return null; @@ -161,30 +240,10 @@ export class DuplicateResultFlyout extends React.Component this.mapFailedCopy(object)), - ...successfulCopies.map((object) => this.mapCopySuccess(object)), - ], - ['type', 'title'] - ); - + renderItems(items: CopyItem[]) { return ( - -

- -

- - - {this.getCountIndicators(copyItems)} - - {copyItems.map((item, index) => { + {items.map((item, index) => { const { type, title, icon } = item; return ( error.type === 'missing_data_source') + .map((object) => this.mapFailedCopy(object)); + + const missingReferencesItems = failedCopies + .filter(({ error }) => error.type === 'missing_references') + .map((object) => this.mapFailedCopy(object)); + + if (missingReferencesItems.length > 0 || missingDataSourceItems.length > 0) { + return ( + + {missingReferencesItems.length > 0 && ( + <> + {this.indexPatternConflictsWarning} + + {this.renderItems(missingReferencesItems)} + + + )} + {missingDataSourceItems.length > 0 && ( + <> + {this.missingDataSourceWarning} + + {this.renderItems(missingDataSourceItems)} + + )} + + ); + } + + const copyItems: CopyItem[] = _.sortBy( + [ + ...failedCopies.map((object) => this.mapFailedCopy(object)), + ...successfulCopies.map((object) => this.mapCopySuccess(object)), + ], + ['type', 'title'] + ); + + return ( + + +

+ +

+ + + {this.getCountIndicators(copyItems)} + + {this.renderItems(copyItems)} + + ); + } + + duplicateSavedObjects = async () => { + const { successfulCopies, workspaceName, onCopy, targetWorkspace } = this.props; + + const savedObjects: DuplicateObject[] = successfulCopies.map(({ id, meta, type }) => { + return { id, meta, type }; + }); + + this.setState({ + isLoading: true, + }); + + await onCopy(savedObjects, false, targetWorkspace, workspaceName, true); + + this.setState({ + isLoading: false, + showRemainingButton: false, + }); + }; + render() { - const { onClose, failedCopies, successfulCopies, workspaceName } = this.props; + const { onClose, failedCopies, successfulCopies, workspaceName, useUpdatedUX } = this.props; + const { isLoading, showRemainingButton } = this.state; + return ( @@ -225,20 +364,40 @@ export class DuplicateResultFlyout extends React.Component {this.copyResult({ failedCopies, successfulCopies })} - - - + + + + + + + {showRemainingButton && ( + + this.duplicateSavedObjects()} + isLoading={isLoading} + > + + + + )} + ); diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx index 8c77f9dd3eff..47e732090e38 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx @@ -167,6 +167,7 @@ export interface SavedObjectsTableState { failedCopies: SavedObjectsImportError[]; successfulCopies: SavedObjectsImportSuccess[]; targetWorkspaceName: string; + targetWorkspace: string; } export class SavedObjectsTable extends Component { private _isMounted = false; @@ -209,6 +210,7 @@ export class SavedObjectsTable extends Component { + const { notifications, workspaces, useUpdatedUX } = this.props; + const workspaceClient = workspaces.client$.getValue(); + + const showErrorNotification = (text?: string) => { + notifications.toasts.addDanger({ + title: i18n.translate('savedObjectsManagement.objectsTable.duplicate.dangerNotification', { + defaultMessage: + 'Unable to copy {useUpdatedUX, select, true {{errorCount, plural, one {# asset} other {# assets}}} other {{errorCount, plural, one {# saved object} other {# saved objects}}}}.', + values: { errorCount: savedObjects.length, useUpdatedUX }, + }), + ...(text && { text }), + }); + }; + if (!workspaceClient) { + showErrorNotification(); + return; + } + const objectsToDuplicate = savedObjects.map((obj) => ({ id: obj.id, type: obj.type })); + try { + const result = await workspaceClient.copy( + objectsToDuplicate, + targetWorkspace, + includeReferencesDeep + ); + if (result?.error) { + showErrorNotification(result.error); + } else { + if (isSolveErrorsOperation) { + this.setState({ + failedCopies: result?.errors || [], + successfulCopies: result?.successCount > 0 ? result.successResults : [], + targetWorkspaceName, + }); + } else { + this.setState({ + isShowingDuplicateResultFlyout: true, + failedCopies: result?.errors || [], + successfulCopies: result?.successCount > 0 ? result.successResults : [], + targetWorkspace, + targetWorkspaceName, + }); + } + } + } catch (e) { + showErrorNotification(); + } finally { + this.hideDuplicateModal(); + await this.refreshObjects(); + } + }; + + solveCopyError = async ( savedObjects: DuplicateObject[], includeReferencesDeep: boolean, targetWorkspace: string, @@ -830,6 +890,7 @@ export class SavedObjectsTable extends Component ); } diff --git a/src/plugins/workspace/server/routes/duplicate.ts b/src/plugins/workspace/server/routes/duplicate.ts index 079951954ae1..d5ce965ad410 100644 --- a/src/plugins/workspace/server/routes/duplicate.ts +++ b/src/plugins/workspace/server/routes/duplicate.ts @@ -90,6 +90,7 @@ export const registerDuplicateRoute = ( createNewCopies: true, workspaces: [targetWorkspace], dataSourceEnabled: isDataSourceEnabled, + isCopy: true, }); return res.ok({ body: result }); From a86a3f7fc7990efbb11dac4afd89e496c14bbbac Mon Sep 17 00:00:00 2001 From: yubonluo Date: Mon, 11 Nov 2024 11:03:24 +0800 Subject: [PATCH 03/13] optimize the code Signed-off-by: yubonluo --- src/core/server/saved_objects/import/types.ts | 2 +- src/core/server/saved_objects/import/utils.ts | 7 +------ .../server/saved_objects/import/validate_data_sources.ts | 8 +++++--- .../objects_table/components/duplicate_result_flyout.tsx | 4 ++-- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 55d4d8d7e83e..f844d99d2cd5 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -111,7 +111,7 @@ export interface SavedObjectsImportMissingReferencesError { */ export interface SavedObjectsImportMissingDataSourceError { type: 'missing_data_source'; - dataSourceName: string; + dataSource: string; } /** diff --git a/src/core/server/saved_objects/import/utils.ts b/src/core/server/saved_objects/import/utils.ts index 8a637d0db179..c80bca9b2c89 100644 --- a/src/core/server/saved_objects/import/utils.ts +++ b/src/core/server/saved_objects/import/utils.ts @@ -187,12 +187,7 @@ const parseJSONSpec = (spec: string) => { export function findReferenceDataSourceForObject( savedObject: SavedObject, - savedObjects: Map< - string, - SavedObject<{ - title?: string | undefined; - }> - >, + savedObjects: Map, visited = new Set() ): SavedObjectReference | null { const { references } = savedObject; diff --git a/src/core/server/saved_objects/import/validate_data_sources.ts b/src/core/server/saved_objects/import/validate_data_sources.ts index a0cb2dbbf80c..ac5bab270ded 100644 --- a/src/core/server/saved_objects/import/validate_data_sources.ts +++ b/src/core/server/saved_objects/import/validate_data_sources.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { fetchNestedDependencies } from '../export/inject_nested_depdendencies'; import { SavedObject, SavedObjectsBaseOptions, SavedObjectsClientContract } from '../types'; import { SavedObjectsImportError } from './types'; import { findReferenceDataSourceForObject } from './utils'; @@ -38,11 +39,12 @@ export async function validateDataSources( return response?.saved_objects?.map((ds) => ds.id) ?? []; }); - const filteredObjectsMap = new Map(savedObjects.map((so) => [so.id, so])); + const nestedDependencies = await fetchNestedDependencies(filteredObjects, savedObjectsClient); + const nestedObjectsMap = new Map(nestedDependencies.objects.map((object) => [object.id, object])); for (const object of filteredObjects) { const { id, type, attributes } = object; - const referenceDS = findReferenceDataSourceForObject(object, filteredObjectsMap); + const referenceDS = findReferenceDataSourceForObject(object, nestedObjectsMap); if (referenceDS && !assignedDataSourcesInTargetWorkspace.includes(referenceDS.id)) { errorMap[`${type}:${id}`] = { id, @@ -51,7 +53,7 @@ export async function validateDataSources( meta: { title: attributes?.title }, error: { type: 'missing_data_source', - dataSourceName: referenceDS.name, + dataSource: referenceDS.id, }, }; } diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx index e3249cf1f608..9044c8b71348 100644 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx +++ b/src/plugins/saved_objects_management/public/management_section/objects_table/components/duplicate_result_flyout.tsx @@ -80,8 +80,8 @@ const getErrorMessage = ({ error }: SavedObjectsImportError) => { return unsupportedTypeErrorMessage; } else if (error.type === 'missing_data_source') { return i18n.translate('savedObjectsManagement.objectsTable.copyResult.missingDataSourceError', { - defaultMessage: 'Missing data source: {dsName}.', - values: { dsName: error.dataSourceName }, + defaultMessage: 'Missing data source: {ds}', + values: { ds: error.dataSource }, }); } else if (error.type === 'missing_references') { return i18n.translate('savedObjectsManagement.objectsTable.copyResult.missingReferencesError', { From 91fdbe6a8e6e1b15a55e3997e68f05305f7f36f7 Mon Sep 17 00:00:00 2001 From: yubonluo Date: Mon, 11 Nov 2024 16:00:04 +0800 Subject: [PATCH 04/13] optimize the code Signed-off-by: yubonluo --- .../server/saved_objects/import/utils.test.ts | 18 +- .../import/validate_data_sources.test.ts | 18 +- .../duplicate_result_flyout.test.tsx.snap | 675 ------------------ .../duplicate_result_flyout.test.tsx | 174 +++-- .../integration_tests/duplicate.test.ts | 71 ++ 5 files changed, 207 insertions(+), 749 deletions(-) delete mode 100644 src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/duplicate_result_flyout.test.tsx.snap diff --git a/src/core/server/saved_objects/import/utils.test.ts b/src/core/server/saved_objects/import/utils.test.ts index e39fa228ed06..72f66d0fd5ec 100644 --- a/src/core/server/saved_objects/import/utils.test.ts +++ b/src/core/server/saved_objects/import/utils.test.ts @@ -370,14 +370,16 @@ describe('findReferenceDataSourceForObject', () => { type: 'non-data-source', attributes: {}, }, + { + id: '6', + references: [{ id: '7', type: 'index-pattern', name: '7' }], + type: 'non-data-source', + attributes: {}, + }, ]; const ObjectsMap = new Map(savedObjects.map((so) => [so.id, so])); - test('returns null if no references exist', () => { - expect(findReferenceDataSourceForObject(savedObjects[2], ObjectsMap)).toBeNull(); - }); - test('returns the data-source reference if it exists in the references', () => { const result = findReferenceDataSourceForObject(savedObjects[0], ObjectsMap); expect(result).toEqual({ id: '5', type: 'data-source', name: '5' }); @@ -387,11 +389,19 @@ describe('findReferenceDataSourceForObject', () => { expect(findReferenceDataSourceForObject(savedObjects[1], ObjectsMap)).toBeNull(); }); + test('returns null if no references exist', () => { + expect(findReferenceDataSourceForObject(savedObjects[2], ObjectsMap)).toBeNull(); + }); + test('returns nested data-source reference if found', () => { const result = findReferenceDataSourceForObject(savedObjects[3], ObjectsMap); expect(result).toEqual({ id: '5', type: 'data-source', name: '5' }); }); + test('returns null if the nested references have no data-source reference', () => { + expect(findReferenceDataSourceForObject(savedObjects[4], ObjectsMap)).toBeNull(); + }); + test('returns null if circular reference', () => { const circularAssets: Array> = [ { diff --git a/src/core/server/saved_objects/import/validate_data_sources.test.ts b/src/core/server/saved_objects/import/validate_data_sources.test.ts index a99f70f6363a..1b0d1fa6c7f5 100644 --- a/src/core/server/saved_objects/import/validate_data_sources.test.ts +++ b/src/core/server/saved_objects/import/validate_data_sources.test.ts @@ -10,14 +10,6 @@ import * as utilsExports from './utils'; describe('validateDataSources', () => { const savedObjectsClient = savedObjectsClientMock.create(); - const errorAccumulator: SavedObjectsImportError[] = [ - { - id: '1', - type: 'dashboards', - meta: {}, - error: { type: 'missing_references', references: [] }, - }, - ]; const workspaces = ['workspace-1']; beforeEach(() => { @@ -25,6 +17,14 @@ describe('validateDataSources', () => { }); it('returns empty array if no valid objects', async () => { + const errorAccumulator: SavedObjectsImportError[] = [ + { + id: '1', + type: 'dashboards', + meta: {}, + error: { type: 'missing_references', references: [] }, + }, + ]; const savedObjects = [{ id: '1', type: 'dashboards', attributes: {}, references: [] }]; const result = await validateDataSources( savedObjects, @@ -72,7 +72,7 @@ describe('validateDataSources', () => { expect(result).toEqual([ { error: { - dataSourceName: 'DataSource 2', + dataSource: 'data-source-2', type: 'missing_data_source', }, id: '1', diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/duplicate_result_flyout.test.tsx.snap b/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/duplicate_result_flyout.test.tsx.snap deleted file mode 100644 index f86aa5155d62..000000000000 --- a/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/duplicate_result_flyout.test.tsx.snap +++ /dev/null @@ -1,675 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`DuplicateResultFlyout copy count is null 1`] = ` -HTMLCollection [ - - - - - - - - - - -