From 50333a0fb90c2addfa589b58434586ff1e691b67 Mon Sep 17 00:00:00 2001 From: gaobinlong Date: Wed, 20 Sep 2023 13:19:49 +0800 Subject: [PATCH 01/15] [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 6f67893104e7..09e8ad8b5407 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 @@ -14,6 +14,7 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -111,6 +112,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; @@ -130,6 +134,7 @@ Object { "thirdType": "510f1f0adb69830cf8a1c5ce2923ed82", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -244,6 +249,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 02dc13b2cd3f..05fb534f7a11 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 @@ -186,6 +186,9 @@ function defaultMapping(): IndexMapping { }, }, }, + workspaces: { + type: 'keyword', + }, permissions: { properties: { read: principals, 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 70f96c2e4daf..dde16977f015 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 @@ -83,6 +83,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -93,6 +94,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, permissions: { properties: { library_read: { @@ -235,6 +239,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -246,6 +251,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, permissions: { properties: { library_read: { @@ -331,6 +339,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -342,6 +351,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, permissions: { properties: { library_read: { 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 5e39af788d79..2748ad2eaf6a 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 @@ -14,6 +14,7 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -119,6 +120,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 d1719f1c1c791ed8df6d6a82aa37170d18aee52a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Sep 2023 13:52:21 +0800 Subject: [PATCH 02/15] 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 77c2afa1411f664a7f5d5b668afa15e7a3539542 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sat, 23 Sep 2023 18:14:18 +0800 Subject: [PATCH 03/15] 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 69dd163ecdb82bc0255d5d886ea26a509dfa795e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sat, 23 Sep 2023 18:33:40 +0800 Subject: [PATCH 04/15] 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 34925ec5569943d20de57796de84d0a8de12e332 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 24 Sep 2023 17:20:00 +0800 Subject: [PATCH 05/15] 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 f06d9b0e21ed3766bb833d219b8744eb40ddb83f Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 16:28:48 +0800 Subject: [PATCH 06/15] 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 2b012ea2c8b88a58c6950ca4a82cb40b6fe5db8d Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 17:28:07 +0800 Subject: [PATCH 07/15] 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 e0161fab2f2abbf23e1f92abfa1e0fca5a7b8749 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 17:51:18 +0800 Subject: [PATCH 08/15] 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 18614075cf4cbee3826ba519fa13dd44cc23aec3 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 14:21:20 +0800 Subject: [PATCH 09/15] 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 9971636722db5433b2478d07e2dc0d82f684b7b3 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 15:20:49 +0800 Subject: [PATCH 10/15] 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 e8c029820df70451a06d4e3997d29b7ed1fa0c78 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 17:39:50 +0800 Subject: [PATCH 11/15] 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 9599c4f7f0a1e89b7506fe35cf498ffae31c14b8 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 19:11:21 +0800 Subject: [PATCH 12/15] 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 43804bce3498e4e9148deb71932b0d8a58c7c08c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 17:14:59 +0800 Subject: [PATCH 13/15] 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 e269075b80b837fcfb95dee87a39d856d3fd29b8 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 17:38:25 +0800 Subject: [PATCH 14/15] 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 5b0e31ba90670be0a876ee135c1c82c48d5b168e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 18:14:46 +0800 Subject: [PATCH 15/15] 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();