From 53d84f21fe6a7f05d2d7f20283fab6ac518b4d65 Mon Sep 17 00:00:00 2001 From: gaobinlong Date: Wed, 20 Sep 2023 13:19:49 +0800 Subject: [PATCH 01/25] [Workspace] Add workspaces parameters to all saved objects API Signed-off-by: gaobinlong --- .../saved_objects/saved_objects_client.ts | 1 + .../get_sorted_objects_for_export.test.ts | 6 + .../export/get_sorted_objects_for_export.ts | 9 +- .../saved_objects/import/check_conflicts.ts | 3 + .../import/create_saved_objects.ts | 5 +- .../import/import_saved_objects.ts | 2 + .../import/resolve_import_errors.ts | 2 + src/core/server/saved_objects/import/types.ts | 4 + .../build_active_mappings.test.ts.snap | 8 ++ .../migrations/core/build_active_mappings.ts | 3 + .../migrations/core/index_migrator.test.ts | 12 ++ ...pensearch_dashboards_migrator.test.ts.snap | 4 + .../saved_objects/routes/bulk_create.ts | 12 +- .../server/saved_objects/routes/create.ts | 12 +- .../server/saved_objects/routes/export.ts | 11 +- src/core/server/saved_objects/routes/find.ts | 8 ++ .../server/saved_objects/routes/import.ts | 9 ++ .../routes/resolve_import_errors.ts | 9 ++ .../saved_objects/serialization/serializer.ts | 5 +- .../saved_objects/serialization/types.ts | 2 + .../saved_objects/service/lib/repository.ts | 103 ++++++++++++++++-- .../service/lib/search_dsl/query_params.ts | 33 ++++++ .../service/lib/search_dsl/search_dsl.ts | 3 + .../server/saved_objects/service/lib/utils.ts | 7 ++ src/core/server/saved_objects/types.ts | 2 + src/core/types/saved_objects.ts | 1 + 26 files changed, 258 insertions(+), 18 deletions(-) diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index 6e5482614e40..d6b6b6b6d89c 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -345,6 +345,7 @@ export class SavedObjectsClient { filter: 'filter', namespaces: 'namespaces', preference: 'preference', + workspaces: 'workspaces', }; const renamedQuery = renameKeys(renameMap, options); diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index cf7e1d8246a7..952a74a76940 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -128,6 +128,7 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], + workspaces: undefined, }, ], ], @@ -218,6 +219,7 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], + workspaces: undefined, }, ], ], @@ -368,6 +370,7 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], + workspaces: undefined, }, ], ], @@ -459,6 +462,7 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], + workspaces: undefined, }, ], ], @@ -666,6 +670,7 @@ describe('getSortedObjectsForExport()', () => { ], Object { namespace: undefined, + workspaces: undefined, }, ], ], @@ -784,6 +789,7 @@ describe('getSortedObjectsForExport()', () => { ], Object { namespace: undefined, + workspaces: undefined, }, ], Array [ diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 7bf6e9f6ccdc..8ca085639f10 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -60,6 +60,8 @@ export interface SavedObjectsExportOptions { excludeExportDetails?: boolean; /** optional namespace to override the namespace used by the savedObjectsClient. */ namespace?: string; + /** optional workspaces to override the workspaces used by the savedObjectsClient. */ + workspaces?: string[]; } /** @@ -87,6 +89,7 @@ async function fetchObjectsToExport({ exportSizeLimit, savedObjectsClient, namespace, + workspaces, }: { objects?: SavedObjectsExportOptions['objects']; types?: string[]; @@ -94,6 +97,7 @@ async function fetchObjectsToExport({ exportSizeLimit: number; savedObjectsClient: SavedObjectsClientContract; namespace?: string; + workspaces?: string[]; }) { if ((types?.length ?? 0) > 0 && (objects?.length ?? 0) > 0) { throw Boom.badRequest(`Can't specify both "types" and "objects" properties when exporting`); @@ -105,7 +109,7 @@ async function fetchObjectsToExport({ if (typeof search === 'string') { throw Boom.badRequest(`Can't specify both "search" and "objects" properties when exporting`); } - const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace }); + const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace, workspaces }); const erroredObjects = bulkGetResult.saved_objects.filter((obj) => !!obj.error); if (erroredObjects.length) { const err = Boom.badRequest(); @@ -121,6 +125,7 @@ async function fetchObjectsToExport({ search, perPage: exportSizeLimit, namespaces: namespace ? [namespace] : undefined, + workspaces, }); if (findResponse.total > exportSizeLimit) { throw Boom.badRequest(`Can't export more than ${exportSizeLimit} objects`); @@ -153,6 +158,7 @@ export async function exportSavedObjectsToStream({ includeReferencesDeep = false, excludeExportDetails = false, namespace, + workspaces, }: SavedObjectsExportOptions) { const rootObjects = await fetchObjectsToExport({ types, @@ -161,6 +167,7 @@ export async function exportSavedObjectsToStream({ savedObjectsClient, exportSizeLimit, namespace, + workspaces, }); let exportedObjects: Array> = []; let missingReferences: SavedObjectsExportResultDetails['missingReferences'] = []; diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index 830f7f55d7c5..f36bcf3a8a92 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -44,6 +44,7 @@ interface CheckConflictsParams { ignoreRegularConflicts?: boolean; retries?: SavedObjectsImportRetry[]; createNewCopies?: boolean; + workspaces?: string[]; } const isUnresolvableConflict = (error: SavedObjectError) => @@ -56,6 +57,7 @@ export async function checkConflicts({ ignoreRegularConflicts, retries = [], createNewCopies, + workspaces, }: CheckConflictsParams) { const filteredObjects: Array> = []; const errors: SavedObjectsImportError[] = []; @@ -77,6 +79,7 @@ export async function checkConflicts({ }); const checkConflictsResult = await savedObjectsClient.checkConflicts(objectsToCheck, { namespace, + workspaces, }); const errorMap = checkConflictsResult.errors.reduce( (acc, { type, id, error }) => acc.set(`${type}:${id}`, error), 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 a3a1eebbd2ab..b67cffce1e96 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -39,6 +39,7 @@ interface CreateSavedObjectsParams { importIdMap: Map; namespace?: string; overwrite?: boolean; + workspaces?: string[]; } interface CreateSavedObjectsResult { createdObjects: Array>; @@ -56,6 +57,7 @@ export const createSavedObjects = async ({ importIdMap, namespace, overwrite, + workspaces, }: CreateSavedObjectsParams): Promise> => { // filter out any objects that resulted in errors const errorSet = accumulatedErrors.reduce( @@ -103,6 +105,7 @@ export const createSavedObjects = async ({ const bulkCreateResponse = await savedObjectsClient.bulkCreate(objectsToCreate, { namespace, overwrite, + workspaces, }); expectedResults = bulkCreateResponse.saved_objects; } @@ -110,7 +113,7 @@ export const createSavedObjects = async ({ // remap results to reflect the object IDs that were submitted for import // this ensures that consumers understand the results const remappedResults = expectedResults.map>((result) => { - const { id } = objectIdMap.get(`${result.type}:${result.id}`)!; + const { id } = objectIdMap.get(`${result.type}:${result.id}`) || ({} as SavedObject); // also, include a `destinationId` field if the object create attempt was made with a different ID return { ...result, id, ...(id !== result.id && { destinationId: result.id }) }; }); 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 cd250fc5f65f..68104db85e6f 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -54,6 +54,7 @@ export async function importSavedObjectsFromStream({ savedObjectsClient, typeRegistry, namespace, + workspaces, }: SavedObjectsImportOptions): Promise { let errorAccumulator: SavedObjectsImportError[] = []; const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); @@ -118,6 +119,7 @@ export async function importSavedObjectsFromStream({ importIdMap, overwrite, namespace, + workspaces, }; const createSavedObjectsResult = await createSavedObjects(createSavedObjectsParams); errorAccumulator = [...errorAccumulator, ...createSavedObjectsResult.errors]; diff --git a/src/core/server/saved_objects/import/resolve_import_errors.ts b/src/core/server/saved_objects/import/resolve_import_errors.ts index 162410c4ce9b..207410136645 100644 --- a/src/core/server/saved_objects/import/resolve_import_errors.ts +++ b/src/core/server/saved_objects/import/resolve_import_errors.ts @@ -59,6 +59,7 @@ export async function resolveSavedObjectsImportErrors({ typeRegistry, namespace, createNewCopies, + workspaces, }: SavedObjectsResolveImportErrorsOptions): Promise { // throw a BadRequest error if we see invalid retries validateRetries(retries); @@ -157,6 +158,7 @@ export async function resolveSavedObjectsImportErrors({ importIdMap, namespace, overwrite, + workspaces, }; const { createdObjects, errors: bulkCreateErrors } = await createSavedObjects( createSavedObjectsParams diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 88beacb9d2fd..924d1b18895a 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -187,6 +187,8 @@ 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 */ + workspaces?: string[]; } /** @@ -208,6 +210,8 @@ 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 */ + workspaces?: string[]; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap index f8ef47cae894..254bdbb4b2c6 100644 --- a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap +++ b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap @@ -13,6 +13,7 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -56,6 +57,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; @@ -74,6 +78,7 @@ Object { "thirdType": "510f1f0adb69830cf8a1c5ce2923ed82", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -134,6 +139,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; diff --git a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts index bf377a13a42e..812cc1fd5eb1 100644 --- a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts +++ b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts @@ -175,6 +175,9 @@ function defaultMapping(): IndexMapping { }, }, }, + workspaces: { + type: 'keyword', + }, }, }; } diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index 4bacfda3bd5a..4b65da5250d6 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -82,6 +82,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -92,6 +93,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, references: { type: 'nested', properties: { @@ -199,6 +203,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -210,6 +215,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, references: { type: 'nested', properties: { @@ -260,6 +268,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -271,6 +280,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, references: { type: 'nested', properties: { diff --git a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap index baebb7848798..811d5f3594bf 100644 --- a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap +++ b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap @@ -13,6 +13,7 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -64,6 +65,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; diff --git a/src/core/server/saved_objects/routes/bulk_create.ts b/src/core/server/saved_objects/routes/bulk_create.ts index 5c2844d64813..36fd4bda5bff 100644 --- a/src/core/server/saved_objects/routes/bulk_create.ts +++ b/src/core/server/saved_objects/routes/bulk_create.ts @@ -38,6 +38,9 @@ export const registerBulkCreateRoute = (router: IRouter) => { validate: { query: schema.object({ overwrite: schema.boolean({ defaultValue: false }), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }), body: schema.arrayOf( schema.object({ @@ -62,7 +65,14 @@ export const registerBulkCreateRoute = (router: IRouter) => { }, router.handleLegacyErrors(async (context, req, res) => { const { overwrite } = req.query; - const result = await context.core.savedObjects.client.bulkCreate(req.body, { overwrite }); + let workspaces = req.query.workspaces; + if (typeof workspaces === 'string') { + workspaces = [workspaces]; + } + const result = await context.core.savedObjects.client.bulkCreate(req.body, { + overwrite, + workspaces, + }); return res.ok({ body: result }); }) ); diff --git a/src/core/server/saved_objects/routes/create.ts b/src/core/server/saved_objects/routes/create.ts index c8c330ba7774..4d22bd244a03 100644 --- a/src/core/server/saved_objects/routes/create.ts +++ b/src/core/server/saved_objects/routes/create.ts @@ -56,15 +56,23 @@ export const registerCreateRoute = (router: IRouter) => { ) ), initialNamespaces: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1 })), + workspaces: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1 })), }), }, }, router.handleLegacyErrors(async (context, req, res) => { const { type, id } = req.params; const { overwrite } = req.query; - const { attributes, migrationVersion, references, initialNamespaces } = req.body; + const { attributes, migrationVersion, references, initialNamespaces, workspaces } = req.body; - const options = { id, overwrite, migrationVersion, references, initialNamespaces }; + const options = { + id, + overwrite, + migrationVersion, + references, + initialNamespaces, + workspaces, + }; const result = await context.core.savedObjects.client.create(type, attributes, options); return res.ok({ body: result }); }) diff --git a/src/core/server/saved_objects/routes/export.ts b/src/core/server/saved_objects/routes/export.ts index 2c808b731b4e..9325b632e40f 100644 --- a/src/core/server/saved_objects/routes/export.ts +++ b/src/core/server/saved_objects/routes/export.ts @@ -57,12 +57,20 @@ export const registerExportRoute = (router: IRouter, config: SavedObjectConfig) search: schema.maybe(schema.string()), includeReferencesDeep: schema.boolean({ defaultValue: false }), excludeExportDetails: schema.boolean({ defaultValue: false }), + workspaces: schema.maybe(schema.arrayOf(schema.string())), }), }, }, router.handleLegacyErrors(async (context, req, res) => { const savedObjectsClient = context.core.savedObjects.client; - const { type, objects, search, excludeExportDetails, includeReferencesDeep } = req.body; + const { + type, + objects, + search, + excludeExportDetails, + includeReferencesDeep, + workspaces, + } = req.body; const types = typeof type === 'string' ? [type] : type; // need to access the registry for type validation, can't use the schema for this @@ -98,6 +106,7 @@ export const registerExportRoute = (router: IRouter, config: SavedObjectConfig) exportSizeLimit: maxImportExportSize, includeReferencesDeep, excludeExportDetails, + workspaces, }); const docsToExport: string[] = await createPromiseFromStreams([ diff --git a/src/core/server/saved_objects/routes/find.ts b/src/core/server/saved_objects/routes/find.ts index dbc9bf9e3a0d..6cfb2d780036 100644 --- a/src/core/server/saved_objects/routes/find.ts +++ b/src/core/server/saved_objects/routes/find.ts @@ -59,6 +59,9 @@ export const registerFindRoute = (router: IRouter) => { namespaces: schema.maybe( schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) ), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }), }, }, @@ -67,6 +70,10 @@ export const registerFindRoute = (router: IRouter) => { const namespaces = typeof req.query.namespaces === 'string' ? [req.query.namespaces] : req.query.namespaces; + let workspaces = req.query.workspaces; + if (typeof workspaces === 'string') { + workspaces = [workspaces]; + } const result = await context.core.savedObjects.client.find({ perPage: query.per_page, @@ -81,6 +88,7 @@ export const registerFindRoute = (router: IRouter) => { fields: typeof query.fields === 'string' ? [query.fields] : query.fields, filter: query.filter, namespaces, + workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/routes/import.ts b/src/core/server/saved_objects/routes/import.ts index b157feb0860e..2d221d2d47bc 100644 --- a/src/core/server/saved_objects/routes/import.ts +++ b/src/core/server/saved_objects/routes/import.ts @@ -60,6 +60,9 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) { overwrite: schema.boolean({ defaultValue: false }), createNewCopies: schema.boolean({ defaultValue: false }), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }, { validate: (object) => { @@ -91,6 +94,11 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) }); } + let workspaces = req.query.workspaces; + if (typeof workspaces === 'string') { + workspaces = [workspaces]; + } + const result = await importSavedObjectsFromStream({ savedObjectsClient: context.core.savedObjects.client, typeRegistry: context.core.savedObjects.typeRegistry, @@ -98,6 +106,7 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) objectLimit: maxImportExportSize, overwrite, createNewCopies, + workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/routes/resolve_import_errors.ts b/src/core/server/saved_objects/routes/resolve_import_errors.ts index 5e07125671f1..32d67b5ae8ab 100644 --- a/src/core/server/saved_objects/routes/resolve_import_errors.ts +++ b/src/core/server/saved_objects/routes/resolve_import_errors.ts @@ -58,6 +58,9 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO validate: { query: schema.object({ createNewCopies: schema.boolean({ defaultValue: false }), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }), body: schema.object({ file: schema.stream(), @@ -98,6 +101,11 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO }); } + let workspaces = req.query.workspaces; + if (typeof workspaces === 'string') { + workspaces = [workspaces]; + } + const result = await resolveSavedObjectsImportErrors({ typeRegistry: context.core.savedObjects.typeRegistry, savedObjectsClient: context.core.savedObjects.client, @@ -105,6 +113,7 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO retries: req.body.retries, objectLimit: maxImportExportSize, createNewCopies: req.query.createNewCopies, + workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/serialization/serializer.ts b/src/core/server/saved_objects/serialization/serializer.ts index ff840a1fac60..5c3e22ac646a 100644 --- a/src/core/server/saved_objects/serialization/serializer.ts +++ b/src/core/server/saved_objects/serialization/serializer.ts @@ -73,7 +73,7 @@ export class SavedObjectsSerializer { */ public rawToSavedObject(doc: SavedObjectsRawDoc): SavedObjectSanitizedDoc { const { _id, _source, _seq_no, _primary_term } = doc; - const { type, namespace, namespaces, originId } = _source; + const { type, namespace, namespaces, originId, workspaces } = _source; const version = _seq_no != null || _primary_term != null @@ -91,6 +91,7 @@ export class SavedObjectsSerializer { ...(_source.migrationVersion && { migrationVersion: _source.migrationVersion }), ...(_source.updated_at && { updated_at: _source.updated_at }), ...(version && { version }), + ...(workspaces && { workspaces }), }; } @@ -112,6 +113,7 @@ export class SavedObjectsSerializer { updated_at, version, references, + workspaces, } = savedObj; const source = { [type]: attributes, @@ -122,6 +124,7 @@ export class SavedObjectsSerializer { ...(originId && { originId }), ...(migrationVersion && { migrationVersion }), ...(updated_at && { updated_at }), + ...(workspaces && { workspaces }), }; return { diff --git a/src/core/server/saved_objects/serialization/types.ts b/src/core/server/saved_objects/serialization/types.ts index d10ec75cdf41..473a63cf65f4 100644 --- a/src/core/server/saved_objects/serialization/types.ts +++ b/src/core/server/saved_objects/serialization/types.ts @@ -52,6 +52,7 @@ export interface SavedObjectsRawDocSource { updated_at?: string; references?: SavedObjectReference[]; originId?: string; + workspaces?: string[]; [typeMapping: string]: any; } @@ -69,6 +70,7 @@ interface SavedObjectDoc { version?: string; updated_at?: string; originId?: string; + workspaces?: string[]; } interface Referencable { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index bccfd8ff2265..a49f356bbb81 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -243,6 +243,7 @@ export class SavedObjectsRepository { originId, initialNamespaces, version, + workspaces, } = options; const namespace = normalizeNamespace(options.namespace); @@ -279,6 +280,20 @@ export class SavedObjectsRepository { } } + let savedObjectWorkspaces = workspaces; + + if (id && overwrite && workspaces) { + try { + const currentItem = await this.get(type, id); + if (currentItem && currentItem.workspaces) { + // do not overwrite workspaces + savedObjectWorkspaces = currentItem.workspaces; + } + } catch (e) { + // this.get will throw an error when no items can be found + } + } + const migrated = this._migrator.migrateDocument({ id, type, @@ -289,6 +304,7 @@ export class SavedObjectsRepository { migrationVersion, updated_at: time, ...(Array.isArray(references) && { references }), + ...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }), }); const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc); @@ -402,12 +418,16 @@ export class SavedObjectsRepository { object: { initialNamespaces, version, ...object }, method, } = expectedBulkGetResult.value; + let savedObjectWorkspaces: string[] | undefined; + let finalMethod = method; + let finalObjectId = object.id; if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound ? bulkGetResponse?.body.docs[opensearchRequestIndex] : undefined; const docFound = indexFound && actualResult?.found === true; + let hasSetNamespace = false; // @ts-expect-error MultiGetHit._source is optional if (docFound && !this.rawDocExistsInNamespace(actualResult!, namespace)) { const { id, type } = object; @@ -422,11 +442,20 @@ export class SavedObjectsRepository { }, }, }; + } else { + hasSetNamespace = true; + if (this._registry.isSingleNamespace(object.type)) { + savedObjectNamespace = initialNamespaces ? initialNamespaces[0] : namespace; + } else if (this._registry.isMultiNamespace(object.type)) { + savedObjectNamespaces = initialNamespaces || getSavedObjectNamespaces(namespace); + } + } + if (!hasSetNamespace) { + savedObjectNamespaces = + initialNamespaces || + // @ts-expect-error MultiGetHit._source is optional + getSavedObjectNamespaces(namespace, docFound ? actualResult : undefined); } - savedObjectNamespaces = - initialNamespaces || - // @ts-expect-error MultiGetHit._source is optional - getSavedObjectNamespaces(namespace, docFound ? actualResult : undefined); // @ts-expect-error MultiGetHit._source is optional versionProperties = getExpectedVersionProperties(version, actualResult); } else { @@ -438,12 +467,57 @@ export class SavedObjectsRepository { versionProperties = getExpectedVersionProperties(version); } + if (expectedBulkGetResult.value.method === 'create') { + savedObjectWorkspaces = options.workspaces; + } else { + const changeToCreate = () => { + finalMethod = 'create'; + finalObjectId = object.id; + savedObjectWorkspaces = options.workspaces; + versionProperties = {}; + }; + /** + * When overwrite, need to check if the object is workspace-specific + * if so, copy object to target workspace instead of refering it. + */ + 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 + ); + /** + * We need to create a new object when the object + * is about to import into workspaces it is not belong to + */ + if (filteredWorkspaces.length) { + /** + * Create a new object but only belong to the set of (target workspaces - original workspace) + */ + changeToCreate(); + finalObjectId = uuid.v1(); + savedObjectWorkspaces = filteredWorkspaces; + } else { + savedObjectWorkspaces = transformedObject.workspaces; + } + } else { + savedObjectWorkspaces = options.workspaces; + } + } + const expectedResult = { opensearchRequestIndex: bulkRequestIndexCounter++, - requestedId: object.id, + requestedId: finalObjectId, rawMigratedDoc: this._serializer.savedObjectToRaw( this._migrator.migrateDocument({ - id: object.id, + id: finalObjectId, type: object.type, attributes: object.attributes, migrationVersion: object.migrationVersion, @@ -452,13 +526,14 @@ export class SavedObjectsRepository { updated_at: time, references: object.references || [], originId: object.originId, + workspaces: savedObjectWorkspaces, }) as SavedObjectSanitizedDoc ), }; bulkCreateParams.push( { - [method]: { + [finalMethod]: { _id: expectedResult.rawMigratedDoc._id, _index: this.getIndexForType(object.type), ...(overwrite && versionProperties), @@ -736,6 +811,7 @@ export class SavedObjectsRepository { typeToNamespacesMap, filter, preference, + workspaces, } = options; if (!type && !typeToNamespacesMap) { @@ -809,6 +885,7 @@ export class SavedObjectsRepository { typeToNamespacesMap, hasReference, kueryNode, + workspaces, }), }, }; @@ -976,7 +1053,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId, updated_at: updatedAt } = body._source; + const { originId, updated_at: updatedAt, workspaces } = body._source; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { @@ -991,6 +1068,7 @@ export class SavedObjectsRepository { namespaces, ...(originId && { originId }), ...(updatedAt && { updated_at: updatedAt }), + ...(workspaces && { workspaces }), version: encodeHitVersion(body), attributes: body._source[type], references: body._source.references || [], @@ -1055,7 +1133,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId } = body.get?._source ?? {}; + const { originId, workspaces } = body.get?._source ?? {}; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { namespaces = body.get?._source.namespaces ?? [ @@ -1070,6 +1148,7 @@ export class SavedObjectsRepository { version: encodeHitVersion(body), namespaces, ...(originId && { originId }), + ...(workspaces && { workspaces }), references, attributes, }; @@ -1452,12 +1531,13 @@ export class SavedObjectsRepository { }; } - const { originId } = get._source; + const { originId, workspaces } = get._source; return { id, type, ...(namespaces && { namespaces }), ...(originId && { originId }), + ...(workspaces && { workspaces }), updated_at, version: encodeVersion(seqNo, primaryTerm), attributes, @@ -1754,7 +1834,7 @@ function getSavedObjectFromSource( id: string, doc: { _seq_no?: number; _primary_term?: number; _source: SavedObjectsRawDocSource } ): SavedObject { - const { originId, updated_at: updatedAt } = doc._source; + const { originId, updated_at: updatedAt, workspaces } = doc._source; let namespaces: string[] = []; if (!registry.isNamespaceAgnostic(type)) { @@ -1769,6 +1849,7 @@ function getSavedObjectFromSource( namespaces, ...(originId && { originId }), ...(updatedAt && { updated_at: updatedAt }), + ...(workspaces && { workspaces }), version: encodeHitVersion(doc), attributes: doc._source[type], references: doc._source.references || [], diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 5bbb0a1fe24f..521149c4e646 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -127,6 +127,26 @@ function getClauseForType( }, }; } +/** + * Gets the clause that will filter for the workspace. + */ +function getClauseForWorkspace(workspace: string) { + if (workspace === '*') { + return { + bool: { + must: { + match_all: {}, + }, + }, + }; + } + + return { + bool: { + must: [{ term: { workspaces: workspace } }], + }, + }; +} interface HasReferenceQueryParams { type: string; @@ -144,6 +164,7 @@ interface QueryParams { defaultSearchOperator?: string; hasReference?: HasReferenceQueryParams; kueryNode?: KueryNode; + workspaces?: string[]; } export function getClauseForReference(reference: HasReferenceQueryParams) { @@ -200,6 +221,7 @@ export function getQueryParams({ defaultSearchOperator, hasReference, kueryNode, + workspaces, }: QueryParams) { const types = getTypes( registry, @@ -224,6 +246,17 @@ export function getQueryParams({ ], }; + if (workspaces) { + bool.filter.push({ + bool: { + should: workspaces.map((workspace) => { + return getClauseForWorkspace(workspace); + }), + minimum_should_match: 1, + }, + }); + } + if (search) { const useMatchPhrasePrefix = shouldUseMatchPhrasePrefix(search); const simpleQueryStringClause = getSimpleQueryStringClause({ diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index 8b54141a4c3c..df6109eb9d0a 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -52,6 +52,7 @@ interface GetSearchDslOptions { id: string; }; kueryNode?: KueryNode; + workspaces?: string[]; } export function getSearchDsl( @@ -71,6 +72,7 @@ export function getSearchDsl( typeToNamespacesMap, hasReference, kueryNode, + workspaces, } = options; if (!type) { @@ -93,6 +95,7 @@ export function getSearchDsl( defaultSearchOperator, hasReference, kueryNode, + workspaces, }), ...getSortingParams(mappings, type, sortField, sortOrder), }; diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 4823e52d77c9..490c2b7083d2 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -80,4 +80,11 @@ export class SavedObjectsUtils { total: 0, saved_objects: [], }); + + public static filterWorkspacesAccordingToBaseWorkspaces( + targetWorkspaces?: string[], + baseWorkspaces?: string[] + ): string[] { + return targetWorkspaces?.filter((item) => !baseWorkspaces?.includes(item)) || []; + } } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 3e2553b8ce51..33862cb149fb 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -110,6 +110,7 @@ export interface SavedObjectsFindOptions { typeToNamespacesMap?: Map; /** An optional OpenSearch preference value to be used for the query **/ preference?: string; + workspaces?: string[]; } /** @@ -119,6 +120,7 @@ export interface SavedObjectsFindOptions { export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; + workspaces?: string[]; } /** diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index 81e1ed029ddc..47faffb0b922 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -113,6 +113,7 @@ export interface SavedObject { * space. */ originId?: string; + workspaces?: string[]; } export interface SavedObjectError { From c841390d4e471dd0b62592561bc32a26212ed15b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Sep 2023 13:52:21 +0800 Subject: [PATCH 02/25] feat: update snapshot Signed-off-by: SuZhou-Joe --- .../export/get_sorted_objects_for_export.test.ts | 6 ------ .../saved_objects/export/get_sorted_objects_for_export.ts | 7 +++++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index 952a74a76940..cf7e1d8246a7 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -128,7 +128,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -219,7 +218,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -370,7 +368,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -462,7 +459,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -670,7 +666,6 @@ describe('getSortedObjectsForExport()', () => { ], Object { namespace: undefined, - workspaces: undefined, }, ], ], @@ -789,7 +784,6 @@ describe('getSortedObjectsForExport()', () => { ], Object { namespace: undefined, - workspaces: undefined, }, ], Array [ diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 8ca085639f10..0336fc702973 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -109,7 +109,10 @@ async function fetchObjectsToExport({ if (typeof search === 'string') { throw Boom.badRequest(`Can't specify both "search" and "objects" properties when exporting`); } - const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace, workspaces }); + const bulkGetResult = await savedObjectsClient.bulkGet(objects, { + namespace, + ...(workspaces ? { workspaces } : {}), + }); const erroredObjects = bulkGetResult.saved_objects.filter((obj) => !!obj.error); if (erroredObjects.length) { const err = Boom.badRequest(); @@ -125,7 +128,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`); From fb43df1d8a3d6af37b72b698c8b662531af73a65 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sat, 23 Sep 2023 18:14:18 +0800 Subject: [PATCH 03/25] feat: optimize logic when checkConflict and bulkCreate (#189) * feat: optimize logic when checkConflict and bulkCreate Signed-off-by: SuZhou-Joe * feat: add options.workspace check Signed-off-by: SuZhou-Joe * feat: throw error when workspace check error in repository create Signed-off-by: SuZhou-Joe * feat: modify judgement Signed-off-by: SuZhou-Joe * feat: always get objects from DB when create-with-override Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.ts | 71 ++++++++++--------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index a49f356bbb81..e8b410350d9e 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -285,8 +285,14 @@ export class SavedObjectsRepository { if (id && overwrite && workspaces) { try { const currentItem = await this.get(type, id); - if (currentItem && currentItem.workspaces) { - // do not overwrite workspaces + if ( + SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + workspaces, + currentItem.workspaces + ).length + ) { + throw SavedObjectsErrorHelpers.createConflictError(type, id); + } else { savedObjectWorkspaces = currentItem.workspaces; } } catch (e) { @@ -419,8 +425,6 @@ export class SavedObjectsRepository { method, } = expectedBulkGetResult.value; let savedObjectWorkspaces: string[] | undefined; - let finalMethod = method; - let finalObjectId = object.id; if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound @@ -467,19 +471,9 @@ export class SavedObjectsRepository { versionProperties = getExpectedVersionProperties(version); } - if (expectedBulkGetResult.value.method === 'create') { - savedObjectWorkspaces = options.workspaces; - } else { - const changeToCreate = () => { - finalMethod = 'create'; - finalObjectId = object.id; - savedObjectWorkspaces = options.workspaces; - versionProperties = {}; - }; - /** - * When overwrite, need to check if the object is workspace-specific - * if so, copy object to target workspace instead of refering it. - */ + savedObjectWorkspaces = options.workspaces; + + if (expectedBulkGetResult.value.method !== 'create') { const rawId = this._serializer.generateRawId(namespace, object.type, object.id); const findObject = bulkGetResponse?.statusCode !== 404 @@ -493,31 +487,31 @@ export class SavedObjectsRepository { options.workspaces, transformedObject.workspaces ); - /** - * We need to create a new object when the object - * is about to import into workspaces it is not belong to - */ if (filteredWorkspaces.length) { - /** - * Create a new object but only belong to the set of (target workspaces - original workspace) - */ - changeToCreate(); - finalObjectId = uuid.v1(); - savedObjectWorkspaces = filteredWorkspaces; + const { id, type } = object; + return { + tag: 'Left' as 'Left', + error: { + id, + type, + error: { + ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), + metadata: { isNotOverwritable: true }, + }, + }, + }; } else { savedObjectWorkspaces = transformedObject.workspaces; } - } else { - savedObjectWorkspaces = options.workspaces; } } const expectedResult = { opensearchRequestIndex: bulkRequestIndexCounter++, - requestedId: finalObjectId, + requestedId: object.id, rawMigratedDoc: this._serializer.savedObjectToRaw( this._migrator.migrateDocument({ - id: finalObjectId, + id: object.id, type: object.type, attributes: object.attributes, migrationVersion: object.migrationVersion, @@ -533,7 +527,7 @@ export class SavedObjectsRepository { bulkCreateParams.push( { - [finalMethod]: { + [method]: { _id: expectedResult.rawMigratedDoc._id, _index: this.getIndexForType(object.type), ...(overwrite && versionProperties), @@ -647,13 +641,24 @@ export class SavedObjectsRepository { const { type, id, opensearchRequestIndex } = expectedResult.value; const doc = bulkGetResponse?.body.docs[opensearchRequestIndex]; if (doc?.found) { + let workspaceConflict = false; + if (options.workspaces) { + const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc); + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + options.workspaces, + transformedObject.workspaces + ); + if (filteredWorkspaces.length) { + workspaceConflict = true; + } + } errors.push({ id, type, error: { ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), // @ts-expect-error MultiGetHit._source is optional - ...(!this.rawDocExistsInNamespace(doc!, namespace) && { + ...((!this.rawDocExistsInNamespace(doc!, namespace) || workspaceConflict) && { metadata: { isNotOverwritable: true }, }), }, From 7e184859312ccdc40dd6537451c40ed26df406de Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sat, 23 Sep 2023 18:33:40 +0800 Subject: [PATCH 04/25] feat: call get when create with override Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index e8b410350d9e..755ffb63de79 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -282,7 +282,7 @@ export class SavedObjectsRepository { let savedObjectWorkspaces = workspaces; - if (id && overwrite && workspaces) { + if (id && overwrite) { try { const currentItem = await this.get(type, id); if ( From aa69695351ba3366a25540396b3b4d2b8ae07f53 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 24 Sep 2023 17:20:00 +0800 Subject: [PATCH 05/25] feat: update test according to count Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.test.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index fb5d366dd454..c591ccccbf58 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -1846,9 +1846,17 @@ describe('SavedObjectsRepository', () => { const createSuccess = async (type, attributes, options) => { const result = await savedObjectsRepository.create(type, attributes, options); - expect(client.get).toHaveBeenCalledTimes( - registry.isMultiNamespace(type) && options.overwrite ? 1 : 0 - ); + let count = 0; + if (options?.overwrite && options?.id) { + /** + * workspace will call extra one to get latest status of current object + */ + count++; + } + if (registry.isMultiNamespace(type) && options.overwrite) { + count++; + } + expect(client.get).toHaveBeenCalledTimes(count); return result; }; From 674bd0983e62e36b281b6de52d9a65a5562ece1f Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 16:28:48 +0800 Subject: [PATCH 06/25] feat: add integration test Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.ts | 1 + .../routes/integration_tests/find.test.ts | 34 ++ .../lib/integration_tests/repository.test.ts | 303 ++++++++++++++++++ .../saved_objects/service/lib/repository.ts | 28 +- 4 files changed, 360 insertions(+), 6 deletions(-) create mode 100644 src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts 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 68104db85e6f..a2669bc49c9a 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -87,6 +87,7 @@ export async function importSavedObjectsFromStream({ savedObjectsClient, namespace, ignoreRegularConflicts: overwrite, + workspaces, }; const checkConflictsResult = await checkConflicts(checkConflictsParams); errorAccumulator = [...errorAccumulator, ...checkConflictsResult.errors]; diff --git a/src/core/server/saved_objects/routes/integration_tests/find.test.ts b/src/core/server/saved_objects/routes/integration_tests/find.test.ts index fc21eefed434..b21425386400 100644 --- a/src/core/server/saved_objects/routes/integration_tests/find.test.ts +++ b/src/core/server/saved_objects/routes/integration_tests/find.test.ts @@ -288,4 +288,38 @@ describe('GET /api/saved_objects/_find', () => { defaultSearchOperator: 'OR', }); }); + + it('accepts the query parameter workspaces as a string', async () => { + await supertest(httpSetup.server.listener) + .get('/api/saved_objects/_find?type=index-pattern&workspaces=foo') + .expect(200); + + expect(savedObjectsClient.find).toHaveBeenCalledTimes(1); + + const options = savedObjectsClient.find.mock.calls[0][0]; + expect(options).toEqual({ + defaultSearchOperator: 'OR', + perPage: 20, + page: 1, + type: ['index-pattern'], + workspaces: ['foo'], + }); + }); + + it('accepts the query parameter workspaces as an array', async () => { + await supertest(httpSetup.server.listener) + .get('/api/saved_objects/_find?type=index-pattern&workspaces=default&workspaces=foo') + .expect(200); + + expect(savedObjectsClient.find).toHaveBeenCalledTimes(1); + + const options = savedObjectsClient.find.mock.calls[0][0]; + expect(options).toEqual({ + perPage: 20, + page: 1, + type: ['index-pattern'], + workspaces: ['default', 'foo'], + defaultSearchOperator: 'OR', + }); + }); }); diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts new file mode 100644 index 000000000000..50c75e43fdf0 --- /dev/null +++ b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts @@ -0,0 +1,303 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { SavedObject } from 'src/core/types'; +import { isEqual } from 'lodash'; +import * as osdTestServer from '../../../../../test_helpers/osd_server'; +import { Readable } from 'stream'; + +const dashboard: Omit = { + type: 'dashboard', + attributes: {}, + references: [], +}; + +describe('repository integration test', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + beforeAll(async () => { + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + }); + opensearchServer = await startOpenSearch(); + const startOSDResp = await startOpenSearchDashboards(); + root = startOSDResp.root; + }, 30000); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + + const deleteItem = async (object: Pick) => { + await osdTestServer.request + .delete(root, `/api/saved_objects/${object.type}/${object.id}`) + .expect(200); + }; + + const getItem = async (object: Pick) => { + return await osdTestServer.request + .get(root, `/api/saved_objects/${object.type}/${object.id}`) + .expect(200); + }; + + describe('workspace related CRUD', () => { + it('create', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + expect(createResult.body.workspaces).toEqual(['foo']); + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('create-with-override', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + /** + * Override without workspace, in this case workspaces field should be retained. + */ + const correctOverride = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) + .send({ + attributes: { + title: 'foo', + }, + }) + .expect(200); + + expect(correctOverride.body.workspaces).toEqual(['foo']); + expect(correctOverride.body.attributes.title).toEqual('foo'); + + await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) + .send({ + attributes: dashboard.attributes, + workspaces: ['bar'], + }) + .expect(409); + + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('bulk create', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + expect((createResultFoo.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultFoo.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['foo']) + ) + ).toEqual(true); + expect((createResultBar.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultBar.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['bar']) + ) + ).toEqual(true); + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('bulk create with conflict', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + /** + * overwrite without workspaces + */ + const overwriteWithoutWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?overwrite=true`) + .send([ + { + ...dashboard, + id: 'bar', + }, + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const getFooResult = await getItem({ + type: dashboard.type, + id: 'foo', + }); + + const getBarResult = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + expect(getFooResult.body.workspaces).toEqual(['foo']); + expect(getBarResult.body.workspaces).toEqual(['bar']); + expect( + (overwriteWithoutWorkspacesResult.body.saved_objects as any[]).some((item) => item.error) + ).toEqual(false); + + /** + * overwrite with workspaces + */ + const overwriteWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?overwrite=true&workspaces=foo`) + .send([ + { + ...dashboard, + id: 'bar', + }, + { + ...dashboard, + id: 'foo', + attributes: { + title: 'foo', + }, + }, + ]) + .expect(200); + + expect(overwriteWithWorkspacesResult.body.saved_objects[0].error.statusCode).toEqual(409); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].attributes.title).toEqual('foo'); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual(['foo']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('checkConflicts when importing ndjson', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const getResultFoo = await getItem({ + type: dashboard.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + const readableStream = new Readable(); + readableStream.push( + `Content-Disposition: form-data; name="file"; filename="tmp.ndjson"\r\n\r\n` + ); + readableStream.push( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n') + ); + readableStream.push(null); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_import?workspaces=foo&overwrite=false`) + .attach( + 'file', + Buffer.from( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n'), + 'utf-8' + ), + 'tmp.ndjson' + ) + .expect(200); + + expect(importWithWorkspacesResult.body.success).toEqual(false); + expect(importWithWorkspacesResult.body.errors.length).toEqual(1); + expect(importWithWorkspacesResult.body.errors[0].id).toEqual('foo'); + expect(importWithWorkspacesResult.body.errors[0].error.type).toEqual('conflict'); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + }); +}); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 755ffb63de79..05f203a0f079 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -283,8 +283,13 @@ export class SavedObjectsRepository { let savedObjectWorkspaces = workspaces; if (id && overwrite) { + let currentItem; try { - const currentItem = await this.get(type, id); + currentItem = await this.get(type, id); + } catch (e) { + // this.get will throw an error when no items can be found + } + if (currentItem) { if ( SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( workspaces, @@ -295,8 +300,6 @@ export class SavedObjectsRepository { } else { savedObjectWorkspaces = currentItem.workspaces; } - } catch (e) { - // this.get will throw an error when no items can be found } } @@ -377,15 +380,28 @@ export class SavedObjectsRepository { const method = object.id && overwrite ? 'index' : 'create'; const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type); + /** + * Only when importing an object to a target workspace should we check if the object is workspace-specific. + */ + const requiresWorkspaceCheck = object.id; if (object.id == null) object.id = uuid.v1(); + let opensearchRequestIndexPayload = {}; + + if (requiresNamespacesCheck || requiresWorkspaceCheck) { + opensearchRequestIndexPayload = { + opensearchRequestIndex: bulkGetRequestIndexCounter, + }; + bulkGetRequestIndexCounter++; + } + return { tag: 'Right' as 'Right', value: { method, object, - ...(requiresNamespacesCheck && { opensearchRequestIndex: bulkGetRequestIndexCounter++ }), + ...opensearchRequestIndexPayload, }, }; }); @@ -396,7 +412,7 @@ export class SavedObjectsRepository { .map(({ value: { object: { type, id } } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces'], + _source: ['type', 'namespaces', 'workspaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( @@ -618,7 +634,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'], + _source: ['type', 'namespaces', 'workspaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( From 2fb66b781679c803f02eb2cc12a7788d9ad0e983 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 17:28:07 +0800 Subject: [PATCH 07/25] fix: unit test Signed-off-by: SuZhou-Joe --- .../lib/integration_tests/repository.test.ts | 38 +++++++++++++++++++ .../service/lib/repository.test.js | 31 ++++++++++++--- .../saved_objects/service/lib/repository.ts | 6 +-- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts index 50c75e43fdf0..42b0b031006d 100644 --- a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts @@ -299,5 +299,43 @@ describe('repository integration test', () => { ) ); }); + + it('find by workspaces', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const findResult = await osdTestServer.request + .get(root, `/api/saved_objects/_find?workspaces=bar&type=${dashboard.type}`) + .expect(200); + + expect(findResult.body.total).toEqual(1); + expect(findResult.body.saved_objects[0].workspaces).toEqual(['bar']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); }); }); diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index c591ccccbf58..9f0c0aed23b1 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -168,7 +168,7 @@ describe('SavedObjectsRepository', () => { }); const getMockGetResponse = ( - { type, id, references, namespace: objectNamespace, originId }, + { type, id, references, namespace: objectNamespace, originId, workspaces }, namespace ) => { const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; @@ -182,6 +182,7 @@ describe('SavedObjectsRepository', () => { _source: { ...(registry.isSingleNamespace(type) && { namespace: namespaceId }), ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), + workspaces, ...(originId && { originId }), type, [type]: { title: 'Testing' }, @@ -466,6 +467,7 @@ describe('SavedObjectsRepository', () => { }; const bulkCreateSuccess = async (objects, options) => { + const originalObjects = JSON.parse(JSON.stringify(objects)); const multiNamespaceObjects = objects.filter( ({ type, id }) => registry.isMultiNamespace(type) && id ); @@ -480,7 +482,9 @@ describe('SavedObjectsRepository', () => { opensearchClientMock.createSuccessTransportRequestPromise(response) ); const result = await savedObjectsRepository.bulkCreate(objects, options); - expect(client.mget).toHaveBeenCalledTimes(multiNamespaceObjects?.length ? 1 : 0); + expect(client.mget).toHaveBeenCalledTimes( + multiNamespaceObjects?.length || originalObjects?.some((item) => item.id) ? 1 : 0 + ); return result; }; @@ -538,7 +542,10 @@ describe('SavedObjectsRepository', () => { await bulkCreateSuccess(objects); expect(client.bulk).toHaveBeenCalledTimes(1); expect(client.mget).toHaveBeenCalledTimes(1); - const docs = [expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` })]; + const docs = [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` }), + ]; expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); @@ -683,6 +690,7 @@ describe('SavedObjectsRepository', () => { expect.anything() ); client.bulk.mockClear(); + client.mget.mockClear(); }; await test(undefined); await test(namespace); @@ -815,6 +823,12 @@ describe('SavedObjectsRepository', () => { const response1 = { status: 200, docs: [ + { + found: true, + _source: { + type: obj1.type, + }, + }, { found: true, _source: { @@ -837,7 +851,13 @@ describe('SavedObjectsRepository', () => { expect(client.bulk).toHaveBeenCalled(); expect(client.mget).toHaveBeenCalled(); - const body1 = { docs: [expect.objectContaining({ _id: `${obj.type}:${obj.id}` })] }; + const body1 = { + docs: [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ], + }; expect(client.mget).toHaveBeenCalledWith( expect.objectContaining({ body: body1 }), expect.anything() @@ -2194,10 +2214,11 @@ describe('SavedObjectsRepository', () => { const type = 'index-pattern'; const id = 'logstash-*'; const namespace = 'foo-namespace'; + const workspaces = ['bar-workspace']; const deleteSuccess = async (type, id, options) => { if (registry.isMultiNamespace(type)) { - const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace); + const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace, workspaces); client.get.mockResolvedValueOnce( opensearchClientMock.createSuccessTransportRequestPromise(mockGetResponse) ); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 05f203a0f079..e9f6821254d5 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -444,7 +444,7 @@ export class SavedObjectsRepository { if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound - ? bulkGetResponse?.body.docs[opensearchRequestIndex] + ? bulkGetResponse?.body.docs?.[opensearchRequestIndex] : undefined; const docFound = indexFound && actualResult?.found === true; let hasSetNamespace = false; @@ -960,7 +960,7 @@ export class SavedObjectsRepository { */ async bulkGet( objects: SavedObjectsBulkGetObject[] = [], - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { const namespace = normalizeNamespace(options.namespace); @@ -1048,7 +1048,7 @@ export class SavedObjectsRepository { async get( type: string, id: string, - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { if (!this._allowedTypes.includes(type)) { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); From 952f13e9b120b0ab120bde5fb412643e91295965 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 17:51:18 +0800 Subject: [PATCH 08/25] feat: regenerate ids when import Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.test.ts | 12 +++++- .../import/import_saved_objects.ts | 11 ++++- .../saved_objects/import/regenerate_ids.ts | 40 ++++++++++++++++++- 3 files changed, 59 insertions(+), 4 deletions(-) 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 de8fb34dfbed..4bf1717e895c 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 @@ -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 } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; import { validateReferences } from './validate_references'; import { checkConflicts } from './check_conflicts'; import { checkOriginConflicts } from './check_origin_conflicts'; @@ -68,6 +68,7 @@ describe('#importSavedObjectsFromStream', () => { importIdMap: new Map(), }); getMockFn(regenerateIds).mockReturnValue(new Map()); + getMockFn(regenerateIdsWithReference).mockReturnValue(Promise.resolve(new Map())); getMockFn(validateReferences).mockResolvedValue([]); getMockFn(checkConflicts).mockResolvedValue({ errors: [], @@ -240,6 +241,15 @@ describe('#importSavedObjectsFromStream', () => { ]), }); getMockFn(validateReferences).mockResolvedValue([errors[1]]); + getMockFn(regenerateIdsWithReference).mockResolvedValue( + Promise.resolve( + new Map([ + ['foo', {}], + ['bar', {}], + ['baz', {}], + ]) + ) + ); getMockFn(checkConflicts).mockResolvedValue({ errors: [errors[2]], filteredObjects, 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 a2669bc49c9a..fce50bab7abc 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/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 } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; /** * Import saved objects from given stream. See the {@link SavedObjectsImportOptions | options} for more @@ -81,6 +81,13 @@ export async function importSavedObjectsFromStream({ if (createNewCopies) { importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects); } else { + importIdMap = await regenerateIdsWithReference({ + savedObjects: collectSavedObjectsResult.collectedObjects, + savedObjectsClient, + workspaces, + objectLimit, + importIdMap, + }); // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces const checkConflictsParams = { objects: collectSavedObjectsResult.collectedObjects, @@ -120,7 +127,7 @@ export async function importSavedObjectsFromStream({ importIdMap, overwrite, namespace, - workspaces, + ...(workspaces ? { workspaces } : {}), }; const createSavedObjectsResult = await createSavedObjects(createSavedObjectsParams); errorAccumulator = [...errorAccumulator, ...createSavedObjectsResult.errors]; diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index 672a8f030620..27b10e575f44 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -29,7 +29,8 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { SavedObject } from '../types'; +import { SavedObject, SavedObjectsClientContract } from '../types'; +import { SavedObjectsUtils } from '../service'; /** * Takes an array of saved objects and returns an importIdMap of randomly-generated new IDs. @@ -42,3 +43,40 @@ export const regenerateIds = (objects: SavedObject[]) => { }, new Map()); return importIdMap; }; + +export const regenerateIdsWithReference = async (props: { + savedObjects: SavedObject[]; + savedObjectsClient: SavedObjectsClientContract; + workspaces?: string[]; + objectLimit: number; + importIdMap: Map; +}): Promise> => { + 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); +}; From 23481a17ddb26c03da7d4f76b387eca0184d409c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 14:21:20 +0800 Subject: [PATCH 09/25] feat: add more unit test Signed-off-by: SuZhou-Joe --- .../service/lib/repository.test.js | 166 +++++++++++++++++- .../lib/search_dsl/query_params.test.ts | 21 +++ 2 files changed, 186 insertions(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 9f0c0aed23b1..74802561d45b 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -27,7 +27,7 @@ * specific language governing permissions and limitations * under the License. */ - +import { omit } from 'lodash'; import { SavedObjectsRepository } from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; @@ -445,6 +445,7 @@ describe('SavedObjectsRepository', () => { references: [{ name: 'ref_0', type: 'test', id: '2' }], }; const namespace = 'foo-namespace'; + const workspace = 'foo-workspace'; const getMockBulkCreateResponse = (objects, namespace) => { return { @@ -549,6 +550,18 @@ describe('SavedObjectsRepository', () => { expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); + it(`should use the OpenSearch mget action before bulk action for any types, when id is defined`, async () => { + const objects = [obj1, obj2]; + await bulkCreateSuccess(objects); + expect(client.bulk).toHaveBeenCalledTimes(1); + expect(client.mget).toHaveBeenCalledTimes(1); + const docs = [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ]; + expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); + }); + it(`should use the OpenSearch create method if ID is undefined and overwrite=true`, async () => { const objects = [obj1, obj2].map((obj) => ({ ...obj, id: undefined })); await bulkCreateSuccess(objects, { overwrite: true }); @@ -738,6 +751,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', () => { @@ -897,6 +920,74 @@ describe('SavedObjectsRepository', () => { const expectedError = expectErrorResult(obj3, { message: JSON.stringify(opensearchError) }); await bulkCreateError(obj3, opensearchError, expectedError); }); + + it(`returns error when there is a conflict with an existing saved object according to workspaces`, async () => { + const obj = { ...obj3, workspaces: ['foo'] }; + const response1 = { + status: 200, + docs: [ + { + found: true, + _id: `${obj1.type}:${obj1.id}`, + _source: { + type: obj1.type, + workspaces: ['bar'], + }, + }, + { + found: true, + _id: `${obj.type}:${obj.id}`, + _source: { + type: obj.type, + workspaces: obj.workspaces, + }, + }, + { + found: true, + _id: `${obj2.type}:${obj2.id}`, + _source: { + type: obj2.type, + }, + }, + ], + }; + client.mget.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response1) + ); + const response2 = getMockBulkCreateResponse([obj1, obj, obj2]); + client.bulk.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response2) + ); + + const options = { overwrite: true, workspaces: ['bar'] }; + const result = await savedObjectsRepository.bulkCreate([obj1, obj, obj2], options); + expect(client.bulk).toHaveBeenCalled(); + expect(client.mget).toHaveBeenCalled(); + + const body1 = { + docs: [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ], + }; + expect(client.mget).toHaveBeenCalledWith( + expect.objectContaining({ body: body1 }), + expect.anything() + ); + const body2 = [...expectObjArgs(obj1)]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body: body2 }), + expect.anything() + ); + expect(result).toEqual({ + saved_objects: [ + expectSuccess(obj1), + expectErrorConflict(obj, { metadata: { isNotOverwritable: true } }), + expectErrorConflict(obj2, { metadata: { isNotOverwritable: true } }), + ], + }); + }); }); describe('migration', () => { @@ -1719,6 +1810,8 @@ describe('SavedObjectsRepository', () => { const obj5 = { type: MULTI_NAMESPACE_TYPE, id: 'five' }; const obj6 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'six' }; const obj7 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'seven' }; + const obj8 = { type: 'dashboard', id: 'eight', workspaces: ['foo'] }; + const obj9 = { type: 'dashboard', id: 'nine', workspaces: ['bar'] }; const namespace = 'foo-namespace'; const checkConflicts = async (objects, options) => @@ -1810,6 +1903,8 @@ describe('SavedObjectsRepository', () => { { found: false }, getMockGetResponse(obj6), { found: false }, + getMockGetResponse(obj7), + getMockGetResponse(obj8), ], }; client.mget.mockResolvedValue( @@ -1838,6 +1933,36 @@ describe('SavedObjectsRepository', () => { ], }); }); + + it(`expected results with workspaces`, async () => { + const objects = [obj8, obj9]; + const response = { + status: 200, + docs: [getMockGetResponse(obj8), getMockGetResponse(obj9)], + }; + client.mget.mockResolvedValue( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + + const result = await checkConflicts(objects, { + workspaces: ['foo'], + }); + expect(client.mget).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + errors: [ + { ...omit(obj8, 'workspaces'), error: createConflictError(obj8.type, obj8.id) }, + { + ...omit(obj9, 'workspaces'), + error: { + ...createConflictError(obj9.type, obj9.id), + metadata: { + isNotOverwritable: true, + }, + }, + }, + ], + }); + }); }); }); @@ -1894,6 +2019,7 @@ describe('SavedObjectsRepository', () => { it(`should use the OpenSearch index action if ID is defined and overwrite=true`, async () => { await createSuccess(type, attributes, { id, overwrite: true }); expect(client.index).toHaveBeenCalled(); + expect(client.get).toHaveBeenCalled(); }); it(`should use the OpenSearch index with version if ID and version are defined and overwrite=true`, async () => { @@ -2068,6 +2194,29 @@ describe('SavedObjectsRepository', () => { expect.anything() ); }); + + it(`doesn't modify workspaces when overwrite without target workspaces`, async () => { + const response = getMockGetResponse({ workspaces: ['foo'], id }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + + await savedObjectsRepository.create('dashboard', attributes, { + id, + overwrite: true, + workspaces: [], + }); + + expect(client.index).toHaveBeenCalledWith( + expect.objectContaining({ + id: `dashboard:${id}`, + body: expect.objectContaining({ + workspaces: ['foo'], + }), + }), + expect.anything() + ); + }); }); describe('errors', () => { @@ -2128,6 +2277,21 @@ describe('SavedObjectsRepository', () => { expect(client.get).toHaveBeenCalled(); }); + it(`throws error when there is a conflict with an existing workspaces saved object`, async () => { + const response = getMockGetResponse({ workspaces: ['foo'], id }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + await expect( + savedObjectsRepository.create('dashboard', attributes, { + id, + overwrite: true, + workspaces: ['bar'], + }) + ).rejects.toThrowError(createConflictError('dashboard', id)); + expect(client.get).toHaveBeenCalled(); + }); + it.todo(`throws when automatic index creation fails`); it.todo(`throws when an unexpected failure occurs`); diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index 518e2ff56d0e..a47bc27fcd92 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -625,6 +625,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', () => { From e740165522454e51fdb34300d02f716f13e5e85d Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 15:20:49 +0800 Subject: [PATCH 10/25] feat: minor changes logic on repository Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index e9f6821254d5..a4dcd53b9cdb 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -282,7 +282,7 @@ export class SavedObjectsRepository { let savedObjectWorkspaces = workspaces; - if (id && overwrite) { + if (id && overwrite && workspaces) { let currentItem; try { currentItem = await this.get(type, id); @@ -383,7 +383,7 @@ export class SavedObjectsRepository { /** * Only when importing an object to a target workspace should we check if the object is workspace-specific. */ - const requiresWorkspaceCheck = object.id; + const requiresWorkspaceCheck = object.id && options.workspaces; if (object.id == null) object.id = uuid.v1(); From 1997c730ce4f2d540146ac547aeb731fa7ec24d0 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 17:39:50 +0800 Subject: [PATCH 11/25] feat: update unit test Signed-off-by: SuZhou-Joe --- .../service/lib/repository.test.js | 37 ++----------------- 1 file changed, 4 insertions(+), 33 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 74802561d45b..fc80e62e1b36 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -468,7 +468,6 @@ describe('SavedObjectsRepository', () => { }; const bulkCreateSuccess = async (objects, options) => { - const originalObjects = JSON.parse(JSON.stringify(objects)); const multiNamespaceObjects = objects.filter( ({ type, id }) => registry.isMultiNamespace(type) && id ); @@ -484,7 +483,7 @@ describe('SavedObjectsRepository', () => { ); const result = await savedObjectsRepository.bulkCreate(objects, options); expect(client.mget).toHaveBeenCalledTimes( - multiNamespaceObjects?.length || originalObjects?.some((item) => item.id) ? 1 : 0 + multiNamespaceObjects?.length || options?.workspaces ? 1 : 0 ); return result; }; @@ -543,22 +542,7 @@ describe('SavedObjectsRepository', () => { await bulkCreateSuccess(objects); expect(client.bulk).toHaveBeenCalledTimes(1); expect(client.mget).toHaveBeenCalledTimes(1); - const docs = [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` }), - ]; - expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); - }); - - it(`should use the OpenSearch mget action before bulk action for any types, when id is defined`, async () => { - const objects = [obj1, obj2]; - await bulkCreateSuccess(objects); - expect(client.bulk).toHaveBeenCalledTimes(1); - expect(client.mget).toHaveBeenCalledTimes(1); - const docs = [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), - ]; + const docs = [expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` })]; expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); @@ -846,12 +830,6 @@ describe('SavedObjectsRepository', () => { const response1 = { status: 200, docs: [ - { - found: true, - _source: { - type: obj1.type, - }, - }, { found: true, _source: { @@ -874,13 +852,7 @@ describe('SavedObjectsRepository', () => { expect(client.bulk).toHaveBeenCalled(); expect(client.mget).toHaveBeenCalled(); - const body1 = { - docs: [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), - expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), - ], - }; + const body1 = { docs: [expect.objectContaining({ _id: `${obj.type}:${obj.id}` })] }; expect(client.mget).toHaveBeenCalledWith( expect.objectContaining({ body: body1 }), expect.anything() @@ -1992,7 +1964,7 @@ describe('SavedObjectsRepository', () => { const createSuccess = async (type, attributes, options) => { const result = await savedObjectsRepository.create(type, attributes, options); let count = 0; - if (options?.overwrite && options?.id) { + if (options?.overwrite && options.id && options.workspaces) { /** * workspace will call extra one to get latest status of current object */ @@ -2019,7 +1991,6 @@ describe('SavedObjectsRepository', () => { it(`should use the OpenSearch index action if ID is defined and overwrite=true`, async () => { await createSuccess(type, attributes, { id, overwrite: true }); expect(client.index).toHaveBeenCalled(); - expect(client.get).toHaveBeenCalled(); }); it(`should use the OpenSearch index with version if ID and version are defined and overwrite=true`, async () => { From cae196e1be25139b4370beeb547af36be4f7d5e5 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 19:11:21 +0800 Subject: [PATCH 12/25] feat: update test Signed-off-by: SuZhou-Joe --- .../lib/integration_tests/repository.test.ts | 71 ++++++------------- 1 file changed, 20 insertions(+), 51 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts index 42b0b031006d..b601de985dc0 100644 --- a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts @@ -30,9 +30,12 @@ describe('repository integration test', () => { }); const deleteItem = async (object: Pick) => { - await osdTestServer.request - .delete(root, `/api/saved_objects/${object.type}/${object.id}`) - .expect(200); + expect( + [200, 404].includes( + (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`)) + .statusCode + ) + ); }; const getItem = async (object: Pick) => { @@ -41,6 +44,17 @@ describe('repository integration test', () => { .expect(200); }; + const clearFooAndBar = async () => { + await deleteItem({ + type: dashboard.type, + id: 'foo', + }); + await deleteItem({ + type: dashboard.type, + id: 'bar', + }); + }; + describe('workspace related CRUD', () => { it('create', async () => { const createResult = await osdTestServer.request @@ -67,21 +81,6 @@ describe('repository integration test', () => { }) .expect(200); - /** - * Override without workspace, in this case workspaces field should be retained. - */ - const correctOverride = await osdTestServer.request - .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) - .send({ - attributes: { - title: 'foo', - }, - }) - .expect(200); - - expect(correctOverride.body.workspaces).toEqual(['foo']); - expect(correctOverride.body.attributes.title).toEqual('foo'); - await osdTestServer.request .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) .send({ @@ -97,6 +96,7 @@ describe('repository integration test', () => { }); it('bulk create', async () => { + await clearFooAndBar(); const createResultFoo = await osdTestServer.request .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) .send([ @@ -144,6 +144,7 @@ describe('repository integration test', () => { }); it('bulk create with conflict', async () => { + await clearFooAndBar(); const createResultFoo = await osdTestServer.request .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) .send([ @@ -164,39 +165,6 @@ describe('repository integration test', () => { ]) .expect(200); - /** - * overwrite without workspaces - */ - const overwriteWithoutWorkspacesResult = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?overwrite=true`) - .send([ - { - ...dashboard, - id: 'bar', - }, - { - ...dashboard, - id: 'foo', - }, - ]) - .expect(200); - - const getFooResult = await getItem({ - type: dashboard.type, - id: 'foo', - }); - - const getBarResult = await getItem({ - type: dashboard.type, - id: 'bar', - }); - - expect(getFooResult.body.workspaces).toEqual(['foo']); - expect(getBarResult.body.workspaces).toEqual(['bar']); - expect( - (overwriteWithoutWorkspacesResult.body.saved_objects as any[]).some((item) => item.error) - ).toEqual(false); - /** * overwrite with workspaces */ @@ -232,6 +200,7 @@ describe('repository integration test', () => { }); it('checkConflicts when importing ndjson', async () => { + await clearFooAndBar(); const createResultFoo = await osdTestServer.request .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) .send([ From fd24685b3b3021be354cd2cdb7e05e028c3d0832 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 17:14:59 +0800 Subject: [PATCH 13/25] feat: optimization according to comments Signed-off-by: SuZhou-Joe --- .../export/get_sorted_objects_for_export.ts | 1 - .../import/create_saved_objects.ts | 2 +- .../import/import_saved_objects.ts | 16 +++++---- .../saved_objects/import/regenerate_ids.ts | 7 +--- .../service/lib/repository.test.js | 11 ++++-- .../saved_objects/service/lib/repository.ts | 34 +++++++++---------- .../service/lib/search_dsl/query_params.ts | 20 +++++------ .../server/saved_objects/service/lib/utils.ts | 4 +-- .../service/saved_objects_client.ts | 2 +- src/core/server/saved_objects/types.ts | 2 ++ src/core/types/saved_objects.ts | 1 + 11 files changed, 49 insertions(+), 51 deletions(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 0336fc702973..660f86846137 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -111,7 +111,6 @@ async function fetchObjectsToExport({ } const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace, - ...(workspaces ? { workspaces } : {}), }); const erroredObjects = bulkGetResult.saved_objects.filter((obj) => !!obj.error); if (erroredObjects.length) { 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 b67cffce1e96..6982f21bbf37 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -113,7 +113,7 @@ export const createSavedObjects = async ({ // remap results to reflect the object IDs that were submitted for import // this ensures that consumers understand the results const remappedResults = expectedResults.map>((result) => { - const { id } = objectIdMap.get(`${result.type}:${result.id}`) || ({} as SavedObject); + const { id } = objectIdMap.get(`${result.type}:${result.id}`)!; // also, include a `destinationId` field if the object create attempt was made with a different ID return { ...result, id, ...(id !== result.id && { destinationId: result.id }) }; }); 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 fce50bab7abc..2d87f94b97e3 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -81,13 +81,15 @@ export async function importSavedObjectsFromStream({ if (createNewCopies) { importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects); } else { - importIdMap = await regenerateIdsWithReference({ - savedObjects: collectSavedObjectsResult.collectedObjects, - savedObjectsClient, - workspaces, - objectLimit, - importIdMap, - }); + if (workspaces) { + importIdMap = await regenerateIdsWithReference({ + savedObjects: collectSavedObjectsResult.collectedObjects, + savedObjectsClient, + workspaces, + objectLimit, + importIdMap, + }); + } // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces const checkConflictsParams = { objects: collectSavedObjectsResult.collectedObjects, diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index 27b10e575f44..b309753566e3 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -47,16 +47,11 @@ export const regenerateIds = (objects: SavedObject[]) => { export const regenerateIdsWithReference = async (props: { savedObjects: SavedObject[]; savedObjectsClient: SavedObjectsClientContract; - workspaces?: string[]; + workspaces: string[]; objectLimit: number; importIdMap: Map; }): Promise> => { 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 })) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index fc80e62e1b36..7bb22474ee76 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -27,7 +27,6 @@ * specific language governing permissions and limitations * under the License. */ -import { omit } from 'lodash'; import { SavedObjectsRepository } from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; @@ -54,6 +53,12 @@ const createGenericNotFoundError = (...args) => const createUnsupportedTypeError = (...args) => SavedObjectsErrorHelpers.createUnsupportedTypeError(...args).output.payload; +const omitWorkspace = (object) => { + const newObject = JSON.parse(JSON.stringify(object)); + delete newObject.workspaces; + return newObject; +}; + describe('SavedObjectsRepository', () => { let client; let savedObjectsRepository; @@ -1922,9 +1927,9 @@ describe('SavedObjectsRepository', () => { expect(client.mget).toHaveBeenCalledTimes(1); expect(result).toEqual({ errors: [ - { ...omit(obj8, 'workspaces'), error: createConflictError(obj8.type, obj8.id) }, + { ...omitWorkspace(obj8), error: createConflictError(obj8.type, obj8.id) }, { - ...omit(obj9, 'workspaces'), + ...omitWorkspace(obj9), error: { ...createConflictError(obj9.type, obj9.id), metadata: { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index a4dcd53b9cdb..cad4763ceaab 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -381,9 +381,9 @@ export class SavedObjectsRepository { const method = object.id && overwrite ? 'index' : 'create'; const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type); /** - * Only when importing an object to a target workspace should we check if the object is workspace-specific. + * It requires a check when overwriting objects to target workspaces */ - const requiresWorkspaceCheck = object.id && options.workspaces; + const requiresWorkspaceCheck = !!(object.id && overwrite && options.workspaces); if (object.id == null) object.id = uuid.v1(); @@ -440,14 +440,12 @@ 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 ? bulkGetResponse?.body.docs?.[opensearchRequestIndex] : undefined; const docFound = indexFound && actualResult?.found === true; - let hasSetNamespace = false; // @ts-expect-error MultiGetHit._source is optional if (docFound && !this.rawDocExistsInNamespace(actualResult!, namespace)) { const { id, type } = object; @@ -462,20 +460,11 @@ export class SavedObjectsRepository { }, }, }; - } else { - hasSetNamespace = true; - if (this._registry.isSingleNamespace(object.type)) { - savedObjectNamespace = initialNamespaces ? initialNamespaces[0] : namespace; - } else if (this._registry.isMultiNamespace(object.type)) { - savedObjectNamespaces = initialNamespaces || getSavedObjectNamespaces(namespace); - } - } - if (!hasSetNamespace) { - savedObjectNamespaces = - initialNamespaces || - // @ts-expect-error MultiGetHit._source is optional - getSavedObjectNamespaces(namespace, docFound ? actualResult : undefined); } + savedObjectNamespaces = + initialNamespaces || + // @ts-expect-error MultiGetHit._source is optional + getSavedObjectNamespaces(namespace, docFound ? actualResult : undefined); // @ts-expect-error MultiGetHit._source is optional versionProperties = getExpectedVersionProperties(version, actualResult); } else { @@ -487,7 +476,7 @@ export class SavedObjectsRepository { versionProperties = getExpectedVersionProperties(version); } - savedObjectWorkspaces = options.workspaces; + let savedObjectWorkspaces: string[] | undefined = options.workspaces; if (expectedBulkGetResult.value.method !== 'create') { const rawId = this._serializer.generateRawId(namespace, object.type, object.id); @@ -495,6 +484,11 @@ export class SavedObjectsRepository { bulkGetResponse?.statusCode !== 404 ? bulkGetResponse?.body.docs?.find((item) => item._id === rawId) : null; + /** + * When it is about to overwrite a object into options.workspace. + * We need to check if the options.workspaces is the subset of object.workspaces, + * Or it will be treated as a conflict + */ if (findObject && findObject.found) { const transformedObject = this._serializer.rawToSavedObject( findObject as SavedObjectsRawDoc @@ -504,6 +498,10 @@ export class SavedObjectsRepository { transformedObject.workspaces ); if (filteredWorkspaces.length) { + /** + * options.workspaces is not a subset of object.workspaces, + * return a conflict error. + */ const { id, type } = object; return { tag: 'Left' as 'Left', diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 521149c4e646..4236eaf7fc74 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -131,14 +131,8 @@ function getClauseForType( * Gets the clause that will filter for the workspace. */ function getClauseForWorkspace(workspace: string) { - if (workspace === '*') { - return { - bool: { - must: { - match_all: {}, - }, - }, - }; + if (!workspace) { + return {}; } return { @@ -246,12 +240,14 @@ export function getQueryParams({ ], }; - if (workspaces) { + if (workspaces?.filter((workspace) => workspace).length) { bool.filter.push({ bool: { - should: workspaces.map((workspace) => { - return getClauseForWorkspace(workspace); - }), + should: workspaces + .filter((workspace) => workspace) + .map((workspace) => { + return getClauseForWorkspace(workspace); + }), minimum_should_match: 1, }, }); diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 490c2b7083d2..9fc4a6280b63 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -83,8 +83,8 @@ export class SavedObjectsUtils { public static filterWorkspacesAccordingToBaseWorkspaces( targetWorkspaces?: string[], - baseWorkspaces?: string[] + sourceWorkspaces?: string[] ): string[] { - return targetWorkspaces?.filter((item) => !baseWorkspaces?.includes(item)) || []; + return targetWorkspaces?.filter((item) => !sourceWorkspaces?.includes(item)) || []; } } diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index 5f92dacacf36..a087dc6c388a 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -363,7 +363,7 @@ export class SavedObjectsClient { */ async bulkGet( objects: SavedObjectsBulkGetObject[] = [], - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { return await this._repository.bulkGet(objects, options); } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 33862cb149fb..1b1409570fea 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -110,6 +110,7 @@ export interface SavedObjectsFindOptions { typeToNamespacesMap?: Map; /** An optional OpenSearch preference value to be used for the query **/ preference?: string; + /** If specified, will find all objects belong to specified workspaces **/ workspaces?: string[]; } @@ -120,6 +121,7 @@ export interface SavedObjectsFindOptions { export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; + /** Specify the workspaces for this operation */ workspaces?: string[]; } diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index 47faffb0b922..fa4f5ab97fdf 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -113,6 +113,7 @@ export interface SavedObject { * space. */ originId?: string; + /** Workspaces that this saved object exists in. */ workspaces?: string[]; } From ab92370100bfd4eff9cdcf22f7cb952f4861167e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 17:38:25 +0800 Subject: [PATCH 14/25] feat: update test Signed-off-by: SuZhou-Joe --- .../import/regenerate_ids.test.ts | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/import/regenerate_ids.test.ts b/src/core/server/saved_objects/import/regenerate_ids.test.ts index 11556c8a21c1..605a61774534 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.test.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.test.ts @@ -29,8 +29,10 @@ */ import { mockUuidv4 } from './__mocks__'; -import { regenerateIds } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; import { SavedObject } from '../types'; +import { savedObjectsClientMock } from '../service/saved_objects_client.mock'; +import { SavedObjectsBulkResponse } from '../service'; describe('#regenerateIds', () => { const objects = ([ @@ -62,3 +64,70 @@ describe('#regenerateIds', () => { `); }); }); + +describe('#regenerateIdsWithReference', () => { + const objects = ([ + { type: 'foo', id: '1' }, + { type: 'bar', id: '2' }, + { type: 'baz', id: '3' }, + ] as any) as SavedObject[]; + + test('returns expected values', async () => { + const mockedSavedObjectsClient = savedObjectsClientMock.create(); + mockUuidv4.mockReturnValueOnce('uuidv4 #1'); + const result: SavedObjectsBulkResponse = { + saved_objects: [ + { + error: { + statusCode: 404, + error: '', + message: '', + }, + id: '1', + type: 'foo', + attributes: {}, + references: [], + }, + { + id: '2', + type: 'bar', + attributes: {}, + references: [], + workspaces: ['bar'], + }, + { + id: '3', + type: 'baz', + attributes: {}, + references: [], + workspaces: ['foo'], + }, + ], + }; + mockedSavedObjectsClient.bulkGet.mockResolvedValue(result); + expect( + await regenerateIdsWithReference({ + savedObjects: objects, + savedObjectsClient: mockedSavedObjectsClient, + workspaces: ['bar'], + objectLimit: 1000, + importIdMap: new Map(), + }) + ).toMatchInlineSnapshot(` + Map { + "foo:1" => Object { + "id": "1", + "omitOriginId": true, + }, + "bar:2" => Object { + "id": "2", + "omitOriginId": false, + }, + "baz:3" => Object { + "id": "uuidv4 #1", + "omitOriginId": true, + }, + } + `); + }); +}); From c0cfd1780606e539b4faa232beae1a87ff51d751 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 18:14:46 +0800 Subject: [PATCH 15/25] feat: optimize code Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index cad4763ceaab..daa40075caf9 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -383,7 +383,7 @@ export class SavedObjectsRepository { /** * It requires a check when overwriting objects to target workspaces */ - const requiresWorkspaceCheck = !!(object.id && overwrite && options.workspaces); + const requiresWorkspaceCheck = !!(object.id && options.workspaces); if (object.id == null) object.id = uuid.v1(); From f4b86f0ae192a4f51d991bc9095eb4a8d0141511 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sat, 30 Sep 2023 10:50:34 +0800 Subject: [PATCH 16/25] feat: add changelog Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 623c5b7206a9..7885a742b316 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Adds Data explorer framework and implements Discover using it ([#4806](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4806)) - [Theme] Use themes' definitions to render the initial view ([#4936](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4936/)) - [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854/)) +- [Workspace] Optional workspaces params in repository ([#5162](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5162/)) ### 🐛 Bug Fixes From eb6d98fc1111bb271802aa5adf96903c04c0171c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 4 Oct 2023 22:22:28 +0800 Subject: [PATCH 17/25] feat: modify CHANGELOG Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e06e149db74..884248e19b78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,10 +37,10 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Decouple] Allow plugin manifest config to define semver compatible OpenSearch plugin and verify if it is installed on the cluster([#4612](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4612)) - [Advanced Settings] Consolidate settings into new "Appearance" category and add category IDs ([#4845](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4845)) - Adds Data explorer framework and implements Discover using it ([#4806](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4806)) -- [Theme] Use themes' definitions to render the initial view ([#4936](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4936/)) -- [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854/)) +- [Theme] Use themes' definitions to render the initial view ([#4936](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4936)) +- [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854)) - [Discover] Update embeddable for saved searches ([#5081](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5081)) -- [Workspace] Optional workspaces params in repository ([#5162](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5162/)) +- [Workspace] Optional workspaces params in repository ([#5162](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5162)) ### 🐛 Bug Fixes From 2bf837a2fefdb3e03dad0a3352f017d657ad64ef Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 12 Oct 2023 16:49:48 +0800 Subject: [PATCH 18/25] feat: increase unit test coverage Signed-off-by: SuZhou-Joe --- .../get_sorted_objects_for_export.test.ts | 25 +++++++++ .../import/import_saved_objects.test.ts | 56 +++++++++++++++++++ .../service/lib/repository.test.js | 6 ++ .../service/lib/search_dsl/query_params.ts | 8 +-- 4 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index cf7e1d8246a7..dedcdffaf2ac 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -857,4 +857,29 @@ describe('getSortedObjectsForExport()', () => { `Can't specify both "search" and "objects" properties when exporting` ); }); + + test('rejects when both types and objecys are passed in', () => { + const exportOpts = { + exportSizeLimit: 1, + savedObjectsClient, + objects: [{ type: 'index-pattern', id: '1' }], + types: ['foo'], + }; + + expect(exportSavedObjectsToStream(exportOpts)).rejects.toThrowErrorMatchingInlineSnapshot( + `Can't specify both "types" and "objects" properties when exporting` + ); + }); + + test('rejects when bulkGet returns an error', () => { + const exportOpts = { + exportSizeLimit: 1, + savedObjectsClient, + objects: [{ type: 'index-pattern', id: '1' }], + }; + + expect(exportSavedObjectsToStream(exportOpts)).rejects.toThrowErrorMatchingInlineSnapshot( + `Can't specify both "types" and "objects" properties when exporting` + ); + }); }); 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 4bf1717e895c..1039903a2550 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 @@ -278,6 +278,62 @@ describe('#importSavedObjectsFromStream', () => { }; expect(createSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); }); + + test('creates saved objects when workspaces param is provided', async () => { + const options = setupOptions(); + options.workspaces = ['foo']; + const collectedObjects = [createObject()]; + const filteredObjects = [createObject()]; + const errors = [createError(), createError(), createError(), createError()]; + getMockFn(collectSavedObjects).mockResolvedValue({ + errors: [errors[0]], + collectedObjects, + importIdMap: new Map([ + ['foo', {}], + ['bar', {}], + ['baz', {}], + ]), + }); + getMockFn(validateReferences).mockResolvedValue([errors[1]]); + getMockFn(regenerateIdsWithReference).mockResolvedValue( + Promise.resolve( + new Map([ + ['foo', {}], + ['bar', {}], + ['baz', {}], + ]) + ) + ); + getMockFn(checkConflicts).mockResolvedValue({ + errors: [errors[2]], + filteredObjects, + importIdMap: new Map([['bar', { id: 'newId1' }]]), + pendingOverwrites: new Set(), + }); + getMockFn(checkOriginConflicts).mockResolvedValue({ + errors: [errors[3]], + importIdMap: new Map([['baz', { id: 'newId2' }]]), + pendingOverwrites: new Set(), + }); + + await importSavedObjectsFromStream(options); + const importIdMap = new Map([ + ['foo', {}], + ['bar', { id: 'newId1' }], + ['baz', { id: 'newId2' }], + ]); + const createSavedObjectsParams = { + objects: collectedObjects, + accumulatedErrors: errors, + savedObjectsClient, + importIdMap, + overwrite, + namespace, + workspaces: options.workspaces, + }; + expect(createSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); + expect(regenerateIdsWithReference).toBeCalledTimes(1); + }); }); describe('with createNewCopies enabled', () => { diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 7bb22474ee76..81ab1fb778dd 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -1259,6 +1259,12 @@ describe('SavedObjectsRepository', () => { migrationVersion: doc._source.migrationVersion, }); + it(`returns early for undefined objects argument`, async () => { + const result = await savedObjectsRepository.bulkGet(); + expect(result).toEqual({ saved_objects: [] }); + expect(client.mget).not.toHaveBeenCalled(); + }); + it(`returns early for empty objects argument`, async () => { const result = await bulkGet([]); expect(result).toEqual({ saved_objects: [] }); diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 4236eaf7fc74..e660f299f460 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -131,10 +131,6 @@ function getClauseForType( * Gets the clause that will filter for the workspace. */ function getClauseForWorkspace(workspace: string) { - if (!workspace) { - return {}; - } - return { bool: { must: [{ term: { workspaces: workspace } }], @@ -245,9 +241,7 @@ export function getQueryParams({ bool: { should: workspaces .filter((workspace) => workspace) - .map((workspace) => { - return getClauseForWorkspace(workspace); - }), + .map((workspace) => getClauseForWorkspace(workspace)), minimum_should_match: 1, }, }); From dbb12484ce0807ffb79e61c400df5cbbc1457d93 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 13 Oct 2023 10:51:22 +0800 Subject: [PATCH 19/25] feat: add comment Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index daa40075caf9..17a66fceb44a 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -811,6 +811,7 @@ export class SavedObjectsRepository { * @property {string} [options.namespace] * @property {object} [options.hasReference] - { type, id } * @property {string} [options.preference] + * @property {Array} [options.workspaces] * @returns {promise} - { saved_objects: [{ id, type, version, attributes }], total, per_page, page } */ async find(options: SavedObjectsFindOptions): Promise> { From 0ba8df5d00236d0fbce32b1f67b5bf895785e913 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 17:28:45 +0800 Subject: [PATCH 20/25] feat: remove useless generateId method Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.test.ts | 22 +--- .../import/import_saved_objects.ts | 16 +-- .../saved_objects/import/regenerate_ids.ts | 35 +----- .../import/validate_references.test.ts | 104 ++++++++++++++++++ .../import/validate_references.ts | 29 ++++- .../saved_objects/service/lib/repository.ts | 6 +- .../server/saved_objects/service/lib/utils.ts | 2 +- 7 files changed, 140 insertions(+), 74 deletions(-) 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 1039903a2550..4506867a6863 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 @@ -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'; @@ -68,7 +68,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: [], @@ -241,15 +240,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, @@ -295,15 +285,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, @@ -332,7 +313,6 @@ describe('#importSavedObjectsFromStream', () => { workspaces: options.workspaces, }; expect(createSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); - expect(regenerateIdsWithReference).toBeCalledTimes(1); }); }); 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 2d87f94b97e3..b283f10c2364 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/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 saved objects from given stream. See the {@link SavedObjectsImportOptions | options} for more @@ -74,23 +74,17 @@ export async function importSavedObjectsFromStream({ const validateReferencesResult = await validateReferences( collectSavedObjectsResult.collectedObjects, savedObjectsClient, - namespace + namespace, + undefined, + workspaces ); errorAccumulator = [...errorAccumulator, ...validateReferencesResult]; if (createNewCopies) { importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects); } else { - if (workspaces) { - importIdMap = await regenerateIdsWithReference({ - savedObjects: collectSavedObjectsResult.collectedObjects, - savedObjectsClient, - workspaces, - objectLimit, - importIdMap, - }); - } // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces + // Check for conflicts across all workspaces const checkConflictsParams = { objects: collectSavedObjectsResult.collectedObjects, savedObjectsClient, diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index b309753566e3..672a8f030620 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/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. @@ -43,35 +42,3 @@ export const regenerateIds = (objects: SavedObject[]) => { }, new Map()); return importIdMap; }; - -export const regenerateIdsWithReference = async (props: { - savedObjects: SavedObject[]; - savedObjectsClient: SavedObjectsClientContract; - workspaces: string[]; - objectLimit: number; - importIdMap: Map; -}): Promise> => { - const { savedObjects, savedObjectsClient, workspaces, importIdMap } = props; - - 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); -}; diff --git a/src/core/server/saved_objects/import/validate_references.test.ts b/src/core/server/saved_objects/import/validate_references.test.ts index df9a4e216bbb..c028f7a9ca5f 100644 --- a/src/core/server/saved_objects/import/validate_references.test.ts +++ b/src/core/server/saved_objects/import/validate_references.test.ts @@ -136,6 +136,7 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, + workspaces, ], id: 1, type: index-pattern, @@ -230,6 +231,7 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, + workspaces, ], id: 1, type: index-pattern, @@ -237,6 +239,7 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, + workspaces, ], id: 3, type: search, @@ -419,6 +422,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 3, type: index-pattern, @@ -426,6 +430,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 5, type: index-pattern, @@ -433,6 +438,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 6, type: index-pattern, @@ -440,6 +446,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 7, type: search, @@ -447,6 +454,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 8, type: search, @@ -601,4 +609,100 @@ describe('validateReferences()', () => { validateReferences(savedObjects, savedObjectsClient) ).rejects.toThrowErrorMatchingInlineSnapshot(`Bad Request`); }); + + test('returns errors when references have conflict on workspaces', async () => { + savedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: [ + { + type: 'index-pattern', + id: '3', + error: SavedObjectsErrorHelpers.createGenericNotFoundError('index-pattern', '3').output + .payload, + attributes: {}, + references: [], + workspaces: ['foo'], + }, + { + type: 'index-pattern', + id: '5', + error: SavedObjectsErrorHelpers.createGenericNotFoundError('index-pattern', '5').output + .payload, + attributes: {}, + references: [], + workspaces: ['bar'], + }, + ], + }); + const savedObjects = [ + { + id: '5', + type: 'index-pattern', + attributes: { + title: 'My Visualization 2', + }, + references: [ + { + name: 'ref_0', + type: 'index-pattern', + id: '3', + }, + ], + }, + ]; + const result = await validateReferences( + savedObjects, + savedObjectsClient, + undefined, + undefined, + ['bar'] + ); + expect(result).toMatchInlineSnapshot(` + Array [ + Object { + error: Object { + references: Array [ + Object { + id: 3, + type: index-pattern, + }, + ], + type: missing_references, + }, + id: 5, + meta: Object { + title: My Visualization 2, + }, + title: My Visualization 2, + type: index-pattern, + }, + ] + `); + expect(savedObjectsClient.bulkGet).toMatchInlineSnapshot(` + [MockFunction] { + "calls": Array [ + Array [ + Array [ + Object { + fields: Array [ + id, + workspaces, + ], + id: 3, + type: index-pattern, + }, + ], + Object { + namespace: undefined, + }, + ], + ], + "results": Array [ + Object { + type: return, + value: Promise {}, + }, + ], + } + `); + }); }); diff --git a/src/core/server/saved_objects/import/validate_references.ts b/src/core/server/saved_objects/import/validate_references.ts index fb75eb837443..e0ad4960dfc6 100644 --- a/src/core/server/saved_objects/import/validate_references.ts +++ b/src/core/server/saved_objects/import/validate_references.ts @@ -31,6 +31,7 @@ import Boom from '@hapi/boom'; import { SavedObject, SavedObjectsClientContract } from '../types'; import { SavedObjectsImportError, SavedObjectsImportRetry } from './types'; +import { SavedObjectsUtils } from '../service'; const REF_TYPES_TO_VLIDATE = ['index-pattern', 'search']; @@ -48,7 +49,8 @@ export async function getNonExistingReferenceAsKeys( savedObjects: SavedObject[], savedObjectsClient: SavedObjectsClientContract, namespace?: string, - retries?: SavedObjectsImportRetry[] + retries?: SavedObjectsImportRetry[], + workspaces?: string[] ) { const objectsToSkip = getObjectsToSkip(retries); const collector = new Map(); @@ -73,7 +75,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', 'workspaces'], + })); const bulkGetResponse = await savedObjectsClient.bulkGet(bulkGetOpts, { namespace }); // Error handling @@ -93,6 +98,20 @@ export async function getNonExistingReferenceAsKeys( if (savedObject.error) { continue; } + /** + * If original workspaces have conflict with target workspace + * make it as NonExistingReference + */ + if (workspaces) { + if ( + SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + workspaces, + savedObject.workspaces + ).length + ) { + continue; + } + } collector.delete(`${savedObject.type}:${savedObject.id}`); } @@ -103,7 +122,8 @@ export async function validateReferences( savedObjects: Array>, savedObjectsClient: SavedObjectsClientContract, namespace?: string, - retries?: SavedObjectsImportRetry[] + retries?: SavedObjectsImportRetry[], + workspaces?: string[] ) { const objectsToSkip = getObjectsToSkip(retries); const errorMap: { [key: string]: SavedObjectsImportError } = {}; @@ -111,7 +131,8 @@ export async function validateReferences( savedObjects, savedObjectsClient, namespace, - retries + retries, + workspaces ); // Filter out objects with missing references, add to error object diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 17a66fceb44a..45997e6cf6b9 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -291,7 +291,7 @@ export class SavedObjectsRepository { } if (currentItem) { if ( - SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( workspaces, currentItem.workspaces ).length @@ -493,7 +493,7 @@ export class SavedObjectsRepository { const transformedObject = this._serializer.rawToSavedObject( findObject as SavedObjectsRawDoc ) as SavedObject; - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( options.workspaces, transformedObject.workspaces ); @@ -658,7 +658,7 @@ export class SavedObjectsRepository { let workspaceConflict = false; if (options.workspaces) { const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc); - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( options.workspaces, transformedObject.workspaces ); diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 9fc4a6280b63..d3cb62e02b82 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/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[], sourceWorkspaces?: string[] ): string[] { From 743bf33d40218ecf57d00422bbf8529405727895 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 17:29:46 +0800 Subject: [PATCH 21/25] feat: remove flaky test Signed-off-by: SuZhou-Joe --- .../export/get_sorted_objects_for_export.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index dedcdffaf2ac..0fb602629c55 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -870,16 +870,4 @@ describe('getSortedObjectsForExport()', () => { `Can't specify both "types" and "objects" properties when exporting` ); }); - - test('rejects when bulkGet returns an error', () => { - const exportOpts = { - exportSizeLimit: 1, - savedObjectsClient, - objects: [{ type: 'index-pattern', id: '1' }], - }; - - expect(exportSavedObjectsToStream(exportOpts)).rejects.toThrowErrorMatchingInlineSnapshot( - `Can't specify both "types" and "objects" properties when exporting` - ); - }); }); From 285adad79b26e13c722c4764daa3c679b8f94420 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 17:31:18 +0800 Subject: [PATCH 22/25] feat: remove useless code Signed-off-by: SuZhou-Joe --- .../import/regenerate_ids.test.ts | 71 +------------------ 1 file changed, 1 insertion(+), 70 deletions(-) diff --git a/src/core/server/saved_objects/import/regenerate_ids.test.ts b/src/core/server/saved_objects/import/regenerate_ids.test.ts index 605a61774534..11556c8a21c1 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.test.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.test.ts @@ -29,10 +29,8 @@ */ import { mockUuidv4 } from './__mocks__'; -import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; +import { regenerateIds } from './regenerate_ids'; import { SavedObject } from '../types'; -import { savedObjectsClientMock } from '../service/saved_objects_client.mock'; -import { SavedObjectsBulkResponse } from '../service'; describe('#regenerateIds', () => { const objects = ([ @@ -64,70 +62,3 @@ describe('#regenerateIds', () => { `); }); }); - -describe('#regenerateIdsWithReference', () => { - const objects = ([ - { type: 'foo', id: '1' }, - { type: 'bar', id: '2' }, - { type: 'baz', id: '3' }, - ] as any) as SavedObject[]; - - test('returns expected values', async () => { - const mockedSavedObjectsClient = savedObjectsClientMock.create(); - mockUuidv4.mockReturnValueOnce('uuidv4 #1'); - const result: SavedObjectsBulkResponse = { - saved_objects: [ - { - error: { - statusCode: 404, - error: '', - message: '', - }, - id: '1', - type: 'foo', - attributes: {}, - references: [], - }, - { - id: '2', - type: 'bar', - attributes: {}, - references: [], - workspaces: ['bar'], - }, - { - id: '3', - type: 'baz', - attributes: {}, - references: [], - workspaces: ['foo'], - }, - ], - }; - mockedSavedObjectsClient.bulkGet.mockResolvedValue(result); - expect( - await regenerateIdsWithReference({ - savedObjects: objects, - savedObjectsClient: mockedSavedObjectsClient, - workspaces: ['bar'], - objectLimit: 1000, - importIdMap: new Map(), - }) - ).toMatchInlineSnapshot(` - Map { - "foo:1" => Object { - "id": "1", - "omitOriginId": true, - }, - "bar:2" => Object { - "id": "2", - "omitOriginId": false, - }, - "baz:3" => Object { - "id": "uuidv4 #1", - "omitOriginId": true, - }, - } - `); - }); -}); From 142e9c000302cab244679844b8e80c30ad83b400 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 10:22:31 +0800 Subject: [PATCH 23/25] fix: unit test Signed-off-by: SuZhou-Joe --- .../saved_objects/import/import_saved_objects.test.ts | 4 +++- .../server/saved_objects/import/validate_references.ts | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) 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 4506867a6863..d0e4eda7ab3c 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 @@ -165,7 +165,9 @@ describe('#importSavedObjectsFromStream', () => { expect(validateReferences).toHaveBeenCalledWith( collectedObjects, savedObjectsClient, - namespace + namespace, + undefined, + undefined ); }); diff --git a/src/core/server/saved_objects/import/validate_references.ts b/src/core/server/saved_objects/import/validate_references.ts index e0ad4960dfc6..1f9b4460b2f8 100644 --- a/src/core/server/saved_objects/import/validate_references.ts +++ b/src/core/server/saved_objects/import/validate_references.ts @@ -74,10 +74,15 @@ export async function getNonExistingReferenceAsKeys( return []; } + const fields = ['id']; + if (workspaces?.length) { + fields.push('workspaces'); + } + // Fetch references to see if they exist const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ ...obj, - fields: ['id', 'workspaces'], + fields, })); const bulkGetResponse = await savedObjectsClient.bulkGet(bulkGetOpts, { namespace }); From e8aa3a4594ab7598195e00c923964555263c0296 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 16:45:57 +0800 Subject: [PATCH 24/25] feat: update snapshot Signed-off-by: SuZhou-Joe --- .../saved_objects/import/validate_references.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/core/server/saved_objects/import/validate_references.test.ts b/src/core/server/saved_objects/import/validate_references.test.ts index c028f7a9ca5f..f53922c27ac0 100644 --- a/src/core/server/saved_objects/import/validate_references.test.ts +++ b/src/core/server/saved_objects/import/validate_references.test.ts @@ -136,7 +136,6 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, - workspaces, ], id: 1, type: index-pattern, @@ -231,7 +230,6 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, - workspaces, ], id: 1, type: index-pattern, @@ -239,7 +237,6 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, - workspaces, ], id: 3, type: search, @@ -422,7 +419,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 3, type: index-pattern, @@ -430,7 +426,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 5, type: index-pattern, @@ -438,7 +433,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 6, type: index-pattern, @@ -446,7 +440,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 7, type: search, @@ -454,7 +447,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 8, type: search, From 32889618e36a1e7e7c50e78f0d866121e3e8d6df Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 17:58:07 +0800 Subject: [PATCH 25/25] feat: increase code coverage Signed-off-by: SuZhou-Joe --- .../server/saved_objects/import/validate_references.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/core/server/saved_objects/import/validate_references.test.ts b/src/core/server/saved_objects/import/validate_references.test.ts index f53922c27ac0..809b75c9320c 100644 --- a/src/core/server/saved_objects/import/validate_references.test.ts +++ b/src/core/server/saved_objects/import/validate_references.test.ts @@ -608,8 +608,6 @@ describe('validateReferences()', () => { { type: 'index-pattern', id: '3', - error: SavedObjectsErrorHelpers.createGenericNotFoundError('index-pattern', '3').output - .payload, attributes: {}, references: [], workspaces: ['foo'], @@ -617,8 +615,6 @@ describe('validateReferences()', () => { { type: 'index-pattern', id: '5', - error: SavedObjectsErrorHelpers.createGenericNotFoundError('index-pattern', '5').output - .payload, attributes: {}, references: [], workspaces: ['bar'],