From 53d84f21fe6a7f05d2d7f20283fab6ac518b4d65 Mon Sep 17 00:00:00 2001 From: gaobinlong Date: Wed, 20 Sep 2023 13:19:49 +0800 Subject: [PATCH 01/43] [Workspace] Add workspaces parameters to all saved objects API Signed-off-by: gaobinlong --- .../saved_objects/saved_objects_client.ts | 1 + .../get_sorted_objects_for_export.test.ts | 6 + .../export/get_sorted_objects_for_export.ts | 9 +- .../saved_objects/import/check_conflicts.ts | 3 + .../import/create_saved_objects.ts | 5 +- .../import/import_saved_objects.ts | 2 + .../import/resolve_import_errors.ts | 2 + src/core/server/saved_objects/import/types.ts | 4 + .../build_active_mappings.test.ts.snap | 8 ++ .../migrations/core/build_active_mappings.ts | 3 + .../migrations/core/index_migrator.test.ts | 12 ++ ...pensearch_dashboards_migrator.test.ts.snap | 4 + .../saved_objects/routes/bulk_create.ts | 12 +- .../server/saved_objects/routes/create.ts | 12 +- .../server/saved_objects/routes/export.ts | 11 +- src/core/server/saved_objects/routes/find.ts | 8 ++ .../server/saved_objects/routes/import.ts | 9 ++ .../routes/resolve_import_errors.ts | 9 ++ .../saved_objects/serialization/serializer.ts | 5 +- .../saved_objects/serialization/types.ts | 2 + .../saved_objects/service/lib/repository.ts | 103 ++++++++++++++++-- .../service/lib/search_dsl/query_params.ts | 33 ++++++ .../service/lib/search_dsl/search_dsl.ts | 3 + .../server/saved_objects/service/lib/utils.ts | 7 ++ src/core/server/saved_objects/types.ts | 2 + src/core/types/saved_objects.ts | 1 + 26 files changed, 258 insertions(+), 18 deletions(-) diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index 6e5482614e40..d6b6b6b6d89c 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -345,6 +345,7 @@ export class SavedObjectsClient { filter: 'filter', namespaces: 'namespaces', preference: 'preference', + workspaces: 'workspaces', }; const renamedQuery = renameKeys(renameMap, options); diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index cf7e1d8246a7..952a74a76940 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -128,6 +128,7 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], + workspaces: undefined, }, ], ], @@ -218,6 +219,7 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], + workspaces: undefined, }, ], ], @@ -368,6 +370,7 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], + workspaces: undefined, }, ], ], @@ -459,6 +462,7 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], + workspaces: undefined, }, ], ], @@ -666,6 +670,7 @@ describe('getSortedObjectsForExport()', () => { ], Object { namespace: undefined, + workspaces: undefined, }, ], ], @@ -784,6 +789,7 @@ describe('getSortedObjectsForExport()', () => { ], Object { namespace: undefined, + workspaces: undefined, }, ], Array [ diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 7bf6e9f6ccdc..8ca085639f10 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -60,6 +60,8 @@ export interface SavedObjectsExportOptions { excludeExportDetails?: boolean; /** optional namespace to override the namespace used by the savedObjectsClient. */ namespace?: string; + /** optional workspaces to override the workspaces used by the savedObjectsClient. */ + workspaces?: string[]; } /** @@ -87,6 +89,7 @@ async function fetchObjectsToExport({ exportSizeLimit, savedObjectsClient, namespace, + workspaces, }: { objects?: SavedObjectsExportOptions['objects']; types?: string[]; @@ -94,6 +97,7 @@ async function fetchObjectsToExport({ exportSizeLimit: number; savedObjectsClient: SavedObjectsClientContract; namespace?: string; + workspaces?: string[]; }) { if ((types?.length ?? 0) > 0 && (objects?.length ?? 0) > 0) { throw Boom.badRequest(`Can't specify both "types" and "objects" properties when exporting`); @@ -105,7 +109,7 @@ async function fetchObjectsToExport({ if (typeof search === 'string') { throw Boom.badRequest(`Can't specify both "search" and "objects" properties when exporting`); } - const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace }); + const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace, workspaces }); const erroredObjects = bulkGetResult.saved_objects.filter((obj) => !!obj.error); if (erroredObjects.length) { const err = Boom.badRequest(); @@ -121,6 +125,7 @@ async function fetchObjectsToExport({ search, perPage: exportSizeLimit, namespaces: namespace ? [namespace] : undefined, + workspaces, }); if (findResponse.total > exportSizeLimit) { throw Boom.badRequest(`Can't export more than ${exportSizeLimit} objects`); @@ -153,6 +158,7 @@ export async function exportSavedObjectsToStream({ includeReferencesDeep = false, excludeExportDetails = false, namespace, + workspaces, }: SavedObjectsExportOptions) { const rootObjects = await fetchObjectsToExport({ types, @@ -161,6 +167,7 @@ export async function exportSavedObjectsToStream({ savedObjectsClient, exportSizeLimit, namespace, + workspaces, }); let exportedObjects: Array> = []; let missingReferences: SavedObjectsExportResultDetails['missingReferences'] = []; diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index 830f7f55d7c5..f36bcf3a8a92 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -44,6 +44,7 @@ interface CheckConflictsParams { ignoreRegularConflicts?: boolean; retries?: SavedObjectsImportRetry[]; createNewCopies?: boolean; + workspaces?: string[]; } const isUnresolvableConflict = (error: SavedObjectError) => @@ -56,6 +57,7 @@ export async function checkConflicts({ ignoreRegularConflicts, retries = [], createNewCopies, + workspaces, }: CheckConflictsParams) { const filteredObjects: Array> = []; const errors: SavedObjectsImportError[] = []; @@ -77,6 +79,7 @@ export async function checkConflicts({ }); const checkConflictsResult = await savedObjectsClient.checkConflicts(objectsToCheck, { namespace, + workspaces, }); const errorMap = checkConflictsResult.errors.reduce( (acc, { type, id, error }) => acc.set(`${type}:${id}`, error), diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts index a3a1eebbd2ab..b67cffce1e96 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -39,6 +39,7 @@ interface CreateSavedObjectsParams { importIdMap: Map; namespace?: string; overwrite?: boolean; + workspaces?: string[]; } interface CreateSavedObjectsResult { createdObjects: Array>; @@ -56,6 +57,7 @@ export const createSavedObjects = async ({ importIdMap, namespace, overwrite, + workspaces, }: CreateSavedObjectsParams): Promise> => { // filter out any objects that resulted in errors const errorSet = accumulatedErrors.reduce( @@ -103,6 +105,7 @@ export const createSavedObjects = async ({ const bulkCreateResponse = await savedObjectsClient.bulkCreate(objectsToCreate, { namespace, overwrite, + workspaces, }); expectedResults = bulkCreateResponse.saved_objects; } @@ -110,7 +113,7 @@ export const createSavedObjects = async ({ // remap results to reflect the object IDs that were submitted for import // this ensures that consumers understand the results const remappedResults = expectedResults.map>((result) => { - const { id } = objectIdMap.get(`${result.type}:${result.id}`)!; + const { id } = objectIdMap.get(`${result.type}:${result.id}`) || ({} as SavedObject); // also, include a `destinationId` field if the object create attempt was made with a different ID return { ...result, id, ...(id !== result.id && { destinationId: result.id }) }; }); diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index cd250fc5f65f..68104db85e6f 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -54,6 +54,7 @@ export async function importSavedObjectsFromStream({ savedObjectsClient, typeRegistry, namespace, + workspaces, }: SavedObjectsImportOptions): Promise { let errorAccumulator: SavedObjectsImportError[] = []; const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name); @@ -118,6 +119,7 @@ export async function importSavedObjectsFromStream({ importIdMap, overwrite, namespace, + workspaces, }; const createSavedObjectsResult = await createSavedObjects(createSavedObjectsParams); errorAccumulator = [...errorAccumulator, ...createSavedObjectsResult.errors]; diff --git a/src/core/server/saved_objects/import/resolve_import_errors.ts b/src/core/server/saved_objects/import/resolve_import_errors.ts index 162410c4ce9b..207410136645 100644 --- a/src/core/server/saved_objects/import/resolve_import_errors.ts +++ b/src/core/server/saved_objects/import/resolve_import_errors.ts @@ -59,6 +59,7 @@ export async function resolveSavedObjectsImportErrors({ typeRegistry, namespace, createNewCopies, + workspaces, }: SavedObjectsResolveImportErrorsOptions): Promise { // throw a BadRequest error if we see invalid retries validateRetries(retries); @@ -157,6 +158,7 @@ export async function resolveSavedObjectsImportErrors({ importIdMap, namespace, overwrite, + workspaces, }; const { createdObjects, errors: bulkCreateErrors } = await createSavedObjects( createSavedObjectsParams diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 88beacb9d2fd..924d1b18895a 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -187,6 +187,8 @@ export interface SavedObjectsImportOptions { namespace?: string; /** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */ createNewCopies: boolean; + /** if specified, will import in given workspaces, else will import as global object */ + workspaces?: string[]; } /** @@ -208,6 +210,8 @@ export interface SavedObjectsResolveImportErrorsOptions { namespace?: string; /** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */ createNewCopies: boolean; + /** if specified, will import in given workspaces, else will import as global object */ + workspaces?: string[]; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap index f8ef47cae894..254bdbb4b2c6 100644 --- a/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap +++ b/src/core/server/saved_objects/migrations/core/__snapshots__/build_active_mappings.test.ts.snap @@ -13,6 +13,7 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -56,6 +57,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; @@ -74,6 +78,7 @@ Object { "thirdType": "510f1f0adb69830cf8a1c5ce2923ed82", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -134,6 +139,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; diff --git a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts index bf377a13a42e..812cc1fd5eb1 100644 --- a/src/core/server/saved_objects/migrations/core/build_active_mappings.ts +++ b/src/core/server/saved_objects/migrations/core/build_active_mappings.ts @@ -175,6 +175,9 @@ function defaultMapping(): IndexMapping { }, }, }, + workspaces: { + type: 'keyword', + }, }, }; } diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index 4bacfda3bd5a..4b65da5250d6 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -82,6 +82,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -92,6 +93,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, references: { type: 'nested', properties: { @@ -199,6 +203,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -210,6 +215,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, references: { type: 'nested', properties: { @@ -260,6 +268,7 @@ describe('IndexMigrator', () => { references: '7997cf5a56cc02bdc9c93361bde732b0', type: '2f4316de49999235636386fe51dc06c1', updated_at: '00da57df13e94e9d98437d13ace4bfe0', + workspaces: '2f4316de49999235636386fe51dc06c1', }, }, properties: { @@ -271,6 +280,9 @@ describe('IndexMigrator', () => { originId: { type: 'keyword' }, type: { type: 'keyword' }, updated_at: { type: 'date' }, + workspaces: { + type: 'keyword', + }, references: { type: 'nested', properties: { diff --git a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap index baebb7848798..811d5f3594bf 100644 --- a/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap +++ b/src/core/server/saved_objects/migrations/opensearch_dashboards/__snapshots__/opensearch_dashboards_migrator.test.ts.snap @@ -13,6 +13,7 @@ Object { "references": "7997cf5a56cc02bdc9c93361bde732b0", "type": "2f4316de49999235636386fe51dc06c1", "updated_at": "00da57df13e94e9d98437d13ace4bfe0", + "workspaces": "2f4316de49999235636386fe51dc06c1", }, }, "dynamic": "strict", @@ -64,6 +65,9 @@ Object { "updated_at": Object { "type": "date", }, + "workspaces": Object { + "type": "keyword", + }, }, } `; diff --git a/src/core/server/saved_objects/routes/bulk_create.ts b/src/core/server/saved_objects/routes/bulk_create.ts index 5c2844d64813..36fd4bda5bff 100644 --- a/src/core/server/saved_objects/routes/bulk_create.ts +++ b/src/core/server/saved_objects/routes/bulk_create.ts @@ -38,6 +38,9 @@ export const registerBulkCreateRoute = (router: IRouter) => { validate: { query: schema.object({ overwrite: schema.boolean({ defaultValue: false }), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }), body: schema.arrayOf( schema.object({ @@ -62,7 +65,14 @@ export const registerBulkCreateRoute = (router: IRouter) => { }, router.handleLegacyErrors(async (context, req, res) => { const { overwrite } = req.query; - const result = await context.core.savedObjects.client.bulkCreate(req.body, { overwrite }); + let workspaces = req.query.workspaces; + if (typeof workspaces === 'string') { + workspaces = [workspaces]; + } + const result = await context.core.savedObjects.client.bulkCreate(req.body, { + overwrite, + workspaces, + }); return res.ok({ body: result }); }) ); diff --git a/src/core/server/saved_objects/routes/create.ts b/src/core/server/saved_objects/routes/create.ts index c8c330ba7774..4d22bd244a03 100644 --- a/src/core/server/saved_objects/routes/create.ts +++ b/src/core/server/saved_objects/routes/create.ts @@ -56,15 +56,23 @@ export const registerCreateRoute = (router: IRouter) => { ) ), initialNamespaces: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1 })), + workspaces: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1 })), }), }, }, router.handleLegacyErrors(async (context, req, res) => { const { type, id } = req.params; const { overwrite } = req.query; - const { attributes, migrationVersion, references, initialNamespaces } = req.body; + const { attributes, migrationVersion, references, initialNamespaces, workspaces } = req.body; - const options = { id, overwrite, migrationVersion, references, initialNamespaces }; + const options = { + id, + overwrite, + migrationVersion, + references, + initialNamespaces, + workspaces, + }; const result = await context.core.savedObjects.client.create(type, attributes, options); return res.ok({ body: result }); }) diff --git a/src/core/server/saved_objects/routes/export.ts b/src/core/server/saved_objects/routes/export.ts index 2c808b731b4e..9325b632e40f 100644 --- a/src/core/server/saved_objects/routes/export.ts +++ b/src/core/server/saved_objects/routes/export.ts @@ -57,12 +57,20 @@ export const registerExportRoute = (router: IRouter, config: SavedObjectConfig) search: schema.maybe(schema.string()), includeReferencesDeep: schema.boolean({ defaultValue: false }), excludeExportDetails: schema.boolean({ defaultValue: false }), + workspaces: schema.maybe(schema.arrayOf(schema.string())), }), }, }, router.handleLegacyErrors(async (context, req, res) => { const savedObjectsClient = context.core.savedObjects.client; - const { type, objects, search, excludeExportDetails, includeReferencesDeep } = req.body; + const { + type, + objects, + search, + excludeExportDetails, + includeReferencesDeep, + workspaces, + } = req.body; const types = typeof type === 'string' ? [type] : type; // need to access the registry for type validation, can't use the schema for this @@ -98,6 +106,7 @@ export const registerExportRoute = (router: IRouter, config: SavedObjectConfig) exportSizeLimit: maxImportExportSize, includeReferencesDeep, excludeExportDetails, + workspaces, }); const docsToExport: string[] = await createPromiseFromStreams([ diff --git a/src/core/server/saved_objects/routes/find.ts b/src/core/server/saved_objects/routes/find.ts index dbc9bf9e3a0d..6cfb2d780036 100644 --- a/src/core/server/saved_objects/routes/find.ts +++ b/src/core/server/saved_objects/routes/find.ts @@ -59,6 +59,9 @@ export const registerFindRoute = (router: IRouter) => { namespaces: schema.maybe( schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) ), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }), }, }, @@ -67,6 +70,10 @@ export const registerFindRoute = (router: IRouter) => { const namespaces = typeof req.query.namespaces === 'string' ? [req.query.namespaces] : req.query.namespaces; + let workspaces = req.query.workspaces; + if (typeof workspaces === 'string') { + workspaces = [workspaces]; + } const result = await context.core.savedObjects.client.find({ perPage: query.per_page, @@ -81,6 +88,7 @@ export const registerFindRoute = (router: IRouter) => { fields: typeof query.fields === 'string' ? [query.fields] : query.fields, filter: query.filter, namespaces, + workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/routes/import.ts b/src/core/server/saved_objects/routes/import.ts index b157feb0860e..2d221d2d47bc 100644 --- a/src/core/server/saved_objects/routes/import.ts +++ b/src/core/server/saved_objects/routes/import.ts @@ -60,6 +60,9 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) { overwrite: schema.boolean({ defaultValue: false }), createNewCopies: schema.boolean({ defaultValue: false }), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }, { validate: (object) => { @@ -91,6 +94,11 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) }); } + let workspaces = req.query.workspaces; + if (typeof workspaces === 'string') { + workspaces = [workspaces]; + } + const result = await importSavedObjectsFromStream({ savedObjectsClient: context.core.savedObjects.client, typeRegistry: context.core.savedObjects.typeRegistry, @@ -98,6 +106,7 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig) objectLimit: maxImportExportSize, overwrite, createNewCopies, + workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/routes/resolve_import_errors.ts b/src/core/server/saved_objects/routes/resolve_import_errors.ts index 5e07125671f1..32d67b5ae8ab 100644 --- a/src/core/server/saved_objects/routes/resolve_import_errors.ts +++ b/src/core/server/saved_objects/routes/resolve_import_errors.ts @@ -58,6 +58,9 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO validate: { query: schema.object({ createNewCopies: schema.boolean({ defaultValue: false }), + workspaces: schema.maybe( + schema.oneOf([schema.string(), schema.arrayOf(schema.string())]) + ), }), body: schema.object({ file: schema.stream(), @@ -98,6 +101,11 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO }); } + let workspaces = req.query.workspaces; + if (typeof workspaces === 'string') { + workspaces = [workspaces]; + } + const result = await resolveSavedObjectsImportErrors({ typeRegistry: context.core.savedObjects.typeRegistry, savedObjectsClient: context.core.savedObjects.client, @@ -105,6 +113,7 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO retries: req.body.retries, objectLimit: maxImportExportSize, createNewCopies: req.query.createNewCopies, + workspaces, }); return res.ok({ body: result }); diff --git a/src/core/server/saved_objects/serialization/serializer.ts b/src/core/server/saved_objects/serialization/serializer.ts index ff840a1fac60..5c3e22ac646a 100644 --- a/src/core/server/saved_objects/serialization/serializer.ts +++ b/src/core/server/saved_objects/serialization/serializer.ts @@ -73,7 +73,7 @@ export class SavedObjectsSerializer { */ public rawToSavedObject(doc: SavedObjectsRawDoc): SavedObjectSanitizedDoc { const { _id, _source, _seq_no, _primary_term } = doc; - const { type, namespace, namespaces, originId } = _source; + const { type, namespace, namespaces, originId, workspaces } = _source; const version = _seq_no != null || _primary_term != null @@ -91,6 +91,7 @@ export class SavedObjectsSerializer { ...(_source.migrationVersion && { migrationVersion: _source.migrationVersion }), ...(_source.updated_at && { updated_at: _source.updated_at }), ...(version && { version }), + ...(workspaces && { workspaces }), }; } @@ -112,6 +113,7 @@ export class SavedObjectsSerializer { updated_at, version, references, + workspaces, } = savedObj; const source = { [type]: attributes, @@ -122,6 +124,7 @@ export class SavedObjectsSerializer { ...(originId && { originId }), ...(migrationVersion && { migrationVersion }), ...(updated_at && { updated_at }), + ...(workspaces && { workspaces }), }; return { diff --git a/src/core/server/saved_objects/serialization/types.ts b/src/core/server/saved_objects/serialization/types.ts index d10ec75cdf41..473a63cf65f4 100644 --- a/src/core/server/saved_objects/serialization/types.ts +++ b/src/core/server/saved_objects/serialization/types.ts @@ -52,6 +52,7 @@ export interface SavedObjectsRawDocSource { updated_at?: string; references?: SavedObjectReference[]; originId?: string; + workspaces?: string[]; [typeMapping: string]: any; } @@ -69,6 +70,7 @@ interface SavedObjectDoc { version?: string; updated_at?: string; originId?: string; + workspaces?: string[]; } interface Referencable { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index bccfd8ff2265..a49f356bbb81 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -243,6 +243,7 @@ export class SavedObjectsRepository { originId, initialNamespaces, version, + workspaces, } = options; const namespace = normalizeNamespace(options.namespace); @@ -279,6 +280,20 @@ export class SavedObjectsRepository { } } + let savedObjectWorkspaces = workspaces; + + if (id && overwrite && workspaces) { + try { + const currentItem = await this.get(type, id); + if (currentItem && currentItem.workspaces) { + // do not overwrite workspaces + savedObjectWorkspaces = currentItem.workspaces; + } + } catch (e) { + // this.get will throw an error when no items can be found + } + } + const migrated = this._migrator.migrateDocument({ id, type, @@ -289,6 +304,7 @@ export class SavedObjectsRepository { migrationVersion, updated_at: time, ...(Array.isArray(references) && { references }), + ...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }), }); const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc); @@ -402,12 +418,16 @@ export class SavedObjectsRepository { object: { initialNamespaces, version, ...object }, method, } = expectedBulkGetResult.value; + let savedObjectWorkspaces: string[] | undefined; + let finalMethod = method; + let finalObjectId = object.id; if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound ? bulkGetResponse?.body.docs[opensearchRequestIndex] : undefined; const docFound = indexFound && actualResult?.found === true; + let hasSetNamespace = false; // @ts-expect-error MultiGetHit._source is optional if (docFound && !this.rawDocExistsInNamespace(actualResult!, namespace)) { const { id, type } = object; @@ -422,11 +442,20 @@ export class SavedObjectsRepository { }, }, }; + } else { + hasSetNamespace = true; + if (this._registry.isSingleNamespace(object.type)) { + savedObjectNamespace = initialNamespaces ? initialNamespaces[0] : namespace; + } else if (this._registry.isMultiNamespace(object.type)) { + savedObjectNamespaces = initialNamespaces || getSavedObjectNamespaces(namespace); + } + } + if (!hasSetNamespace) { + savedObjectNamespaces = + initialNamespaces || + // @ts-expect-error MultiGetHit._source is optional + getSavedObjectNamespaces(namespace, docFound ? actualResult : undefined); } - savedObjectNamespaces = - initialNamespaces || - // @ts-expect-error MultiGetHit._source is optional - getSavedObjectNamespaces(namespace, docFound ? actualResult : undefined); // @ts-expect-error MultiGetHit._source is optional versionProperties = getExpectedVersionProperties(version, actualResult); } else { @@ -438,12 +467,57 @@ export class SavedObjectsRepository { versionProperties = getExpectedVersionProperties(version); } + if (expectedBulkGetResult.value.method === 'create') { + savedObjectWorkspaces = options.workspaces; + } else { + const changeToCreate = () => { + finalMethod = 'create'; + finalObjectId = object.id; + savedObjectWorkspaces = options.workspaces; + versionProperties = {}; + }; + /** + * When overwrite, need to check if the object is workspace-specific + * if so, copy object to target workspace instead of refering it. + */ + const rawId = this._serializer.generateRawId(namespace, object.type, object.id); + const findObject = + bulkGetResponse?.statusCode !== 404 + ? bulkGetResponse?.body.docs?.find((item) => item._id === rawId) + : null; + if (findObject && findObject.found) { + const transformedObject = this._serializer.rawToSavedObject( + findObject as SavedObjectsRawDoc + ) as SavedObject; + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + options.workspaces, + transformedObject.workspaces + ); + /** + * We need to create a new object when the object + * is about to import into workspaces it is not belong to + */ + if (filteredWorkspaces.length) { + /** + * Create a new object but only belong to the set of (target workspaces - original workspace) + */ + changeToCreate(); + finalObjectId = uuid.v1(); + savedObjectWorkspaces = filteredWorkspaces; + } else { + savedObjectWorkspaces = transformedObject.workspaces; + } + } else { + savedObjectWorkspaces = options.workspaces; + } + } + const expectedResult = { opensearchRequestIndex: bulkRequestIndexCounter++, - requestedId: object.id, + requestedId: finalObjectId, rawMigratedDoc: this._serializer.savedObjectToRaw( this._migrator.migrateDocument({ - id: object.id, + id: finalObjectId, type: object.type, attributes: object.attributes, migrationVersion: object.migrationVersion, @@ -452,13 +526,14 @@ export class SavedObjectsRepository { updated_at: time, references: object.references || [], originId: object.originId, + workspaces: savedObjectWorkspaces, }) as SavedObjectSanitizedDoc ), }; bulkCreateParams.push( { - [method]: { + [finalMethod]: { _id: expectedResult.rawMigratedDoc._id, _index: this.getIndexForType(object.type), ...(overwrite && versionProperties), @@ -736,6 +811,7 @@ export class SavedObjectsRepository { typeToNamespacesMap, filter, preference, + workspaces, } = options; if (!type && !typeToNamespacesMap) { @@ -809,6 +885,7 @@ export class SavedObjectsRepository { typeToNamespacesMap, hasReference, kueryNode, + workspaces, }), }, }; @@ -976,7 +1053,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId, updated_at: updatedAt } = body._source; + const { originId, updated_at: updatedAt, workspaces } = body._source; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { @@ -991,6 +1068,7 @@ export class SavedObjectsRepository { namespaces, ...(originId && { originId }), ...(updatedAt && { updated_at: updatedAt }), + ...(workspaces && { workspaces }), version: encodeHitVersion(body), attributes: body._source[type], references: body._source.references || [], @@ -1055,7 +1133,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId } = body.get?._source ?? {}; + const { originId, workspaces } = body.get?._source ?? {}; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { namespaces = body.get?._source.namespaces ?? [ @@ -1070,6 +1148,7 @@ export class SavedObjectsRepository { version: encodeHitVersion(body), namespaces, ...(originId && { originId }), + ...(workspaces && { workspaces }), references, attributes, }; @@ -1452,12 +1531,13 @@ export class SavedObjectsRepository { }; } - const { originId } = get._source; + const { originId, workspaces } = get._source; return { id, type, ...(namespaces && { namespaces }), ...(originId && { originId }), + ...(workspaces && { workspaces }), updated_at, version: encodeVersion(seqNo, primaryTerm), attributes, @@ -1754,7 +1834,7 @@ function getSavedObjectFromSource( id: string, doc: { _seq_no?: number; _primary_term?: number; _source: SavedObjectsRawDocSource } ): SavedObject { - const { originId, updated_at: updatedAt } = doc._source; + const { originId, updated_at: updatedAt, workspaces } = doc._source; let namespaces: string[] = []; if (!registry.isNamespaceAgnostic(type)) { @@ -1769,6 +1849,7 @@ function getSavedObjectFromSource( namespaces, ...(originId && { originId }), ...(updatedAt && { updated_at: updatedAt }), + ...(workspaces && { workspaces }), version: encodeHitVersion(doc), attributes: doc._source[type], references: doc._source.references || [], diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 5bbb0a1fe24f..521149c4e646 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -127,6 +127,26 @@ function getClauseForType( }, }; } +/** + * Gets the clause that will filter for the workspace. + */ +function getClauseForWorkspace(workspace: string) { + if (workspace === '*') { + return { + bool: { + must: { + match_all: {}, + }, + }, + }; + } + + return { + bool: { + must: [{ term: { workspaces: workspace } }], + }, + }; +} interface HasReferenceQueryParams { type: string; @@ -144,6 +164,7 @@ interface QueryParams { defaultSearchOperator?: string; hasReference?: HasReferenceQueryParams; kueryNode?: KueryNode; + workspaces?: string[]; } export function getClauseForReference(reference: HasReferenceQueryParams) { @@ -200,6 +221,7 @@ export function getQueryParams({ defaultSearchOperator, hasReference, kueryNode, + workspaces, }: QueryParams) { const types = getTypes( registry, @@ -224,6 +246,17 @@ export function getQueryParams({ ], }; + if (workspaces) { + bool.filter.push({ + bool: { + should: workspaces.map((workspace) => { + return getClauseForWorkspace(workspace); + }), + minimum_should_match: 1, + }, + }); + } + if (search) { const useMatchPhrasePrefix = shouldUseMatchPhrasePrefix(search); const simpleQueryStringClause = getSimpleQueryStringClause({ diff --git a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts index 8b54141a4c3c..df6109eb9d0a 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts @@ -52,6 +52,7 @@ interface GetSearchDslOptions { id: string; }; kueryNode?: KueryNode; + workspaces?: string[]; } export function getSearchDsl( @@ -71,6 +72,7 @@ export function getSearchDsl( typeToNamespacesMap, hasReference, kueryNode, + workspaces, } = options; if (!type) { @@ -93,6 +95,7 @@ export function getSearchDsl( defaultSearchOperator, hasReference, kueryNode, + workspaces, }), ...getSortingParams(mappings, type, sortField, sortOrder), }; diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 4823e52d77c9..490c2b7083d2 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -80,4 +80,11 @@ export class SavedObjectsUtils { total: 0, saved_objects: [], }); + + public static filterWorkspacesAccordingToBaseWorkspaces( + targetWorkspaces?: string[], + baseWorkspaces?: string[] + ): string[] { + return targetWorkspaces?.filter((item) => !baseWorkspaces?.includes(item)) || []; + } } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 3e2553b8ce51..33862cb149fb 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -110,6 +110,7 @@ export interface SavedObjectsFindOptions { typeToNamespacesMap?: Map; /** An optional OpenSearch preference value to be used for the query **/ preference?: string; + workspaces?: string[]; } /** @@ -119,6 +120,7 @@ export interface SavedObjectsFindOptions { export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; + workspaces?: string[]; } /** diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index 81e1ed029ddc..47faffb0b922 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -113,6 +113,7 @@ export interface SavedObject { * space. */ originId?: string; + workspaces?: string[]; } export interface SavedObjectError { From c841390d4e471dd0b62592561bc32a26212ed15b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Sep 2023 13:52:21 +0800 Subject: [PATCH 02/43] feat: update snapshot Signed-off-by: SuZhou-Joe --- .../export/get_sorted_objects_for_export.test.ts | 6 ------ .../saved_objects/export/get_sorted_objects_for_export.ts | 7 +++++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index 952a74a76940..cf7e1d8246a7 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -128,7 +128,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -219,7 +218,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -370,7 +368,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -462,7 +459,6 @@ describe('getSortedObjectsForExport()', () => { index-pattern, search, ], - workspaces: undefined, }, ], ], @@ -670,7 +666,6 @@ describe('getSortedObjectsForExport()', () => { ], Object { namespace: undefined, - workspaces: undefined, }, ], ], @@ -789,7 +784,6 @@ describe('getSortedObjectsForExport()', () => { ], Object { namespace: undefined, - workspaces: undefined, }, ], Array [ diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 8ca085639f10..0336fc702973 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -109,7 +109,10 @@ async function fetchObjectsToExport({ if (typeof search === 'string') { throw Boom.badRequest(`Can't specify both "search" and "objects" properties when exporting`); } - const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace, workspaces }); + const bulkGetResult = await savedObjectsClient.bulkGet(objects, { + namespace, + ...(workspaces ? { workspaces } : {}), + }); const erroredObjects = bulkGetResult.saved_objects.filter((obj) => !!obj.error); if (erroredObjects.length) { const err = Boom.badRequest(); @@ -125,7 +128,7 @@ async function fetchObjectsToExport({ search, perPage: exportSizeLimit, namespaces: namespace ? [namespace] : undefined, - workspaces, + ...(workspaces ? { workspaces } : {}), }); if (findResponse.total > exportSizeLimit) { throw Boom.badRequest(`Can't export more than ${exportSizeLimit} objects`); From fb43df1d8a3d6af37b72b698c8b662531af73a65 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sat, 23 Sep 2023 18:14:18 +0800 Subject: [PATCH 03/43] feat: optimize logic when checkConflict and bulkCreate (#189) * feat: optimize logic when checkConflict and bulkCreate Signed-off-by: SuZhou-Joe * feat: add options.workspace check Signed-off-by: SuZhou-Joe * feat: throw error when workspace check error in repository create Signed-off-by: SuZhou-Joe * feat: modify judgement Signed-off-by: SuZhou-Joe * feat: always get objects from DB when create-with-override Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.ts | 71 ++++++++++--------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index a49f356bbb81..e8b410350d9e 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -285,8 +285,14 @@ export class SavedObjectsRepository { if (id && overwrite && workspaces) { try { const currentItem = await this.get(type, id); - if (currentItem && currentItem.workspaces) { - // do not overwrite workspaces + if ( + SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + workspaces, + currentItem.workspaces + ).length + ) { + throw SavedObjectsErrorHelpers.createConflictError(type, id); + } else { savedObjectWorkspaces = currentItem.workspaces; } } catch (e) { @@ -419,8 +425,6 @@ export class SavedObjectsRepository { method, } = expectedBulkGetResult.value; let savedObjectWorkspaces: string[] | undefined; - let finalMethod = method; - let finalObjectId = object.id; if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound @@ -467,19 +471,9 @@ export class SavedObjectsRepository { versionProperties = getExpectedVersionProperties(version); } - if (expectedBulkGetResult.value.method === 'create') { - savedObjectWorkspaces = options.workspaces; - } else { - const changeToCreate = () => { - finalMethod = 'create'; - finalObjectId = object.id; - savedObjectWorkspaces = options.workspaces; - versionProperties = {}; - }; - /** - * When overwrite, need to check if the object is workspace-specific - * if so, copy object to target workspace instead of refering it. - */ + savedObjectWorkspaces = options.workspaces; + + if (expectedBulkGetResult.value.method !== 'create') { const rawId = this._serializer.generateRawId(namespace, object.type, object.id); const findObject = bulkGetResponse?.statusCode !== 404 @@ -493,31 +487,31 @@ export class SavedObjectsRepository { options.workspaces, transformedObject.workspaces ); - /** - * We need to create a new object when the object - * is about to import into workspaces it is not belong to - */ if (filteredWorkspaces.length) { - /** - * Create a new object but only belong to the set of (target workspaces - original workspace) - */ - changeToCreate(); - finalObjectId = uuid.v1(); - savedObjectWorkspaces = filteredWorkspaces; + const { id, type } = object; + return { + tag: 'Left' as 'Left', + error: { + id, + type, + error: { + ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), + metadata: { isNotOverwritable: true }, + }, + }, + }; } else { savedObjectWorkspaces = transformedObject.workspaces; } - } else { - savedObjectWorkspaces = options.workspaces; } } const expectedResult = { opensearchRequestIndex: bulkRequestIndexCounter++, - requestedId: finalObjectId, + requestedId: object.id, rawMigratedDoc: this._serializer.savedObjectToRaw( this._migrator.migrateDocument({ - id: finalObjectId, + id: object.id, type: object.type, attributes: object.attributes, migrationVersion: object.migrationVersion, @@ -533,7 +527,7 @@ export class SavedObjectsRepository { bulkCreateParams.push( { - [finalMethod]: { + [method]: { _id: expectedResult.rawMigratedDoc._id, _index: this.getIndexForType(object.type), ...(overwrite && versionProperties), @@ -647,13 +641,24 @@ export class SavedObjectsRepository { const { type, id, opensearchRequestIndex } = expectedResult.value; const doc = bulkGetResponse?.body.docs[opensearchRequestIndex]; if (doc?.found) { + let workspaceConflict = false; + if (options.workspaces) { + const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc); + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + options.workspaces, + transformedObject.workspaces + ); + if (filteredWorkspaces.length) { + workspaceConflict = true; + } + } errors.push({ id, type, error: { ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), // @ts-expect-error MultiGetHit._source is optional - ...(!this.rawDocExistsInNamespace(doc!, namespace) && { + ...((!this.rawDocExistsInNamespace(doc!, namespace) || workspaceConflict) && { metadata: { isNotOverwritable: true }, }), }, From 7e184859312ccdc40dd6537451c40ed26df406de Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sat, 23 Sep 2023 18:33:40 +0800 Subject: [PATCH 04/43] feat: call get when create with override Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index e8b410350d9e..755ffb63de79 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -282,7 +282,7 @@ export class SavedObjectsRepository { let savedObjectWorkspaces = workspaces; - if (id && overwrite && workspaces) { + if (id && overwrite) { try { const currentItem = await this.get(type, id); if ( From aa69695351ba3366a25540396b3b4d2b8ae07f53 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 24 Sep 2023 17:20:00 +0800 Subject: [PATCH 05/43] feat: update test according to count Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.test.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index fb5d366dd454..c591ccccbf58 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -1846,9 +1846,17 @@ describe('SavedObjectsRepository', () => { const createSuccess = async (type, attributes, options) => { const result = await savedObjectsRepository.create(type, attributes, options); - expect(client.get).toHaveBeenCalledTimes( - registry.isMultiNamespace(type) && options.overwrite ? 1 : 0 - ); + let count = 0; + if (options?.overwrite && options?.id) { + /** + * workspace will call extra one to get latest status of current object + */ + count++; + } + if (registry.isMultiNamespace(type) && options.overwrite) { + count++; + } + expect(client.get).toHaveBeenCalledTimes(count); return result; }; From 674bd0983e62e36b281b6de52d9a65a5562ece1f Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 16:28:48 +0800 Subject: [PATCH 06/43] feat: add integration test Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.ts | 1 + .../routes/integration_tests/find.test.ts | 34 ++ .../lib/integration_tests/repository.test.ts | 303 ++++++++++++++++++ .../saved_objects/service/lib/repository.ts | 28 +- 4 files changed, 360 insertions(+), 6 deletions(-) create mode 100644 src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index 68104db85e6f..a2669bc49c9a 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -87,6 +87,7 @@ export async function importSavedObjectsFromStream({ savedObjectsClient, namespace, ignoreRegularConflicts: overwrite, + workspaces, }; const checkConflictsResult = await checkConflicts(checkConflictsParams); errorAccumulator = [...errorAccumulator, ...checkConflictsResult.errors]; diff --git a/src/core/server/saved_objects/routes/integration_tests/find.test.ts b/src/core/server/saved_objects/routes/integration_tests/find.test.ts index fc21eefed434..b21425386400 100644 --- a/src/core/server/saved_objects/routes/integration_tests/find.test.ts +++ b/src/core/server/saved_objects/routes/integration_tests/find.test.ts @@ -288,4 +288,38 @@ describe('GET /api/saved_objects/_find', () => { defaultSearchOperator: 'OR', }); }); + + it('accepts the query parameter workspaces as a string', async () => { + await supertest(httpSetup.server.listener) + .get('/api/saved_objects/_find?type=index-pattern&workspaces=foo') + .expect(200); + + expect(savedObjectsClient.find).toHaveBeenCalledTimes(1); + + const options = savedObjectsClient.find.mock.calls[0][0]; + expect(options).toEqual({ + defaultSearchOperator: 'OR', + perPage: 20, + page: 1, + type: ['index-pattern'], + workspaces: ['foo'], + }); + }); + + it('accepts the query parameter workspaces as an array', async () => { + await supertest(httpSetup.server.listener) + .get('/api/saved_objects/_find?type=index-pattern&workspaces=default&workspaces=foo') + .expect(200); + + expect(savedObjectsClient.find).toHaveBeenCalledTimes(1); + + const options = savedObjectsClient.find.mock.calls[0][0]; + expect(options).toEqual({ + perPage: 20, + page: 1, + type: ['index-pattern'], + workspaces: ['default', 'foo'], + defaultSearchOperator: 'OR', + }); + }); }); diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts new file mode 100644 index 000000000000..50c75e43fdf0 --- /dev/null +++ b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts @@ -0,0 +1,303 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { SavedObject } from 'src/core/types'; +import { isEqual } from 'lodash'; +import * as osdTestServer from '../../../../../test_helpers/osd_server'; +import { Readable } from 'stream'; + +const dashboard: Omit = { + type: 'dashboard', + attributes: {}, + references: [], +}; + +describe('repository integration test', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + beforeAll(async () => { + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + }); + opensearchServer = await startOpenSearch(); + const startOSDResp = await startOpenSearchDashboards(); + root = startOSDResp.root; + }, 30000); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + + const deleteItem = async (object: Pick) => { + await osdTestServer.request + .delete(root, `/api/saved_objects/${object.type}/${object.id}`) + .expect(200); + }; + + const getItem = async (object: Pick) => { + return await osdTestServer.request + .get(root, `/api/saved_objects/${object.type}/${object.id}`) + .expect(200); + }; + + describe('workspace related CRUD', () => { + it('create', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + expect(createResult.body.workspaces).toEqual(['foo']); + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('create-with-override', async () => { + const createResult = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + workspaces: ['foo'], + }) + .expect(200); + + /** + * Override without workspace, in this case workspaces field should be retained. + */ + const correctOverride = await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) + .send({ + attributes: { + title: 'foo', + }, + }) + .expect(200); + + expect(correctOverride.body.workspaces).toEqual(['foo']); + expect(correctOverride.body.attributes.title).toEqual('foo'); + + await osdTestServer.request + .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) + .send({ + attributes: dashboard.attributes, + workspaces: ['bar'], + }) + .expect(409); + + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('bulk create', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + expect((createResultFoo.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultFoo.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['foo']) + ) + ).toEqual(true); + expect((createResultBar.body.saved_objects as any[]).some((item) => item.error)).toEqual( + false + ); + expect( + (createResultBar.body.saved_objects as any[]).every((item) => + isEqual(item.workspaces, ['bar']) + ) + ).toEqual(true); + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('bulk create with conflict', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + /** + * overwrite without workspaces + */ + const overwriteWithoutWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?overwrite=true`) + .send([ + { + ...dashboard, + id: 'bar', + }, + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const getFooResult = await getItem({ + type: dashboard.type, + id: 'foo', + }); + + const getBarResult = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + expect(getFooResult.body.workspaces).toEqual(['foo']); + expect(getBarResult.body.workspaces).toEqual(['bar']); + expect( + (overwriteWithoutWorkspacesResult.body.saved_objects as any[]).some((item) => item.error) + ).toEqual(false); + + /** + * overwrite with workspaces + */ + const overwriteWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?overwrite=true&workspaces=foo`) + .send([ + { + ...dashboard, + id: 'bar', + }, + { + ...dashboard, + id: 'foo', + attributes: { + title: 'foo', + }, + }, + ]) + .expect(200); + + expect(overwriteWithWorkspacesResult.body.saved_objects[0].error.statusCode).toEqual(409); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].attributes.title).toEqual('foo'); + expect(overwriteWithWorkspacesResult.body.saved_objects[1].workspaces).toEqual(['foo']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('checkConflicts when importing ndjson', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const getResultFoo = await getItem({ + type: dashboard.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + const readableStream = new Readable(); + readableStream.push( + `Content-Disposition: form-data; name="file"; filename="tmp.ndjson"\r\n\r\n` + ); + readableStream.push( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n') + ); + readableStream.push(null); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post(root, `/api/saved_objects/_import?workspaces=foo&overwrite=false`) + .attach( + 'file', + Buffer.from( + [JSON.stringify(getResultFoo.body), JSON.stringify(getResultBar.body)].join('\n'), + 'utf-8' + ), + 'tmp.ndjson' + ) + .expect(200); + + expect(importWithWorkspacesResult.body.success).toEqual(false); + expect(importWithWorkspacesResult.body.errors.length).toEqual(1); + expect(importWithWorkspacesResult.body.errors[0].id).toEqual('foo'); + expect(importWithWorkspacesResult.body.errors[0].error.type).toEqual('conflict'); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + }); +}); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 755ffb63de79..05f203a0f079 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -283,8 +283,13 @@ export class SavedObjectsRepository { let savedObjectWorkspaces = workspaces; if (id && overwrite) { + let currentItem; try { - const currentItem = await this.get(type, id); + currentItem = await this.get(type, id); + } catch (e) { + // this.get will throw an error when no items can be found + } + if (currentItem) { if ( SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( workspaces, @@ -295,8 +300,6 @@ export class SavedObjectsRepository { } else { savedObjectWorkspaces = currentItem.workspaces; } - } catch (e) { - // this.get will throw an error when no items can be found } } @@ -377,15 +380,28 @@ export class SavedObjectsRepository { const method = object.id && overwrite ? 'index' : 'create'; const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type); + /** + * Only when importing an object to a target workspace should we check if the object is workspace-specific. + */ + const requiresWorkspaceCheck = object.id; if (object.id == null) object.id = uuid.v1(); + let opensearchRequestIndexPayload = {}; + + if (requiresNamespacesCheck || requiresWorkspaceCheck) { + opensearchRequestIndexPayload = { + opensearchRequestIndex: bulkGetRequestIndexCounter, + }; + bulkGetRequestIndexCounter++; + } + return { tag: 'Right' as 'Right', value: { method, object, - ...(requiresNamespacesCheck && { opensearchRequestIndex: bulkGetRequestIndexCounter++ }), + ...opensearchRequestIndexPayload, }, }; }); @@ -396,7 +412,7 @@ export class SavedObjectsRepository { .map(({ value: { object: { type, id } } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces'], + _source: ['type', 'namespaces', 'workspaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( @@ -618,7 +634,7 @@ export class SavedObjectsRepository { const bulkGetDocs = expectedBulkGetResults.filter(isRight).map(({ value: { type, id } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces'], + _source: ['type', 'namespaces', 'workspaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( From 2fb66b781679c803f02eb2cc12a7788d9ad0e983 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 17:28:07 +0800 Subject: [PATCH 07/43] fix: unit test Signed-off-by: SuZhou-Joe --- .../lib/integration_tests/repository.test.ts | 38 +++++++++++++++++++ .../service/lib/repository.test.js | 31 ++++++++++++--- .../saved_objects/service/lib/repository.ts | 6 +-- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts index 50c75e43fdf0..42b0b031006d 100644 --- a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts @@ -299,5 +299,43 @@ describe('repository integration test', () => { ) ); }); + + it('find by workspaces', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/api/saved_objects/_bulk_create?workspaces=bar`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const findResult = await osdTestServer.request + .get(root, `/api/saved_objects/_find?workspaces=bar&type=${dashboard.type}`) + .expect(200); + + expect(findResult.body.total).toEqual(1); + expect(findResult.body.saved_objects[0].workspaces).toEqual(['bar']); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); }); }); diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index c591ccccbf58..9f0c0aed23b1 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -168,7 +168,7 @@ describe('SavedObjectsRepository', () => { }); const getMockGetResponse = ( - { type, id, references, namespace: objectNamespace, originId }, + { type, id, references, namespace: objectNamespace, originId, workspaces }, namespace ) => { const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; @@ -182,6 +182,7 @@ describe('SavedObjectsRepository', () => { _source: { ...(registry.isSingleNamespace(type) && { namespace: namespaceId }), ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), + workspaces, ...(originId && { originId }), type, [type]: { title: 'Testing' }, @@ -466,6 +467,7 @@ describe('SavedObjectsRepository', () => { }; const bulkCreateSuccess = async (objects, options) => { + const originalObjects = JSON.parse(JSON.stringify(objects)); const multiNamespaceObjects = objects.filter( ({ type, id }) => registry.isMultiNamespace(type) && id ); @@ -480,7 +482,9 @@ describe('SavedObjectsRepository', () => { opensearchClientMock.createSuccessTransportRequestPromise(response) ); const result = await savedObjectsRepository.bulkCreate(objects, options); - expect(client.mget).toHaveBeenCalledTimes(multiNamespaceObjects?.length ? 1 : 0); + expect(client.mget).toHaveBeenCalledTimes( + multiNamespaceObjects?.length || originalObjects?.some((item) => item.id) ? 1 : 0 + ); return result; }; @@ -538,7 +542,10 @@ describe('SavedObjectsRepository', () => { await bulkCreateSuccess(objects); expect(client.bulk).toHaveBeenCalledTimes(1); expect(client.mget).toHaveBeenCalledTimes(1); - const docs = [expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` })]; + const docs = [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` }), + ]; expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); @@ -683,6 +690,7 @@ describe('SavedObjectsRepository', () => { expect.anything() ); client.bulk.mockClear(); + client.mget.mockClear(); }; await test(undefined); await test(namespace); @@ -815,6 +823,12 @@ describe('SavedObjectsRepository', () => { const response1 = { status: 200, docs: [ + { + found: true, + _source: { + type: obj1.type, + }, + }, { found: true, _source: { @@ -837,7 +851,13 @@ describe('SavedObjectsRepository', () => { expect(client.bulk).toHaveBeenCalled(); expect(client.mget).toHaveBeenCalled(); - const body1 = { docs: [expect.objectContaining({ _id: `${obj.type}:${obj.id}` })] }; + const body1 = { + docs: [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ], + }; expect(client.mget).toHaveBeenCalledWith( expect.objectContaining({ body: body1 }), expect.anything() @@ -2194,10 +2214,11 @@ describe('SavedObjectsRepository', () => { const type = 'index-pattern'; const id = 'logstash-*'; const namespace = 'foo-namespace'; + const workspaces = ['bar-workspace']; const deleteSuccess = async (type, id, options) => { if (registry.isMultiNamespace(type)) { - const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace); + const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace, workspaces); client.get.mockResolvedValueOnce( opensearchClientMock.createSuccessTransportRequestPromise(mockGetResponse) ); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 05f203a0f079..e9f6821254d5 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -444,7 +444,7 @@ export class SavedObjectsRepository { if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound - ? bulkGetResponse?.body.docs[opensearchRequestIndex] + ? bulkGetResponse?.body.docs?.[opensearchRequestIndex] : undefined; const docFound = indexFound && actualResult?.found === true; let hasSetNamespace = false; @@ -960,7 +960,7 @@ export class SavedObjectsRepository { */ async bulkGet( objects: SavedObjectsBulkGetObject[] = [], - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { const namespace = normalizeNamespace(options.namespace); @@ -1048,7 +1048,7 @@ export class SavedObjectsRepository { async get( type: string, id: string, - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { if (!this._allowedTypes.includes(type)) { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); From 952f13e9b120b0ab120bde5fb412643e91295965 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Sep 2023 17:51:18 +0800 Subject: [PATCH 08/43] feat: regenerate ids when import Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.test.ts | 12 +++++- .../import/import_saved_objects.ts | 11 ++++- .../saved_objects/import/regenerate_ids.ts | 40 ++++++++++++++++++- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index de8fb34dfbed..4bf1717e895c 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -42,7 +42,7 @@ import { typeRegistryMock } from '../saved_objects_type_registry.mock'; import { importSavedObjectsFromStream } from './import_saved_objects'; import { collectSavedObjects } from './collect_saved_objects'; -import { regenerateIds } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; import { validateReferences } from './validate_references'; import { checkConflicts } from './check_conflicts'; import { checkOriginConflicts } from './check_origin_conflicts'; @@ -68,6 +68,7 @@ describe('#importSavedObjectsFromStream', () => { importIdMap: new Map(), }); getMockFn(regenerateIds).mockReturnValue(new Map()); + getMockFn(regenerateIdsWithReference).mockReturnValue(Promise.resolve(new Map())); getMockFn(validateReferences).mockResolvedValue([]); getMockFn(checkConflicts).mockResolvedValue({ errors: [], @@ -240,6 +241,15 @@ describe('#importSavedObjectsFromStream', () => { ]), }); getMockFn(validateReferences).mockResolvedValue([errors[1]]); + getMockFn(regenerateIdsWithReference).mockResolvedValue( + Promise.resolve( + new Map([ + ['foo', {}], + ['bar', {}], + ['baz', {}], + ]) + ) + ); getMockFn(checkConflicts).mockResolvedValue({ errors: [errors[2]], filteredObjects, diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index a2669bc49c9a..fce50bab7abc 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -38,7 +38,7 @@ import { validateReferences } from './validate_references'; import { checkOriginConflicts } from './check_origin_conflicts'; import { createSavedObjects } from './create_saved_objects'; import { checkConflicts } from './check_conflicts'; -import { regenerateIds } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; /** * Import saved objects from given stream. See the {@link SavedObjectsImportOptions | options} for more @@ -81,6 +81,13 @@ export async function importSavedObjectsFromStream({ if (createNewCopies) { importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects); } else { + importIdMap = await regenerateIdsWithReference({ + savedObjects: collectSavedObjectsResult.collectedObjects, + savedObjectsClient, + workspaces, + objectLimit, + importIdMap, + }); // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces const checkConflictsParams = { objects: collectSavedObjectsResult.collectedObjects, @@ -120,7 +127,7 @@ export async function importSavedObjectsFromStream({ importIdMap, overwrite, namespace, - workspaces, + ...(workspaces ? { workspaces } : {}), }; const createSavedObjectsResult = await createSavedObjects(createSavedObjectsParams); errorAccumulator = [...errorAccumulator, ...createSavedObjectsResult.errors]; diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index 672a8f030620..27b10e575f44 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -29,7 +29,8 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { SavedObject } from '../types'; +import { SavedObject, SavedObjectsClientContract } from '../types'; +import { SavedObjectsUtils } from '../service'; /** * Takes an array of saved objects and returns an importIdMap of randomly-generated new IDs. @@ -42,3 +43,40 @@ export const regenerateIds = (objects: SavedObject[]) => { }, new Map()); return importIdMap; }; + +export const regenerateIdsWithReference = async (props: { + savedObjects: SavedObject[]; + savedObjectsClient: SavedObjectsClientContract; + workspaces?: string[]; + objectLimit: number; + importIdMap: Map; +}): Promise> => { + const { savedObjects, savedObjectsClient, workspaces, importIdMap } = props; + if (!workspaces || !workspaces.length) { + return savedObjects.reduce((acc, object) => { + return acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false }); + }, importIdMap); + } + + const bulkGetResult = await savedObjectsClient.bulkGet( + savedObjects.map((item) => ({ type: item.type, id: item.id })) + ); + + return bulkGetResult.saved_objects.reduce((acc, object) => { + if (object.error?.statusCode === 404) { + acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: true }); + return acc; + } + + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + workspaces, + object.workspaces + ); + if (filteredWorkspaces.length) { + acc.set(`${object.type}:${object.id}`, { id: uuidv4(), omitOriginId: true }); + } else { + acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false }); + } + return acc; + }, importIdMap); +}; From 23481a17ddb26c03da7d4f76b387eca0184d409c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 14:21:20 +0800 Subject: [PATCH 09/43] feat: add more unit test Signed-off-by: SuZhou-Joe --- .../service/lib/repository.test.js | 166 +++++++++++++++++- .../lib/search_dsl/query_params.test.ts | 21 +++ 2 files changed, 186 insertions(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 9f0c0aed23b1..74802561d45b 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -27,7 +27,7 @@ * specific language governing permissions and limitations * under the License. */ - +import { omit } from 'lodash'; import { SavedObjectsRepository } from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; @@ -445,6 +445,7 @@ describe('SavedObjectsRepository', () => { references: [{ name: 'ref_0', type: 'test', id: '2' }], }; const namespace = 'foo-namespace'; + const workspace = 'foo-workspace'; const getMockBulkCreateResponse = (objects, namespace) => { return { @@ -549,6 +550,18 @@ describe('SavedObjectsRepository', () => { expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); + it(`should use the OpenSearch mget action before bulk action for any types, when id is defined`, async () => { + const objects = [obj1, obj2]; + await bulkCreateSuccess(objects); + expect(client.bulk).toHaveBeenCalledTimes(1); + expect(client.mget).toHaveBeenCalledTimes(1); + const docs = [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ]; + expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); + }); + it(`should use the OpenSearch create method if ID is undefined and overwrite=true`, async () => { const objects = [obj1, obj2].map((obj) => ({ ...obj, id: undefined })); await bulkCreateSuccess(objects, { overwrite: true }); @@ -738,6 +751,16 @@ describe('SavedObjectsRepository', () => { await bulkCreateSuccess(objects, { namespace }); expectClientCallArgsAction(objects, { method: 'create', getId }); }); + + it(`adds workspaces to request body for any types`, async () => { + await bulkCreateSuccess([obj1, obj2], { workspaces: [workspace] }); + const expected = expect.objectContaining({ workspaces: [workspace] }); + const body = [expect.any(Object), expected, expect.any(Object), expected]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body }), + expect.anything() + ); + }); }); describe('errors', () => { @@ -897,6 +920,74 @@ describe('SavedObjectsRepository', () => { const expectedError = expectErrorResult(obj3, { message: JSON.stringify(opensearchError) }); await bulkCreateError(obj3, opensearchError, expectedError); }); + + it(`returns error when there is a conflict with an existing saved object according to workspaces`, async () => { + const obj = { ...obj3, workspaces: ['foo'] }; + const response1 = { + status: 200, + docs: [ + { + found: true, + _id: `${obj1.type}:${obj1.id}`, + _source: { + type: obj1.type, + workspaces: ['bar'], + }, + }, + { + found: true, + _id: `${obj.type}:${obj.id}`, + _source: { + type: obj.type, + workspaces: obj.workspaces, + }, + }, + { + found: true, + _id: `${obj2.type}:${obj2.id}`, + _source: { + type: obj2.type, + }, + }, + ], + }; + client.mget.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response1) + ); + const response2 = getMockBulkCreateResponse([obj1, obj, obj2]); + client.bulk.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response2) + ); + + const options = { overwrite: true, workspaces: ['bar'] }; + const result = await savedObjectsRepository.bulkCreate([obj1, obj, obj2], options); + expect(client.bulk).toHaveBeenCalled(); + expect(client.mget).toHaveBeenCalled(); + + const body1 = { + docs: [ + expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), + expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), + expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), + ], + }; + expect(client.mget).toHaveBeenCalledWith( + expect.objectContaining({ body: body1 }), + expect.anything() + ); + const body2 = [...expectObjArgs(obj1)]; + expect(client.bulk).toHaveBeenCalledWith( + expect.objectContaining({ body: body2 }), + expect.anything() + ); + expect(result).toEqual({ + saved_objects: [ + expectSuccess(obj1), + expectErrorConflict(obj, { metadata: { isNotOverwritable: true } }), + expectErrorConflict(obj2, { metadata: { isNotOverwritable: true } }), + ], + }); + }); }); describe('migration', () => { @@ -1719,6 +1810,8 @@ describe('SavedObjectsRepository', () => { const obj5 = { type: MULTI_NAMESPACE_TYPE, id: 'five' }; const obj6 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'six' }; const obj7 = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'seven' }; + const obj8 = { type: 'dashboard', id: 'eight', workspaces: ['foo'] }; + const obj9 = { type: 'dashboard', id: 'nine', workspaces: ['bar'] }; const namespace = 'foo-namespace'; const checkConflicts = async (objects, options) => @@ -1810,6 +1903,8 @@ describe('SavedObjectsRepository', () => { { found: false }, getMockGetResponse(obj6), { found: false }, + getMockGetResponse(obj7), + getMockGetResponse(obj8), ], }; client.mget.mockResolvedValue( @@ -1838,6 +1933,36 @@ describe('SavedObjectsRepository', () => { ], }); }); + + it(`expected results with workspaces`, async () => { + const objects = [obj8, obj9]; + const response = { + status: 200, + docs: [getMockGetResponse(obj8), getMockGetResponse(obj9)], + }; + client.mget.mockResolvedValue( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + + const result = await checkConflicts(objects, { + workspaces: ['foo'], + }); + expect(client.mget).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + errors: [ + { ...omit(obj8, 'workspaces'), error: createConflictError(obj8.type, obj8.id) }, + { + ...omit(obj9, 'workspaces'), + error: { + ...createConflictError(obj9.type, obj9.id), + metadata: { + isNotOverwritable: true, + }, + }, + }, + ], + }); + }); }); }); @@ -1894,6 +2019,7 @@ describe('SavedObjectsRepository', () => { it(`should use the OpenSearch index action if ID is defined and overwrite=true`, async () => { await createSuccess(type, attributes, { id, overwrite: true }); expect(client.index).toHaveBeenCalled(); + expect(client.get).toHaveBeenCalled(); }); it(`should use the OpenSearch index with version if ID and version are defined and overwrite=true`, async () => { @@ -2068,6 +2194,29 @@ describe('SavedObjectsRepository', () => { expect.anything() ); }); + + it(`doesn't modify workspaces when overwrite without target workspaces`, async () => { + const response = getMockGetResponse({ workspaces: ['foo'], id }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + + await savedObjectsRepository.create('dashboard', attributes, { + id, + overwrite: true, + workspaces: [], + }); + + expect(client.index).toHaveBeenCalledWith( + expect.objectContaining({ + id: `dashboard:${id}`, + body: expect.objectContaining({ + workspaces: ['foo'], + }), + }), + expect.anything() + ); + }); }); describe('errors', () => { @@ -2128,6 +2277,21 @@ describe('SavedObjectsRepository', () => { expect(client.get).toHaveBeenCalled(); }); + it(`throws error when there is a conflict with an existing workspaces saved object`, async () => { + const response = getMockGetResponse({ workspaces: ['foo'], id }); + client.get.mockResolvedValueOnce( + opensearchClientMock.createSuccessTransportRequestPromise(response) + ); + await expect( + savedObjectsRepository.create('dashboard', attributes, { + id, + overwrite: true, + workspaces: ['bar'], + }) + ).rejects.toThrowError(createConflictError('dashboard', id)); + expect(client.get).toHaveBeenCalled(); + }); + it.todo(`throws when automatic index creation fails`); it.todo(`throws when an unexpected failure occurs`); diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index 518e2ff56d0e..a47bc27fcd92 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -625,6 +625,27 @@ describe('#getQueryParams', () => { ]); }); }); + + describe('when using workspace search', () => { + it('using normal workspaces', () => { + const result: Result = getQueryParams({ + registry, + workspaces: ['foo'], + }); + expect(result.query.bool.filter[1]).toEqual({ + bool: { + should: [ + { + bool: { + must: [{ term: { workspaces: 'foo' } }], + }, + }, + ], + minimum_should_match: 1, + }, + }); + }); + }); }); describe('namespaces property', () => { From e740165522454e51fdb34300d02f716f13e5e85d Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 15:20:49 +0800 Subject: [PATCH 10/43] feat: minor changes logic on repository Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index e9f6821254d5..a4dcd53b9cdb 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -282,7 +282,7 @@ export class SavedObjectsRepository { let savedObjectWorkspaces = workspaces; - if (id && overwrite) { + if (id && overwrite && workspaces) { let currentItem; try { currentItem = await this.get(type, id); @@ -383,7 +383,7 @@ export class SavedObjectsRepository { /** * Only when importing an object to a target workspace should we check if the object is workspace-specific. */ - const requiresWorkspaceCheck = object.id; + const requiresWorkspaceCheck = object.id && options.workspaces; if (object.id == null) object.id = uuid.v1(); From 1997c730ce4f2d540146ac547aeb731fa7ec24d0 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 17:39:50 +0800 Subject: [PATCH 11/43] feat: update unit test Signed-off-by: SuZhou-Joe --- .../service/lib/repository.test.js | 37 ++----------------- 1 file changed, 4 insertions(+), 33 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 74802561d45b..fc80e62e1b36 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -468,7 +468,6 @@ describe('SavedObjectsRepository', () => { }; const bulkCreateSuccess = async (objects, options) => { - const originalObjects = JSON.parse(JSON.stringify(objects)); const multiNamespaceObjects = objects.filter( ({ type, id }) => registry.isMultiNamespace(type) && id ); @@ -484,7 +483,7 @@ describe('SavedObjectsRepository', () => { ); const result = await savedObjectsRepository.bulkCreate(objects, options); expect(client.mget).toHaveBeenCalledTimes( - multiNamespaceObjects?.length || originalObjects?.some((item) => item.id) ? 1 : 0 + multiNamespaceObjects?.length || options?.workspaces ? 1 : 0 ); return result; }; @@ -543,22 +542,7 @@ describe('SavedObjectsRepository', () => { await bulkCreateSuccess(objects); expect(client.bulk).toHaveBeenCalledTimes(1); expect(client.mget).toHaveBeenCalledTimes(1); - const docs = [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` }), - ]; - expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); - }); - - it(`should use the OpenSearch mget action before bulk action for any types, when id is defined`, async () => { - const objects = [obj1, obj2]; - await bulkCreateSuccess(objects); - expect(client.bulk).toHaveBeenCalledTimes(1); - expect(client.mget).toHaveBeenCalledTimes(1); - const docs = [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), - ]; + const docs = [expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` })]; expect(client.mget.mock.calls[0][0].body).toEqual({ docs }); }); @@ -846,12 +830,6 @@ describe('SavedObjectsRepository', () => { const response1 = { status: 200, docs: [ - { - found: true, - _source: { - type: obj1.type, - }, - }, { found: true, _source: { @@ -874,13 +852,7 @@ describe('SavedObjectsRepository', () => { expect(client.bulk).toHaveBeenCalled(); expect(client.mget).toHaveBeenCalled(); - const body1 = { - docs: [ - expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }), - expect.objectContaining({ _id: `${obj.type}:${obj.id}` }), - expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }), - ], - }; + const body1 = { docs: [expect.objectContaining({ _id: `${obj.type}:${obj.id}` })] }; expect(client.mget).toHaveBeenCalledWith( expect.objectContaining({ body: body1 }), expect.anything() @@ -1992,7 +1964,7 @@ describe('SavedObjectsRepository', () => { const createSuccess = async (type, attributes, options) => { const result = await savedObjectsRepository.create(type, attributes, options); let count = 0; - if (options?.overwrite && options?.id) { + if (options?.overwrite && options.id && options.workspaces) { /** * workspace will call extra one to get latest status of current object */ @@ -2019,7 +1991,6 @@ describe('SavedObjectsRepository', () => { it(`should use the OpenSearch index action if ID is defined and overwrite=true`, async () => { await createSuccess(type, attributes, { id, overwrite: true }); expect(client.index).toHaveBeenCalled(); - expect(client.get).toHaveBeenCalled(); }); it(`should use the OpenSearch index with version if ID and version are defined and overwrite=true`, async () => { From cae196e1be25139b4370beeb547af36be4f7d5e5 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 26 Sep 2023 19:11:21 +0800 Subject: [PATCH 12/43] feat: update test Signed-off-by: SuZhou-Joe --- .../lib/integration_tests/repository.test.ts | 71 ++++++------------- 1 file changed, 20 insertions(+), 51 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts index 42b0b031006d..b601de985dc0 100644 --- a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts +++ b/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts @@ -30,9 +30,12 @@ describe('repository integration test', () => { }); const deleteItem = async (object: Pick) => { - await osdTestServer.request - .delete(root, `/api/saved_objects/${object.type}/${object.id}`) - .expect(200); + expect( + [200, 404].includes( + (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`)) + .statusCode + ) + ); }; const getItem = async (object: Pick) => { @@ -41,6 +44,17 @@ describe('repository integration test', () => { .expect(200); }; + const clearFooAndBar = async () => { + await deleteItem({ + type: dashboard.type, + id: 'foo', + }); + await deleteItem({ + type: dashboard.type, + id: 'bar', + }); + }; + describe('workspace related CRUD', () => { it('create', async () => { const createResult = await osdTestServer.request @@ -67,21 +81,6 @@ describe('repository integration test', () => { }) .expect(200); - /** - * Override without workspace, in this case workspaces field should be retained. - */ - const correctOverride = await osdTestServer.request - .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) - .send({ - attributes: { - title: 'foo', - }, - }) - .expect(200); - - expect(correctOverride.body.workspaces).toEqual(['foo']); - expect(correctOverride.body.attributes.title).toEqual('foo'); - await osdTestServer.request .post(root, `/api/saved_objects/${dashboard.type}/${createResult.body.id}?overwrite=true`) .send({ @@ -97,6 +96,7 @@ describe('repository integration test', () => { }); it('bulk create', async () => { + await clearFooAndBar(); const createResultFoo = await osdTestServer.request .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) .send([ @@ -144,6 +144,7 @@ describe('repository integration test', () => { }); it('bulk create with conflict', async () => { + await clearFooAndBar(); const createResultFoo = await osdTestServer.request .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) .send([ @@ -164,39 +165,6 @@ describe('repository integration test', () => { ]) .expect(200); - /** - * overwrite without workspaces - */ - const overwriteWithoutWorkspacesResult = await osdTestServer.request - .post(root, `/api/saved_objects/_bulk_create?overwrite=true`) - .send([ - { - ...dashboard, - id: 'bar', - }, - { - ...dashboard, - id: 'foo', - }, - ]) - .expect(200); - - const getFooResult = await getItem({ - type: dashboard.type, - id: 'foo', - }); - - const getBarResult = await getItem({ - type: dashboard.type, - id: 'bar', - }); - - expect(getFooResult.body.workspaces).toEqual(['foo']); - expect(getBarResult.body.workspaces).toEqual(['bar']); - expect( - (overwriteWithoutWorkspacesResult.body.saved_objects as any[]).some((item) => item.error) - ).toEqual(false); - /** * overwrite with workspaces */ @@ -232,6 +200,7 @@ describe('repository integration test', () => { }); it('checkConflicts when importing ndjson', async () => { + await clearFooAndBar(); const createResultFoo = await osdTestServer.request .post(root, `/api/saved_objects/_bulk_create?workspaces=foo`) .send([ From fd24685b3b3021be354cd2cdb7e05e028c3d0832 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 17:14:59 +0800 Subject: [PATCH 13/43] feat: optimization according to comments Signed-off-by: SuZhou-Joe --- .../export/get_sorted_objects_for_export.ts | 1 - .../import/create_saved_objects.ts | 2 +- .../import/import_saved_objects.ts | 16 +++++---- .../saved_objects/import/regenerate_ids.ts | 7 +--- .../service/lib/repository.test.js | 11 ++++-- .../saved_objects/service/lib/repository.ts | 34 +++++++++---------- .../service/lib/search_dsl/query_params.ts | 20 +++++------ .../server/saved_objects/service/lib/utils.ts | 4 +-- .../service/saved_objects_client.ts | 2 +- src/core/server/saved_objects/types.ts | 2 ++ src/core/types/saved_objects.ts | 1 + 11 files changed, 49 insertions(+), 51 deletions(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts index 0336fc702973..660f86846137 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.ts @@ -111,7 +111,6 @@ async function fetchObjectsToExport({ } const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace, - ...(workspaces ? { workspaces } : {}), }); const erroredObjects = bulkGetResult.saved_objects.filter((obj) => !!obj.error); if (erroredObjects.length) { diff --git a/src/core/server/saved_objects/import/create_saved_objects.ts b/src/core/server/saved_objects/import/create_saved_objects.ts index b67cffce1e96..6982f21bbf37 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -113,7 +113,7 @@ export const createSavedObjects = async ({ // remap results to reflect the object IDs that were submitted for import // this ensures that consumers understand the results const remappedResults = expectedResults.map>((result) => { - const { id } = objectIdMap.get(`${result.type}:${result.id}`) || ({} as SavedObject); + const { id } = objectIdMap.get(`${result.type}:${result.id}`)!; // also, include a `destinationId` field if the object create attempt was made with a different ID return { ...result, id, ...(id !== result.id && { destinationId: result.id }) }; }); diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index fce50bab7abc..2d87f94b97e3 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -81,13 +81,15 @@ export async function importSavedObjectsFromStream({ if (createNewCopies) { importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects); } else { - importIdMap = await regenerateIdsWithReference({ - savedObjects: collectSavedObjectsResult.collectedObjects, - savedObjectsClient, - workspaces, - objectLimit, - importIdMap, - }); + if (workspaces) { + importIdMap = await regenerateIdsWithReference({ + savedObjects: collectSavedObjectsResult.collectedObjects, + savedObjectsClient, + workspaces, + objectLimit, + importIdMap, + }); + } // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces const checkConflictsParams = { objects: collectSavedObjectsResult.collectedObjects, diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index 27b10e575f44..b309753566e3 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -47,16 +47,11 @@ export const regenerateIds = (objects: SavedObject[]) => { export const regenerateIdsWithReference = async (props: { savedObjects: SavedObject[]; savedObjectsClient: SavedObjectsClientContract; - workspaces?: string[]; + workspaces: string[]; objectLimit: number; importIdMap: Map; }): Promise> => { const { savedObjects, savedObjectsClient, workspaces, importIdMap } = props; - if (!workspaces || !workspaces.length) { - return savedObjects.reduce((acc, object) => { - return acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false }); - }, importIdMap); - } const bulkGetResult = await savedObjectsClient.bulkGet( savedObjects.map((item) => ({ type: item.type, id: item.id })) diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index fc80e62e1b36..7bb22474ee76 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -27,7 +27,6 @@ * specific language governing permissions and limitations * under the License. */ -import { omit } from 'lodash'; import { SavedObjectsRepository } from './repository'; import * as getSearchDslNS from './search_dsl/search_dsl'; import { SavedObjectsErrorHelpers } from './errors'; @@ -54,6 +53,12 @@ const createGenericNotFoundError = (...args) => const createUnsupportedTypeError = (...args) => SavedObjectsErrorHelpers.createUnsupportedTypeError(...args).output.payload; +const omitWorkspace = (object) => { + const newObject = JSON.parse(JSON.stringify(object)); + delete newObject.workspaces; + return newObject; +}; + describe('SavedObjectsRepository', () => { let client; let savedObjectsRepository; @@ -1922,9 +1927,9 @@ describe('SavedObjectsRepository', () => { expect(client.mget).toHaveBeenCalledTimes(1); expect(result).toEqual({ errors: [ - { ...omit(obj8, 'workspaces'), error: createConflictError(obj8.type, obj8.id) }, + { ...omitWorkspace(obj8), error: createConflictError(obj8.type, obj8.id) }, { - ...omit(obj9, 'workspaces'), + ...omitWorkspace(obj9), error: { ...createConflictError(obj9.type, obj9.id), metadata: { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index a4dcd53b9cdb..cad4763ceaab 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -381,9 +381,9 @@ export class SavedObjectsRepository { const method = object.id && overwrite ? 'index' : 'create'; const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type); /** - * Only when importing an object to a target workspace should we check if the object is workspace-specific. + * It requires a check when overwriting objects to target workspaces */ - const requiresWorkspaceCheck = object.id && options.workspaces; + const requiresWorkspaceCheck = !!(object.id && overwrite && options.workspaces); if (object.id == null) object.id = uuid.v1(); @@ -440,14 +440,12 @@ export class SavedObjectsRepository { object: { initialNamespaces, version, ...object }, method, } = expectedBulkGetResult.value; - let savedObjectWorkspaces: string[] | undefined; if (opensearchRequestIndex !== undefined) { const indexFound = bulkGetResponse?.statusCode !== 404; const actualResult = indexFound ? bulkGetResponse?.body.docs?.[opensearchRequestIndex] : undefined; const docFound = indexFound && actualResult?.found === true; - let hasSetNamespace = false; // @ts-expect-error MultiGetHit._source is optional if (docFound && !this.rawDocExistsInNamespace(actualResult!, namespace)) { const { id, type } = object; @@ -462,20 +460,11 @@ export class SavedObjectsRepository { }, }, }; - } else { - hasSetNamespace = true; - if (this._registry.isSingleNamespace(object.type)) { - savedObjectNamespace = initialNamespaces ? initialNamespaces[0] : namespace; - } else if (this._registry.isMultiNamespace(object.type)) { - savedObjectNamespaces = initialNamespaces || getSavedObjectNamespaces(namespace); - } - } - if (!hasSetNamespace) { - savedObjectNamespaces = - initialNamespaces || - // @ts-expect-error MultiGetHit._source is optional - getSavedObjectNamespaces(namespace, docFound ? actualResult : undefined); } + savedObjectNamespaces = + initialNamespaces || + // @ts-expect-error MultiGetHit._source is optional + getSavedObjectNamespaces(namespace, docFound ? actualResult : undefined); // @ts-expect-error MultiGetHit._source is optional versionProperties = getExpectedVersionProperties(version, actualResult); } else { @@ -487,7 +476,7 @@ export class SavedObjectsRepository { versionProperties = getExpectedVersionProperties(version); } - savedObjectWorkspaces = options.workspaces; + let savedObjectWorkspaces: string[] | undefined = options.workspaces; if (expectedBulkGetResult.value.method !== 'create') { const rawId = this._serializer.generateRawId(namespace, object.type, object.id); @@ -495,6 +484,11 @@ export class SavedObjectsRepository { bulkGetResponse?.statusCode !== 404 ? bulkGetResponse?.body.docs?.find((item) => item._id === rawId) : null; + /** + * When it is about to overwrite a object into options.workspace. + * We need to check if the options.workspaces is the subset of object.workspaces, + * Or it will be treated as a conflict + */ if (findObject && findObject.found) { const transformedObject = this._serializer.rawToSavedObject( findObject as SavedObjectsRawDoc @@ -504,6 +498,10 @@ export class SavedObjectsRepository { transformedObject.workspaces ); if (filteredWorkspaces.length) { + /** + * options.workspaces is not a subset of object.workspaces, + * return a conflict error. + */ const { id, type } = object; return { tag: 'Left' as 'Left', diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 521149c4e646..4236eaf7fc74 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -131,14 +131,8 @@ function getClauseForType( * Gets the clause that will filter for the workspace. */ function getClauseForWorkspace(workspace: string) { - if (workspace === '*') { - return { - bool: { - must: { - match_all: {}, - }, - }, - }; + if (!workspace) { + return {}; } return { @@ -246,12 +240,14 @@ export function getQueryParams({ ], }; - if (workspaces) { + if (workspaces?.filter((workspace) => workspace).length) { bool.filter.push({ bool: { - should: workspaces.map((workspace) => { - return getClauseForWorkspace(workspace); - }), + should: workspaces + .filter((workspace) => workspace) + .map((workspace) => { + return getClauseForWorkspace(workspace); + }), minimum_should_match: 1, }, }); diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 490c2b7083d2..9fc4a6280b63 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -83,8 +83,8 @@ export class SavedObjectsUtils { public static filterWorkspacesAccordingToBaseWorkspaces( targetWorkspaces?: string[], - baseWorkspaces?: string[] + sourceWorkspaces?: string[] ): string[] { - return targetWorkspaces?.filter((item) => !baseWorkspaces?.includes(item)) || []; + return targetWorkspaces?.filter((item) => !sourceWorkspaces?.includes(item)) || []; } } diff --git a/src/core/server/saved_objects/service/saved_objects_client.ts b/src/core/server/saved_objects/service/saved_objects_client.ts index 5f92dacacf36..a087dc6c388a 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -363,7 +363,7 @@ export class SavedObjectsClient { */ async bulkGet( objects: SavedObjectsBulkGetObject[] = [], - options: SavedObjectsBaseOptions = {} + options: Omit = {} ): Promise> { return await this._repository.bulkGet(objects, options); } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 33862cb149fb..1b1409570fea 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -110,6 +110,7 @@ export interface SavedObjectsFindOptions { typeToNamespacesMap?: Map; /** An optional OpenSearch preference value to be used for the query **/ preference?: string; + /** If specified, will find all objects belong to specified workspaces **/ workspaces?: string[]; } @@ -120,6 +121,7 @@ export interface SavedObjectsFindOptions { export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; + /** Specify the workspaces for this operation */ workspaces?: string[]; } diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index 47faffb0b922..fa4f5ab97fdf 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -113,6 +113,7 @@ export interface SavedObject { * space. */ originId?: string; + /** Workspaces that this saved object exists in. */ workspaces?: string[]; } From ab92370100bfd4eff9cdcf22f7cb952f4861167e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 17:38:25 +0800 Subject: [PATCH 14/43] feat: update test Signed-off-by: SuZhou-Joe --- .../import/regenerate_ids.test.ts | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/import/regenerate_ids.test.ts b/src/core/server/saved_objects/import/regenerate_ids.test.ts index 11556c8a21c1..605a61774534 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.test.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.test.ts @@ -29,8 +29,10 @@ */ import { mockUuidv4 } from './__mocks__'; -import { regenerateIds } from './regenerate_ids'; +import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; import { SavedObject } from '../types'; +import { savedObjectsClientMock } from '../service/saved_objects_client.mock'; +import { SavedObjectsBulkResponse } from '../service'; describe('#regenerateIds', () => { const objects = ([ @@ -62,3 +64,70 @@ describe('#regenerateIds', () => { `); }); }); + +describe('#regenerateIdsWithReference', () => { + const objects = ([ + { type: 'foo', id: '1' }, + { type: 'bar', id: '2' }, + { type: 'baz', id: '3' }, + ] as any) as SavedObject[]; + + test('returns expected values', async () => { + const mockedSavedObjectsClient = savedObjectsClientMock.create(); + mockUuidv4.mockReturnValueOnce('uuidv4 #1'); + const result: SavedObjectsBulkResponse = { + saved_objects: [ + { + error: { + statusCode: 404, + error: '', + message: '', + }, + id: '1', + type: 'foo', + attributes: {}, + references: [], + }, + { + id: '2', + type: 'bar', + attributes: {}, + references: [], + workspaces: ['bar'], + }, + { + id: '3', + type: 'baz', + attributes: {}, + references: [], + workspaces: ['foo'], + }, + ], + }; + mockedSavedObjectsClient.bulkGet.mockResolvedValue(result); + expect( + await regenerateIdsWithReference({ + savedObjects: objects, + savedObjectsClient: mockedSavedObjectsClient, + workspaces: ['bar'], + objectLimit: 1000, + importIdMap: new Map(), + }) + ).toMatchInlineSnapshot(` + Map { + "foo:1" => Object { + "id": "1", + "omitOriginId": true, + }, + "bar:2" => Object { + "id": "2", + "omitOriginId": false, + }, + "baz:3" => Object { + "id": "uuidv4 #1", + "omitOriginId": true, + }, + } + `); + }); +}); From c0cfd1780606e539b4faa232beae1a87ff51d751 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Sep 2023 18:14:46 +0800 Subject: [PATCH 15/43] feat: optimize code Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index cad4763ceaab..daa40075caf9 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -383,7 +383,7 @@ export class SavedObjectsRepository { /** * It requires a check when overwriting objects to target workspaces */ - const requiresWorkspaceCheck = !!(object.id && overwrite && options.workspaces); + const requiresWorkspaceCheck = !!(object.id && options.workspaces); if (object.id == null) object.id = uuid.v1(); From f4b86f0ae192a4f51d991bc9095eb4a8d0141511 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sat, 30 Sep 2023 10:50:34 +0800 Subject: [PATCH 16/43] feat: add changelog Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 623c5b7206a9..7885a742b316 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Adds Data explorer framework and implements Discover using it ([#4806](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4806)) - [Theme] Use themes' definitions to render the initial view ([#4936](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4936/)) - [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854/)) +- [Workspace] Optional workspaces params in repository ([#5162](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5162/)) ### 🐛 Bug Fixes From eb6d98fc1111bb271802aa5adf96903c04c0171c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 4 Oct 2023 22:22:28 +0800 Subject: [PATCH 17/43] feat: modify CHANGELOG Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e06e149db74..884248e19b78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,10 +37,10 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Decouple] Allow plugin manifest config to define semver compatible OpenSearch plugin and verify if it is installed on the cluster([#4612](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4612)) - [Advanced Settings] Consolidate settings into new "Appearance" category and add category IDs ([#4845](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4845)) - Adds Data explorer framework and implements Discover using it ([#4806](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4806)) -- [Theme] Use themes' definitions to render the initial view ([#4936](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4936/)) -- [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854/)) +- [Theme] Use themes' definitions to render the initial view ([#4936](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4936)) +- [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854)) - [Discover] Update embeddable for saved searches ([#5081](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5081)) -- [Workspace] Optional workspaces params in repository ([#5162](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5162/)) +- [Workspace] Optional workspaces params in repository ([#5162](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5162)) ### 🐛 Bug Fixes From 2bf837a2fefdb3e03dad0a3352f017d657ad64ef Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 12 Oct 2023 16:49:48 +0800 Subject: [PATCH 18/43] feat: increase unit test coverage Signed-off-by: SuZhou-Joe --- .../get_sorted_objects_for_export.test.ts | 25 +++++++++ .../import/import_saved_objects.test.ts | 56 +++++++++++++++++++ .../service/lib/repository.test.js | 6 ++ .../service/lib/search_dsl/query_params.ts | 8 +-- 4 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index cf7e1d8246a7..dedcdffaf2ac 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -857,4 +857,29 @@ describe('getSortedObjectsForExport()', () => { `Can't specify both "search" and "objects" properties when exporting` ); }); + + test('rejects when both types and objecys are passed in', () => { + const exportOpts = { + exportSizeLimit: 1, + savedObjectsClient, + objects: [{ type: 'index-pattern', id: '1' }], + types: ['foo'], + }; + + expect(exportSavedObjectsToStream(exportOpts)).rejects.toThrowErrorMatchingInlineSnapshot( + `Can't specify both "types" and "objects" properties when exporting` + ); + }); + + test('rejects when bulkGet returns an error', () => { + const exportOpts = { + exportSizeLimit: 1, + savedObjectsClient, + objects: [{ type: 'index-pattern', id: '1' }], + }; + + expect(exportSavedObjectsToStream(exportOpts)).rejects.toThrowErrorMatchingInlineSnapshot( + `Can't specify both "types" and "objects" properties when exporting` + ); + }); }); diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index 4bf1717e895c..1039903a2550 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -278,6 +278,62 @@ describe('#importSavedObjectsFromStream', () => { }; expect(createSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); }); + + test('creates saved objects when workspaces param is provided', async () => { + const options = setupOptions(); + options.workspaces = ['foo']; + const collectedObjects = [createObject()]; + const filteredObjects = [createObject()]; + const errors = [createError(), createError(), createError(), createError()]; + getMockFn(collectSavedObjects).mockResolvedValue({ + errors: [errors[0]], + collectedObjects, + importIdMap: new Map([ + ['foo', {}], + ['bar', {}], + ['baz', {}], + ]), + }); + getMockFn(validateReferences).mockResolvedValue([errors[1]]); + getMockFn(regenerateIdsWithReference).mockResolvedValue( + Promise.resolve( + new Map([ + ['foo', {}], + ['bar', {}], + ['baz', {}], + ]) + ) + ); + getMockFn(checkConflicts).mockResolvedValue({ + errors: [errors[2]], + filteredObjects, + importIdMap: new Map([['bar', { id: 'newId1' }]]), + pendingOverwrites: new Set(), + }); + getMockFn(checkOriginConflicts).mockResolvedValue({ + errors: [errors[3]], + importIdMap: new Map([['baz', { id: 'newId2' }]]), + pendingOverwrites: new Set(), + }); + + await importSavedObjectsFromStream(options); + const importIdMap = new Map([ + ['foo', {}], + ['bar', { id: 'newId1' }], + ['baz', { id: 'newId2' }], + ]); + const createSavedObjectsParams = { + objects: collectedObjects, + accumulatedErrors: errors, + savedObjectsClient, + importIdMap, + overwrite, + namespace, + workspaces: options.workspaces, + }; + expect(createSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); + expect(regenerateIdsWithReference).toBeCalledTimes(1); + }); }); describe('with createNewCopies enabled', () => { diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index 7bb22474ee76..81ab1fb778dd 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -1259,6 +1259,12 @@ describe('SavedObjectsRepository', () => { migrationVersion: doc._source.migrationVersion, }); + it(`returns early for undefined objects argument`, async () => { + const result = await savedObjectsRepository.bulkGet(); + expect(result).toEqual({ saved_objects: [] }); + expect(client.mget).not.toHaveBeenCalled(); + }); + it(`returns early for empty objects argument`, async () => { const result = await bulkGet([]); expect(result).toEqual({ saved_objects: [] }); diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index 4236eaf7fc74..e660f299f460 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -131,10 +131,6 @@ function getClauseForType( * Gets the clause that will filter for the workspace. */ function getClauseForWorkspace(workspace: string) { - if (!workspace) { - return {}; - } - return { bool: { must: [{ term: { workspaces: workspace } }], @@ -245,9 +241,7 @@ export function getQueryParams({ bool: { should: workspaces .filter((workspace) => workspace) - .map((workspace) => { - return getClauseForWorkspace(workspace); - }), + .map((workspace) => getClauseForWorkspace(workspace)), minimum_should_match: 1, }, }); From dbb12484ce0807ffb79e61c400df5cbbc1457d93 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 13 Oct 2023 10:51:22 +0800 Subject: [PATCH 19/43] feat: add comment Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/service/lib/repository.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index daa40075caf9..17a66fceb44a 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -811,6 +811,7 @@ export class SavedObjectsRepository { * @property {string} [options.namespace] * @property {object} [options.hasReference] - { type, id } * @property {string} [options.preference] + * @property {Array} [options.workspaces] * @returns {promise} - { saved_objects: [{ id, type, version, attributes }], total, per_page, page } */ async find(options: SavedObjectsFindOptions): Promise> { From 0ba8df5d00236d0fbce32b1f67b5bf895785e913 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 17:28:45 +0800 Subject: [PATCH 20/43] feat: remove useless generateId method Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.test.ts | 22 +--- .../import/import_saved_objects.ts | 16 +-- .../saved_objects/import/regenerate_ids.ts | 35 +----- .../import/validate_references.test.ts | 104 ++++++++++++++++++ .../import/validate_references.ts | 29 ++++- .../saved_objects/service/lib/repository.ts | 6 +- .../server/saved_objects/service/lib/utils.ts | 2 +- 7 files changed, 140 insertions(+), 74 deletions(-) diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index 1039903a2550..4506867a6863 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -42,7 +42,7 @@ import { typeRegistryMock } from '../saved_objects_type_registry.mock'; import { importSavedObjectsFromStream } from './import_saved_objects'; import { collectSavedObjects } from './collect_saved_objects'; -import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; +import { regenerateIds } from './regenerate_ids'; import { validateReferences } from './validate_references'; import { checkConflicts } from './check_conflicts'; import { checkOriginConflicts } from './check_origin_conflicts'; @@ -68,7 +68,6 @@ describe('#importSavedObjectsFromStream', () => { importIdMap: new Map(), }); getMockFn(regenerateIds).mockReturnValue(new Map()); - getMockFn(regenerateIdsWithReference).mockReturnValue(Promise.resolve(new Map())); getMockFn(validateReferences).mockResolvedValue([]); getMockFn(checkConflicts).mockResolvedValue({ errors: [], @@ -241,15 +240,6 @@ describe('#importSavedObjectsFromStream', () => { ]), }); getMockFn(validateReferences).mockResolvedValue([errors[1]]); - getMockFn(regenerateIdsWithReference).mockResolvedValue( - Promise.resolve( - new Map([ - ['foo', {}], - ['bar', {}], - ['baz', {}], - ]) - ) - ); getMockFn(checkConflicts).mockResolvedValue({ errors: [errors[2]], filteredObjects, @@ -295,15 +285,6 @@ describe('#importSavedObjectsFromStream', () => { ]), }); getMockFn(validateReferences).mockResolvedValue([errors[1]]); - getMockFn(regenerateIdsWithReference).mockResolvedValue( - Promise.resolve( - new Map([ - ['foo', {}], - ['bar', {}], - ['baz', {}], - ]) - ) - ); getMockFn(checkConflicts).mockResolvedValue({ errors: [errors[2]], filteredObjects, @@ -332,7 +313,6 @@ describe('#importSavedObjectsFromStream', () => { workspaces: options.workspaces, }; expect(createSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams); - expect(regenerateIdsWithReference).toBeCalledTimes(1); }); }); diff --git a/src/core/server/saved_objects/import/import_saved_objects.ts b/src/core/server/saved_objects/import/import_saved_objects.ts index 2d87f94b97e3..b283f10c2364 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.ts @@ -38,7 +38,7 @@ import { validateReferences } from './validate_references'; import { checkOriginConflicts } from './check_origin_conflicts'; import { createSavedObjects } from './create_saved_objects'; import { checkConflicts } from './check_conflicts'; -import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; +import { regenerateIds } from './regenerate_ids'; /** * Import saved objects from given stream. See the {@link SavedObjectsImportOptions | options} for more @@ -74,23 +74,17 @@ export async function importSavedObjectsFromStream({ const validateReferencesResult = await validateReferences( collectSavedObjectsResult.collectedObjects, savedObjectsClient, - namespace + namespace, + undefined, + workspaces ); errorAccumulator = [...errorAccumulator, ...validateReferencesResult]; if (createNewCopies) { importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects); } else { - if (workspaces) { - importIdMap = await regenerateIdsWithReference({ - savedObjects: collectSavedObjectsResult.collectedObjects, - savedObjectsClient, - workspaces, - objectLimit, - importIdMap, - }); - } // Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces + // Check for conflicts across all workspaces const checkConflictsParams = { objects: collectSavedObjectsResult.collectedObjects, savedObjectsClient, diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index b309753566e3..672a8f030620 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.ts @@ -29,8 +29,7 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { SavedObject, SavedObjectsClientContract } from '../types'; -import { SavedObjectsUtils } from '../service'; +import { SavedObject } from '../types'; /** * Takes an array of saved objects and returns an importIdMap of randomly-generated new IDs. @@ -43,35 +42,3 @@ export const regenerateIds = (objects: SavedObject[]) => { }, new Map()); return importIdMap; }; - -export const regenerateIdsWithReference = async (props: { - savedObjects: SavedObject[]; - savedObjectsClient: SavedObjectsClientContract; - workspaces: string[]; - objectLimit: number; - importIdMap: Map; -}): Promise> => { - const { savedObjects, savedObjectsClient, workspaces, importIdMap } = props; - - const bulkGetResult = await savedObjectsClient.bulkGet( - savedObjects.map((item) => ({ type: item.type, id: item.id })) - ); - - return bulkGetResult.saved_objects.reduce((acc, object) => { - if (object.error?.statusCode === 404) { - acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: true }); - return acc; - } - - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( - workspaces, - object.workspaces - ); - if (filteredWorkspaces.length) { - acc.set(`${object.type}:${object.id}`, { id: uuidv4(), omitOriginId: true }); - } else { - acc.set(`${object.type}:${object.id}`, { id: object.id, omitOriginId: false }); - } - return acc; - }, importIdMap); -}; diff --git a/src/core/server/saved_objects/import/validate_references.test.ts b/src/core/server/saved_objects/import/validate_references.test.ts index df9a4e216bbb..c028f7a9ca5f 100644 --- a/src/core/server/saved_objects/import/validate_references.test.ts +++ b/src/core/server/saved_objects/import/validate_references.test.ts @@ -136,6 +136,7 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, + workspaces, ], id: 1, type: index-pattern, @@ -230,6 +231,7 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, + workspaces, ], id: 1, type: index-pattern, @@ -237,6 +239,7 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, + workspaces, ], id: 3, type: search, @@ -419,6 +422,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 3, type: index-pattern, @@ -426,6 +430,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 5, type: index-pattern, @@ -433,6 +438,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 6, type: index-pattern, @@ -440,6 +446,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 7, type: search, @@ -447,6 +454,7 @@ describe('validateReferences()', () => { Object { fields: Array [ id, + workspaces, ], id: 8, type: search, @@ -601,4 +609,100 @@ describe('validateReferences()', () => { validateReferences(savedObjects, savedObjectsClient) ).rejects.toThrowErrorMatchingInlineSnapshot(`Bad Request`); }); + + test('returns errors when references have conflict on workspaces', async () => { + savedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: [ + { + type: 'index-pattern', + id: '3', + error: SavedObjectsErrorHelpers.createGenericNotFoundError('index-pattern', '3').output + .payload, + attributes: {}, + references: [], + workspaces: ['foo'], + }, + { + type: 'index-pattern', + id: '5', + error: SavedObjectsErrorHelpers.createGenericNotFoundError('index-pattern', '5').output + .payload, + attributes: {}, + references: [], + workspaces: ['bar'], + }, + ], + }); + const savedObjects = [ + { + id: '5', + type: 'index-pattern', + attributes: { + title: 'My Visualization 2', + }, + references: [ + { + name: 'ref_0', + type: 'index-pattern', + id: '3', + }, + ], + }, + ]; + const result = await validateReferences( + savedObjects, + savedObjectsClient, + undefined, + undefined, + ['bar'] + ); + expect(result).toMatchInlineSnapshot(` + Array [ + Object { + error: Object { + references: Array [ + Object { + id: 3, + type: index-pattern, + }, + ], + type: missing_references, + }, + id: 5, + meta: Object { + title: My Visualization 2, + }, + title: My Visualization 2, + type: index-pattern, + }, + ] + `); + expect(savedObjectsClient.bulkGet).toMatchInlineSnapshot(` + [MockFunction] { + "calls": Array [ + Array [ + Array [ + Object { + fields: Array [ + id, + workspaces, + ], + id: 3, + type: index-pattern, + }, + ], + Object { + namespace: undefined, + }, + ], + ], + "results": Array [ + Object { + type: return, + value: Promise {}, + }, + ], + } + `); + }); }); diff --git a/src/core/server/saved_objects/import/validate_references.ts b/src/core/server/saved_objects/import/validate_references.ts index fb75eb837443..e0ad4960dfc6 100644 --- a/src/core/server/saved_objects/import/validate_references.ts +++ b/src/core/server/saved_objects/import/validate_references.ts @@ -31,6 +31,7 @@ import Boom from '@hapi/boom'; import { SavedObject, SavedObjectsClientContract } from '../types'; import { SavedObjectsImportError, SavedObjectsImportRetry } from './types'; +import { SavedObjectsUtils } from '../service'; const REF_TYPES_TO_VLIDATE = ['index-pattern', 'search']; @@ -48,7 +49,8 @@ export async function getNonExistingReferenceAsKeys( savedObjects: SavedObject[], savedObjectsClient: SavedObjectsClientContract, namespace?: string, - retries?: SavedObjectsImportRetry[] + retries?: SavedObjectsImportRetry[], + workspaces?: string[] ) { const objectsToSkip = getObjectsToSkip(retries); const collector = new Map(); @@ -73,7 +75,10 @@ export async function getNonExistingReferenceAsKeys( } // Fetch references to see if they exist - const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ ...obj, fields: ['id'] })); + const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ + ...obj, + fields: ['id', 'workspaces'], + })); const bulkGetResponse = await savedObjectsClient.bulkGet(bulkGetOpts, { namespace }); // Error handling @@ -93,6 +98,20 @@ export async function getNonExistingReferenceAsKeys( if (savedObject.error) { continue; } + /** + * If original workspaces have conflict with target workspace + * make it as NonExistingReference + */ + if (workspaces) { + if ( + SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + workspaces, + savedObject.workspaces + ).length + ) { + continue; + } + } collector.delete(`${savedObject.type}:${savedObject.id}`); } @@ -103,7 +122,8 @@ export async function validateReferences( savedObjects: Array>, savedObjectsClient: SavedObjectsClientContract, namespace?: string, - retries?: SavedObjectsImportRetry[] + retries?: SavedObjectsImportRetry[], + workspaces?: string[] ) { const objectsToSkip = getObjectsToSkip(retries); const errorMap: { [key: string]: SavedObjectsImportError } = {}; @@ -111,7 +131,8 @@ export async function validateReferences( savedObjects, savedObjectsClient, namespace, - retries + retries, + workspaces ); // Filter out objects with missing references, add to error object diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 17a66fceb44a..45997e6cf6b9 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -291,7 +291,7 @@ export class SavedObjectsRepository { } if (currentItem) { if ( - SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( workspaces, currentItem.workspaces ).length @@ -493,7 +493,7 @@ export class SavedObjectsRepository { const transformedObject = this._serializer.rawToSavedObject( findObject as SavedObjectsRawDoc ) as SavedObject; - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( options.workspaces, transformedObject.workspaces ); @@ -658,7 +658,7 @@ export class SavedObjectsRepository { let workspaceConflict = false; if (options.workspaces) { const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc); - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces( + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( options.workspaces, transformedObject.workspaces ); diff --git a/src/core/server/saved_objects/service/lib/utils.ts b/src/core/server/saved_objects/service/lib/utils.ts index 9fc4a6280b63..d3cb62e02b82 100644 --- a/src/core/server/saved_objects/service/lib/utils.ts +++ b/src/core/server/saved_objects/service/lib/utils.ts @@ -81,7 +81,7 @@ export class SavedObjectsUtils { saved_objects: [], }); - public static filterWorkspacesAccordingToBaseWorkspaces( + public static filterWorkspacesAccordingToSourceWorkspaces( targetWorkspaces?: string[], sourceWorkspaces?: string[] ): string[] { From 743bf33d40218ecf57d00422bbf8529405727895 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 17:29:46 +0800 Subject: [PATCH 21/43] feat: remove flaky test Signed-off-by: SuZhou-Joe --- .../export/get_sorted_objects_for_export.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts index dedcdffaf2ac..0fb602629c55 100644 --- a/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts +++ b/src/core/server/saved_objects/export/get_sorted_objects_for_export.test.ts @@ -870,16 +870,4 @@ describe('getSortedObjectsForExport()', () => { `Can't specify both "types" and "objects" properties when exporting` ); }); - - test('rejects when bulkGet returns an error', () => { - const exportOpts = { - exportSizeLimit: 1, - savedObjectsClient, - objects: [{ type: 'index-pattern', id: '1' }], - }; - - expect(exportSavedObjectsToStream(exportOpts)).rejects.toThrowErrorMatchingInlineSnapshot( - `Can't specify both "types" and "objects" properties when exporting` - ); - }); }); From 285adad79b26e13c722c4764daa3c679b8f94420 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 17:31:18 +0800 Subject: [PATCH 22/43] feat: remove useless code Signed-off-by: SuZhou-Joe --- .../import/regenerate_ids.test.ts | 71 +------------------ 1 file changed, 1 insertion(+), 70 deletions(-) diff --git a/src/core/server/saved_objects/import/regenerate_ids.test.ts b/src/core/server/saved_objects/import/regenerate_ids.test.ts index 605a61774534..11556c8a21c1 100644 --- a/src/core/server/saved_objects/import/regenerate_ids.test.ts +++ b/src/core/server/saved_objects/import/regenerate_ids.test.ts @@ -29,10 +29,8 @@ */ import { mockUuidv4 } from './__mocks__'; -import { regenerateIds, regenerateIdsWithReference } from './regenerate_ids'; +import { regenerateIds } from './regenerate_ids'; import { SavedObject } from '../types'; -import { savedObjectsClientMock } from '../service/saved_objects_client.mock'; -import { SavedObjectsBulkResponse } from '../service'; describe('#regenerateIds', () => { const objects = ([ @@ -64,70 +62,3 @@ describe('#regenerateIds', () => { `); }); }); - -describe('#regenerateIdsWithReference', () => { - const objects = ([ - { type: 'foo', id: '1' }, - { type: 'bar', id: '2' }, - { type: 'baz', id: '3' }, - ] as any) as SavedObject[]; - - test('returns expected values', async () => { - const mockedSavedObjectsClient = savedObjectsClientMock.create(); - mockUuidv4.mockReturnValueOnce('uuidv4 #1'); - const result: SavedObjectsBulkResponse = { - saved_objects: [ - { - error: { - statusCode: 404, - error: '', - message: '', - }, - id: '1', - type: 'foo', - attributes: {}, - references: [], - }, - { - id: '2', - type: 'bar', - attributes: {}, - references: [], - workspaces: ['bar'], - }, - { - id: '3', - type: 'baz', - attributes: {}, - references: [], - workspaces: ['foo'], - }, - ], - }; - mockedSavedObjectsClient.bulkGet.mockResolvedValue(result); - expect( - await regenerateIdsWithReference({ - savedObjects: objects, - savedObjectsClient: mockedSavedObjectsClient, - workspaces: ['bar'], - objectLimit: 1000, - importIdMap: new Map(), - }) - ).toMatchInlineSnapshot(` - Map { - "foo:1" => Object { - "id": "1", - "omitOriginId": true, - }, - "bar:2" => Object { - "id": "2", - "omitOriginId": false, - }, - "baz:3" => Object { - "id": "uuidv4 #1", - "omitOriginId": true, - }, - } - `); - }); -}); From 142e9c000302cab244679844b8e80c30ad83b400 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 10:22:31 +0800 Subject: [PATCH 23/43] fix: unit test Signed-off-by: SuZhou-Joe --- .../saved_objects/import/import_saved_objects.test.ts | 4 +++- .../server/saved_objects/import/validate_references.ts | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/import/import_saved_objects.test.ts b/src/core/server/saved_objects/import/import_saved_objects.test.ts index 4506867a6863..d0e4eda7ab3c 100644 --- a/src/core/server/saved_objects/import/import_saved_objects.test.ts +++ b/src/core/server/saved_objects/import/import_saved_objects.test.ts @@ -165,7 +165,9 @@ describe('#importSavedObjectsFromStream', () => { expect(validateReferences).toHaveBeenCalledWith( collectedObjects, savedObjectsClient, - namespace + namespace, + undefined, + undefined ); }); diff --git a/src/core/server/saved_objects/import/validate_references.ts b/src/core/server/saved_objects/import/validate_references.ts index e0ad4960dfc6..1f9b4460b2f8 100644 --- a/src/core/server/saved_objects/import/validate_references.ts +++ b/src/core/server/saved_objects/import/validate_references.ts @@ -74,10 +74,15 @@ export async function getNonExistingReferenceAsKeys( return []; } + const fields = ['id']; + if (workspaces?.length) { + fields.push('workspaces'); + } + // Fetch references to see if they exist const bulkGetOpts = Array.from(collector.values()).map((obj) => ({ ...obj, - fields: ['id', 'workspaces'], + fields, })); const bulkGetResponse = await savedObjectsClient.bulkGet(bulkGetOpts, { namespace }); From e8aa3a4594ab7598195e00c923964555263c0296 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 16:45:57 +0800 Subject: [PATCH 24/43] feat: update snapshot Signed-off-by: SuZhou-Joe --- .../saved_objects/import/validate_references.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/core/server/saved_objects/import/validate_references.test.ts b/src/core/server/saved_objects/import/validate_references.test.ts index c028f7a9ca5f..f53922c27ac0 100644 --- a/src/core/server/saved_objects/import/validate_references.test.ts +++ b/src/core/server/saved_objects/import/validate_references.test.ts @@ -136,7 +136,6 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, - workspaces, ], id: 1, type: index-pattern, @@ -231,7 +230,6 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, - workspaces, ], id: 1, type: index-pattern, @@ -239,7 +237,6 @@ describe('getNonExistingReferenceAsKeys()', () => { Object { fields: Array [ id, - workspaces, ], id: 3, type: search, @@ -422,7 +419,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 3, type: index-pattern, @@ -430,7 +426,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 5, type: index-pattern, @@ -438,7 +433,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 6, type: index-pattern, @@ -446,7 +440,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 7, type: search, @@ -454,7 +447,6 @@ describe('validateReferences()', () => { Object { fields: Array [ id, - workspaces, ], id: 8, type: search, From dc43f707fb58e64dd3caf243ca6aac0dfe63ea89 Mon Sep 17 00:00:00 2001 From: Zhou Su Date: Wed, 6 Sep 2023 03:17:17 +0000 Subject: [PATCH 25/43] feature: setup workspace skeleton and implement basic CRUD API on workspace Signed-off-by: Zhou Su --- src/core/server/index.ts | 4 +- src/core/utils/constants.ts | 6 + src/core/utils/index.ts | 1 + .../workspace/opensearch_dashboards.json | 9 + src/plugins/workspace/server/index.ts | 13 ++ src/plugins/workspace/server/plugin.ts | 45 ++++ src/plugins/workspace/server/routes/index.ts | 181 ++++++++++++++++ .../workspace/server/saved_objects/index.ts | 6 + .../server/saved_objects/workspace.ts | 41 ++++ src/plugins/workspace/server/types.ts | 65 ++++++ src/plugins/workspace/server/utils.test.ts | 21 ++ src/plugins/workspace/server/utils.ts | 13 ++ .../workspace/server/workspace_client.ts | 193 ++++++++++++++++++ 13 files changed, 596 insertions(+), 2 deletions(-) create mode 100644 src/core/utils/constants.ts create mode 100644 src/plugins/workspace/opensearch_dashboards.json create mode 100644 src/plugins/workspace/server/index.ts create mode 100644 src/plugins/workspace/server/plugin.ts create mode 100644 src/plugins/workspace/server/routes/index.ts create mode 100644 src/plugins/workspace/server/saved_objects/index.ts create mode 100644 src/plugins/workspace/server/saved_objects/workspace.ts create mode 100644 src/plugins/workspace/server/types.ts create mode 100644 src/plugins/workspace/server/utils.test.ts create mode 100644 src/plugins/workspace/server/utils.ts create mode 100644 src/plugins/workspace/server/workspace_client.ts diff --git a/src/core/server/index.ts b/src/core/server/index.ts index ca55cc8dc1d5..379411398fca 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -345,8 +345,8 @@ export { MetricsServiceStart, } from './metrics'; -export { AppCategory } from '../types'; -export { DEFAULT_APP_CATEGORIES } from '../utils'; +export { AppCategory, WorkspaceAttribute } from '../types'; +export { DEFAULT_APP_CATEGORIES, WORKSPACE_TYPE } from '../utils'; export { SavedObject, diff --git a/src/core/utils/constants.ts b/src/core/utils/constants.ts new file mode 100644 index 000000000000..73c2d6010846 --- /dev/null +++ b/src/core/utils/constants.ts @@ -0,0 +1,6 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export const WORKSPACE_TYPE = 'workspace'; diff --git a/src/core/utils/index.ts b/src/core/utils/index.ts index b3b6ce4aab02..af4f9a17ae58 100644 --- a/src/core/utils/index.ts +++ b/src/core/utils/index.ts @@ -37,3 +37,4 @@ export { IContextProvider, } from './context'; export { DEFAULT_APP_CATEGORIES } from './default_app_categories'; +export { WORKSPACE_TYPE } from './constants'; diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json new file mode 100644 index 000000000000..a898536ebb79 --- /dev/null +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -0,0 +1,9 @@ +{ + "id": "workspace", + "version": "opensearchDashboards", + "server": true, + "ui": true, + "requiredPlugins": ["savedObjects"], + "optionalPlugins": [], + "requiredBundles": [] +} diff --git a/src/plugins/workspace/server/index.ts b/src/plugins/workspace/server/index.ts new file mode 100644 index 000000000000..936d3cf3ecec --- /dev/null +++ b/src/plugins/workspace/server/index.ts @@ -0,0 +1,13 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { PluginInitializerContext } from '../../../core/server'; +import { WorkspacePlugin } from './plugin'; + +// This exports static code and TypeScript types, +// as well as, OpenSearch Dashboards Platform `plugin()` initializer. + +export function plugin(initializerContext: PluginInitializerContext) { + return new WorkspacePlugin(initializerContext); +} diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts new file mode 100644 index 000000000000..e15a9ef25f7e --- /dev/null +++ b/src/plugins/workspace/server/plugin.ts @@ -0,0 +1,45 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { PluginInitializerContext, CoreSetup, Plugin, Logger } from '../../../core/server'; +import { IWorkspaceDBImpl } from './types'; +import { WorkspaceClientWithSavedObject } from './workspace_client'; +import { registerRoutes } from './routes'; + +export class WorkspacePlugin implements Plugin<{}, {}> { + private readonly logger: Logger; + private client?: IWorkspaceDBImpl; + + constructor(initializerContext: PluginInitializerContext) { + this.logger = initializerContext.logger.get('plugins', 'workspace'); + } + + public async setup(core: CoreSetup) { + this.logger.debug('Setting up Workspaces service'); + + this.client = new WorkspaceClientWithSavedObject(core); + + await this.client.setup(core); + + registerRoutes({ + http: core.http, + logger: this.logger, + client: this.client as IWorkspaceDBImpl, + }); + + return { + client: this.client, + }; + } + + public start() { + this.logger.debug('Starting SavedObjects service'); + + return { + client: this.client as IWorkspaceDBImpl, + }; + } + + public stop() {} +} diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts new file mode 100644 index 000000000000..39cc5877b469 --- /dev/null +++ b/src/plugins/workspace/server/routes/index.ts @@ -0,0 +1,181 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { schema } from '@osd/config-schema'; + +import { CoreSetup, Logger } from '../../../../core/server'; +import { IWorkspaceDBImpl } from '../types'; + +const WORKSPACES_API_BASE_URL = '/api/workspaces'; + +const workspaceAttributesSchema = schema.object({ + description: schema.maybe(schema.string()), + name: schema.string(), + features: schema.maybe(schema.arrayOf(schema.string())), + color: schema.maybe(schema.string()), + icon: schema.maybe(schema.string()), + defaultVISTheme: schema.maybe(schema.string()), +}); + +export function registerRoutes({ + client, + logger, + http, +}: { + client: IWorkspaceDBImpl; + logger: Logger; + http: CoreSetup['http']; +}) { + const router = http.createRouter(); + router.post( + { + path: `${WORKSPACES_API_BASE_URL}/_list`, + validate: { + body: schema.object({ + search: schema.maybe(schema.string()), + sortOrder: schema.maybe(schema.string()), + perPage: schema.number({ min: 0, defaultValue: 20 }), + page: schema.number({ min: 0, defaultValue: 1 }), + sortField: schema.maybe(schema.string()), + searchFields: schema.maybe(schema.arrayOf(schema.string())), + }), + }, + }, + router.handleLegacyErrors(async (context, req, res) => { + const result = await client.list( + { + context, + request: req, + logger, + }, + req.body + ); + if (!result.success) { + return res.ok({ body: result }); + } + return res.ok({ + body: { + ...result, + result: { + ...result.result, + workspaces: result.result.workspaces.map((workspace) => ({ + ...workspace, + })), + }, + }, + }); + }) + ); + router.get( + { + path: `${WORKSPACES_API_BASE_URL}/{id}`, + validate: { + params: schema.object({ + id: schema.string(), + }), + }, + }, + router.handleLegacyErrors(async (context, req, res) => { + const { id } = req.params; + const result = await client.get( + { + context, + request: req, + logger, + }, + id + ); + if (!result.success) { + return res.ok({ body: result }); + } + + return res.ok({ + body: { + ...result, + result: { + ...result.result, + }, + }, + }); + }) + ); + router.post( + { + path: `${WORKSPACES_API_BASE_URL}`, + validate: { + body: schema.object({ + attributes: workspaceAttributesSchema, + }), + }, + }, + router.handleLegacyErrors(async (context, req, res) => { + const { attributes } = req.body; + + const result = await client.create( + { + context, + request: req, + logger, + }, + { + ...attributes, + } + ); + return res.ok({ body: result }); + }) + ); + router.put( + { + path: `${WORKSPACES_API_BASE_URL}/{id?}`, + validate: { + params: schema.object({ + id: schema.string(), + }), + body: schema.object({ + attributes: workspaceAttributesSchema, + }), + }, + }, + router.handleLegacyErrors(async (context, req, res) => { + const { id } = req.params; + const { attributes } = req.body; + + const result = await client.update( + { + context, + request: req, + logger, + }, + id, + { + ...attributes, + } + ); + return res.ok({ body: result }); + }) + ); + router.delete( + { + path: `${WORKSPACES_API_BASE_URL}/{id?}`, + validate: { + params: schema.object({ + id: schema.string(), + }), + }, + }, + router.handleLegacyErrors(async (context, req, res) => { + const { id } = req.params; + + const result = await client.delete( + { + context, + request: req, + logger, + }, + id + ); + return res.ok({ body: result }); + }) + ); +} diff --git a/src/plugins/workspace/server/saved_objects/index.ts b/src/plugins/workspace/server/saved_objects/index.ts new file mode 100644 index 000000000000..51653c50681e --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/index.ts @@ -0,0 +1,6 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export { workspace } from './workspace'; diff --git a/src/plugins/workspace/server/saved_objects/workspace.ts b/src/plugins/workspace/server/saved_objects/workspace.ts new file mode 100644 index 000000000000..5142185b0c2d --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace.ts @@ -0,0 +1,41 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObjectsType, WORKSPACE_TYPE } from '../../../../core/server'; + +export const workspace: SavedObjectsType = { + name: WORKSPACE_TYPE, + namespaceType: 'agnostic', + hidden: false, + /** + * workspace won't appear in management page. + */ + mappings: { + dynamic: false, + properties: { + name: { + type: 'keyword', + }, + description: { + type: 'text', + }, + /** + * In opensearch, string[] is also mapped to text + */ + features: { + type: 'text', + }, + color: { + type: 'text', + }, + icon: { + type: 'text', + }, + defaultVISTheme: { + type: 'text', + }, + }, + }, +}; diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts new file mode 100644 index 000000000000..d57cecd4a3c9 --- /dev/null +++ b/src/plugins/workspace/server/types.ts @@ -0,0 +1,65 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { + Logger, + OpenSearchDashboardsRequest, + RequestHandlerContext, + SavedObjectsFindResponse, + CoreSetup, + WorkspaceAttribute, + ISavedObjectsRepository, +} from '../../../core/server'; + +export interface WorkspaceFindOptions { + page?: number; + perPage?: number; + search?: string; + searchFields?: string[]; + sortField?: string; + sortOrder?: string; +} + +export interface IRequestDetail { + request: OpenSearchDashboardsRequest; + context: RequestHandlerContext; + logger: Logger; +} + +export interface IWorkspaceDBImpl { + setup(dep: CoreSetup): Promise>; + setInternalRepository(repository: ISavedObjectsRepository): void; + create( + requestDetail: IRequestDetail, + payload: Omit + ): Promise>; + list( + requestDetail: IRequestDetail, + options: WorkspaceFindOptions + ): Promise< + IResponse< + { + workspaces: WorkspaceAttribute[]; + } & Pick + > + >; + get(requestDetail: IRequestDetail, id: string): Promise>; + update( + requestDetail: IRequestDetail, + id: string, + payload: Omit + ): Promise>; + delete(requestDetail: IRequestDetail, id: string): Promise>; + destroy(): Promise>; +} + +export type IResponse = + | { + result: T; + success: true; + } + | { + success: false; + error?: string; + }; diff --git a/src/plugins/workspace/server/utils.test.ts b/src/plugins/workspace/server/utils.test.ts new file mode 100644 index 000000000000..119b8889f715 --- /dev/null +++ b/src/plugins/workspace/server/utils.test.ts @@ -0,0 +1,21 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { generateRandomId } from './utils'; + +describe('workspace utils', () => { + it('should generate id with the specified size', () => { + expect(generateRandomId(6)).toHaveLength(6); + }); + + it('should generate random IDs', () => { + const NUM_OF_ID = 10000; + const ids = new Set(); + for (let i = 0; i < NUM_OF_ID; i++) { + ids.add(generateRandomId(6)); + } + expect(ids.size).toBe(NUM_OF_ID); + }); +}); diff --git a/src/plugins/workspace/server/utils.ts b/src/plugins/workspace/server/utils.ts new file mode 100644 index 000000000000..89bfabd52657 --- /dev/null +++ b/src/plugins/workspace/server/utils.ts @@ -0,0 +1,13 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import crypto from 'crypto'; + +/** + * Generate URL friendly random ID + */ +export const generateRandomId = (size: number) => { + return crypto.randomBytes(size).toString('base64url').slice(0, size); +}; diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts new file mode 100644 index 000000000000..ec49daf83b6a --- /dev/null +++ b/src/plugins/workspace/server/workspace_client.ts @@ -0,0 +1,193 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { i18n } from '@osd/i18n'; +import type { + SavedObject, + SavedObjectsClientContract, + CoreSetup, + WorkspaceAttribute, + ISavedObjectsRepository, +} from '../../../core/server'; +import { WORKSPACE_TYPE } from '../../../core/server'; +import { IWorkspaceDBImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; +import { workspace } from './saved_objects'; +import { generateRandomId } from './utils'; + +const WORKSPACE_ID_SIZE = 6; + +const DUPLICATE_WORKSPACE_NAME_ERROR = i18n.translate('workspace.duplicate.name.error', { + defaultMessage: 'workspace name has already been used, try with a different name', +}); + +export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { + private setupDep: CoreSetup; + + private internalSavedObjectsRepository?: ISavedObjectsRepository; + setInternalRepository(repository: ISavedObjectsRepository) { + this.internalSavedObjectsRepository = repository; + } + + constructor(core: CoreSetup) { + this.setupDep = core; + } + private getSavedObjectClientsFromRequestDetail( + requestDetail: IRequestDetail + ): SavedObjectsClientContract { + return requestDetail.context.core.savedObjects.client; + } + private getFlattenedResultWithSavedObject( + savedObject: SavedObject + ): WorkspaceAttribute { + return { + ...savedObject.attributes, + id: savedObject.id, + }; + } + private formatError(error: Error | any): string { + return error.message || error.error || 'Error'; + } + public async setup(core: CoreSetup): Promise> { + this.setupDep.savedObjects.registerType(workspace); + return { + success: true, + result: true, + }; + } + public async create( + requestDetail: IRequestDetail, + payload: Omit + ): ReturnType { + try { + const attributes = payload; + const id = generateRandomId(WORKSPACE_ID_SIZE); + const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); + const existingWorkspaceRes = await this.internalSavedObjectsRepository?.find({ + type: WORKSPACE_TYPE, + search: attributes.name, + searchFields: ['name'], + }); + if (existingWorkspaceRes && existingWorkspaceRes.total > 0) { + throw new Error(DUPLICATE_WORKSPACE_NAME_ERROR); + } + const result = await client.create>( + WORKSPACE_TYPE, + attributes, + { + id, + } + ); + return { + success: true, + result: { + id: result.id, + }, + }; + } catch (e: unknown) { + return { + success: false, + error: this.formatError(e), + }; + } + } + public async list( + requestDetail: IRequestDetail, + options: WorkspaceFindOptions + ): ReturnType { + try { + const { + saved_objects: savedObjects, + ...others + } = await this.getSavedObjectClientsFromRequestDetail(requestDetail).find( + { + ...options, + type: WORKSPACE_TYPE, + } + ); + return { + success: true, + result: { + ...others, + workspaces: savedObjects.map((item) => this.getFlattenedResultWithSavedObject(item)), + }, + }; + } catch (e: unknown) { + return { + success: false, + error: this.formatError(e), + }; + } + } + public async get( + requestDetail: IRequestDetail, + id: string + ): Promise> { + try { + const result = await this.getSavedObjectClientsFromRequestDetail(requestDetail).get< + WorkspaceAttribute + >(WORKSPACE_TYPE, id); + return { + success: true, + result: this.getFlattenedResultWithSavedObject(result), + }; + } catch (e: unknown) { + return { + success: false, + error: this.formatError(e), + }; + } + } + public async update( + requestDetail: IRequestDetail, + id: string, + payload: Omit + ): Promise> { + const attributes = payload; + try { + const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); + const workspaceInDB: SavedObject = await client.get(WORKSPACE_TYPE, id); + if (workspaceInDB.attributes.name !== attributes.name) { + const existingWorkspaceRes = await this.internalSavedObjectsRepository?.find({ + type: WORKSPACE_TYPE, + search: attributes.name, + searchFields: ['name'], + fields: ['_id'], + }); + if (existingWorkspaceRes && existingWorkspaceRes.total > 0) { + throw new Error(DUPLICATE_WORKSPACE_NAME_ERROR); + } + } + await client.update>(WORKSPACE_TYPE, id, attributes, {}); + return { + success: true, + result: true, + }; + } catch (e: unknown) { + return { + success: false, + error: this.formatError(e), + }; + } + } + public async delete(requestDetail: IRequestDetail, id: string): Promise> { + try { + await this.getSavedObjectClientsFromRequestDetail(requestDetail).delete(WORKSPACE_TYPE, id); + return { + success: true, + result: true, + }; + } catch (e: unknown) { + return { + success: false, + error: this.formatError(e), + }; + } + } + public async destroy(): Promise> { + return { + success: true, + result: true, + }; + } +} From 2bc18b0b8a13a3e75e2328dba9a13eed6420c352 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 6 Sep 2023 14:36:46 +0800 Subject: [PATCH 26/43] feat: remove useless required plugins and logger typo Signed-off-by: SuZhou-Joe --- src/plugins/workspace/opensearch_dashboards.json | 2 +- src/plugins/workspace/server/plugin.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index a898536ebb79..ea2fe1cbed49 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -3,7 +3,7 @@ "version": "opensearchDashboards", "server": true, "ui": true, - "requiredPlugins": ["savedObjects"], + "requiredPlugins": [], "optionalPlugins": [], "requiredBundles": [] } diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index e15a9ef25f7e..515ccc399f8e 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -34,7 +34,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { } public start() { - this.logger.debug('Starting SavedObjects service'); + this.logger.debug('Starting Workspace service'); return { client: this.client as IWorkspaceDBImpl, From 56b3343092dd58b23f43a52d4ae6d51bf196366c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 7 Sep 2023 10:40:12 +0800 Subject: [PATCH 27/43] feat: setup public side skeleton Signed-off-by: SuZhou-Joe --- src/plugins/workspace/public/index.ts | 10 ++++++++++ src/plugins/workspace/public/plugin.ts | 17 +++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 src/plugins/workspace/public/index.ts create mode 100644 src/plugins/workspace/public/plugin.ts diff --git a/src/plugins/workspace/public/index.ts b/src/plugins/workspace/public/index.ts new file mode 100644 index 000000000000..99161a7edbd7 --- /dev/null +++ b/src/plugins/workspace/public/index.ts @@ -0,0 +1,10 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { WorkspacePlugin } from './plugin'; + +export function plugin() { + return new WorkspacePlugin(); +} diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts new file mode 100644 index 000000000000..933afa5858be --- /dev/null +++ b/src/plugins/workspace/public/plugin.ts @@ -0,0 +1,17 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { Plugin } from '../../../core/public'; + +export class WorkspacePlugin implements Plugin<{}, {}, {}> { + public async setup() { + return {}; + } + + public start() { + return {}; + } + + public stop() {} +} From 436e66fcd80e39dc5ca4311caca1d77c7596ae79 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 19 Sep 2023 18:14:35 +0800 Subject: [PATCH 28/43] temp: add unit test Signed-off-by: SuZhou-Joe --- test/api_integration/apis/workspace/index.ts | 39 ++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 test/api_integration/apis/workspace/index.ts diff --git a/test/api_integration/apis/workspace/index.ts b/test/api_integration/apis/workspace/index.ts new file mode 100644 index 000000000000..18b0f9b7f511 --- /dev/null +++ b/test/api_integration/apis/workspace/index.ts @@ -0,0 +1,39 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import expect from '@osd/expect'; +import { WorkspaceAttribute } from 'opensearch-dashboards/server'; +import { FtrProviderContext } from '../../ftr_provider_context'; + +const testWorkspace: WorkspaceAttribute = { + id: 'fake_id', + name: 'test_workspace', + description: 'test_workspace_description', +}; + +export default function ({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const opensearch = getService('legacyOpenSearch'); + + const MILLISECOND_IN_WEEK = 1000 * 60 * 60 * 24 * 7; + + describe('Workspace CRUD apis', () => { + it('basic CRUD', async () => { + const resp = await supertest + .post(`/api/workspaces`) + .set('osd-xsrf', 'opensearch-dashboards') + .send( + JSON.stringify({ + attributes: testWorkspace, + }) + ) + .expect(200); + + expect(resp.body).to.be.an('array'); + expect(resp.body.length).to.be.above(0); + expect(resp.body[0].status).to.be('not_installed'); + }); + }); +} From a62c38595b689e53e11ed9a7e838592fbd9c45ab Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 20 Sep 2023 09:42:02 +0000 Subject: [PATCH 29/43] feat: add function test for workspace CRUD routes Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/routes/index.ts | 10 +- test/api_integration/apis/index.js | 1 + test/api_integration/apis/workspace/index.ts | 121 ++++++++++++++++--- 3 files changed, 109 insertions(+), 23 deletions(-) diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index 39cc5877b469..f968c853fc90 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -55,15 +55,7 @@ export function registerRoutes({ return res.ok({ body: result }); } return res.ok({ - body: { - ...result, - result: { - ...result.result, - workspaces: result.result.workspaces.map((workspace) => ({ - ...workspace, - })), - }, - }, + body: result, }); }) ); diff --git a/test/api_integration/apis/index.js b/test/api_integration/apis/index.js index 54ffe6e774a5..2d870d88251d 100644 --- a/test/api_integration/apis/index.js +++ b/test/api_integration/apis/index.js @@ -45,5 +45,6 @@ export default function ({ loadTestFile }) { loadTestFile(require.resolve('./stats')); loadTestFile(require.resolve('./ui_metric')); loadTestFile(require.resolve('./telemetry')); + loadTestFile(require.resolve('./workspace')); }); } diff --git a/test/api_integration/apis/workspace/index.ts b/test/api_integration/apis/workspace/index.ts index 18b0f9b7f511..553ee0dce1af 100644 --- a/test/api_integration/apis/workspace/index.ts +++ b/test/api_integration/apis/workspace/index.ts @@ -5,6 +5,7 @@ import expect from '@osd/expect'; import { WorkspaceAttribute } from 'opensearch-dashboards/server'; +import { omit } from 'lodash'; import { FtrProviderContext } from '../../ftr_provider_context'; const testWorkspace: WorkspaceAttribute = { @@ -15,25 +16,117 @@ const testWorkspace: WorkspaceAttribute = { export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); - const opensearch = getService('legacyOpenSearch'); - - const MILLISECOND_IN_WEEK = 1000 * 60 * 60 * 24 * 7; describe('Workspace CRUD apis', () => { - it('basic CRUD', async () => { - const resp = await supertest - .post(`/api/workspaces`) + afterEach(async () => { + const listResult = await supertest + .post(`/api/workspaces/_list`) + .send({ + page: 1, + }) .set('osd-xsrf', 'opensearch-dashboards') - .send( - JSON.stringify({ - attributes: testWorkspace, - }) + .expect(200); + await Promise.all( + listResult.body.result.workspaces.map((item: WorkspaceAttribute) => + supertest + .delete(`/api/workspaces/${item.id}`) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200) ) + ); + }); + it('create', async () => { + await supertest + .post(`/api/workspaces`) + .send({ + attributes: testWorkspace, + }) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(400); + + const result: any = await supertest + .post(`/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200); + + expect(result.body.success).equal(true); + expect(result.body.result.id).to.be.a('string'); + }); + it('get', async () => { + const result = await supertest + .post(`/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200); + + const getResult = await supertest.get(`/api/workspaces/${result.body.result.id}`); + expect(getResult.body.result.name).equal(testWorkspace.name); + }); + it('update', async () => { + const result: any = await supertest + .post(`/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .set('osd-xsrf', 'opensearch-dashboards') .expect(200); - expect(resp.body).to.be.an('array'); - expect(resp.body.length).to.be.above(0); - expect(resp.body[0].status).to.be('not_installed'); + await supertest + .put(`/api/workspaces/${result.body.result.id}`) + .send({ + attributes: { + ...omit(testWorkspace, 'id'), + name: 'updated', + }, + }) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200); + + const getResult = await supertest.get(`/api/workspaces/${result.body.result.id}`); + + expect(getResult.body.success).equal(true); + expect(getResult.body.result.name).equal('updated'); + }); + it('delete', async () => { + const result: any = await supertest + .post(`/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200); + + await supertest + .delete(`/api/workspaces/${result.body.result.id}`) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200); + + const getResult = await supertest.get(`/api/workspaces/${result.body.result.id}`); + + expect(getResult.body.success).equal(false); + }); + it('list', async () => { + await supertest + .post(`/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200); + + const listResult = await supertest + .post(`/api/workspaces/_list`) + .send({ + page: 1, + }) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200); + expect(listResult.body.result.total).equal(1); }); - }); + }).tags('is:workspace'); } From 338c3706e32565d7c31f0f5e45e3dec1911fe28b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 20 Sep 2023 09:53:03 +0000 Subject: [PATCH 30/43] feat: use saved objects client instead of internal repository Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 11 ++++++++-- src/plugins/workspace/server/types.ts | 4 ++-- .../workspace/server/workspace_client.ts | 20 +++++++++++-------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 515ccc399f8e..568f536d65e8 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -2,7 +2,13 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import { PluginInitializerContext, CoreSetup, Plugin, Logger } from '../../../core/server'; +import { + PluginInitializerContext, + CoreSetup, + Plugin, + Logger, + CoreStart, +} from '../../../core/server'; import { IWorkspaceDBImpl } from './types'; import { WorkspaceClientWithSavedObject } from './workspace_client'; import { registerRoutes } from './routes'; @@ -33,8 +39,9 @@ export class WorkspacePlugin implements Plugin<{}, {}> { }; } - public start() { + public start(core: CoreStart) { this.logger.debug('Starting Workspace service'); + this.client?.setSavedObjects(core.savedObjects); return { client: this.client as IWorkspaceDBImpl, diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index d57cecd4a3c9..28d8c25fd3b0 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -9,7 +9,7 @@ import { SavedObjectsFindResponse, CoreSetup, WorkspaceAttribute, - ISavedObjectsRepository, + SavedObjectsServiceStart, } from '../../../core/server'; export interface WorkspaceFindOptions { @@ -29,7 +29,7 @@ export interface IRequestDetail { export interface IWorkspaceDBImpl { setup(dep: CoreSetup): Promise>; - setInternalRepository(repository: ISavedObjectsRepository): void; + setSavedObjects(savedObjects: SavedObjectsServiceStart): void; create( requestDetail: IRequestDetail, payload: Omit diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index ec49daf83b6a..e8e6b01c85c9 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -8,7 +8,7 @@ import type { SavedObjectsClientContract, CoreSetup, WorkspaceAttribute, - ISavedObjectsRepository, + SavedObjectsServiceStart, } from '../../../core/server'; import { WORKSPACE_TYPE } from '../../../core/server'; import { IWorkspaceDBImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; @@ -23,15 +23,16 @@ const DUPLICATE_WORKSPACE_NAME_ERROR = i18n.translate('workspace.duplicate.name. export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { private setupDep: CoreSetup; - - private internalSavedObjectsRepository?: ISavedObjectsRepository; - setInternalRepository(repository: ISavedObjectsRepository) { - this.internalSavedObjectsRepository = repository; - } + private savedObjects?: SavedObjectsServiceStart; constructor(core: CoreSetup) { this.setupDep = core; } + + private getScopedClient(requestDetail: IRequestDetail): SavedObjectsClientContract | undefined { + return this.savedObjects?.getScopedClient(requestDetail.request); + } + private getSavedObjectClientsFromRequestDetail( requestDetail: IRequestDetail ): SavedObjectsClientContract { @@ -63,7 +64,7 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { const attributes = payload; const id = generateRandomId(WORKSPACE_ID_SIZE); const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); - const existingWorkspaceRes = await this.internalSavedObjectsRepository?.find({ + const existingWorkspaceRes = await this.getScopedClient(requestDetail)?.find({ type: WORKSPACE_TYPE, search: attributes.name, searchFields: ['name'], @@ -148,7 +149,7 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); const workspaceInDB: SavedObject = await client.get(WORKSPACE_TYPE, id); if (workspaceInDB.attributes.name !== attributes.name) { - const existingWorkspaceRes = await this.internalSavedObjectsRepository?.find({ + const existingWorkspaceRes = await this.getScopedClient(requestDetail)?.find({ type: WORKSPACE_TYPE, search: attributes.name, searchFields: ['name'], @@ -184,6 +185,9 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { }; } } + public setSavedObjects(savedObjects: SavedObjectsServiceStart) { + this.savedObjects = savedObjects; + } public async destroy(): Promise> { return { success: true, From 6a80cced6e1c8df6386a5f1508f33c63dc66adae Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 20 Sep 2023 17:59:11 +0800 Subject: [PATCH 31/43] feat: update CHANGELOG Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index be2777365bd4..7c70f9824b99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854)) - [Discover] Update embeddable for saved searches ([#5081](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5081)) - [Workspace] Add core workspace service module to enable the implementation of workspace features within OSD plugins ([#5092](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5092)) +- [Workspace] Setup workspace skeleton and implement basic CRUD API ([#5075](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5075)) - [Workspace] Optional workspaces params in repository ([#5162](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5162)) ### 🐛 Bug Fixes From 8df65068fc3b5cb2f60e265bc58d63a7f3a0ef5c Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 20 Sep 2023 10:07:58 +0000 Subject: [PATCH 32/43] feat: exclude permission check wrapper Signed-off-by: SuZhou-Joe --- src/plugins/workspace/common/constants.ts | 6 +++++ .../workspace/server/workspace_client.ts | 25 +++++++++++++------ 2 files changed, 23 insertions(+), 8 deletions(-) create mode 100644 src/plugins/workspace/common/constants.ts diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts new file mode 100644 index 000000000000..b6bd7b00f676 --- /dev/null +++ b/src/plugins/workspace/common/constants.ts @@ -0,0 +1,6 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace'; diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index e8e6b01c85c9..9dcbc2906d43 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -14,6 +14,7 @@ import { WORKSPACE_TYPE } from '../../../core/server'; import { IWorkspaceDBImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; const WORKSPACE_ID_SIZE = 6; @@ -29,8 +30,12 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { this.setupDep = core; } - private getScopedClient(requestDetail: IRequestDetail): SavedObjectsClientContract | undefined { - return this.savedObjects?.getScopedClient(requestDetail.request); + private getScopedClientWithoutPermission( + requestDetail: IRequestDetail + ): SavedObjectsClientContract | undefined { + return this.savedObjects?.getScopedClient(requestDetail.request, { + excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], + }); } private getSavedObjectClientsFromRequestDetail( @@ -64,11 +69,13 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { const attributes = payload; const id = generateRandomId(WORKSPACE_ID_SIZE); const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); - const existingWorkspaceRes = await this.getScopedClient(requestDetail)?.find({ - type: WORKSPACE_TYPE, - search: attributes.name, - searchFields: ['name'], - }); + const existingWorkspaceRes = await this.getScopedClientWithoutPermission(requestDetail)?.find( + { + type: WORKSPACE_TYPE, + search: attributes.name, + searchFields: ['name'], + } + ); if (existingWorkspaceRes && existingWorkspaceRes.total > 0) { throw new Error(DUPLICATE_WORKSPACE_NAME_ERROR); } @@ -149,7 +156,9 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); const workspaceInDB: SavedObject = await client.get(WORKSPACE_TYPE, id); if (workspaceInDB.attributes.name !== attributes.name) { - const existingWorkspaceRes = await this.getScopedClient(requestDetail)?.find({ + const existingWorkspaceRes = await this.getScopedClientWithoutPermission( + requestDetail + )?.find({ type: WORKSPACE_TYPE, search: attributes.name, searchFields: ['name'], From c4588d4d3a2b65ca5aeb3e4c4fbbd3d3dc4a702b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 21 Sep 2023 13:20:29 +0800 Subject: [PATCH 33/43] feat: add integration test Signed-off-by: SuZhou-Joe --- .../server/integration_tests/routes.test.ts | 141 ++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 src/plugins/workspace/server/integration_tests/routes.test.ts diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts new file mode 100644 index 000000000000..bdb342e00ade --- /dev/null +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -0,0 +1,141 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { WorkspaceAttribute } from 'src/core/types'; +import { omit } from 'lodash'; +import * as osdTestServer from '../../../../core/test_helpers/osd_server'; + +const testWorkspace: WorkspaceAttribute = { + id: 'fake_id', + name: 'test_workspace', + description: 'test_workspace_description', +}; + +describe('workspace service', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + beforeAll(async () => { + const { startOpenSearch } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + }); + opensearchServer = await startOpenSearch(); + root = osdTestServer.createRootWithCorePlugins(); + + await root.setup(); + await root.start(); + }, 30000); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + describe('Workspace CRUD apis', () => { + afterEach(async () => { + const listResult = await osdTestServer.request + .post(root, `/api/workspaces/_list`) + .send({ + page: 1, + }) + .expect(200); + await Promise.all( + listResult.body.result.workspaces.map((item: WorkspaceAttribute) => + osdTestServer.request.delete(root, `/api/workspaces/${item.id}`).expect(200) + ) + ); + }); + it('create', async () => { + await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: testWorkspace, + }) + .expect(400); + + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .expect(200); + + expect(result.body.success).toEqual(true); + expect(typeof result.body.result.id).toBe('string'); + }); + it('get', async () => { + const result = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .expect(200); + + const getResult = await osdTestServer.request.get( + root, + `/api/workspaces/${result.body.result.id}` + ); + expect(getResult.body.result.name).toEqual(testWorkspace.name); + }); + it('update', async () => { + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .expect(200); + + await osdTestServer.request + .put(root, `/api/workspaces/${result.body.result.id}`) + .send({ + attributes: { + ...omit(testWorkspace, 'id'), + name: 'updated', + }, + }) + .expect(200); + + const getResult = await osdTestServer.request.get( + root, + `/api/workspaces/${result.body.result.id}` + ); + + expect(getResult.body.success).toEqual(true); + expect(getResult.body.result.name).toEqual('updated'); + }); + it('delete', async () => { + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .expect(200); + + await osdTestServer.request + .delete(root, `/api/workspaces/${result.body.result.id}`) + .expect(200); + + const getResult = await osdTestServer.request.get( + root, + `/api/workspaces/${result.body.result.id}` + ); + + expect(getResult.body.success).toEqual(false); + }); + it('list', async () => { + await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omit(testWorkspace, 'id'), + }) + .expect(200); + + const listResult = await osdTestServer.request + .post(root, `/api/workspaces/_list`) + .send({ + page: 1, + }) + .expect(200); + expect(listResult.body.result.total).toEqual(1); + }); + }); +}); From c11f0aa7bf72545bc6f6d4cb38da20671a214b85 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 21 Sep 2023 13:43:01 +0800 Subject: [PATCH 34/43] feat: add configuration Signed-off-by: SuZhou-Joe --- src/plugins/workspace/config.ts | 12 ++++++++++++ src/plugins/workspace/server/index.ts | 9 ++++++++- .../workspace/server/saved_objects/workspace.ts | 11 +++++++---- 3 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 src/plugins/workspace/config.ts diff --git a/src/plugins/workspace/config.ts b/src/plugins/workspace/config.ts new file mode 100644 index 000000000000..79412f5c02ee --- /dev/null +++ b/src/plugins/workspace/config.ts @@ -0,0 +1,12 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { schema, TypeOf } from '@osd/config-schema'; + +export const configSchema = schema.object({ + enabled: schema.boolean({ defaultValue: false }), +}); + +export type ConfigSchema = TypeOf; diff --git a/src/plugins/workspace/server/index.ts b/src/plugins/workspace/server/index.ts index 936d3cf3ecec..9447b7c6dc8c 100644 --- a/src/plugins/workspace/server/index.ts +++ b/src/plugins/workspace/server/index.ts @@ -2,8 +2,9 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import { PluginInitializerContext } from '../../../core/server'; +import { PluginConfigDescriptor, PluginInitializerContext } from '../../../core/server'; import { WorkspacePlugin } from './plugin'; +import { configSchema } from '../config'; // This exports static code and TypeScript types, // as well as, OpenSearch Dashboards Platform `plugin()` initializer. @@ -11,3 +12,9 @@ import { WorkspacePlugin } from './plugin'; export function plugin(initializerContext: PluginInitializerContext) { return new WorkspacePlugin(initializerContext); } + +export const config: PluginConfigDescriptor = { + schema: configSchema, +}; + +export { WorkspaceFindOptions } from './types'; diff --git a/src/plugins/workspace/server/saved_objects/workspace.ts b/src/plugins/workspace/server/saved_objects/workspace.ts index 5142185b0c2d..a26e695d83cb 100644 --- a/src/plugins/workspace/server/saved_objects/workspace.ts +++ b/src/plugins/workspace/server/saved_objects/workspace.ts @@ -25,16 +25,19 @@ export const workspace: SavedObjectsType = { * In opensearch, string[] is also mapped to text */ features: { - type: 'text', + type: 'keyword', }, color: { - type: 'text', + type: 'keyword', }, icon: { - type: 'text', + type: 'keyword', }, defaultVISTheme: { - type: 'text', + type: 'keyword', + }, + reserved: { + type: 'boolean', }, }, }, From a9456152a446a87195c9367a6cd20488275affd5 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 21 Sep 2023 15:02:28 +0800 Subject: [PATCH 35/43] feat: enable workspace flag when run workspace related test Signed-off-by: SuZhou-Joe --- .../server/integration_tests/routes.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index bdb342e00ade..e4d29b86ac55 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -17,14 +17,19 @@ describe('workspace service', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; beforeAll(async () => { - const { startOpenSearch } = osdTestServer.createTestServers({ + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + osd: { + workspace: { + enabled: true, + }, + }, + }, }); opensearchServer = await startOpenSearch(); - root = osdTestServer.createRootWithCorePlugins(); - - await root.setup(); - await root.start(); + const startOSDResp = await startOpenSearchDashboards(); + root = startOSDResp.root; }, 30000); afterAll(async () => { await root.shutdown(); From 32b44e240bab420377437911067f40196560cd2e Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Sep 2023 10:55:04 +0800 Subject: [PATCH 36/43] feat: optimization according to PR comments Signed-off-by: SuZhou-Joe --- .../workspace/opensearch_dashboards.json | 6 +++-- src/plugins/workspace/public/plugin.ts | 1 + src/plugins/workspace/server/index.ts | 1 + src/plugins/workspace/server/plugin.ts | 13 +++++----- src/plugins/workspace/server/routes/index.ts | 24 +++++-------------- .../server/saved_objects/workspace.ts | 3 --- src/plugins/workspace/server/types.ts | 3 ++- .../workspace/server/workspace_client.ts | 9 +++---- test/api_integration/apis/workspace/index.ts | 18 ++++++++------ 9 files changed, 37 insertions(+), 41 deletions(-) diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index ea2fe1cbed49..40a7eb5c3f9f 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -2,8 +2,10 @@ "id": "workspace", "version": "opensearchDashboards", "server": true, - "ui": true, - "requiredPlugins": [], + "ui": false, + "requiredPlugins": [ + "savedObjects" + ], "optionalPlugins": [], "requiredBundles": [] } diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index 933afa5858be..18e84e3a6f35 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -2,6 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ + import { Plugin } from '../../../core/public'; export class WorkspacePlugin implements Plugin<{}, {}, {}> { diff --git a/src/plugins/workspace/server/index.ts b/src/plugins/workspace/server/index.ts index 9447b7c6dc8c..fe44b4d71757 100644 --- a/src/plugins/workspace/server/index.ts +++ b/src/plugins/workspace/server/index.ts @@ -2,6 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ + import { PluginConfigDescriptor, PluginInitializerContext } from '../../../core/server'; import { WorkspacePlugin } from './plugin'; import { configSchema } from '../config'; diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 568f536d65e8..38e8a3c18f8c 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -2,6 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ + import { PluginInitializerContext, CoreSetup, @@ -9,13 +10,13 @@ import { Logger, CoreStart, } from '../../../core/server'; -import { IWorkspaceDBImpl } from './types'; -import { WorkspaceClientWithSavedObject } from './workspace_client'; +import { IWorkspaceClientImpl } from './types'; +import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; - private client?: IWorkspaceDBImpl; + private client?: IWorkspaceClientImpl; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); @@ -24,14 +25,14 @@ export class WorkspacePlugin implements Plugin<{}, {}> { public async setup(core: CoreSetup) { this.logger.debug('Setting up Workspaces service'); - this.client = new WorkspaceClientWithSavedObject(core); + this.client = new WorkspaceClient(core); await this.client.setup(core); registerRoutes({ http: core.http, logger: this.logger, - client: this.client as IWorkspaceDBImpl, + client: this.client as IWorkspaceClientImpl, }); return { @@ -44,7 +45,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.client?.setSavedObjects(core.savedObjects); return { - client: this.client as IWorkspaceDBImpl, + client: this.client as IWorkspaceClientImpl, }; } diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index f968c853fc90..5789aa0481fa 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -2,10 +2,10 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ -import { schema } from '@osd/config-schema'; +import { schema } from '@osd/config-schema'; import { CoreSetup, Logger } from '../../../../core/server'; -import { IWorkspaceDBImpl } from '../types'; +import { IWorkspaceClientImpl } from '../types'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -23,7 +23,7 @@ export function registerRoutes({ logger, http, }: { - client: IWorkspaceDBImpl; + client: IWorkspaceClientImpl; logger: Logger; http: CoreSetup['http']; }) { @@ -78,17 +78,9 @@ export function registerRoutes({ }, id ); - if (!result.success) { - return res.ok({ body: result }); - } return res.ok({ - body: { - ...result, - result: { - ...result.result, - }, - }, + body: result, }); }) ); @@ -110,9 +102,7 @@ export function registerRoutes({ request: req, logger, }, - { - ...attributes, - } + attributes ); return res.ok({ body: result }); }) @@ -140,9 +130,7 @@ export function registerRoutes({ logger, }, id, - { - ...attributes, - } + attributes ); return res.ok({ body: result }); }) diff --git a/src/plugins/workspace/server/saved_objects/workspace.ts b/src/plugins/workspace/server/saved_objects/workspace.ts index a26e695d83cb..af90a0faae5c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace.ts +++ b/src/plugins/workspace/server/saved_objects/workspace.ts @@ -21,9 +21,6 @@ export const workspace: SavedObjectsType = { description: { type: 'text', }, - /** - * In opensearch, string[] is also mapped to text - */ features: { type: 'keyword', }, diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index 28d8c25fd3b0..f51de6b7c988 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -2,6 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ + import { Logger, OpenSearchDashboardsRequest, @@ -27,7 +28,7 @@ export interface IRequestDetail { logger: Logger; } -export interface IWorkspaceDBImpl { +export interface IWorkspaceClientImpl { setup(dep: CoreSetup): Promise>; setSavedObjects(savedObjects: SavedObjectsServiceStart): void; create( diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 9dcbc2906d43..295a7f3981f6 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -2,6 +2,7 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ + import { i18n } from '@osd/i18n'; import type { SavedObject, @@ -11,7 +12,7 @@ import type { SavedObjectsServiceStart, } from '../../../core/server'; import { WORKSPACE_TYPE } from '../../../core/server'; -import { IWorkspaceDBImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; +import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; @@ -22,7 +23,7 @@ const DUPLICATE_WORKSPACE_NAME_ERROR = i18n.translate('workspace.duplicate.name. defaultMessage: 'workspace name has already been used, try with a different name', }); -export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { +export class WorkspaceClient implements IWorkspaceClientImpl { private setupDep: CoreSetup; private savedObjects?: SavedObjectsServiceStart; @@ -64,7 +65,7 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { public async create( requestDetail: IRequestDetail, payload: Omit - ): ReturnType { + ): ReturnType { try { const attributes = payload; const id = generateRandomId(WORKSPACE_ID_SIZE); @@ -102,7 +103,7 @@ export class WorkspaceClientWithSavedObject implements IWorkspaceDBImpl { public async list( requestDetail: IRequestDetail, options: WorkspaceFindOptions - ): ReturnType { + ): ReturnType { try { const { saved_objects: savedObjects, diff --git a/test/api_integration/apis/workspace/index.ts b/test/api_integration/apis/workspace/index.ts index 553ee0dce1af..94138bb47f00 100644 --- a/test/api_integration/apis/workspace/index.ts +++ b/test/api_integration/apis/workspace/index.ts @@ -5,9 +5,13 @@ import expect from '@osd/expect'; import { WorkspaceAttribute } from 'opensearch-dashboards/server'; -import { omit } from 'lodash'; import { FtrProviderContext } from '../../ftr_provider_context'; +const omitId = (object: T): Omit => { + const { id, ...others } = object; + return others; +}; + const testWorkspace: WorkspaceAttribute = { id: 'fake_id', name: 'test_workspace', @@ -47,7 +51,7 @@ export default function ({ getService }: FtrProviderContext) { const result: any = await supertest .post(`/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .set('osd-xsrf', 'opensearch-dashboards') .expect(200); @@ -59,7 +63,7 @@ export default function ({ getService }: FtrProviderContext) { const result = await supertest .post(`/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .set('osd-xsrf', 'opensearch-dashboards') .expect(200); @@ -71,7 +75,7 @@ export default function ({ getService }: FtrProviderContext) { const result: any = await supertest .post(`/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .set('osd-xsrf', 'opensearch-dashboards') .expect(200); @@ -80,7 +84,7 @@ export default function ({ getService }: FtrProviderContext) { .put(`/api/workspaces/${result.body.result.id}`) .send({ attributes: { - ...omit(testWorkspace, 'id'), + ...omitId(testWorkspace), name: 'updated', }, }) @@ -96,7 +100,7 @@ export default function ({ getService }: FtrProviderContext) { const result: any = await supertest .post(`/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .set('osd-xsrf', 'opensearch-dashboards') .expect(200); @@ -114,7 +118,7 @@ export default function ({ getService }: FtrProviderContext) { await supertest .post(`/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .set('osd-xsrf', 'opensearch-dashboards') .expect(200); From 83c3258948fbff9f459fe4a9a1f3e91f2a7d7fae Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 4 Oct 2023 13:59:48 +0800 Subject: [PATCH 37/43] feat: add JSDoc for workspace client Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/types.ts | 53 +++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index f51de6b7c988..0f60597a7a8a 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -29,12 +29,38 @@ export interface IRequestDetail { } export interface IWorkspaceClientImpl { + /** + * Setup function for workspace client, need to be called before any other methods. + * @param dep {@link CoreSetup} + * @returns a promise indicate if the setup has successed. + * @public + */ setup(dep: CoreSetup): Promise>; + /** + * Set saved objects client that will be used inside the workspace client. + * @param savedObjects {@link SavedObjectsServiceStart} + * @returns void + * @public + */ setSavedObjects(savedObjects: SavedObjectsServiceStart): void; + /** + * Create a workspace + * @param requestDetail {@link IRequestDetail} + * @param payload {@link WorkspaceAttribute} + * @returns a Promise with a new-created id for the workspace + * @public + */ create( requestDetail: IRequestDetail, payload: Omit ): Promise>; + /** + * List workspaces + * @param requestDetail {@link IRequestDetail} + * @param options {@link WorkspaceFindOptions} + * @returns a Promise with workspaces list + * @public + */ list( requestDetail: IRequestDetail, options: WorkspaceFindOptions @@ -45,13 +71,40 @@ export interface IWorkspaceClientImpl { } & Pick > >; + /** + * Get the detail of a given workspace id + * @param requestDetail {@link IRequestDetail} + * @param id workspace id + * @returns a Promise with the detail of {@link WorkspaceAttribute} + * @public + */ get(requestDetail: IRequestDetail, id: string): Promise>; + /** + * Update the detail of a given workspace + * @param requestDetail {@link IRequestDetail} + * @param id workspace id + * @param payload {@link WorkspaceAttribute} + * @returns a Promise with a boolean result indicating if the update operation successed. + * @public + */ update( requestDetail: IRequestDetail, id: string, payload: Omit ): Promise>; + /** + * Delete a given workspace + * @param requestDetail {@link IRequestDetail} + * @param id workspace id + * @returns a Promise with a boolean result indicating if the delete operation successed. + * @public + */ delete(requestDetail: IRequestDetail, id: string): Promise>; + /** + * Destroy the workspace client, should be called after the server disposes. + * @returns a Promise with a boolean result indicating if the destroy operation successed. + * @public + */ destroy(): Promise>; } From 9f350dd1ced130a79467def48c27da7fb6c13a1b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 13:01:56 +0800 Subject: [PATCH 38/43] Update src/plugins/workspace/server/integration_tests/routes.test.ts Co-authored-by: Josh Romero Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/integration_tests/routes.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index e4d29b86ac55..3fc8fc8484a8 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -35,7 +35,7 @@ describe('workspace service', () => { await root.shutdown(); await opensearchServer.stop(); }); - describe('Workspace CRUD apis', () => { + describe('Workspace CRUD APIs', () => { afterEach(async () => { const listResult = await osdTestServer.request .post(root, `/api/workspaces/_list`) From ceb3960a355e284d520331787afafb6906e742a0 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 13:32:14 +0800 Subject: [PATCH 39/43] feat: remove hard-coded delay Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/integration_tests/routes.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 3fc8fc8484a8..b8fdfaa98435 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -30,7 +30,7 @@ describe('workspace service', () => { opensearchServer = await startOpenSearch(); const startOSDResp = await startOpenSearchDashboards(); root = startOSDResp.root; - }, 30000); + }); afterAll(async () => { await root.shutdown(); await opensearchServer.stop(); From 970549350855eceda1ebd4231492c288a6851a51 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 17 Oct 2023 13:51:02 +0800 Subject: [PATCH 40/43] feat: optimize unit test Signed-off-by: SuZhou-Joe --- .../server/integration_tests/routes.test.ts | 28 ++++++++++++++----- test/api_integration/apis/workspace/index.ts | 15 ++++++++-- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index b8fdfaa98435..0f6c027ea61d 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -4,9 +4,13 @@ */ import { WorkspaceAttribute } from 'src/core/types'; -import { omit } from 'lodash'; import * as osdTestServer from '../../../../core/test_helpers/osd_server'; +const omitId = (object: T): Omit => { + const { id, ...others } = object; + return others; +}; + const testWorkspace: WorkspaceAttribute = { id: 'fake_id', name: 'test_workspace', @@ -60,7 +64,7 @@ describe('workspace service', () => { const result: any = await osdTestServer.request .post(root, `/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .expect(200); @@ -71,7 +75,7 @@ describe('workspace service', () => { const result = await osdTestServer.request .post(root, `/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .expect(200); @@ -85,7 +89,7 @@ describe('workspace service', () => { const result: any = await osdTestServer.request .post(root, `/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .expect(200); @@ -93,7 +97,7 @@ describe('workspace service', () => { .put(root, `/api/workspaces/${result.body.result.id}`) .send({ attributes: { - ...omit(testWorkspace, 'id'), + ...omitId(testWorkspace), name: 'updated', }, }) @@ -111,7 +115,7 @@ describe('workspace service', () => { const result: any = await osdTestServer.request .post(root, `/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), }) .expect(200); @@ -130,7 +134,17 @@ describe('workspace service', () => { await osdTestServer.request .post(root, `/api/workspaces`) .send({ - attributes: omit(testWorkspace, 'id'), + attributes: omitId(testWorkspace), + }) + .expect(200); + + await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: { + ...omitId(testWorkspace), + name: 'another test workspace', + }, }) .expect(200); diff --git a/test/api_integration/apis/workspace/index.ts b/test/api_integration/apis/workspace/index.ts index 94138bb47f00..9d4919b852bf 100644 --- a/test/api_integration/apis/workspace/index.ts +++ b/test/api_integration/apis/workspace/index.ts @@ -123,6 +123,17 @@ export default function ({ getService }: FtrProviderContext) { .set('osd-xsrf', 'opensearch-dashboards') .expect(200); + await supertest + .post(`/api/workspaces`) + .send({ + attributes: { + ...omitId(testWorkspace), + name: 'another test workspace', + }, + }) + .set('osd-xsrf', 'opensearch-dashboards') + .expect(200); + const listResult = await supertest .post(`/api/workspaces/_list`) .send({ @@ -130,7 +141,7 @@ export default function ({ getService }: FtrProviderContext) { }) .set('osd-xsrf', 'opensearch-dashboards') .expect(200); - expect(listResult.body.result.total).equal(1); + expect(listResult.body.result.total).equal(2); }); - }).tags('is:workspace'); + }); } From 721ed4aa04e5e911b1182831d14bdffa72a51599 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 10:32:03 +0800 Subject: [PATCH 41/43] fix: unit test Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/integration_tests/routes.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 0f6c027ea61d..a2ce8161786e 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -154,7 +154,7 @@ describe('workspace service', () => { page: 1, }) .expect(200); - expect(listResult.body.result.total).toEqual(1); + expect(listResult.body.result.total).toEqual(2); }); }); }); From 2a8635eafa7128fe162df3042018001c8dc2d335 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 16:22:34 +0800 Subject: [PATCH 42/43] refact: move workspace specific logic to savedObjectWrapper Signed-off-by: SuZhou-Joe --- .../saved_objects/service/lib/repository.ts | 90 +----- .../service/saved_objects_client.ts | 8 + src/plugins/workspace/common/constants.ts | 2 + src/plugins/workspace/server/plugin.ts | 12 + ...pper_for_check_workspace_conflict.test.ts} | 14 +- ...ts_wrapper_for_check_workspace_conflict.ts | 257 ++++++++++++++++++ 6 files changed, 297 insertions(+), 86 deletions(-) rename src/{core/server/saved_objects/service/lib/integration_tests/repository.test.ts => plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts} (96%) create mode 100644 src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 45997e6cf6b9..15e32336eb3b 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -280,29 +280,6 @@ export class SavedObjectsRepository { } } - let savedObjectWorkspaces = workspaces; - - if (id && overwrite && workspaces) { - let currentItem; - try { - currentItem = await this.get(type, id); - } catch (e) { - // this.get will throw an error when no items can be found - } - if (currentItem) { - if ( - SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - workspaces, - currentItem.workspaces - ).length - ) { - throw SavedObjectsErrorHelpers.createConflictError(type, id); - } else { - savedObjectWorkspaces = currentItem.workspaces; - } - } - } - const migrated = this._migrator.migrateDocument({ id, type, @@ -313,7 +290,7 @@ export class SavedObjectsRepository { migrationVersion, updated_at: time, ...(Array.isArray(references) && { references }), - ...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }), + ...(Array.isArray(workspaces) && { workspaces }), }); const raw = this._serializer.savedObjectToRaw(migrated as SavedObjectSanitizedDoc); @@ -380,16 +357,12 @@ export class SavedObjectsRepository { const method = object.id && overwrite ? 'index' : 'create'; const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type); - /** - * It requires a check when overwriting objects to target workspaces - */ - const requiresWorkspaceCheck = !!(object.id && options.workspaces); if (object.id == null) object.id = uuid.v1(); let opensearchRequestIndexPayload = {}; - if (requiresNamespacesCheck || requiresWorkspaceCheck) { + if (requiresNamespacesCheck) { opensearchRequestIndexPayload = { opensearchRequestIndex: bulkGetRequestIndexCounter, }; @@ -412,7 +385,7 @@ export class SavedObjectsRepository { .map(({ value: { object: { type, id } } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces', 'workspaces'], + _source: ['type', 'namespaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( @@ -479,45 +452,7 @@ export class SavedObjectsRepository { let savedObjectWorkspaces: string[] | undefined = options.workspaces; if (expectedBulkGetResult.value.method !== 'create') { - const rawId = this._serializer.generateRawId(namespace, object.type, object.id); - const findObject = - bulkGetResponse?.statusCode !== 404 - ? bulkGetResponse?.body.docs?.find((item) => item._id === rawId) - : null; - /** - * 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 - ) as SavedObject; - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - options.workspaces, - 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', - error: { - id, - type, - error: { - ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), - metadata: { isNotOverwritable: true }, - }, - }, - }; - } else { - savedObjectWorkspaces = transformedObject.workspaces; - } - } + savedObjectWorkspaces = object.workspaces; } const expectedResult = { @@ -534,7 +469,7 @@ export class SavedObjectsRepository { updated_at: time, references: object.references || [], originId: object.originId, - workspaces: savedObjectWorkspaces, + ...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }), }) as SavedObjectSanitizedDoc ), }; @@ -632,7 +567,7 @@ export class SavedObjectsRepository { const bulkGetDocs = expectedBulkGetResults.filter(isRight).map(({ value: { type, id } }) => ({ _id: this._serializer.generateRawId(namespace, type, id), _index: this.getIndexForType(type), - _source: ['type', 'namespaces', 'workspaces'], + _source: ['type', 'namespaces'], })); const bulkGetResponse = bulkGetDocs.length ? await this.client.mget( @@ -655,24 +590,13 @@ export class SavedObjectsRepository { const { type, id, opensearchRequestIndex } = expectedResult.value; const doc = bulkGetResponse?.body.docs[opensearchRequestIndex]; if (doc?.found) { - let workspaceConflict = false; - if (options.workspaces) { - const transformedObject = this._serializer.rawToSavedObject(doc as SavedObjectsRawDoc); - const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( - options.workspaces, - transformedObject.workspaces - ); - if (filteredWorkspaces.length) { - workspaceConflict = true; - } - } errors.push({ id, type, error: { ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), // @ts-expect-error MultiGetHit._source is optional - ...((!this.rawDocExistsInNamespace(doc!, namespace) || workspaceConflict) && { + ...(!this.rawDocExistsInNamespace(doc!, namespace) && { metadata: { isNotOverwritable: true }, }), }, 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 a087dc6c388a..19c337b9172f 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -68,6 +68,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions { * Note: this can only be used for multi-namespace object types. */ initialNamespaces?: string[]; + /** + * workspaces the new created objects belong to + */ + workspaces?: string[]; } /** @@ -91,6 +95,10 @@ export interface SavedObjectsBulkCreateObject { * Note: this can only be used for multi-namespace object types. */ initialNamespaces?: string[]; + /** + * workspaces the objects belong to, will only be used when overwrite is enabled. + */ + workspaces?: string[]; } /** diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index b6bd7b00f676..e60bb6aea0eb 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -4,3 +4,5 @@ */ export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace'; +export const WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID = + 'workspace_conflict_control'; diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 38e8a3c18f8c..daaed047d15f 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -13,10 +13,13 @@ import { import { IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; +import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; +import { WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; private client?: IWorkspaceClientImpl; + private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); @@ -29,6 +32,14 @@ export class WorkspacePlugin implements Plugin<{}, {}> { await this.client.setup(core); + this.workspaceConflictControl = new WorkspaceConflictSavedObjectsClientWrapper(); + + core.savedObjects.addClientWrapper( + -1, + WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + this.workspaceConflictControl.wrapperFactory + ); + registerRoutes({ http: core.http, logger: this.logger, @@ -43,6 +54,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { public start(core: CoreStart) { this.logger.debug('Starting Workspace service'); this.client?.setSavedObjects(core.savedObjects); + this.workspaceConflictControl?.setSerializer(core.savedObjects.createSerializer()); return { client: this.client as IWorkspaceClientImpl, diff --git a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts similarity index 96% rename from src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts rename to src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts index b601de985dc0..b41f5a9eb08b 100644 --- a/src/core/server/saved_objects/service/lib/integration_tests/repository.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/saved_objects_wrapper_for_check_workspace_conflict.test.ts @@ -2,10 +2,11 @@ * 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'; +import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; const dashboard: Omit = { type: 'dashboard', @@ -13,12 +14,19 @@ const dashboard: Omit = { references: [], }; -describe('repository integration test', () => { +describe('saved_objects_wrapper_for_check_workspace_conflict integration test', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; beforeAll(async () => { const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + osd: { + workspace: { + enabled: true, + }, + }, + }, }); opensearchServer = await startOpenSearch(); const startOSDResp = await startOpenSearchDashboards(); @@ -35,7 +43,7 @@ describe('repository integration test', () => { (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`)) .statusCode ) - ); + ).toEqual(true); }; const getItem = async (object: Pick) => { diff --git a/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts new file mode 100644 index 000000000000..6d8392f3b191 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts @@ -0,0 +1,257 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import Boom from '@hapi/boom'; +import { + SavedObject, + SavedObjectsBaseOptions, + SavedObjectsBulkCreateObject, + SavedObjectsBulkResponse, + SavedObjectsClientWrapperFactory, + SavedObjectsCreateOptions, + SavedObjectsErrorHelpers, + SavedObjectsUtils, + SavedObjectsSerializer, + SavedObjectsCheckConflictsObject, + SavedObjectsCheckConflictsResponse, +} from '../../../../core/server'; + +const errorContent = (error: Boom.Boom) => error.output.payload; + +export class WorkspaceConflictSavedObjectsClientWrapper { + private _serializer?: SavedObjectsSerializer; + public setSerializer(serializer: SavedObjectsSerializer) { + this._serializer = serializer; + } + private getRawId(props: { namespace?: string; id: string; type: string }) { + return ( + this._serializer?.generateRawId(props.namespace, props.type, props.id) || + `${props.type}:${props.id}` + ); + } + public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { + const createWithWorkspaceConflictCheck = async ( + type: string, + attributes: T, + options: SavedObjectsCreateOptions = {} + ) => { + const { workspaces, id, overwrite } = options; + let savedObjectWorkspaces = options?.workspaces; + + if (id && overwrite && workspaces) { + let currentItem; + try { + currentItem = await wrapperOptions.client.get(type, id); + } catch (e) { + // this.get will throw an error when no items can be found + } + if (currentItem) { + if ( + SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + workspaces, + currentItem.workspaces + ).length + ) { + throw SavedObjectsErrorHelpers.createConflictError(type, id); + } else { + savedObjectWorkspaces = currentItem.workspaces; + } + } + } + + return await wrapperOptions.client.create(type, attributes, { + ...options, + workspaces: savedObjectWorkspaces, + }); + }; + + const bulkCreateWithWorkspaceConflictCheck = async ( + objects: Array>, + options: SavedObjectsCreateOptions = {} + ): Promise> => { + const { overwrite, namespace } = options; + const bulkGetDocs = objects + .filter((object) => !!(object.id && options.workspaces)) + .map((object) => { + const { type, id } = object; + /** + * It requires a check when overwriting objects to target workspaces + */ + return { + type, + id: id as string, + fields: ['id', 'workspaces'], + }; + }); + const objectsConflictWithWorkspace: SavedObject[] = []; + const objectsMapWorkspaces: Record = {}; + if (bulkGetDocs.length) { + const bulkGetResult = await wrapperOptions.client.bulkGet(bulkGetDocs); + + bulkGetResult.saved_objects.forEach((object) => { + if (!object.error && object.id && overwrite) { + /** + * 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 + */ + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + options.workspaces, + object.workspaces + ); + const { id, type } = object; + if (filteredWorkspaces.length) { + /** + * options.workspaces is not a subset of object.workspaces, + * return a conflict error. + */ + objectsConflictWithWorkspace.push({ + id, + type, + attributes: {}, + references: [], + error: { + ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), + metadata: { isNotOverwritable: true }, + }, + }); + } else { + objectsMapWorkspaces[this.getRawId({ namespace, type, id })] = object.workspaces; + } + } + }); + } + + const objectsFilteredByError = objects.filter( + (item) => + !objectsConflictWithWorkspace.find( + (errorItems) => + this.getRawId({ namespace, type: errorItems.type, id: errorItems.id }) === + this.getRawId({ namespace, type: item.type, id: item.id as string }) + ) + ); + let objectsPayload = objectsFilteredByError; + if (overwrite) { + objectsPayload = objectsPayload.map((item) => { + if (item.id) { + item.workspaces = + objectsMapWorkspaces[ + this.getRawId({ + namespace, + id: item.id, + type: item.type, + }) + ]; + } + + return item; + }); + } + const realBulkCreateResult = await wrapperOptions.client.bulkCreate(objectsPayload, options); + const result: SavedObjectsBulkResponse = { + ...realBulkCreateResult, + saved_objects: [...objectsConflictWithWorkspace, ...realBulkCreateResult.saved_objects], + }; + return result as SavedObjectsBulkResponse; + }; + + const checkConflictWithWorkspaceConflictCheck = async ( + objects: SavedObjectsCheckConflictsObject[] = [], + options: SavedObjectsBaseOptions = {} + ) => { + const objectsConflictWithWorkspace: SavedObjectsCheckConflictsResponse['errors'] = []; + if (options.workspaces) { + if (objects.length === 0) { + return { errors: [] }; + } + + const bulkGetDocs: any[] = objects.map((object) => { + const { type, id } = object; + + return { + type, + id, + fields: ['id', 'workspaces'], + }; + }); + + if (bulkGetDocs.length) { + const bulkGetResult = await wrapperOptions.client.bulkGet(bulkGetDocs); + + bulkGetResult.saved_objects.forEach((object) => { + const { id, type } = object; + if (!object.error) { + let workspaceConflict = false; + if (options.workspaces) { + const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces( + options.workspaces, + object.workspaces + ); + if (filteredWorkspaces.length) { + workspaceConflict = true; + } + } + if (workspaceConflict) { + objectsConflictWithWorkspace.push({ + id, + type, + error: { + ...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)), + metadata: { isNotOverwritable: true }, + }, + }); + } + } + }); + } + } + + const objectsFilteredByError = objects.filter( + (item) => + !objectsConflictWithWorkspace.find( + (errorItems) => + this.getRawId({ + namespace: options.namespace, + type: errorItems.type, + id: errorItems.id, + }) === + this.getRawId({ + namespace: options.namespace, + type: item.type, + id: item.id as string, + }) + ) + ); + const realBulkCreateResult = await wrapperOptions.client.checkConflicts( + objectsFilteredByError, + options + ); + const result: SavedObjectsCheckConflictsResponse = { + ...realBulkCreateResult, + errors: [...objectsConflictWithWorkspace, ...realBulkCreateResult.errors], + }; + + return result; + }; + + return { + ...wrapperOptions.client, + create: createWithWorkspaceConflictCheck, + bulkCreate: bulkCreateWithWorkspaceConflictCheck, + checkConflicts: checkConflictWithWorkspaceConflictCheck, + delete: wrapperOptions.client.delete, + find: wrapperOptions.client.find, + bulkGet: wrapperOptions.client.bulkGet, + get: wrapperOptions.client.get, + update: wrapperOptions.client.update, + bulkUpdate: wrapperOptions.client.bulkUpdate, + errors: wrapperOptions.client.errors, + addToNamespaces: wrapperOptions.client.addToNamespaces, + deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + }; + }; + + constructor() {} +} From 2d22c9673463022120e02551d6a525e44ccd4fed Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 18 Oct 2023 16:53:42 +0800 Subject: [PATCH 43/43] feat: remove unnecessary changes Signed-off-by: SuZhou-Joe --- .../server/saved_objects/service/lib/repository.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 15e32336eb3b..88e8cec97408 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -360,21 +360,12 @@ export class SavedObjectsRepository { if (object.id == null) object.id = uuid.v1(); - let opensearchRequestIndexPayload = {}; - - if (requiresNamespacesCheck) { - opensearchRequestIndexPayload = { - opensearchRequestIndex: bulkGetRequestIndexCounter, - }; - bulkGetRequestIndexCounter++; - } - return { tag: 'Right' as 'Right', value: { method, object, - ...opensearchRequestIndexPayload, + ...(requiresNamespacesCheck && { opensearchRequestIndex: bulkGetRequestIndexCounter++ }), }, }; });