From e3ce2e276a99fe2598d4a5a5ea9b4677cf396afb Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Mar 2024 13:25:12 +0800 Subject: [PATCH 01/36] feat: remove useless code (#310) Signed-off-by: SuZhou-Joe --- .../service/lib/repository.test.js | 41 +------------------ .../saved_objects/service/lib/repository.ts | 38 ++++------------- 2 files changed, 8 insertions(+), 71 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 3cf067ff6465..d26589882273 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -53,12 +53,6 @@ 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; @@ -499,9 +493,7 @@ describe('SavedObjectsRepository', () => { opensearchClientMock.createSuccessTransportRequestPromise(response) ); const result = await savedObjectsRepository.bulkCreate(objects, options); - expect(client.mget).toHaveBeenCalledTimes( - multiNamespaceObjects?.length || options?.workspaces ? 1 : 0 - ); + expect(client.mget).toHaveBeenCalledTimes(multiNamespaceObjects?.length ? 1 : 0); return result; }; @@ -1801,7 +1793,6 @@ describe('SavedObjectsRepository', () => { 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) => @@ -1923,36 +1914,6 @@ 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: [ - { ...omitWorkspace(obj8), error: createConflictError(obj8.type, obj8.id) }, - { - ...omitWorkspace(obj9), - error: { - ...createConflictError(obj9.type, obj9.id), - metadata: { - isNotOverwritable: true, - }, - }, - }, - ], - }); - }); }); }); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index ad79282fee54..c3de94bf84b9 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -360,28 +360,15 @@ 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) { - opensearchRequestIndexPayload = { - opensearchRequestIndex: bulkGetRequestIndexCounter, - }; - bulkGetRequestIndexCounter++; - } - return { tag: 'Right' as 'Right', value: { method, object, - ...opensearchRequestIndexPayload, + ...(requiresNamespacesCheck && { opensearchRequestIndex: bulkGetRequestIndexCounter++ }), }, }; }); @@ -392,7 +379,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( @@ -423,7 +410,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; // @ts-expect-error MultiGetHit._source is optional @@ -575,7 +562,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( @@ -598,24 +585,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.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) || workspaceConflict) && { + ...(!this.rawDocExistsInNamespace(doc!, namespace) && { metadata: { isNotOverwritable: true }, }), }, @@ -740,9 +716,9 @@ export class SavedObjectsRepository { } /** - * Deletes all objects from the provided workspace. It used when deleting a workspace. + * Deletes all objects from the provided workspace. * - * @param {string} workspace + * @param {string} workspace - workspace id * @param options SavedObjectsDeleteByWorkspaceOptions * @returns {promise} - { took, timed_out, total, deleted, batches, version_conflicts, noops, retries, failures } */ From a2cead4ec2ea0eb0da969cb8fec4318fb2548baa Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Mar 2024 18:17:12 +0800 Subject: [PATCH 02/36] feat: remove useless code (#311) Signed-off-by: SuZhou-Joe --- .../import/import_saved_objects.test.ts | 12 +--- .../import/import_saved_objects.ts | 13 +--- .../import/regenerate_ids.test.ts | 71 +------------------ .../saved_objects/import/regenerate_ids.ts | 35 +-------- 4 files changed, 4 insertions(+), 127 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 dcb8d685d42c..3dda6931bd1e 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'; @@ -70,7 +70,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: [], @@ -279,15 +278,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, 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 0222223a1751..46db9dc3a9ce 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 { checkConflictsForDataSource } from './check_conflict_for_data_source'; /** @@ -86,17 +86,6 @@ export async function importSavedObjectsFromStream({ // randomly generated id importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects, dataSourceId); } else { - // in check conclict and override mode - - 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.test.ts b/src/core/server/saved_objects/import/regenerate_ids.test.ts index 895b90a89324..c7dbfb8b50bc 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 = ([ @@ -70,70 +68,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, - }, - } - `); - }); -}); diff --git a/src/core/server/saved_objects/import/regenerate_ids.ts b/src/core/server/saved_objects/import/regenerate_ids.ts index bf89fad824d6..f1092bed7f55 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. @@ -48,35 +47,3 @@ export const regenerateIds = (objects: SavedObject[], dataSourceId: string | und }, 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); -}; From e2d36bac334dde7cb2994cf3ea2397d955b62bb8 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 1 Apr 2024 09:43:53 +0800 Subject: [PATCH 03/36] feat: add APIs to support plugin state in request (#312) * feat: add APIs to support plugin state in request Signed-off-by: SuZhou-Joe * feat: add APIs to support plugin state in request Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe --- src/core/server/utils/index.ts | 1 + src/core/server/utils/workspace.test.ts | 19 +++++++++++ src/core/server/utils/workspace.ts | 43 +++++++++++++++++++++++++ src/legacy/server/osd_server.d.ts | 8 +++++ 4 files changed, 71 insertions(+) create mode 100644 src/core/server/utils/workspace.test.ts create mode 100644 src/core/server/utils/workspace.ts diff --git a/src/core/server/utils/index.ts b/src/core/server/utils/index.ts index 42b01e72b0d1..a20b8c4c4e5b 100644 --- a/src/core/server/utils/index.ts +++ b/src/core/server/utils/index.ts @@ -33,3 +33,4 @@ export * from './from_root'; export * from './package_json'; export * from './streams'; export { getWorkspaceIdFromUrl, cleanWorkspaceId } from '../../utils'; +export { updateWorkspaceState, getWorkspaceState } from './workspace'; diff --git a/src/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts new file mode 100644 index 000000000000..49382cfac38f --- /dev/null +++ b/src/core/server/utils/workspace.test.ts @@ -0,0 +1,19 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { httpServerMock } from '../mocks'; +import { getWorkspaceState, updateWorkspaceState } from './workspace'; + +describe('updateWorkspaceState', () => { + it('update with payload', () => { + const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); + updateWorkspaceState(requestMock, { + id: 'foo', + }); + expect(getWorkspaceState(requestMock)).toEqual({ + id: 'foo', + }); + }); +}); diff --git a/src/core/server/utils/workspace.ts b/src/core/server/utils/workspace.ts new file mode 100644 index 000000000000..c5dcf84b92d9 --- /dev/null +++ b/src/core/server/utils/workspace.ts @@ -0,0 +1,43 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * This file is using {@link PluginsStates} to store workspace info into request. + * The best practice would be using {@link Server.register} to register plugins into the hapi server + * but OSD is wrappering the hapi server and the hapi server instance is hidden as internal implementation. + */ +import { PluginsStates } from '@hapi/hapi'; +import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; + +/** + * This function will be used as a proxy + * because `ensureRequest` is only importable from core module. + * + * @param workspaceId string + * @returns void + */ +export const updateWorkspaceState = ( + request: OpenSearchDashboardsRequest, + payload: Partial +) => { + const rawRequest = ensureRawRequest(request); + + if (!rawRequest.plugins) { + rawRequest.plugins = {}; + } + + if (!rawRequest.plugins.workspace) { + rawRequest.plugins.workspace = {}; + } + + rawRequest.plugins.workspace = { + ...rawRequest.plugins.workspace, + ...payload, + }; +}; + +export const getWorkspaceState = (request: OpenSearchDashboardsRequest) => { + return ensureRawRequest(request).plugins?.workspace; +}; diff --git a/src/legacy/server/osd_server.d.ts b/src/legacy/server/osd_server.d.ts index 1f8535b59e87..9d94349cf1c0 100644 --- a/src/legacy/server/osd_server.d.ts +++ b/src/legacy/server/osd_server.d.ts @@ -60,6 +60,14 @@ declare module 'hapi' { } } +declare module '@hapi/hapi' { + interface PluginsStates { + workspace?: { + id?: string; + }; + } +} + type OsdMixinFunc = (osdServer: OsdServer, server: Server, config: any) => Promise | void; export interface PluginsSetup { From 66d6096cceec516e2eaadfbf9a5cee4738fd4f2a Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:33:09 +0800 Subject: [PATCH 04/36] feat: POC implementation Signed-off-by: SuZhou-Joe --- src/plugins/workspace/common/constants.ts | 2 + .../workspace/opensearch_dashboards.json | 4 +- src/plugins/workspace/server/constant.ts | 11 ++ src/plugins/workspace/server/plugin.ts | 13 ++ .../workspace_id_consumer_wrapper.ts | 117 ++++++++++++++++++ .../workspace/server/workspace_client.ts | 1 + 6 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 src/plugins/workspace/server/constant.ts create mode 100644 src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts diff --git a/src/plugins/workspace/common/constants.ts b/src/plugins/workspace/common/constants.ts index 2be4a9d121db..1781b7e32771 100644 --- a/src/plugins/workspace/common/constants.ts +++ b/src/plugins/workspace/common/constants.ts @@ -20,3 +20,5 @@ export enum WorkspacePermissionMode { LibraryRead = 'library_read', LibraryWrite = 'library_write', } + +export const WORKSPACE_ID_CONSUMER_WRAPPER_ID = 'workspace_id_consumer'; diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 7d94a7491a00..0f1d851d5d76 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -3,9 +3,7 @@ "version": "opensearchDashboards", "server": true, "ui": true, - "requiredPlugins": [ - "savedObjects" - ], + "requiredPlugins": [], "optionalPlugins": ["savedObjectsManagement"], "requiredBundles": ["opensearchDashboardsReact"] } diff --git a/src/plugins/workspace/server/constant.ts b/src/plugins/workspace/server/constant.ts new file mode 100644 index 000000000000..d3cd904b1cf5 --- /dev/null +++ b/src/plugins/workspace/server/constant.ts @@ -0,0 +1,11 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * A symbol used for imply the workspace id in original url + * + * @Internal + */ +export const workspaceIdInUrlSymbol = Symbol('workspace_id_in_url'); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 0fff0082476b..5c42ae476033 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -15,6 +15,7 @@ import { import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, } from '../common/constants'; import { IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; @@ -27,6 +28,10 @@ import { SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; +import { workspaceIdInUrlSymbol } from './constant'; +import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; +// eslint-disable-next-line @osd/eslint/no-restricted-paths +import { ensureRawRequest } from '../../../core/server/http/router'; // will be an issue as the ensureRawRequest is an internal implementation export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -47,6 +52,8 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); if (workspaceId) { + const rawRequest = ensureRawRequest(request); + rawRequest.headers[workspaceIdInUrlSymbol.toString()] = workspaceId; const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); return toolkit.rewriteUrl(requestUrl.toString()); @@ -94,6 +101,12 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } + core.savedObjects.addClientWrapper( + -2, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + new WorkspaceIdConsumerWrapper().wrapperFactory + ); + registerRoutes({ http: core.http, logger: this.logger, diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts new file mode 100644 index 000000000000..4c302fc93518 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -0,0 +1,117 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + SavedObjectsBaseOptions, + SavedObjectsBulkCreateObject, + SavedObjectsClientWrapperFactory, + SavedObjectsCreateOptions, + SavedObjectsCheckConflictsObject, + OpenSearchDashboardsRequest, + SavedObjectsFindOptions, + SavedObjectsUpdateOptions, + SavedObjectsBulkUpdateOptions, + SavedObjectsBulkUpdateObject, + WORKSPACE_TYPE, +} from '../../../../core/server'; +import { workspaceIdInUrlSymbol } from '../constant'; + +type WorkspaceOptions = + | { + workspaces?: string[]; + } + | undefined; + +export class WorkspaceIdConsumerWrapper { + private typeRelatedToWorkspace(type: string | string[]): boolean { + if (Array.isArray(type)) { + return type.some((item) => item === WORKSPACE_TYPE); + } + + return type === WORKSPACE_TYPE; + } + private formatWorkspaceIdParams( + request: OpenSearchDashboardsRequest, + options: T + ): T { + if (!options) { + return options; + } + const { workspaces, ...others } = options; + const workspaceIdParsedFromUrl = request.headers[workspaceIdInUrlSymbol.toString()] as string; + const workspaceIdInUserOptions = options.workspaces; + let finalWorkspaces: string[] = []; + if (workspaceIdInUserOptions?.length) { + finalWorkspaces = workspaceIdInUserOptions; + } else if (workspaceIdParsedFromUrl) { + finalWorkspaces = [workspaceIdParsedFromUrl]; + } + + return { + ...(others as T), + ...(finalWorkspaces.length ? { workspaces: finalWorkspaces } : {}), + }; + } + public wrapperFactory: SavedObjectsClientWrapperFactory = (wrapperOptions) => { + return { + ...wrapperOptions.client, + create: (type: string, attributes: T, options: SavedObjectsCreateOptions = {}) => + wrapperOptions.client.create( + type, + attributes, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkCreate: ( + objects: Array>, + options: SavedObjectsCreateOptions = {} + ) => + wrapperOptions.client.bulkCreate( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + checkConflicts: ( + objects: SavedObjectsCheckConflictsObject[] = [], + options: SavedObjectsBaseOptions = {} + ) => + wrapperOptions.client.checkConflicts( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + delete: wrapperOptions.client.delete, + find: (options: SavedObjectsFindOptions) => + wrapperOptions.client.find( + this.typeRelatedToWorkspace(options.type) + ? options + : this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkGet: wrapperOptions.client.bulkGet, + get: wrapperOptions.client.get, + update: ( + type: string, + id: string, + attributes: Partial, + options: SavedObjectsUpdateOptions = {} + ) => + wrapperOptions.client.update( + type, + id, + attributes, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + bulkUpdate: ( + objects: Array>, + options?: SavedObjectsBulkUpdateOptions + ) => + wrapperOptions.client.bulkUpdate( + objects, + this.formatWorkspaceIdParams(wrapperOptions.request, options) + ), + addToNamespaces: wrapperOptions.client.addToNamespaces, + deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + }; + }; + + constructor() {} +} diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index e7bdf97b54ec..0ffb72f016f2 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -21,6 +21,7 @@ import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; import { + WORKSPACE_ID_CONSUMER_WRAPPER_ID, WORKSPACE_OVERVIEW_APP_ID, WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_UPDATE_APP_ID, From d275f6173e5e68986505e2a8c050b29682102ae1 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:37:46 +0800 Subject: [PATCH 05/36] feat: add some comment Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 5c42ae476033..ecea0bb84e71 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -101,6 +101,8 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } + // TOO many client wrapper inside a single workspace plugin + // Try to combine conflict wrapper and this consumer wrapper. core.savedObjects.addClientWrapper( -2, WORKSPACE_ID_CONSUMER_WRAPPER_ID, From 65f6be3b585ea0c7e77445e2621525286567b406 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:39:15 +0800 Subject: [PATCH 06/36] feat: revert dependency Signed-off-by: SuZhou-Joe --- src/plugins/workspace/opensearch_dashboards.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plugins/workspace/opensearch_dashboards.json b/src/plugins/workspace/opensearch_dashboards.json index 0f1d851d5d76..7d94a7491a00 100644 --- a/src/plugins/workspace/opensearch_dashboards.json +++ b/src/plugins/workspace/opensearch_dashboards.json @@ -3,7 +3,9 @@ "version": "opensearchDashboards", "server": true, "ui": true, - "requiredPlugins": [], + "requiredPlugins": [ + "savedObjects" + ], "optionalPlugins": ["savedObjectsManagement"], "requiredBundles": ["opensearchDashboardsReact"] } From 9c5d2d9851474760abe4fde95cae9c5a321c5f59 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 13 Mar 2024 17:41:40 +0800 Subject: [PATCH 07/36] feat: update comment Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index ecea0bb84e71..3cedf77a4637 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -31,7 +31,7 @@ import { WorkspacePluginConfigType } from '../config'; import { workspaceIdInUrlSymbol } from './constant'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; // eslint-disable-next-line @osd/eslint/no-restricted-paths -import { ensureRawRequest } from '../../../core/server/http/router'; // will be an issue as the ensureRawRequest is an internal implementation +import { ensureRawRequest } from '../../../core/server/http/router'; // TODO: move the ensureRawRequest to a core module or the import will be an issue as the ensureRawRequest can only be import from core module export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -101,7 +101,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } - // TOO many client wrapper inside a single workspace plugin + // TODO: TOO many client wrapper inside a single workspace plugin // Try to combine conflict wrapper and this consumer wrapper. core.savedObjects.addClientWrapper( -2, From d5638ff248adbb6fc4d777d5fe7645c1792a5233 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 14:53:01 +0800 Subject: [PATCH 08/36] feat: address one TODO Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/constant.ts | 11 ----------- src/plugins/workspace/server/plugin.ts | 14 ++++++++------ .../workspace_id_consumer_wrapper.ts | 15 ++++++++------- 3 files changed, 16 insertions(+), 24 deletions(-) delete mode 100644 src/plugins/workspace/server/constant.ts diff --git a/src/plugins/workspace/server/constant.ts b/src/plugins/workspace/server/constant.ts deleted file mode 100644 index d3cd904b1cf5..000000000000 --- a/src/plugins/workspace/server/constant.ts +++ /dev/null @@ -1,11 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * A symbol used for imply the workspace id in original url - * - * @Internal - */ -export const workspaceIdInUrlSymbol = Symbol('workspace_id_in_url'); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 3cedf77a4637..21de1deeca10 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -21,17 +21,18 @@ import { IWorkspaceClientImpl } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; -import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; +import { + cleanWorkspaceId, + getWorkspaceIdFromUrl, + updateWorkspaceState, +} from '../../../core/server/utils'; import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; import { WorkspacePluginConfigType } from '../config'; -import { workspaceIdInUrlSymbol } from './constant'; import { WorkspaceIdConsumerWrapper } from './saved_objects/workspace_id_consumer_wrapper'; -// eslint-disable-next-line @osd/eslint/no-restricted-paths -import { ensureRawRequest } from '../../../core/server/http/router'; // TODO: move the ensureRawRequest to a core module or the import will be an issue as the ensureRawRequest can only be import from core module export class WorkspacePlugin implements Plugin<{}, {}> { private readonly logger: Logger; @@ -52,8 +53,9 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); if (workspaceId) { - const rawRequest = ensureRawRequest(request); - rawRequest.headers[workspaceIdInUrlSymbol.toString()] = workspaceId; + updateWorkspaceState(request, { + id: workspaceId, + }); const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); return toolkit.rewriteUrl(requestUrl.toString()); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 4c302fc93518..1135b8ca2eed 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { getWorkspaceState } from '../../../../core/server/utils'; import { SavedObjectsBaseOptions, SavedObjectsBulkCreateObject, @@ -16,7 +17,6 @@ import { SavedObjectsBulkUpdateObject, WORKSPACE_TYPE, } from '../../../../core/server'; -import { workspaceIdInUrlSymbol } from '../constant'; type WorkspaceOptions = | { @@ -40,13 +40,14 @@ export class WorkspaceIdConsumerWrapper { return options; } const { workspaces, ...others } = options; - const workspaceIdParsedFromUrl = request.headers[workspaceIdInUrlSymbol.toString()] as string; - const workspaceIdInUserOptions = options.workspaces; + const workspaceState = getWorkspaceState(request); + const workspaceIdParsedFromRequest = workspaceState?.id; + const workspaceIdsInUserOptions = options.workspaces; let finalWorkspaces: string[] = []; - if (workspaceIdInUserOptions?.length) { - finalWorkspaces = workspaceIdInUserOptions; - } else if (workspaceIdParsedFromUrl) { - finalWorkspaces = [workspaceIdParsedFromUrl]; + if (options.hasOwnProperty('workspaces')) { + finalWorkspaces = workspaceIdsInUserOptions || []; + } else if (workspaceIdParsedFromRequest) { + finalWorkspaces = [workspaceIdParsedFromRequest]; } return { From 45589cff80409190f98a403da659fa6e34d0ec40 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 15:24:25 +0800 Subject: [PATCH 09/36] feat: address TODO Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 14 +++++------ .../workspace_id_consumer_wrapper.ts | 25 ++----------------- .../workspace/server/workspace_client.ts | 7 +----- 3 files changed, 9 insertions(+), 37 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 21de1deeca10..088096df4d35 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -88,6 +88,12 @@ export class WorkspacePlugin implements Plugin<{}, {}> { this.workspaceConflictControl.wrapperFactory ); + core.savedObjects.addClientWrapper( + -2, + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + new WorkspaceIdConsumerWrapper().wrapperFactory + ); + this.logger.info('Workspace permission control enabled:' + isPermissionControlEnabled); if (isPermissionControlEnabled) { this.permissionControl = new SavedObjectsPermissionControl(this.logger); @@ -103,14 +109,6 @@ export class WorkspacePlugin implements Plugin<{}, {}> { ); } - // TODO: TOO many client wrapper inside a single workspace plugin - // Try to combine conflict wrapper and this consumer wrapper. - core.savedObjects.addClientWrapper( - -2, - WORKSPACE_ID_CONSUMER_WRAPPER_ID, - new WorkspaceIdConsumerWrapper().wrapperFactory - ); - registerRoutes({ http: core.http, logger: this.logger, diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 1135b8ca2eed..321a026d943e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,9 +12,6 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - SavedObjectsUpdateOptions, - SavedObjectsBulkUpdateOptions, - SavedObjectsBulkUpdateObject, WORKSPACE_TYPE, } from '../../../../core/server'; @@ -89,26 +86,8 @@ export class WorkspaceIdConsumerWrapper { ), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, - update: ( - type: string, - id: string, - attributes: Partial, - options: SavedObjectsUpdateOptions = {} - ) => - wrapperOptions.client.update( - type, - id, - attributes, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), - bulkUpdate: ( - objects: Array>, - options?: SavedObjectsBulkUpdateOptions - ) => - wrapperOptions.client.bulkUpdate( - objects, - this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), + update: wrapperOptions.client.update, + bulkUpdate: wrapperOptions.client.bulkUpdate, addToNamespaces: wrapperOptions.client.addToNamespaces, deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, }; diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 0ffb72f016f2..05e323f63f02 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -20,12 +20,7 @@ import { import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { - WORKSPACE_ID_CONSUMER_WRAPPER_ID, - WORKSPACE_OVERVIEW_APP_ID, - WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, - WORKSPACE_UPDATE_APP_ID, -} from '../common/constants'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; const WORKSPACE_ID_SIZE = 6; From 5d990c8211931563612b03f1ce487fa177451e6b Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 16:46:15 +0800 Subject: [PATCH 10/36] feat: add unit test Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.test.ts | 65 +++++++++- .../workspace_id_consumer_wrapper.test.ts | 118 ++++++++++++++++++ .../workspace_id_consumer_wrapper.ts | 25 +--- .../workspace/server/workspace_client.ts | 12 +- 4 files changed, 198 insertions(+), 22 deletions(-) create mode 100644 src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index c448fcf209f9..300b5e982a01 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -3,8 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { coreMock } from '../../../core/server/mocks'; +import { OnPreRoutingHandler } from 'src/core/server'; +import { coreMock, httpServerMock } from '../../../core/server/mocks'; import { WorkspacePlugin } from './plugin'; +import { getWorkspaceState } from '../../../core/server/utils'; describe('Workspace server plugin', () => { it('#setup', async () => { @@ -27,5 +29,66 @@ describe('Workspace server plugin', () => { }, } `); + expect(setupMock.savedObjects.addClientWrapper).toBeCalledTimes(3); + }); + + it('#proxyWorkspaceTrafficToRealHandler', async () => { + const setupMock = coreMock.createSetup(); + const initializerContextConfigMock = coreMock.createPluginInitializerContext({ + enabled: true, + permission: { + enabled: true, + }, + }); + let onPreRoutingFn: OnPreRoutingHandler = () => httpServerMock.createResponseFactory().ok(); + setupMock.http.registerOnPreRouting.mockImplementation((fn) => { + onPreRoutingFn = fn; + return fn; + }); + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + await workspacePlugin.setup(setupMock); + const toolKitMock = httpServerMock.createToolkit(); + + const requestWithWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/w/foo/app', + }); + onPreRoutingFn(requestWithWorkspaceInUrl, httpServerMock.createResponseFactory(), toolKitMock); + expect(toolKitMock.rewriteUrl).toBeCalledWith('http://localhost/app'); + expect(toolKitMock.next).toBeCalledTimes(0); + expect(getWorkspaceState(requestWithWorkspaceInUrl)).toEqual({ + id: 'foo', + }); + + const requestWithoutWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ + path: '/app', + }); + onPreRoutingFn( + requestWithoutWorkspaceInUrl, + httpServerMock.createResponseFactory(), + toolKitMock + ); + expect(toolKitMock.next).toBeCalledTimes(1); + }); + + it('#start', async () => { + const setupMock = coreMock.createSetup(); + const startMock = coreMock.createStart(); + const initializerContextConfigMock = coreMock.createPluginInitializerContext({ + enabled: true, + permission: { + enabled: true, + }, + }); + + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + await workspacePlugin.setup(setupMock); + await workspacePlugin.start(startMock); + expect(startMock.savedObjects.createSerializer).toBeCalledTimes(1); + }); + + it('#stop', () => { + const initializerContextConfigMock = coreMock.createPluginInitializerContext(); + const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); + workspacePlugin.stop(); }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts new file mode 100644 index 000000000000..8715606da681 --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -0,0 +1,118 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { updateWorkspaceState } from '../../../../core/server/utils'; +import { SavedObject } from '../../../../core/public'; +import { httpServerMock, savedObjectsClientMock, coreMock } from '../../../../core/server/mocks'; +import { WorkspaceIdConsumerWrapper } from './workspace_id_consumer_wrapper'; + +describe('WorkspaceIdConsumerWrapper', () => { + const requestHandlerContext = coreMock.createRequestHandlerContext(); + const wrapperInstance = new WorkspaceIdConsumerWrapper(); + const mockedClient = savedObjectsClientMock.create(); + const workspaceEnabledMockRequest = httpServerMock.createOpenSearchDashboardsRequest(); + updateWorkspaceState(workspaceEnabledMockRequest, { + id: 'foo', + }); + const wrapperClient = wrapperInstance.wrapperFactory({ + client: mockedClient, + typeRegistry: requestHandlerContext.savedObjects.typeRegistry, + request: workspaceEnabledMockRequest, + }); + const getSavedObject = (savedObject: Partial) => { + const payload: SavedObject = { + references: [], + id: '', + type: 'dashboard', + attributes: {}, + ...savedObject, + }; + + return payload; + }; + describe('create', () => { + beforeEach(() => { + mockedClient.create.mockClear(); + }); + it(`Should add workspaces parameters when create`, async () => { + await wrapperClient.create('dashboard', { + name: 'foo', + }); + + expect(mockedClient.create).toBeCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({ + workspaces: ['foo'], + }) + ); + }); + + it(`Should use options.workspaces there is workspaces inside options`, async () => { + await wrapperClient.create( + 'dashboard', + { + name: 'foo', + }, + { + id: 'dashboard:foo', + overwrite: true, + workspaces: undefined, + } + ); + + expect(mockedClient.create.mock.calls[0][2]?.hasOwnProperty('workspaces')).toEqual(false); + }); + }); + + describe('bulkCreate', () => { + beforeEach(() => { + mockedClient.bulkCreate.mockClear(); + }); + it(`Should add workspaces parameters when bulk create`, async () => { + await wrapperClient.bulkCreate([ + getSavedObject({ + id: 'foo', + }), + ]); + + expect(mockedClient.bulkCreate).toBeCalledWith( + [{ attributes: {}, id: 'foo', references: [], type: 'dashboard' }], + { + workspaces: ['foo'], + } + ); + }); + }); + + describe('checkConflict', () => { + beforeEach(() => { + mockedClient.checkConflicts.mockClear(); + }); + + it(`Should add workspaces parameters when checkConflict`, async () => { + await wrapperClient.checkConflicts([]); + expect(mockedClient.checkConflicts).toBeCalledWith([], { + workspaces: ['foo'], + }); + }); + }); + + describe('find', () => { + beforeEach(() => { + mockedClient.find.mockClear(); + }); + + it(`Should add workspaces parameters when find`, async () => { + await wrapperClient.find({ + type: 'dashboard', + }); + expect(mockedClient.find).toBeCalledWith({ + type: 'dashboard', + workspaces: ['foo'], + }); + }); + }); +}); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 321a026d943e..bf450841f001 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -12,7 +12,6 @@ import { SavedObjectsCheckConflictsObject, OpenSearchDashboardsRequest, SavedObjectsFindOptions, - WORKSPACE_TYPE, } from '../../../../core/server'; type WorkspaceOptions = @@ -22,26 +21,16 @@ type WorkspaceOptions = | undefined; export class WorkspaceIdConsumerWrapper { - private typeRelatedToWorkspace(type: string | string[]): boolean { - if (Array.isArray(type)) { - return type.some((item) => item === WORKSPACE_TYPE); - } - - return type === WORKSPACE_TYPE; - } private formatWorkspaceIdParams( request: OpenSearchDashboardsRequest, - options: T + options?: T ): T { - if (!options) { - return options; - } - const { workspaces, ...others } = options; + const { workspaces, ...others } = options || {}; const workspaceState = getWorkspaceState(request); const workspaceIdParsedFromRequest = workspaceState?.id; - const workspaceIdsInUserOptions = options.workspaces; + const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (options.hasOwnProperty('workspaces')) { + if (options?.hasOwnProperty('workspaces')) { finalWorkspaces = workspaceIdsInUserOptions || []; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; @@ -79,11 +68,7 @@ export class WorkspaceIdConsumerWrapper { ), delete: wrapperOptions.client.delete, find: (options: SavedObjectsFindOptions) => - wrapperOptions.client.find( - this.typeRelatedToWorkspace(options.type) - ? options - : this.formatWorkspaceIdParams(wrapperOptions.request, options) - ), + wrapperOptions.client.find(this.formatWorkspaceIdParams(wrapperOptions.request, options)), bulkGet: wrapperOptions.client.bulkGet, get: wrapperOptions.client.get, update: wrapperOptions.client.update, diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 05e323f63f02..3378ab650cd1 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -20,7 +20,10 @@ import { 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'; +import { + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, +} from '../common/constants'; const WORKSPACE_ID_SIZE = 6; @@ -56,6 +59,13 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract { return this.savedObjects?.getScopedClient(requestDetail.request, { + /** + * workspace object does not have workspaces field + * so need to bypass the consumer wrapper + * or it will append workspaces into the options.workspaces + * when list all the workspaces inside a workspace + */ + excludedWrappers: [WORKSPACE_ID_CONSUMER_WRAPPER_ID], includedHiddenTypes: [WORKSPACE_TYPE], }) as SavedObjectsClientContract; } From 08dc9e2be28177a037c14d4668cca0bfac042ffc Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 18:27:09 +0800 Subject: [PATCH 11/36] feat: some special logic on specific operation Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.ts | 4 ++-- .../workspace_saved_objects_client_wrapper.ts | 3 +++ src/plugins/workspace/server/workspace_client.ts | 16 +++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index bf450841f001..d24496ed31ef 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -30,8 +30,8 @@ export class WorkspaceIdConsumerWrapper { const workspaceIdParsedFromRequest = workspaceState?.id; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (options?.hasOwnProperty('workspaces')) { - finalWorkspaces = workspaceIdsInUserOptions || []; + if (workspaceIdsInUserOptions) { + finalWorkspaces = workspaceIdsInUserOptions; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; } diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index c515f555fa4b..1a97b2dac69a 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -468,6 +468,9 @@ export class WorkspaceSavedObjectsClientWrapper { [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite] ), }, + // By declaring workspaces as empty array, + // workspaces won't be appended automatically into the options. + workspaces: [], }) ).saved_objects.map((item) => item.id); diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index 3378ab650cd1..a8b006c99a87 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -50,7 +50,15 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract | undefined { return this.savedObjects?.getScopedClient(requestDetail.request, { - excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], + excludedWrappers: [ + WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, + /** + * workspace object does not have workspaces field + * so need to bypass workspace id consumer wrapper + * for any kind of operation to saved objects client. + */ + WORKSPACE_ID_CONSUMER_WRAPPER_ID, + ], includedHiddenTypes: [WORKSPACE_TYPE], }); } @@ -59,12 +67,6 @@ export class WorkspaceClient implements IWorkspaceClientImpl { requestDetail: IRequestDetail ): SavedObjectsClientContract { return this.savedObjects?.getScopedClient(requestDetail.request, { - /** - * workspace object does not have workspaces field - * so need to bypass the consumer wrapper - * or it will append workspaces into the options.workspaces - * when list all the workspaces inside a workspace - */ excludedWrappers: [WORKSPACE_ID_CONSUMER_WRAPPER_ID], includedHiddenTypes: [WORKSPACE_TYPE], }) as SavedObjectsClientContract; From d3cccb8585197ab93e92a2e66f8acf354b9732b8 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Fri, 22 Mar 2024 18:27:47 +0800 Subject: [PATCH 12/36] feat: add integration test Signed-off-by: SuZhou-Joe --- .../workspace_id_consumer_wrapper.test.ts | 242 ++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts new file mode 100644 index 000000000000..381da5227d8d --- /dev/null +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_id_consumer_wrapper.test.ts @@ -0,0 +1,242 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SavedObject } from 'src/core/types'; +import { isEqual } from 'lodash'; +import * as osdTestServer from '../../../../../core/test_helpers/osd_server'; + +const dashboard: Omit = { + type: 'dashboard', + attributes: {}, + references: [], +}; + +interface WorkspaceAttributes { + id: string; + name?: string; +} + +describe('workspace_id_consumer integration test', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + let createdFooWorkspace: WorkspaceAttributes = { + id: '', + }; + let createdBarWorkspace: WorkspaceAttributes = { + id: '', + }; + beforeAll(async () => { + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + osd: { + workspace: { + enabled: true, + }, + savedObjects: { + permission: { + enabled: true, + }, + }, + migrations: { + skip: false, + }, + }, + }, + }); + opensearchServer = await startOpenSearch(); + const startOSDResp = await startOpenSearchDashboards(); + root = startOSDResp.root; + const createWorkspace = (workspaceAttribute: Omit) => + osdTestServer.request.post(root, `/api/workspaces`).send({ + attributes: workspaceAttribute, + }); + + createdFooWorkspace = await createWorkspace({ + name: 'foo', + }).then((resp) => { + return resp.body.result; + }); + createdBarWorkspace = await createWorkspace({ + name: 'bar', + }).then((resp) => resp.body.result); + }, 30000); + afterAll(async () => { + await root.shutdown(); + await opensearchServer.stop(); + }); + + const deleteItem = async (object: Pick) => { + expect( + [200, 404].includes( + (await osdTestServer.request.delete(root, `/api/saved_objects/${object.type}/${object.id}`)) + .statusCode + ) + ).toEqual(true); + }; + + const getItem = async (object: Pick) => { + return await osdTestServer.request + .get(root, `/api/saved_objects/${object.type}/${object.id}`) + .expect(200); + }; + + const clearFooAndBar = async () => { + await deleteItem({ + type: dashboard.type, + id: 'foo', + }); + await deleteItem({ + type: dashboard.type, + id: 'bar', + }); + }; + + describe('saved objects client related CRUD', () => { + it('create', async () => { + const createResult = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/${dashboard.type}`) + .send({ + attributes: dashboard.attributes, + }) + .expect(200); + + expect(createResult.body.workspaces).toEqual([createdFooWorkspace.id]); + await deleteItem({ + type: dashboard.type, + id: createResult.body.id, + }); + }); + + it('bulk create', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .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, [createdFooWorkspace.id]) + ) + ).toEqual(true); + await Promise.all( + [...createResultFoo.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + + it('checkConflicts when importing ndjson', async () => { + await clearFooAndBar(); + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const getResultFoo = await getItem({ + type: dashboard.type, + id: 'foo', + }); + const getResultBar = await getItem({ + type: dashboard.type, + id: 'bar', + }); + + /** + * import with workspaces when conflicts + */ + const importWithWorkspacesResult = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_import?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, + }) + ) + ); + }); + + it('find by workspaces', async () => { + const createResultFoo = await osdTestServer.request + .post(root, `/w/${createdFooWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'foo', + }, + ]) + .expect(200); + + const createResultBar = await osdTestServer.request + .post(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_bulk_create`) + .send([ + { + ...dashboard, + id: 'bar', + }, + ]) + .expect(200); + + const findResult = await osdTestServer.request + .get(root, `/w/${createdBarWorkspace.id}/api/saved_objects/_find?type=${dashboard.type}`) + .expect(200); + + expect(findResult.body.total).toEqual(1); + expect(findResult.body.saved_objects[0].workspaces).toEqual([createdBarWorkspace.id]); + + await Promise.all( + [...createResultFoo.body.saved_objects, ...createResultBar.body.saved_objects].map((item) => + deleteItem({ + type: item.type, + id: item.id, + }) + ) + ); + }); + }); +}); From 298c2cbd100381d681cbd1b9632019233ed58074 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 24 Mar 2024 22:58:05 +0800 Subject: [PATCH 13/36] feat: declare workspaces as empty array for advanced settings Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 639dd09249ff..10d33713c878 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -63,7 +63,12 @@ export async function createOrUpgradeSavedConfig( try { // create the new SavedConfig - await savedObjectsClient.create('config', attributes, { id: version }); + await savedObjectsClient.create('config', attributes, { + id: version, + // config is a global object that should not belong to any of the workspaces + // declare it as empty array to prevent it being created as a workspace object + workspaces: [], + }); } catch (error) { if (handleWriteErrors) { if (SavedObjectsErrorHelpers.isConflictError(error)) { From 85c522dfd3f8887e4771fa964ab39f378f114ec9 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Mar 2024 10:41:25 +0800 Subject: [PATCH 14/36] feat: unified workspaces parameters when parsing from router Signed-off-by: SuZhou-Joe --- .../server/saved_objects/import/check_conflicts.ts | 2 +- src/core/server/saved_objects/routes/bulk_create.ts | 2 +- src/core/server/saved_objects/routes/create.ts | 2 +- src/core/server/saved_objects/routes/export.ts | 2 +- src/core/server/saved_objects/routes/find.ts | 2 +- .../saved_objects/service/saved_objects_client.ts | 4 ---- src/core/server/saved_objects/types.ts | 2 +- .../create_or_upgrade_saved_config.test.ts | 2 ++ .../create_or_upgrade_saved_config.ts | 4 ++-- .../workspace_id_consumer_wrapper.test.ts | 4 ++-- .../saved_objects/workspace_id_consumer_wrapper.ts | 10 +++------- 11 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index f36bcf3a8a92..2c35ba147c5d 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -79,7 +79,7 @@ export async function checkConflicts({ }); const checkConflictsResult = await savedObjectsClient.checkConflicts(objectsToCheck, { namespace, - workspaces, + ...(workspaces ? { workspaces } : {}), }); const errorMap = checkConflictsResult.errors.reduce( (acc, { type, id, error }) => acc.set(`${type}:${id}`, error), diff --git a/src/core/server/saved_objects/routes/bulk_create.ts b/src/core/server/saved_objects/routes/bulk_create.ts index 056b1b795550..b8a020b4ea24 100644 --- a/src/core/server/saved_objects/routes/bulk_create.ts +++ b/src/core/server/saved_objects/routes/bulk_create.ts @@ -70,7 +70,7 @@ export const registerBulkCreateRoute = (router: IRouter) => { : undefined; const result = await context.core.savedObjects.client.bulkCreate(req.body, { overwrite, - workspaces, + ...(workspaces ? { 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 4d22bd244a03..2a7958aa9b78 100644 --- a/src/core/server/saved_objects/routes/create.ts +++ b/src/core/server/saved_objects/routes/create.ts @@ -71,7 +71,7 @@ export const registerCreateRoute = (router: IRouter) => { migrationVersion, references, initialNamespaces, - workspaces, + ...(workspaces ? { 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 9325b632e40f..4d9d5b7e8ec3 100644 --- a/src/core/server/saved_objects/routes/export.ts +++ b/src/core/server/saved_objects/routes/export.ts @@ -106,7 +106,7 @@ export const registerExportRoute = (router: IRouter, config: SavedObjectConfig) exportSizeLimit: maxImportExportSize, includeReferencesDeep, excludeExportDetails, - workspaces, + ...(workspaces ? { 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 36fa7c2cd9f5..42b47cf950fc 100644 --- a/src/core/server/saved_objects/routes/find.ts +++ b/src/core/server/saved_objects/routes/find.ts @@ -85,7 +85,7 @@ export const registerFindRoute = (router: IRouter) => { fields: typeof query.fields === 'string' ? [query.fields] : query.fields, filter: query.filter, namespaces, - workspaces, + ...(workspaces ? { workspaces } : {}), }); return res.ok({ body: result }); 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 e1c3d16a9258..1b268e033d97 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -69,10 +69,6 @@ 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[]; /** permission control describe by ACL object */ permissions?: Permissions; } diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index d21421dbe253..23170e0a1c47 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: string[]; + workspaces?: string[] | null; } /** diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts index eb23e78b1756..fc5a55da0091 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts @@ -98,6 +98,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, + workspaces: null, } ); }); @@ -133,6 +134,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, + workspaces: null, } ); }); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 10d33713c878..6ca3baeb7a9f 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -66,8 +66,8 @@ export async function createOrUpgradeSavedConfig( await savedObjectsClient.create('config', attributes, { id: version, // config is a global object that should not belong to any of the workspaces - // declare it as empty array to prevent it being created as a workspace object - workspaces: [], + // declare it as null to prevent it being created as a workspace object + workspaces: null, }); } catch (error) { if (handleWriteErrors) { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 8715606da681..ea8445a4ccaf 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -50,7 +50,7 @@ describe('WorkspaceIdConsumerWrapper', () => { ); }); - it(`Should use options.workspaces there is workspaces inside options`, async () => { + it(`Should use options.workspaces when there is no workspaces inside options`, async () => { await wrapperClient.create( 'dashboard', { @@ -59,7 +59,7 @@ describe('WorkspaceIdConsumerWrapper', () => { { id: 'dashboard:foo', overwrite: true, - workspaces: undefined, + workspaces: null, } ); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index d24496ed31ef..bcf6fe4d5e8c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -14,11 +14,7 @@ import { SavedObjectsFindOptions, } from '../../../../core/server'; -type WorkspaceOptions = - | { - workspaces?: string[]; - } - | undefined; +type WorkspaceOptions = Pick | undefined; export class WorkspaceIdConsumerWrapper { private formatWorkspaceIdParams( @@ -30,8 +26,8 @@ export class WorkspaceIdConsumerWrapper { const workspaceIdParsedFromRequest = workspaceState?.id; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; - if (workspaceIdsInUserOptions) { - finalWorkspaces = workspaceIdsInUserOptions; + if (options?.hasOwnProperty('workspaces')) { + finalWorkspaces = workspaceIdsInUserOptions || []; } else if (workspaceIdParsedFromRequest) { finalWorkspaces = [workspaceIdParsedFromRequest]; } From cdbe9a83a5c87f4dce96e1550da94738dd86e3ac Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 25 Mar 2024 11:30:12 +0800 Subject: [PATCH 15/36] feat: improve code coverage Signed-off-by: SuZhou-Joe --- .../server/saved_objects/workspace_id_consumer_wrapper.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index ea8445a4ccaf..d4b79868d0e5 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -93,7 +93,7 @@ describe('WorkspaceIdConsumerWrapper', () => { }); it(`Should add workspaces parameters when checkConflict`, async () => { - await wrapperClient.checkConflicts([]); + await wrapperClient.checkConflicts(); expect(mockedClient.checkConflicts).toBeCalledWith([], { workspaces: ['foo'], }); From cb7ebd96d134b6b505f544757d6535dbb2776326 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Mar 2024 11:04:07 +0800 Subject: [PATCH 16/36] feat: declare workspaces as null Signed-off-by: SuZhou-Joe --- src/core/server/saved_objects/types.ts | 2 +- .../saved_objects/workspace_saved_objects_client_wrapper.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 23170e0a1c47..0b31baf5c111 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -112,7 +112,7 @@ export interface SavedObjectsFindOptions { /** An optional OpenSearch preference value to be used for the query **/ preference?: string; /** If specified, will only retrieve objects that are in the workspaces */ - workspaces?: string[]; + workspaces?: string[] | null; /** By default the operator will be 'AND' */ workspacesSearchOperator?: 'AND' | 'OR'; /** diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 1a97b2dac69a..48160c31af63 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -468,9 +468,9 @@ export class WorkspaceSavedObjectsClientWrapper { [WorkspacePermissionMode.LibraryRead, WorkspacePermissionMode.LibraryWrite] ), }, - // By declaring workspaces as empty array, + // By declaring workspaces as null, // workspaces won't be appended automatically into the options. - workspaces: [], + workspaces: null, }) ).saved_objects.map((item) => item.id); From 294d2c557dd5d36e78455af3f3a7d2a426299584 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 27 Mar 2024 11:31:05 +0800 Subject: [PATCH 17/36] feat: use unified types Signed-off-by: SuZhou-Joe --- src/core/public/saved_objects/saved_objects_client.ts | 4 ++-- .../saved_objects/export/get_sorted_objects_for_export.ts | 4 ++-- src/core/server/saved_objects/import/check_conflicts.ts | 2 +- .../server/saved_objects/import/create_saved_objects.ts | 2 +- src/core/server/saved_objects/import/types.ts | 4 ++-- src/core/server/saved_objects/serialization/types.ts | 6 +++--- .../saved_objects/service/lib/search_dsl/query_params.ts | 2 +- .../saved_objects/service/lib/search_dsl/search_dsl.ts | 2 +- .../server/saved_objects/service/saved_objects_client.ts | 2 +- src/core/server/saved_objects/types.ts | 4 ++-- src/core/types/saved_objects.ts | 2 +- src/plugins/workspace/server/permission_control/client.ts | 2 +- 12 files changed, 18 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 f415e9c58596..ce3ebc710ed1 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -65,7 +65,7 @@ export interface SavedObjectsCreateOptions { /** {@inheritDoc SavedObjectsMigrationVersion} */ migrationVersion?: SavedObjectsMigrationVersion; references?: SavedObjectReference[]; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -83,7 +83,7 @@ export interface SavedObjectsBulkCreateObject extends SavedObjectsC export interface SavedObjectsBulkCreateOptions { /** If a document with the given `id` already exists, overwrite it's contents (default=false). */ overwrite?: boolean; - workspaces?: string[]; + workspaces?: SavedObjectsCreateOptions['workspaces']; } /** @public */ 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..6c99db6c24af 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 @@ -61,7 +61,7 @@ export interface SavedObjectsExportOptions { /** optional namespace to override the namespace used by the savedObjectsClient. */ namespace?: string; /** optional workspaces to override the workspaces used by the savedObjectsClient. */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -97,7 +97,7 @@ async function fetchObjectsToExport({ exportSizeLimit: number; savedObjectsClient: SavedObjectsClientContract; namespace?: string; - workspaces?: string[]; + workspaces?: SavedObjectsExportOptions['workspaces']; }) { if ((types?.length ?? 0) > 0 && (objects?.length ?? 0) > 0) { throw Boom.badRequest(`Can't specify both "types" and "objects" properties when exporting`); diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index 2c35ba147c5d..d173af45f761 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -44,7 +44,7 @@ interface CheckConflictsParams { ignoreRegularConflicts?: boolean; retries?: SavedObjectsImportRetry[]; createNewCopies?: boolean; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } const isUnresolvableConflict = (error: SavedObjectError) => 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 6b0015851baf..f98e2e816b75 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -41,7 +41,7 @@ interface CreateSavedObjectsParams { overwrite?: boolean; dataSourceId?: string; dataSourceTitle?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } interface CreateSavedObjectsResult { createdObjects: Array>; diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 994b7e627189..689b8222b510 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -190,7 +190,7 @@ export interface SavedObjectsImportOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** @@ -215,7 +215,7 @@ export interface SavedObjectsResolveImportErrorsOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/serialization/types.ts b/src/core/server/saved_objects/serialization/types.ts index f882596ce529..6b1f025e856d 100644 --- a/src/core/server/saved_objects/serialization/types.ts +++ b/src/core/server/saved_objects/serialization/types.ts @@ -29,7 +29,7 @@ */ import { Permissions } from '../permission_control'; -import { SavedObjectsMigrationVersion, SavedObjectReference } from '../types'; +import { SavedObjectsMigrationVersion, SavedObjectReference, SavedObject } from '../types'; /** * A raw document as represented directly in the saved object index. @@ -53,7 +53,7 @@ export interface SavedObjectsRawDocSource { updated_at?: string; references?: SavedObjectReference[]; originId?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; [typeMapping: string]: any; } @@ -71,7 +71,7 @@ interface SavedObjectDoc { version?: string; updated_at?: string; originId?: string; - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; permissions?: Permissions; } 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 abbef0850dba..318768fd83c2 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 @@ -167,7 +167,7 @@ interface QueryParams { defaultSearchOperator?: string; hasReference?: HasReferenceQueryParams; kueryNode?: KueryNode; - workspaces?: string[]; + workspaces?: SavedObjectsFindOptions['workspaces']; workspacesSearchOperator?: 'AND' | 'OR'; ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams']; } 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 fa4311576638..626fc7efba3e 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 @@ -53,7 +53,7 @@ interface GetSearchDslOptions { id: string; }; kueryNode?: KueryNode; - workspaces?: string[]; + workspaces?: SavedObjectsFindOptions['workspaces']; workspacesSearchOperator?: 'AND' | 'OR'; ACLSearchParams?: SavedObjectsFindOptions['ACLSearchParams']; } 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 1b268e033d97..be02ef247d04 100644 --- a/src/core/server/saved_objects/service/saved_objects_client.ts +++ b/src/core/server/saved_objects/service/saved_objects_client.ts @@ -97,7 +97,7 @@ export interface SavedObjectsBulkCreateObject { /** * workspaces the objects belong to, will only be used when overwrite is enabled. */ - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; } /** diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 0b31baf5c111..22093b90d6b3 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -112,7 +112,7 @@ export interface SavedObjectsFindOptions { /** An optional OpenSearch preference value to be used for the query **/ preference?: string; /** If specified, will only retrieve objects that are in the workspaces */ - workspaces?: string[] | null; + workspaces?: SavedObjectsBaseOptions['workspaces']; /** By default the operator will be 'AND' */ workspacesSearchOperator?: 'AND' | 'OR'; /** @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: string[] | null; + workspaces?: SavedObject['workspaces']; } /** diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index b37309338c9e..e91a5012098e 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -115,7 +115,7 @@ export interface SavedObject { */ originId?: string; /** Workspaces that this saved object exists in. */ - workspaces?: string[]; + workspaces?: string[] | null; /** Permissions that this saved objects exists in. */ permissions?: Permissions; } diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index bad46eb156a6..03a8fb0aaf3d 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -147,7 +147,7 @@ export class SavedObjectsPermissionControl { const principals = this.getPrincipalsFromRequest(request); const deniedObjects: Array< Pick & { - workspaces?: string[]; + workspaces?: SavedObject['workspaces']; permissions?: Permissions; } > = []; From 42e0e051896c900ea9cc573c71b66f7a173c7e17 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Thu, 28 Mar 2024 10:46:18 +0800 Subject: [PATCH 18/36] feat: update comment Signed-off-by: SuZhou-Joe --- .../saved_objects/workspace_saved_objects_client_wrapper.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index 48160c31af63..d5497bc6475c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -470,6 +470,7 @@ export class WorkspaceSavedObjectsClientWrapper { }, // By declaring workspaces as null, // workspaces won't be appended automatically into the options. + // or workspaces can not be found because workspace object do not have `workspaces` field. workspaces: null, }) ).saved_objects.map((item) => item.id); From 1b323162ecb8e844f2f6a345c5e507c0d71491d6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 1 Apr 2024 17:13:00 +0800 Subject: [PATCH 19/36] feat: remove null Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.test.ts | 2 -- .../create_or_upgrade_saved_config.ts | 3 --- 2 files changed, 5 deletions(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts index fc5a55da0091..eb23e78b1756 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts @@ -98,7 +98,6 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, - workspaces: null, } ); }); @@ -134,7 +133,6 @@ describe('uiSettings/createOrUpgradeSavedConfig', function () { }, { id: version, - workspaces: null, } ); }); diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 6ca3baeb7a9f..6649966d8952 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -65,9 +65,6 @@ export async function createOrUpgradeSavedConfig( // create the new SavedConfig await savedObjectsClient.create('config', attributes, { id: version, - // config is a global object that should not belong to any of the workspaces - // declare it as null to prevent it being created as a workspace object - workspaces: null, }); } catch (error) { if (handleWriteErrors) { From b7a6cb7bec00944e89a9e51c25543f4be29f5ea6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Tue, 2 Apr 2024 15:37:50 +0800 Subject: [PATCH 20/36] feat: address comments Signed-off-by: SuZhou-Joe --- .../create_or_upgrade_saved_config.ts | 4 +--- .../saved_objects/workspace_id_consumer_wrapper.test.ts | 2 +- .../server/saved_objects/workspace_id_consumer_wrapper.ts | 1 + 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts index 6649966d8952..639dd09249ff 100644 --- a/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts +++ b/src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts @@ -63,9 +63,7 @@ export async function createOrUpgradeSavedConfig( try { // create the new SavedConfig - await savedObjectsClient.create('config', attributes, { - id: version, - }); + await savedObjectsClient.create('config', attributes, { id: version }); } catch (error) { if (handleWriteErrors) { if (SavedObjectsErrorHelpers.isConflictError(error)) { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index d4b79868d0e5..21ef63c04867 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -50,7 +50,7 @@ describe('WorkspaceIdConsumerWrapper', () => { ); }); - it(`Should use options.workspaces when there is no workspaces inside options`, async () => { + it(`Should not use options.workspaces when there is no workspaces inside options`, async () => { await wrapperClient.create( 'dashboard', { diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index bcf6fe4d5e8c..8ebd6b301563 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -71,6 +71,7 @@ export class WorkspaceIdConsumerWrapper { bulkUpdate: wrapperOptions.client.bulkUpdate, addToNamespaces: wrapperOptions.client.addToNamespaces, deleteFromNamespaces: wrapperOptions.client.deleteFromNamespaces, + deleteByWorkspace: wrapperOptions.client.deleteByWorkspace, }; }; From 55d9b820e54089d5b169adcda0a44e6810d6a626 Mon Sep 17 00:00:00 2001 From: tygao Date: Tue, 2 Apr 2024 16:39:05 +0800 Subject: [PATCH 21/36] Add features quantity in workspace list page (#309) * feat: unite feature set Signed-off-by: tygao * update feature utils Signed-off-by: tygao * update feature utils Signed-off-by: tygao * use featureMatchConfig Signed-off-by: tygao * update Signed-off-by: tygao * add util test Signed-off-by: tygao * unite feature set Signed-off-by: tygao --------- Signed-off-by: tygao --- .../workspace_feature_selector.tsx | 22 +++------ .../components/workspace_list/index.tsx | 7 +++ src/plugins/workspace/public/hooks.ts | 9 ++-- src/plugins/workspace/public/utils.test.ts | 48 ++++++++++++++++++- src/plugins/workspace/public/utils.ts | 30 +++++++++++- 5 files changed, 94 insertions(+), 22 deletions(-) diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx index 61181a7a749e..e172b851f1f3 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_feature_selector.tsx @@ -16,14 +16,11 @@ import { import { i18n } from '@osd/i18n'; import { groupBy } from 'lodash'; -import { - AppNavLinkStatus, - DEFAULT_APP_CATEGORIES, - PublicAppInfo, -} from '../../../../../core/public'; +import { DEFAULT_APP_CATEGORIES, PublicAppInfo } from '../../../../../core/public'; import { WorkspaceFeature, WorkspaceFeatureGroup } from './types'; import { isDefaultCheckedFeatureId, isWorkspaceFeatureGroup } from './utils'; +import { getAllExcludingManagementApps } from '../../utils'; const libraryCategoryLabel = i18n.translate('core.ui.libraryNavList.label', { defaultMessage: 'Library', @@ -58,17 +55,10 @@ export const WorkspaceFeatureSelector = ({ Array >((previousValue, currentKey) => { const apps = category2Applications[currentKey]; - const features = apps - .filter( - ({ navLinkStatus, chromeless, category }) => - navLinkStatus !== AppNavLinkStatus.hidden && - !chromeless && - category?.id !== DEFAULT_APP_CATEGORIES.management.id - ) - .map(({ id, title }) => ({ - id, - name: title, - })); + const features = getAllExcludingManagementApps(apps).map(({ id, title }) => ({ + id, + name: title, + })); if (features.length === 0) { return previousValue; } diff --git a/src/plugins/workspace/public/components/workspace_list/index.tsx b/src/plugins/workspace/public/components/workspace_list/index.tsx index bc92a01f8f58..aac721c236a6 100644 --- a/src/plugins/workspace/public/components/workspace_list/index.tsx +++ b/src/plugins/workspace/public/components/workspace_list/index.tsx @@ -26,6 +26,8 @@ import { WORKSPACE_CREATE_APP_ID } from '../../../common/constants'; import { cleanWorkspaceId } from '../../../../../core/public'; import { DeleteWorkspaceModal } from '../delete_workspace_modal'; +import { useApplications } from '././../../hooks'; +import { getSelectedFeatureQuantities } from '../../utils'; const WORKSPACE_LIST_PAGE_DESCRIPTIOIN = i18n.translate('workspace.list.description', { defaultMessage: @@ -47,6 +49,7 @@ export const WorkspaceList = () => { pageSizeOptions: [5, 10, 20], }); const [deletedWorkspace, setDeletedWorkspace] = useState(null); + const applications = useApplications(application); const handleSwitchWorkspace = useCallback( (id: string) => { @@ -106,6 +109,10 @@ export const WorkspaceList = () => { name: 'Features', isExpander: true, hasActions: true, + render: (features: string[]) => { + const { total, selected } = getSelectedFeatureQuantities(features, applications); + return `${selected}/${total}`; + }, }, { name: 'Actions', diff --git a/src/plugins/workspace/public/hooks.ts b/src/plugins/workspace/public/hooks.ts index e84ee46507ef..a63dc8f83d3d 100644 --- a/src/plugins/workspace/public/hooks.ts +++ b/src/plugins/workspace/public/hooks.ts @@ -3,15 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { ApplicationStart, PublicAppInfo } from 'opensearch-dashboards/public'; import { useObservable } from 'react-use'; import { useMemo } from 'react'; +import { of } from 'rxjs'; +import { ApplicationStart, PublicAppInfo } from '../../../core/public'; -export function useApplications(application: ApplicationStart) { - const applications = useObservable(application.applications$); +export function useApplications(application?: ApplicationStart) { + const applications = useObservable(application?.applications$ ?? of(new Map()), new Map()); return useMemo(() => { const apps: PublicAppInfo[] = []; - applications?.forEach((app) => { + applications.forEach((app) => { apps.push(app); }); return apps; diff --git a/src/plugins/workspace/public/utils.test.ts b/src/plugins/workspace/public/utils.test.ts index 510a775cd745..5ce89d9fffc6 100644 --- a/src/plugins/workspace/public/utils.test.ts +++ b/src/plugins/workspace/public/utils.test.ts @@ -3,7 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { featureMatchesConfig } from './utils'; +import { featureMatchesConfig, getSelectedFeatureQuantities } from './utils'; +import { PublicAppInfo } from '../../../core/public'; describe('workspace utils: featureMatchesConfig', () => { it('feature configured with `*` should match any features', () => { @@ -91,3 +92,48 @@ describe('workspace utils: featureMatchesConfig', () => { ); }); }); + +describe('workspace utils: getSelectedFeatureQuantities', () => { + const defaultApplications = [ + { + appRoute: '/app/dashboards', + id: 'dashboards', + title: 'Dashboards', + category: { + id: 'opensearchDashboards', + label: 'OpenSearch Dashboards', + euiIconType: 'inputOutput', + order: 1000, + }, + status: 0, + navLinkStatus: 1, + }, + { + appRoute: '/app/dev_tools', + id: 'dev_tools', + title: 'Dev Tools', + category: { + id: 'management', + label: 'Management', + order: 5000, + euiIconType: 'managementApp', + }, + status: 0, + navLinkStatus: 1, + }, + ] as PublicAppInfo[]; + it('should support * rules and exclude management category', () => { + const { total, selected } = getSelectedFeatureQuantities(['*'], defaultApplications); + expect(total).toBe(1); + expect(selected).toBe(1); + }); + + it('should get quantity normally and exclude management category', () => { + const { total, selected } = getSelectedFeatureQuantities( + ['dev_tools', '!@management'], + defaultApplications + ); + expect(total).toBe(1); + expect(selected).toBe(0); + }); +}); diff --git a/src/plugins/workspace/public/utils.ts b/src/plugins/workspace/public/utils.ts index 444b3aadadf3..2c0ad62d7775 100644 --- a/src/plugins/workspace/public/utils.ts +++ b/src/plugins/workspace/public/utils.ts @@ -3,7 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { AppCategory } from '../../../core/public'; +import { + AppCategory, + PublicAppInfo, + AppNavLinkStatus, + DEFAULT_APP_CATEGORIES, +} from '../../../core/public'; /** * Checks if a given feature matches the provided feature configuration. @@ -55,3 +60,26 @@ export const featureMatchesConfig = (featureConfigs: string[]) => ({ return matched; }; + +// Get all apps excluding management category +export const getAllExcludingManagementApps = (applications: PublicAppInfo[]): PublicAppInfo[] => { + return applications.filter( + ({ navLinkStatus, chromeless, category }) => + navLinkStatus !== AppNavLinkStatus.hidden && + !chromeless && + category?.id !== DEFAULT_APP_CATEGORIES.management.id + ); +}; + +export const getSelectedFeatureQuantities = ( + featuresConfig: string[], + applications: PublicAppInfo[] +) => { + const visibleApplications = getAllExcludingManagementApps(applications); + const featureFilter = featureMatchesConfig(featuresConfig); + const selectedApplications = visibleApplications.filter((app) => featureFilter(app)); + return { + total: visibleApplications.length, + selected: selectedApplications.length, + }; +}; From dd6cebfca07f3f216578a407fef8cec279927337 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 3 Apr 2024 09:34:21 +0800 Subject: [PATCH 22/36] feat: use request app to store request workspace id Signed-off-by: SuZhou-Joe --- src/core/server/utils/workspace.test.ts | 4 ++-- src/core/server/utils/workspace.ts | 31 +++++++++++-------------- src/legacy/server/osd_server.d.ts | 8 ------- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts index 49382cfac38f..7dfcff9e5d18 100644 --- a/src/core/server/utils/workspace.test.ts +++ b/src/core/server/utils/workspace.test.ts @@ -10,10 +10,10 @@ describe('updateWorkspaceState', () => { it('update with payload', () => { const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(requestMock, { - id: 'foo', + requestWorkspaceId: 'foo', }); expect(getWorkspaceState(requestMock)).toEqual({ - id: 'foo', + requestWorkspaceId: 'foo', }); }); }); diff --git a/src/core/server/utils/workspace.ts b/src/core/server/utils/workspace.ts index c5dcf84b92d9..831755c414a5 100644 --- a/src/core/server/utils/workspace.ts +++ b/src/core/server/utils/workspace.ts @@ -3,14 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -/** - * This file is using {@link PluginsStates} to store workspace info into request. - * The best practice would be using {@link Server.register} to register plugins into the hapi server - * but OSD is wrappering the hapi server and the hapi server instance is hidden as internal implementation. - */ -import { PluginsStates } from '@hapi/hapi'; import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; +export interface WorkspaceState { + requestWorkspaceId?: string; +} + /** * This function will be used as a proxy * because `ensureRequest` is only importable from core module. @@ -20,24 +18,23 @@ import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; */ export const updateWorkspaceState = ( request: OpenSearchDashboardsRequest, - payload: Partial + payload: Partial ) => { const rawRequest = ensureRawRequest(request); - if (!rawRequest.plugins) { - rawRequest.plugins = {}; + if (!rawRequest.app) { + rawRequest.app = {}; } - if (!rawRequest.plugins.workspace) { - rawRequest.plugins.workspace = {}; - } - - rawRequest.plugins.workspace = { - ...rawRequest.plugins.workspace, + rawRequest.app = { + ...rawRequest.app, ...payload, }; }; -export const getWorkspaceState = (request: OpenSearchDashboardsRequest) => { - return ensureRawRequest(request).plugins?.workspace; +export const getWorkspaceState = (request: OpenSearchDashboardsRequest): WorkspaceState => { + const { requestWorkspaceId } = ensureRawRequest(request).app as WorkspaceState; + return { + requestWorkspaceId, + }; }; diff --git a/src/legacy/server/osd_server.d.ts b/src/legacy/server/osd_server.d.ts index 9d94349cf1c0..1f8535b59e87 100644 --- a/src/legacy/server/osd_server.d.ts +++ b/src/legacy/server/osd_server.d.ts @@ -60,14 +60,6 @@ declare module 'hapi' { } } -declare module '@hapi/hapi' { - interface PluginsStates { - workspace?: { - id?: string; - }; - } -} - type OsdMixinFunc = (osdServer: OsdServer, server: Server, config: any) => Promise | void; export interface PluginsSetup { From 7fd5160d3275661fc29ba48858e3ba1349ef75b6 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Wed, 3 Apr 2024 09:51:20 +0800 Subject: [PATCH 23/36] feat: use app state to store request workspace id Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.ts | 2 +- .../server/saved_objects/workspace_id_consumer_wrapper.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 088096df4d35..e7029f8387f6 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -54,7 +54,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { if (workspaceId) { updateWorkspaceState(request, { - id: workspaceId, + requestWorkspaceId: workspaceId, }); const requestUrl = new URL(request.url.toString()); requestUrl.pathname = cleanWorkspaceId(requestUrl.pathname); diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts index 8ebd6b301563..74e8e99af71e 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.ts @@ -23,7 +23,7 @@ export class WorkspaceIdConsumerWrapper { ): T { const { workspaces, ...others } = options || {}; const workspaceState = getWorkspaceState(request); - const workspaceIdParsedFromRequest = workspaceState?.id; + const workspaceIdParsedFromRequest = workspaceState?.requestWorkspaceId; const workspaceIdsInUserOptions = options?.workspaces; let finalWorkspaces: string[] = []; if (options?.hasOwnProperty('workspaces')) { From 43c4e9678abece4274ea75aba40eedc6e80966ea Mon Sep 17 00:00:00 2001 From: Hailong Cui Date: Sun, 7 Apr 2024 10:39:46 +0800 Subject: [PATCH 24/36] Workspace overview blank (#317) * Add workspace overview page (#19) * feat: add workspace overview page Signed-off-by: Lin Wang * refactor: move paths to common constants Signed-off-by: Lin Wang * feat: add workspace overview item by custom nav in start phase Signed-off-by: Lin Wang * refactor: change to currentWorkspace$ in workspace client Signed-off-by: Lin Wang --------- Signed-off-by: Lin Wang * restore yml Signed-off-by: Hailong Cui --------- Signed-off-by: Lin Wang Signed-off-by: Hailong Cui Co-authored-by: Lin Wang --- src/plugins/workspace/public/application.tsx | 14 ++++++++ .../workspace_overview/workspace_overview.tsx | 31 ++++++++++++++++ .../components/workspace_overview_app.tsx | 35 +++++++++++++++++++ src/plugins/workspace/public/plugin.ts | 13 +++++++ 4 files changed, 93 insertions(+) create mode 100644 src/plugins/workspace/public/components/workspace_overview/workspace_overview.tsx create mode 100644 src/plugins/workspace/public/components/workspace_overview_app.tsx diff --git a/src/plugins/workspace/public/application.tsx b/src/plugins/workspace/public/application.tsx index a0af9e695c31..87fcbc555264 100644 --- a/src/plugins/workspace/public/application.tsx +++ b/src/plugins/workspace/public/application.tsx @@ -12,6 +12,7 @@ import { WorkspaceFatalError } from './components/workspace_fatal_error'; import { WorkspaceCreatorApp } from './components/workspace_creator_app'; import { WorkspaceUpdaterApp } from './components/workspace_updater_app'; import { Services } from './types'; +import { WorkspaceOverviewApp } from './components/workspace_overview_app'; export const renderCreatorApp = ({ element }: AppMountParameters, services: Services) => { ReactDOM.render( @@ -66,3 +67,16 @@ export const renderListApp = ({ element }: AppMountParameters, services: Service ReactDOM.unmountComponentAtNode(element); }; }; + +export const renderOverviewApp = ({ element }: AppMountParameters, services: Services) => { + ReactDOM.render( + + + , + element + ); + + return () => { + ReactDOM.unmountComponentAtNode(element); + }; +}; diff --git a/src/plugins/workspace/public/components/workspace_overview/workspace_overview.tsx b/src/plugins/workspace/public/components/workspace_overview/workspace_overview.tsx new file mode 100644 index 000000000000..1a33f3ef233d --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_overview/workspace_overview.tsx @@ -0,0 +1,31 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { EuiPanel, EuiSpacer, EuiTitle } from '@elastic/eui'; +import { useObservable } from 'react-use'; +import { of } from 'rxjs'; + +import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; + +export const WorkspaceOverview = () => { + const { + services: { workspaces }, + } = useOpenSearchDashboards(); + + const currentWorkspace = useObservable(workspaces ? workspaces.currentWorkspace$ : of(null)); + + return ( + <> + + +

Workspace

+ + + {JSON.stringify(currentWorkspace)} + + + ); +}; diff --git a/src/plugins/workspace/public/components/workspace_overview_app.tsx b/src/plugins/workspace/public/components/workspace_overview_app.tsx new file mode 100644 index 000000000000..034dc12e1c62 --- /dev/null +++ b/src/plugins/workspace/public/components/workspace_overview_app.tsx @@ -0,0 +1,35 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React, { useEffect } from 'react'; +import { I18nProvider } from '@osd/i18n/react'; +import { i18n } from '@osd/i18n'; +import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public'; +import { WorkspaceOverview } from './workspace_overview/workspace_overview'; + +export const WorkspaceOverviewApp = () => { + const { + services: { chrome }, + } = useOpenSearchDashboards(); + + /** + * set breadcrumbs to chrome + */ + useEffect(() => { + chrome?.setBreadcrumbs([ + { + text: i18n.translate('workspace.workspaceOverviewTitle', { + defaultMessage: 'Workspace overview', + }), + }, + ]); + }, [chrome]); + + return ( + + + + ); +}; diff --git a/src/plugins/workspace/public/plugin.ts b/src/plugins/workspace/public/plugin.ts index a4910f50d86a..1950d199074b 100644 --- a/src/plugins/workspace/public/plugin.ts +++ b/src/plugins/workspace/public/plugin.ts @@ -190,6 +190,19 @@ export class WorkspacePlugin implements Plugin<{}, {}> { }, }); + // overview + core.application.register({ + id: WORKSPACE_OVERVIEW_APP_ID, + title: i18n.translate('workspace.settings.workspaceOverview', { + defaultMessage: 'Workspace Overview', + }), + navLinkStatus: AppNavLinkStatus.hidden, + async mount(params: AppMountParameters) { + const { renderOverviewApp } = await import('./application'); + return mountWorkspaceApp(params, renderOverviewApp); + }, + }); + // workspace fatal error core.application.register({ id: WORKSPACE_FATAL_ERROR_APP_ID, From b2ab2bd22f6a6b39d833b6ac893c1e35ed95e78c Mon Sep 17 00:00:00 2001 From: Lin Wang Date: Sun, 7 Apr 2024 13:02:44 +0800 Subject: [PATCH 25/36] Backport permission control to pr integr (#314) * [Workspace]Add permission control logic for workspace (#6052) * Add permission control for workspace Signed-off-by: Lin Wang * Add changelog for permission control in workspace Signed-off-by: Lin Wang * Fix integration tests and remove no need type Signed-off-by: Lin Wang * Update permission enabled for workspace CRUD integration tests Signed-off-by: Lin Wang * Change back to config schema Signed-off-by: Lin Wang * feat: do not append workspaces field when no workspaces present (#6) * feat: do not append workspaces field when no workspaces present Signed-off-by: SuZhou-Joe * feat: do not append workspaces field when no workspaces present Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe * fix: authInfo destructure (#7) * fix: authInfo destructure Signed-off-by: SuZhou-Joe * fix: unit test error Signed-off-by: SuZhou-Joe --------- Signed-off-by: SuZhou-Joe * Fix permissions assign in attributes Signed-off-by: Lin Wang * Remove deleteByWorkspace since not exists Signed-off-by: Lin Wang * refactor: remove formatWorkspacePermissionModeToStringArray Signed-off-by: Lin Wang * Remove current not used code Signed-off-by: Lin Wang * Add missing unit tests for permission control Signed-off-by: Lin Wang * Update workspaces API test describe Signed-off-by: Lin Wang * Fix workspace CRUD API integration tests failed Signed-off-by: Lin Wang * Address PR comments Signed-off-by: Lin Wang * Store permissions when savedObjects.permissions.enabled Signed-off-by: Lin Wang * Add permission control for deleteByWorkspace Signed-off-by: Lin Wang * Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe * Update src/plugins/workspace/server/permission_control/client.ts Signed-off-by: SuZhou-Joe * Refactor permissions field in workspace create and update API Signed-off-by: Lin Wang * Fix workspace CRUD API integration tests Signed-off-by: Lin Wang --------- Signed-off-by: Lin Wang Signed-off-by: SuZhou-Joe Co-authored-by: SuZhou-Joe Signed-off-by: Lin Wang * Convert permission settings in client side Signed-off-by: Lin Wang * Fix workspace list always render Signed-off-by: Lin Wang --------- Signed-off-by: Lin Wang Signed-off-by: SuZhou-Joe Co-authored-by: SuZhou-Joe --- src/core/server/index.ts | 3 +- src/core/server/mocks.ts | 3 + .../server/plugins/plugin_context.test.ts | 7 +- src/core/server/plugins/types.ts | 2 +- src/core/server/saved_objects/index.ts | 8 +- .../service/lib/repository.test.js | 15 +- .../saved_objects/service/lib/repository.ts | 4 +- src/core/types/saved_objects.ts | 2 + src/core/types/workspace.ts | 8 +- .../workspace_creator.test.tsx | 13 +- .../workspace_creator/workspace_creator.tsx | 8 +- .../public/components/workspace_form/types.ts | 4 +- .../workspace_form/use_workspace_form.ts | 24 ++- .../public/components/workspace_form/utils.ts | 76 ++++++++ .../workspace_form/workspace_form.tsx | 2 +- .../components/workspace_list/index.tsx | 9 +- .../workspace_updater.test.tsx | 9 +- .../workspace_updater/workspace_updater.tsx | 37 ++-- src/plugins/workspace/public/hooks.ts | 4 +- .../workspace/public/workspace_client.ts | 23 +-- .../server/integration_tests/routes.test.ts | 177 +++++++++++------- .../server/permission_control/client.test.ts | 77 ++++++++ .../server/permission_control/client.ts | 62 +++--- src/plugins/workspace/server/plugin.test.ts | 3 - src/plugins/workspace/server/plugin.ts | 18 +- src/plugins/workspace/server/routes/index.ts | 78 ++++---- ...space_saved_objects_client_wrapper.test.ts | 66 +++++++ ...space_saved_objects_client_wrapper.test.ts | 82 +++++++- .../workspace_saved_objects_client_wrapper.ts | 32 +--- src/plugins/workspace/server/types.ts | 30 +-- .../workspace/server/workspace_client.ts | 25 ++- 31 files changed, 627 insertions(+), 284 deletions(-) diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 84ee65dcb199..f497bed22755 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -321,12 +321,11 @@ export { exportSavedObjectsToStream, importSavedObjectsFromStream, resolveSavedObjectsImportErrors, - SavedObjectsDeleteByWorkspaceOptions, ACL, Principals, - TransformedPermission, PrincipalType, Permissions, + SavedObjectsDeleteByWorkspaceOptions, } from './saved_objects'; export { diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts index 687d408e40a6..dce39d03da7f 100644 --- a/src/core/server/mocks.ts +++ b/src/core/server/mocks.ts @@ -89,6 +89,9 @@ export function pluginInitializerContextConfigMock(config: T) { path: { data: '/tmp' }, savedObjects: { maxImportPayloadBytes: new ByteSizeValue(26214400), + permission: { + enabled: true, + }, }, }; diff --git a/src/core/server/plugins/plugin_context.test.ts b/src/core/server/plugins/plugin_context.test.ts index 7a8ba042825b..57aa372514de 100644 --- a/src/core/server/plugins/plugin_context.test.ts +++ b/src/core/server/plugins/plugin_context.test.ts @@ -108,7 +108,12 @@ describe('createPluginInitializerContext', () => { pingTimeout: duration(30, 's'), }, path: { data: fromRoot('data') }, - savedObjects: { maxImportPayloadBytes: new ByteSizeValue(26214400) }, + savedObjects: { + maxImportPayloadBytes: new ByteSizeValue(26214400), + permission: { + enabled: false, + }, + }, }); }); diff --git a/src/core/server/plugins/types.ts b/src/core/server/plugins/types.ts index 59b9881279c3..c225a24aa386 100644 --- a/src/core/server/plugins/types.ts +++ b/src/core/server/plugins/types.ts @@ -295,7 +295,7 @@ export const SharedGlobalConfigKeys = { ] as const, opensearch: ['shardTimeout', 'requestTimeout', 'pingTimeout'] as const, path: ['data'] as const, - savedObjects: ['maxImportPayloadBytes'] as const, + savedObjects: ['maxImportPayloadBytes', 'permission'] as const, }; /** diff --git a/src/core/server/saved_objects/index.ts b/src/core/server/saved_objects/index.ts index 11809c5b88c9..dccf63d4dcf4 100644 --- a/src/core/server/saved_objects/index.ts +++ b/src/core/server/saved_objects/index.ts @@ -85,10 +85,4 @@ export { export { savedObjectsConfig, savedObjectsMigrationConfig } from './saved_objects_config'; export { SavedObjectTypeRegistry, ISavedObjectTypeRegistry } from './saved_objects_type_registry'; -export { - Permissions, - ACL, - Principals, - TransformedPermission, - PrincipalType, -} from './permission_control/acl'; +export { Permissions, ACL, Principals, PrincipalType } from './permission_control/acl'; 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 d26589882273..087ff2e458d9 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -167,7 +167,7 @@ describe('SavedObjectsRepository', () => { }); const getMockGetResponse = ( - { type, id, references, namespace: objectNamespace, originId, workspaces, permissions }, + { type, id, references, namespace: objectNamespace, originId, permissions, workspaces }, namespace ) => { const namespaceId = objectNamespace === 'default' ? undefined : objectNamespace ?? namespace; @@ -181,9 +181,9 @@ describe('SavedObjectsRepository', () => { _source: { ...(registry.isSingleNamespace(type) && { namespace: namespaceId }), ...(registry.isMultiNamespace(type) && { namespaces: [namespaceId ?? 'default'] }), - workspaces, ...(originId && { originId }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), type, [type]: { title: 'Testing' }, references, @@ -3169,7 +3169,7 @@ describe('SavedObjectsRepository', () => { const namespace = 'foo-namespace'; const originId = 'some-origin-id'; - const getSuccess = async (type, id, options, includeOriginId, permissions) => { + const getSuccess = async (type, id, options, includeOriginId, permissions, workspaces) => { const response = getMockGetResponse( { type, @@ -3178,6 +3178,7 @@ describe('SavedObjectsRepository', () => { // operation will return it in the result. This flag is just used for test purposes to modify the mock cluster call response. ...(includeOriginId && { originId }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), }, options?.namespace ); @@ -3343,6 +3344,14 @@ describe('SavedObjectsRepository', () => { permissions: permissions, }); }); + + it(`includes workspaces property if present`, async () => { + const workspaces = ['workspace-1']; + const result = await getSuccess(type, id, { namespace }, undefined, undefined, workspaces); + expect(result).toMatchObject({ + workspaces: workspaces, + }); + }); }); }); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index c3de94bf84b9..34ff9b1e0d8f 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -1044,7 +1044,7 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id); } - const { originId, updated_at: updatedAt, workspaces, permissions } = body._source; + const { originId, updated_at: updatedAt, permissions, workspaces } = body._source; let namespaces: string[] = []; if (!this._registry.isNamespaceAgnostic(type)) { @@ -1059,8 +1059,8 @@ export class SavedObjectsRepository { namespaces, ...(originId && { originId }), ...(updatedAt && { updated_at: updatedAt }), - ...(workspaces && { workspaces }), ...(permissions && { permissions }), + ...(workspaces && { workspaces }), version: encodeHitVersion(body), attributes: body._source[type], references: body._source.references || [], diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index b37309338c9e..74890bb624a3 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -126,3 +126,5 @@ export interface SavedObjectError { statusCode: number; metadata?: Record; } + +export type SavedObjectPermissions = Permissions; diff --git a/src/core/types/workspace.ts b/src/core/types/workspace.ts index ffad76fb48a2..7cdc3f92382b 100644 --- a/src/core/types/workspace.ts +++ b/src/core/types/workspace.ts @@ -3,6 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { Permissions } from '../server/saved_objects'; + export interface WorkspaceAttribute { id: string; name: string; @@ -14,6 +16,10 @@ export interface WorkspaceAttribute { defaultVISTheme?: string; } -export interface WorkspaceObject extends WorkspaceAttribute { +export interface WorkspaceAttributeWithPermission extends WorkspaceAttribute { + permissions?: Permissions; +} + +export interface WorkspaceObject extends WorkspaceAttributeWithPermission { readonly?: boolean; } diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx index f10fd39cfe9d..b67870b55294 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.test.tsx @@ -152,7 +152,7 @@ describe('WorkspaceCreator', () => { color: '#000000', description: 'test workspace description', }), - expect.any(Array) + undefined ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); @@ -174,7 +174,7 @@ describe('WorkspaceCreator', () => { name: 'test workspace name', features: expect.arrayContaining(['app1', 'app2', 'app3']), }), - expect.any(Array) + undefined ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); @@ -201,7 +201,14 @@ describe('WorkspaceCreator', () => { expect.objectContaining({ name: 'test workspace name', }), - expect.arrayContaining([expect.objectContaining({ type: 'user', userId: 'test user id' })]) + { + read: { + users: ['test user id'], + }, + library_read: { + users: ['test user id'], + }, + } ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); diff --git a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx index 2b3511f18b8b..4b3e6e57c486 100644 --- a/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx +++ b/src/plugins/workspace/public/components/workspace_creator/workspace_creator.tsx @@ -11,6 +11,7 @@ import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from ' import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; import { WorkspaceClient } from '../../workspace_client'; +import { convertPermissionSettingsToPermissions } from '../workspace_form/utils'; export const WorkspaceCreator = () => { const { @@ -22,8 +23,11 @@ export const WorkspaceCreator = () => { async (data: WorkspaceFormSubmitData) => { let result; try { - const { permissions, ...attributes } = data; - result = await workspaceClient.create(attributes, permissions); + const { permissionSettings, ...attributes } = data; + result = await workspaceClient.create( + attributes, + convertPermissionSettingsToPermissions(permissionSettings) + ); } catch (error) { notifications?.toasts.addDanger({ title: i18n.translate('workspace.create.failed', { diff --git a/src/plugins/workspace/public/components/workspace_form/types.ts b/src/plugins/workspace/public/components/workspace_form/types.ts index 15af85965943..5f81ba3e6daa 100644 --- a/src/plugins/workspace/public/components/workspace_form/types.ts +++ b/src/plugins/workspace/public/components/workspace_form/types.ts @@ -5,7 +5,7 @@ import type { WorkspacePermissionItemType, WorkspaceOperationType } from './constants'; import type { WorkspacePermissionMode } from '../../../common/constants'; -import type { App, ApplicationStart } from '../../../../../core/public'; +import type { ApplicationStart } from '../../../../../core/public'; export type WorkspacePermissionSetting = | { type: WorkspacePermissionItemType.User; userId: string; modes: WorkspacePermissionMode[] } @@ -18,7 +18,7 @@ export interface WorkspaceFormSubmitData { color?: string; icon?: string; defaultVISTheme?: string; - permissions: WorkspacePermissionSetting[]; + permissionSettings?: WorkspacePermissionSetting[]; } export interface WorkspaceFormData extends WorkspaceFormSubmitData { diff --git a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts index 7158693aedff..00bee7b3ab37 100644 --- a/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts +++ b/src/plugins/workspace/public/components/workspace_form/use_workspace_form.ts @@ -46,8 +46,8 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works const [permissionSettings, setPermissionSettings] = useState< Array> >( - defaultValues?.permissions && defaultValues.permissions.length > 0 - ? defaultValues.permissions + defaultValues?.permissionSettings && defaultValues.permissionSettings.length > 0 + ? defaultValues.permissionSettings : [] ); @@ -58,7 +58,7 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works description, features: selectedFeatureIds, color, - permissions: permissionSettings, + permissionSettings, }); const getFormDataRef = useRef(getFormData); getFormDataRef.current = getFormData; @@ -96,12 +96,15 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works }), }; } - const permissionErrors: string[] = new Array(formData.permissions.length); - for (let i = 0; i < formData.permissions.length; i++) { - const permission = formData.permissions[i]; + const permissionErrors: string[] = new Array(formData.permissionSettings.length); + for (let i = 0; i < formData.permissionSettings.length; i++) { + const permission = formData.permissionSettings[i]; if (isValidWorkspacePermissionSetting(permission)) { if ( - isUserOrGroupPermissionSettingDuplicated(formData.permissions.slice(0, i), permission) + isUserOrGroupPermissionSettingDuplicated( + formData.permissionSettings.slice(0, i), + permission + ) ) { permissionErrors[i] = i18n.translate('workspace.form.permission.invalidate.group', { defaultMessage: 'Duplicate permission setting', @@ -162,8 +165,11 @@ export const useWorkspaceForm = ({ application, defaultValues, onSubmit }: Works formData.features = defaultValues?.features ?? []; } - const permissions = formData.permissions.filter(isValidWorkspacePermissionSetting); - onSubmit?.({ ...formData, name: formData.name!, permissions }); + onSubmit?.({ + ...formData, + name: formData.name!, + permissionSettings: formData.permissionSettings.filter(isValidWorkspacePermissionSetting), + }); }, [defaultFeatures, onSubmit, defaultValues?.features] ); diff --git a/src/plugins/workspace/public/components/workspace_form/utils.ts b/src/plugins/workspace/public/components/workspace_form/utils.ts index 133a3bc563de..8f06581b7ab0 100644 --- a/src/plugins/workspace/public/components/workspace_form/utils.ts +++ b/src/plugins/workspace/public/components/workspace_form/utils.ts @@ -4,6 +4,7 @@ */ import { WorkspacePermissionMode, DEFAULT_CHECKED_FEATURES_IDS } from '../../../common/constants'; +import type { SavedObjectPermissions } from '../../../../../core/types'; import { WorkspaceFeature, @@ -95,3 +96,78 @@ export const getPermissionModeId = (modes: WorkspacePermissionMode[]) => { } return PermissionModeId.Read; }; + +export const convertPermissionSettingsToPermissions = ( + permissionItems: WorkspacePermissionSetting[] | undefined +) => { + if (!permissionItems || permissionItems.length === 0) { + return undefined; + } + return permissionItems.reduce((previous, current) => { + current.modes.forEach((mode) => { + if (!previous[mode]) { + previous[mode] = {}; + } + switch (current.type) { + case 'user': + previous[mode].users = [...(previous[mode].users || []), current.userId]; + break; + case 'group': + previous[mode].groups = [...(previous[mode].groups || []), current.group]; + break; + } + }); + return previous; + }, {}); +}; + +const isWorkspacePermissionMode = (test: string): test is WorkspacePermissionMode => + test === WorkspacePermissionMode.LibraryRead || + test === WorkspacePermissionMode.LibraryWrite || + test === WorkspacePermissionMode.Read || + test === WorkspacePermissionMode.Write; + +export const convertPermissionsToPermissionSettings = (permissions: SavedObjectPermissions) => { + const userPermissionSettings: WorkspacePermissionSetting[] = []; + const groupPermissionSettings: WorkspacePermissionSetting[] = []; + const settingType2Modes: { [key: string]: WorkspacePermissionMode[] } = {}; + + Object.keys(permissions).forEach((mode) => { + if (!isWorkspacePermissionMode(mode)) { + return; + } + if (permissions[mode].users) { + permissions[mode].users?.forEach((userId) => { + const settingTypeKey = `userId-${userId}`; + const modes = settingType2Modes[settingTypeKey] ? settingType2Modes[settingTypeKey] : []; + + modes.push(mode); + if (modes.length === 1) { + userPermissionSettings.push({ + type: WorkspacePermissionItemType.User, + userId, + modes, + }); + settingType2Modes[settingTypeKey] = modes; + } + }); + permissions[mode].groups?.forEach((group) => { + const settingTypeKey = `group-${group}`; + const modes = settingType2Modes[settingTypeKey] ? settingType2Modes[settingTypeKey] : []; + + modes.push(mode); + if (modes.length === 1) { + userPermissionSettings.push({ + type: WorkspacePermissionItemType.Group, + group, + modes, + }); + } + }); + } + }); + + return [...userPermissionSettings, ...groupPermissionSettings].filter( + isValidWorkspacePermissionSetting + ); +}; diff --git a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx index ec4f2bfed3e0..b340a71588c9 100644 --- a/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx +++ b/src/plugins/workspace/public/components/workspace_form/workspace_form.tsx @@ -170,7 +170,7 @@ export const WorkspaceForm = (props: WorkspaceFormProps) => { diff --git a/src/plugins/workspace/public/components/workspace_list/index.tsx b/src/plugins/workspace/public/components/workspace_list/index.tsx index aac721c236a6..f529524d797e 100644 --- a/src/plugins/workspace/public/components/workspace_list/index.tsx +++ b/src/plugins/workspace/public/components/workspace_list/index.tsx @@ -17,7 +17,7 @@ import { import useObservable from 'react-use/lib/useObservable'; import { of } from 'rxjs'; import { i18n } from '@osd/i18n'; -import { debounce } from '../../../../../core/public'; +import { debounce, WorkspaceObject } from '../../../../../core/public'; import { WorkspaceAttribute } from '../../../../../core/public'; import { useOpenSearchDashboards } from '../../../../../plugins/opensearch_dashboards_react/public'; import { switchWorkspace, updateWorkspace } from '../utils/workspace'; @@ -34,6 +34,8 @@ const WORKSPACE_LIST_PAGE_DESCRIPTIOIN = i18n.translate('workspace.list.descript 'Workspace allow you to save and organize library items, such as index patterns, visualizations, dashboards, saved searches, and share them with other OpenSearch Dashboards users. You can control which features are visible in each workspace, and which users and groups have read and write access to the library items in the workspace.', }); +const emptyWorkspaceList: WorkspaceObject[] = []; + export const WorkspaceList = () => { const { services: { workspaces, application, http }, @@ -41,7 +43,10 @@ export const WorkspaceList = () => { const initialSortField = 'name'; const initialSortDirection = 'asc'; - const workspaceList = useObservable(workspaces?.workspaceList$ ?? of([]), []); + const workspaceList = useObservable( + workspaces?.workspaceList$ ?? of(emptyWorkspaceList), + emptyWorkspaceList + ); const [queryInput, setQueryInput] = useState(''); const [pagination, setPagination] = useState({ pageIndex: 0, diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx index 3c113a71e948..ff07076d99d1 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.test.tsx @@ -169,7 +169,14 @@ describe('WorkspaceUpdater', () => { description: 'test workspace description', features: expect.arrayContaining(['app1', 'app2', 'app3']), }), - expect.arrayContaining([expect.objectContaining({ type: 'user', userId: 'test user id' })]) + { + read: { + users: ['test user id'], + }, + library_read: { + users: ['test user id'], + }, + } ); await waitFor(() => { expect(notificationToastsAddSuccess).toHaveBeenCalled(); diff --git a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx index dcc750f18be8..1f67f2063d9b 100644 --- a/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx +++ b/src/plugins/workspace/public/components/workspace_updater/workspace_updater.tsx @@ -6,27 +6,30 @@ import React, { useCallback, useEffect, useState } from 'react'; import { EuiPage, EuiPageBody, EuiPageHeader, EuiPageContent } from '@elastic/eui'; import { i18n } from '@osd/i18n'; -import { WorkspaceAttribute } from 'opensearch-dashboards/public'; import { useObservable } from 'react-use'; import { of } from 'rxjs'; import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; import { WorkspaceForm, WorkspaceFormSubmitData, WorkspaceOperationType } from '../workspace_form'; import { WORKSPACE_OVERVIEW_APP_ID } from '../../../common/constants'; import { formatUrlWithWorkspaceId } from '../../../../../core/public/utils'; +import { WorkspaceAttributeWithPermission } from '../../../../../core/types'; import { WorkspaceClient } from '../../workspace_client'; -import { WorkspaceFormData, WorkspacePermissionSetting } from '../workspace_form/types'; - -interface WorkspaceWithPermission extends WorkspaceAttribute { - permissions?: WorkspacePermissionSetting[]; -} +import { + convertPermissionSettingsToPermissions, + convertPermissionsToPermissionSettings, +} from '../workspace_form/utils'; function getFormDataFromWorkspace( - currentWorkspace: WorkspaceAttribute | null | undefined -): WorkspaceFormData { - const currentWorkspaceWithPermission = (currentWorkspace || {}) as WorkspaceWithPermission; + currentWorkspace: WorkspaceAttributeWithPermission | null | undefined +) { + if (!currentWorkspace) { + return null; + } return { - ...currentWorkspaceWithPermission, - permissions: currentWorkspaceWithPermission.permissions || [], + ...currentWorkspace, + permissionSettings: currentWorkspace.permissions + ? convertPermissionsToPermissionSettings(currentWorkspace.permissions) + : currentWorkspace.permissions, }; } @@ -38,7 +41,7 @@ export const WorkspaceUpdater = () => { const isPermissionEnabled = application?.capabilities.workspaces.permissionEnabled; const currentWorkspace = useObservable(workspaces ? workspaces.currentWorkspace$ : of(null)); - const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState( + const [currentWorkspaceFormData, setCurrentWorkspaceFormData] = useState( getFormDataFromWorkspace(currentWorkspace) ); @@ -59,8 +62,12 @@ export const WorkspaceUpdater = () => { } try { - const { permissions, ...attributes } = data; - result = await workspaceClient.update(currentWorkspace.id, attributes, permissions); + const { permissionSettings, ...attributes } = data; + result = await workspaceClient.update( + currentWorkspace.id, + attributes, + convertPermissionSettingsToPermissions(permissionSettings) + ); } catch (error) { notifications?.toasts.addDanger({ title: i18n.translate('workspace.update.failed', { @@ -100,7 +107,7 @@ export const WorkspaceUpdater = () => { [notifications?.toasts, currentWorkspace, http, application, workspaceClient] ); - if (!currentWorkspaceFormData.name) { + if (!currentWorkspaceFormData) { return null; } diff --git a/src/plugins/workspace/public/hooks.ts b/src/plugins/workspace/public/hooks.ts index a63dc8f83d3d..4309dab2e5b0 100644 --- a/src/plugins/workspace/public/hooks.ts +++ b/src/plugins/workspace/public/hooks.ts @@ -8,8 +8,10 @@ import { useMemo } from 'react'; import { of } from 'rxjs'; import { ApplicationStart, PublicAppInfo } from '../../../core/public'; +const emptyMap = new Map(); + export function useApplications(application?: ApplicationStart) { - const applications = useObservable(application?.applications$ ?? of(new Map()), new Map()); + const applications = useObservable(application?.applications$ ?? of(emptyMap), emptyMap); return useMemo(() => { const apps: PublicAppInfo[] = []; applications.forEach((app) => { diff --git a/src/plugins/workspace/public/workspace_client.ts b/src/plugins/workspace/public/workspace_client.ts index 31a08b6ae9c2..76bbb618b506 100644 --- a/src/plugins/workspace/public/workspace_client.ts +++ b/src/plugins/workspace/public/workspace_client.ts @@ -11,7 +11,7 @@ import { WorkspaceAttribute, WorkspacesSetup, } from '../../../core/public'; -import { WorkspacePermissionMode } from '../common/constants'; +import { SavedObjectPermissions, WorkspaceAttributeWithPermission } from '../../../core/types'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -31,15 +31,6 @@ type IResponse = error?: string; }; -type WorkspacePermissionItem = { - modes: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Read - | WorkspacePermissionMode.Write - >; -} & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); - interface WorkspaceFindOptions { page?: number; perPage?: number; @@ -195,7 +186,7 @@ export class WorkspaceClient { */ public async create( attributes: Omit, - permissions?: WorkspacePermissionItem[] + permissions?: SavedObjectPermissions ): Promise>> { const path = this.getPath(); @@ -246,7 +237,7 @@ export class WorkspaceClient { options?: WorkspaceFindOptions ): Promise< IResponse<{ - workspaces: WorkspaceAttribute[]; + workspaces: WorkspaceAttributeWithPermission[]; total: number; per_page: number; page: number; @@ -263,9 +254,9 @@ export class WorkspaceClient { * Fetches a single workspace by a workspace id * * @param {string} id - * @returns {Promise>} The metadata of the workspace for the given id. + * @returns {Promise>} The metadata of the workspace for the given id. */ - public get(id: string): Promise> { + public get(id: string): Promise> { const path = this.getPath(id); return this.safeFetch(path, { method: 'GET', @@ -277,13 +268,13 @@ export class WorkspaceClient { * * @param {string} id * @param {object} attributes - * @param {WorkspacePermissionItem[]} permissions + * @param {WorkspacePermissionItem[]} permissionItems * @returns {Promise>} result for this operation */ public async update( id: string, attributes: Partial, - permissions?: WorkspacePermissionItem[] + permissions?: SavedObjectPermissions ): Promise> { const path = this.getPath(id); const body = { diff --git a/src/plugins/workspace/server/integration_tests/routes.test.ts b/src/plugins/workspace/server/integration_tests/routes.test.ts index 0c6e55101b7f..832c43c66399 100644 --- a/src/plugins/workspace/server/integration_tests/routes.test.ts +++ b/src/plugins/workspace/server/integration_tests/routes.test.ts @@ -5,8 +5,7 @@ import { WorkspaceAttribute } from 'src/core/types'; import * as osdTestServer from '../../../../core/test_helpers/osd_server'; -import { WORKSPACE_TYPE } from '../../../../core/server'; -import { WorkspacePermissionItem } from '../types'; +import { WORKSPACE_TYPE, Permissions } from '../../../../core/server'; const omitId = (object: T): Omit => { const { id, ...others } = object; @@ -19,7 +18,7 @@ const testWorkspace: WorkspaceAttribute = { description: 'test_workspace_description', }; -describe('workspace service', () => { +describe('workspace service api integration test', () => { let root: ReturnType; let opensearchServer: osdTestServer.TestOpenSearchUtils; let osd: osdTestServer.TestOpenSearchDashboardsUtils; @@ -36,7 +35,7 @@ describe('workspace service', () => { }, savedObjects: { permission: { - enabled: true, + enabled: false, }, }, migrations: { skip: false }, @@ -89,39 +88,6 @@ describe('workspace service', () => { expect(result.body.success).toEqual(true); expect(typeof result.body.result.id).toBe('string'); }); - it('create with permissions', async () => { - await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'invalid-type', userId: 'foo', modes: ['read'] }], - }) - .expect(400); - - const result: any = await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'user', userId: 'foo', modes: ['read'] }], - }) - .expect(200); - - expect(result.body.success).toEqual(true); - expect(typeof result.body.result.id).toBe('string'); - expect( - ( - await osd.coreStart.savedObjects - .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) - ).attributes.permissions - ).toEqual([ - { - modes: ['read'], - type: 'user', - userId: 'foo', - }, - ]); - }); it('get', async () => { const result = await osdTestServer.request .post(root, `/api/workspaces`) @@ -162,39 +128,6 @@ describe('workspace service', () => { expect(getResult.body.success).toEqual(true); expect(getResult.body.result.name).toEqual('updated'); }); - it('update with permissions', async () => { - const result: any = await osdTestServer.request - .post(root, `/api/workspaces`) - .send({ - attributes: omitId(testWorkspace), - permissions: [{ type: 'user', userId: 'foo', modes: ['read'] }], - }) - .expect(200); - - await osdTestServer.request - .put(root, `/api/workspaces/${result.body.result.id}`) - .send({ - attributes: { - ...omitId(testWorkspace), - }, - permissions: [{ type: 'user', userId: 'foo', modes: ['write'] }], - }) - .expect(200); - - expect( - ( - await osd.coreStart.savedObjects - .createInternalRepository([WORKSPACE_TYPE]) - .get<{ permissions: WorkspacePermissionItem[] }>(WORKSPACE_TYPE, result.body.result.id) - ).attributes.permissions - ).toEqual([ - { - modes: ['write'], - type: 'user', - userId: 'foo', - }, - ]); - }); it('delete', async () => { const result: any = await osdTestServer.request .post(root, `/api/workspaces`) @@ -339,3 +272,107 @@ describe('workspace service', () => { }); }); }); + +describe('workspace service api integration test when savedObjects.permission.enabled equal true', () => { + let root: ReturnType; + let opensearchServer: osdTestServer.TestOpenSearchUtils; + let osd: osdTestServer.TestOpenSearchDashboardsUtils; + beforeAll(async () => { + const { startOpenSearch, startOpenSearchDashboards } = osdTestServer.createTestServers({ + adjustTimeout: (t: number) => jest.setTimeout(t), + settings: { + osd: { + workspace: { + enabled: true, + }, + savedObjects: { + permission: { + enabled: true, + }, + }, + migrations: { skip: false }, + }, + }, + }); + opensearchServer = await startOpenSearch(); + osd = await startOpenSearchDashboards(); + root = osd.root; + }); + 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); + const savedObjectsRepository = osd.coreStart.savedObjects.createInternalRepository([ + WORKSPACE_TYPE, + ]); + await Promise.all( + listResult.body.result.workspaces.map((item: WorkspaceAttribute) => + // this will delete reserved workspace + savedObjectsRepository.delete(WORKSPACE_TYPE, item.id) + ) + ); + }); + it('create', async () => { + await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + permissions: { invalid_type: { users: ['foo'] } }, + }) + .expect(400); + + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + permissions: { read: { users: ['foo'] } }, + }) + .expect(200); + + expect(result.body.success).toEqual(true); + expect(typeof result.body.result.id).toBe('string'); + expect( + ( + await osd.coreStart.savedObjects + .createInternalRepository([WORKSPACE_TYPE]) + .get<{ permissions: Permissions }>(WORKSPACE_TYPE, result.body.result.id) + ).permissions + ).toEqual({ read: { users: ['foo'] } }); + }); + it('update', async () => { + const result: any = await osdTestServer.request + .post(root, `/api/workspaces`) + .send({ + attributes: omitId(testWorkspace), + }) + .expect(200); + + const updateResult = await osdTestServer.request + .put(root, `/api/workspaces/${result.body.result.id}`) + .send({ + attributes: { + ...omitId(testWorkspace), + }, + permissions: { write: { users: ['foo'] } }, + }) + .expect(200); + expect(updateResult.body.result).toBe(true); + + expect( + ( + await osd.coreStart.savedObjects + .createInternalRepository([WORKSPACE_TYPE]) + .get<{ permissions: Permissions }>(WORKSPACE_TYPE, result.body.result.id) + ).permissions + ).toEqual({ write: { users: ['foo'] } }); + }); + }); +}); diff --git a/src/plugins/workspace/server/permission_control/client.test.ts b/src/plugins/workspace/server/permission_control/client.test.ts index e05e299c153b..4d041cc7df56 100644 --- a/src/plugins/workspace/server/permission_control/client.test.ts +++ b/src/plugins/workspace/server/permission_control/client.test.ts @@ -109,6 +109,47 @@ describe('PermissionControl', () => { expect(batchValidateResult.result).toEqual(true); }); + it('should return false and log not permitted saved objects', async () => { + const logger = loggerMock.create(); + const permissionControlClient = new SavedObjectsPermissionControl(logger); + const getScopedClient = jest.fn(); + const clientMock = savedObjectsClientMock.create(); + getScopedClient.mockImplementation((request) => { + return clientMock; + }); + permissionControlClient.setup(getScopedClient, mockAuth); + + clientMock.bulkGet.mockResolvedValue({ + saved_objects: [ + { + id: 'foo', + type: 'dashboard', + references: [], + attributes: {}, + }, + { + id: 'bar', + type: 'dashboard', + references: [], + attributes: {}, + permissions: { + read: { + users: ['foo'], + }, + }, + }, + ], + }); + const batchValidateResult = await permissionControlClient.batchValidate( + httpServerMock.createOpenSearchDashboardsRequest(), + [], + ['read'] + ); + expect(batchValidateResult.success).toEqual(true); + expect(batchValidateResult.result).toEqual(false); + expect(logger.debug).toHaveBeenCalledTimes(1); + }); + describe('getPrincipalsFromRequest', () => { const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); const getScopedClient = jest.fn(); @@ -120,4 +161,40 @@ describe('PermissionControl', () => { expect(result.users).toEqual(['bar']); }); }); + + describe('validateSavedObjectsACL', () => { + it("should return true if saved objects don't have permissions property", () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + expect( + permissionControlClient.validateSavedObjectsACL([{ type: 'workspace', id: 'foo' }], {}, []) + ).toBe(true); + }); + it('should return true if principals permitted to saved objects', () => { + const permissionControlClient = new SavedObjectsPermissionControl(loggerMock.create()); + expect( + permissionControlClient.validateSavedObjectsACL( + [{ type: 'workspace', id: 'foo', permissions: { write: { users: ['bar'] } } }], + { users: ['bar'] }, + ['write'] + ) + ).toBe(true); + }); + it('should return false and log saved objects if not permitted', () => { + const logger = loggerMock.create(); + const permissionControlClient = new SavedObjectsPermissionControl(logger); + expect( + permissionControlClient.validateSavedObjectsACL( + [{ type: 'workspace', id: 'foo', permissions: { write: { users: ['bar'] } } }], + { users: ['foo'] }, + ['write'] + ) + ).toBe(false); + expect(logger.debug).toHaveBeenCalledTimes(1); + expect(logger.debug).toHaveBeenCalledWith( + expect.stringMatching( + /Authorization failed, principals:.*has no.*permissions on the requested saved object:.*foo/ + ) + ); + }); + }); }); diff --git a/src/plugins/workspace/server/permission_control/client.ts b/src/plugins/workspace/server/permission_control/client.ts index bad46eb156a6..ea404e42974f 100644 --- a/src/plugins/workspace/server/permission_control/client.ts +++ b/src/plugins/workspace/server/permission_control/client.ts @@ -6,7 +6,6 @@ import { i18n } from '@osd/i18n'; import { ACL, - TransformedPermission, SavedObjectsBulkGetObject, SavedObjectsServiceStart, Logger, @@ -14,7 +13,6 @@ import { Principals, SavedObject, WORKSPACE_TYPE, - Permissions, HttpAuth, } from '../../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../../common/constants'; @@ -31,6 +29,13 @@ export class SavedObjectsPermissionControl { private readonly logger: Logger; private _getScopedClient?: SavedObjectsServiceStart['getScopedClient']; private auth?: HttpAuth; + /** + * Returns a saved objects client that is able to: + * 1. Read objects whose type is `workspace` because workspace is a hidden type and the permission control client will need to get the metadata of a specific workspace to do the permission check. + * 2. Bypass saved objects permission control wrapper because the permission control client is a dependency of the wrapper to provide the ACL validation capability. It will run into infinite loop if not bypass. + * @param request + * @returns SavedObjectsContract + */ private getScopedClient(request: OpenSearchDashboardsRequest) { return this._getScopedClient?.(request, { excludedWrappers: [WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID], @@ -83,6 +88,15 @@ export class SavedObjectsPermissionControl { return getPrincipalsFromRequest(request, this.auth); } + /** + * Validates the permissions for a collection of saved objects based on their Access Control Lists (ACL). + * This method checks whether the provided principals have the specified permission modes for each saved object. + * If any saved object lacks the required permissions, the function logs details of unauthorized access. + * + * @remarks + * If a saved object doesn't have an ACL (e.g., config objects), it is considered as having the required permissions. + * The function logs detailed information when unauthorized access is detected, including the list of denied saved objects. + */ public validateSavedObjectsACL( savedObjects: Array, 'id' | 'type' | 'workspaces' | 'permissions'>>, principals: Principals, @@ -101,7 +115,12 @@ export class SavedObjectsPermissionControl { const aclInstance = new ACL(savedObject.permissions); const hasPermission = aclInstance.hasPermission(permissionModes, principals); if (!hasPermission) { - notPermittedSavedObjects.push(savedObject); + notPermittedSavedObjects.push({ + id: savedObject.id, + type: savedObject.type, + workspaces: savedObject.workspaces, + permissions: savedObject.permissions, + }); } return hasPermission; }); @@ -145,38 +164,11 @@ export class SavedObjectsPermissionControl { } const principals = this.getPrincipalsFromRequest(request); - const deniedObjects: Array< - Pick & { - workspaces?: string[]; - permissions?: Permissions; - } - > = []; - const hasPermissionToAllObjects = savedObjectsGet.every((item) => { - // for object that doesn't contain ACL like config, return true - if (!item.permissions) { - return true; - } - const aclInstance = new ACL(item.permissions); - const hasPermission = aclInstance.hasPermission(permissionModes, principals); - if (!hasPermission) { - deniedObjects.push({ - id: item.id, - type: item.type, - workspaces: item.workspaces, - permissions: item.permissions, - }); - } - return hasPermission; - }); - if (!hasPermissionToAllObjects) { - this.logger.debug( - `Authorization failed, principals: ${JSON.stringify( - principals - )} has no [${permissionModes}] permissions on the requested saved object: ${JSON.stringify( - deniedObjects - )}` - ); - } + const hasPermissionToAllObjects = this.validateSavedObjectsACL( + savedObjectsGet, + principals, + permissionModes + ); return { success: true, result: hasPermissionToAllObjects, diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index c448fcf209f9..684f754ce9dd 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -12,9 +12,6 @@ describe('Workspace server plugin', () => { const setupMock = coreMock.createSetup(); const initializerContextConfigMock = coreMock.createPluginInitializerContext({ enabled: true, - permission: { - enabled: true, - }, }); setupMock.capabilities.registerProvider.mockImplementationOnce((fn) => (value = fn())); const workspacePlugin = new WorkspacePlugin(initializerContextConfigMock); diff --git a/src/plugins/workspace/server/plugin.ts b/src/plugins/workspace/server/plugin.ts index 0fff0082476b..8e3da6a8cfe5 100644 --- a/src/plugins/workspace/server/plugin.ts +++ b/src/plugins/workspace/server/plugin.ts @@ -11,29 +11,29 @@ import { Plugin, Logger, CoreStart, + SharedGlobalConfig, } from '../../../core/server'; import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID, } from '../common/constants'; -import { IWorkspaceClientImpl } from './types'; +import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; +import { IWorkspaceClientImpl, WorkspacePluginSetup, WorkspacePluginStart } from './types'; import { WorkspaceClient } from './workspace_client'; import { registerRoutes } from './routes'; import { WorkspaceSavedObjectsClientWrapper } from './saved_objects'; -import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils'; import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict'; import { SavedObjectsPermissionControl, SavedObjectsPermissionControlContract, } from './permission_control/client'; -import { WorkspacePluginConfigType } from '../config'; -export class WorkspacePlugin implements Plugin<{}, {}> { +export class WorkspacePlugin implements Plugin { private readonly logger: Logger; private client?: IWorkspaceClientImpl; private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper; private permissionControl?: SavedObjectsPermissionControlContract; - private readonly config$: Observable; + private readonly globalConfig$: Observable; private workspaceSavedObjectsClientWrapper?: WorkspaceSavedObjectsClientWrapper; private proxyWorkspaceTrafficToRealHandler(setupDeps: CoreSetup) { @@ -57,14 +57,13 @@ export class WorkspacePlugin implements Plugin<{}, {}> { constructor(initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get('plugins', 'workspace'); - this.config$ = initializerContext.config.create(); + this.globalConfig$ = initializerContext.config.legacy.globalConfig$; } public async setup(core: CoreSetup) { this.logger.debug('Setting up Workspaces service'); - const config: WorkspacePluginConfigType = await this.config$.pipe(first()).toPromise(); - const isPermissionControlEnabled = - config.permission.enabled === undefined ? true : config.permission.enabled; + const globalConfig = await this.globalConfig$.pipe(first()).toPromise(); + const isPermissionControlEnabled = globalConfig.savedObjects.permission.enabled === true; this.client = new WorkspaceClient(core, this.logger); @@ -99,6 +98,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> { logger: this.logger, client: this.client as IWorkspaceClientImpl, permissionControlClient: this.permissionControl, + isPermissionControlEnabled, }); core.capabilities.registerProvider(() => ({ diff --git a/src/plugins/workspace/server/routes/index.ts b/src/plugins/workspace/server/routes/index.ts index b02bff76c132..701eb8888130 100644 --- a/src/plugins/workspace/server/routes/index.ts +++ b/src/plugins/workspace/server/routes/index.ts @@ -4,9 +4,10 @@ */ import { schema } from '@osd/config-schema'; -import { CoreSetup, Logger } from '../../../../core/server'; +import { CoreSetup, Logger, PrincipalType, ACL } from '../../../../core/server'; +import { WorkspaceAttributeWithPermission } from '../../../../core/types'; import { WorkspacePermissionMode } from '../../common/constants'; -import { IWorkspaceClientImpl, WorkspacePermissionItem } from '../types'; +import { IWorkspaceClientImpl } from '../types'; import { SavedObjectsPermissionControlContract } from '../permission_control/client'; const WORKSPACES_API_BASE_URL = '/api/workspaces'; @@ -18,19 +19,16 @@ const workspacePermissionMode = schema.oneOf([ schema.literal(WorkspacePermissionMode.LibraryWrite), ]); -const workspacePermission = schema.oneOf([ - schema.object({ - type: schema.literal('user'), - userId: schema.string(), - modes: schema.arrayOf(workspacePermissionMode), - }), - schema.object({ - type: schema.literal('group'), - group: schema.string(), - modes: schema.arrayOf(workspacePermissionMode), - }), +const principalType = schema.oneOf([ + schema.literal(PrincipalType.Users), + schema.literal(PrincipalType.Groups), ]); +const workspacePermissions = schema.recordOf( + workspacePermissionMode, + schema.recordOf(principalType, schema.arrayOf(schema.string()), {}) +); + const workspaceAttributesSchema = schema.object({ description: schema.maybe(schema.string()), name: schema.string(), @@ -46,11 +44,13 @@ export function registerRoutes({ logger, http, permissionControlClient, + isPermissionControlEnabled, }: { client: IWorkspaceClientImpl; logger: Logger; http: CoreSetup['http']; permissionControlClient?: SavedObjectsPermissionControlContract; + isPermissionControlEnabled: boolean; }) { const router = http.createRouter(); router.post( @@ -119,29 +119,30 @@ export function registerRoutes({ validate: { body: schema.object({ attributes: workspaceAttributesSchema, - permissions: schema.maybe( - schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)]) - ), + permissions: schema.maybe(workspacePermissions), }), }, }, router.handleLegacyErrors(async (context, req, res) => { - const { attributes, permissions: permissionsInRequest } = req.body; - const authInfo = permissionControlClient?.getPrincipalsFromRequest(req); - let permissions: WorkspacePermissionItem[] = []; - if (permissionsInRequest) { - permissions = Array.isArray(permissionsInRequest) - ? permissionsInRequest - : [permissionsInRequest]; - } + const { attributes, permissions } = req.body; + const principals = permissionControlClient?.getPrincipalsFromRequest(req); + const createPayload: Omit = attributes; - // Assign workspace owner to current user - if (!!authInfo?.users?.length) { - permissions.push({ - type: 'user', - userId: authInfo.users[0], - modes: [WorkspacePermissionMode.LibraryWrite, WorkspacePermissionMode.Write], - }); + if (isPermissionControlEnabled) { + createPayload.permissions = permissions; + // Assign workspace owner to current user + if (!!principals?.users?.length) { + const acl = new ACL(permissions); + const currentUserId = principals.users[0]; + [WorkspacePermissionMode.Write, WorkspacePermissionMode.LibraryWrite].forEach( + (permissionMode) => { + if (!acl.hasPermission([permissionMode], { users: [currentUserId] })) { + acl.addPermission([permissionMode], { users: [currentUserId] }); + } + } + ); + createPayload.permissions = acl.getPermissions(); + } } const result = await client.create( @@ -150,10 +151,7 @@ export function registerRoutes({ request: req, logger, }, - { - ...attributes, - ...(permissions.length ? { permissions } : {}), - } + createPayload ); return res.ok({ body: result }); }) @@ -167,19 +165,13 @@ export function registerRoutes({ }), body: schema.object({ attributes: workspaceAttributesSchema, - permissions: schema.maybe( - schema.oneOf([workspacePermission, schema.arrayOf(workspacePermission)]) - ), + permissions: schema.maybe(workspacePermissions), }), }, }, router.handleLegacyErrors(async (context, req, res) => { const { id } = req.params; const { attributes, permissions } = req.body; - let finalPermissions: WorkspacePermissionItem[] = []; - if (permissions) { - finalPermissions = Array.isArray(permissions) ? permissions : [permissions]; - } const result = await client.update( { @@ -190,7 +182,7 @@ export function registerRoutes({ id, { ...attributes, - ...(finalPermissions.length ? { permissions: finalPermissions } : {}), + ...(isPermissionControlEnabled ? { permissions } : {}), } ); return res.ok({ body: result }); diff --git a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts index 2a7fb0e440b5..b6ea26456f0e 100644 --- a/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/integration_tests/workspace_saved_objects_client_wrapper.test.ts @@ -527,4 +527,70 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(true); }); }); + + describe('deleteByWorkspace', () => { + it('should throw forbidden error when workspace not permitted', async () => { + let error; + try { + await notPermittedSavedObjectedClient.deleteByWorkspace('workspace-1'); + } catch (e) { + error = e; + } + + expect(SavedObjectsErrorHelpers.isForbiddenError(error)).toBe(true); + }); + + it('should be able to delete all data in permitted workspace', async () => { + const deleteWorkspaceId = 'workspace-to-delete'; + await repositoryKit.create( + internalSavedObjectsRepository, + 'workspace', + {}, + { + id: deleteWorkspaceId, + permissions: { + library_read: { users: ['foo'] }, + library_write: { users: ['foo'] }, + }, + } + ); + const dashboardIds = [ + 'inner-delete-workspace-dashboard-1', + 'inner-delete-workspace-dashboard-2', + ]; + await Promise.all( + dashboardIds.map((dashboardId) => + repositoryKit.create( + internalSavedObjectsRepository, + 'dashboard', + {}, + { + id: dashboardId, + workspaces: [deleteWorkspaceId], + } + ) + ) + ); + + expect( + ( + await permittedSavedObjectedClient.find({ + type: 'dashboard', + workspaces: [deleteWorkspaceId], + }) + ).total + ).toBe(2); + + await permittedSavedObjectedClient.deleteByWorkspace(deleteWorkspaceId, { refresh: true }); + + expect( + ( + await permittedSavedObjectedClient.find({ + type: 'dashboard', + workspaces: [deleteWorkspaceId], + }) + ).total + ).toBe(0); + }); + }); }); diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts index 6b40f6e60fa0..07d1e6aff40c 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.test.ts @@ -26,6 +26,15 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { }, permissions: {}, }, + { + type: 'dashboard', + id: 'dashboard-with-empty-workspace-property', + workspaces: [], + attributes: { + bar: 'baz', + }, + permissions: {}, + }, { type: 'workspace', id: 'workspace-1', attributes: { name: 'Workspace - 1' } }, { type: 'workspace', @@ -40,6 +49,9 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { type: 'config', }; } + if (id === 'unknown-error-dashboard') { + throw new Error('Unknown error'); + } return ( savedObjectsStore.find((item) => item.type === type && item.id === id) || SavedObjectsErrorHelpers.createGenericNotFoundError() @@ -82,14 +94,16 @@ const generateWorkspaceSavedObjectsClientWrapper = () => { getPrincipalsFromRequest: jest.fn().mockImplementation(() => ({ users: ['user-1'] })), }; const wrapper = new WorkspaceSavedObjectsClientWrapper(permissionControlMock); - wrapper.setScopedClient(() => ({ + const scopedClientMock = { find: jest.fn().mockImplementation(async () => ({ saved_objects: [{ id: 'workspace-1', type: 'workspace' }], })), - })); + }; + wrapper.setScopedClient(() => scopedClientMock); return { wrapper: wrapper.wrapperFactory(wrapperOptions), clientMock, + scopedClientMock, permissionControlMock, requestMock, }; @@ -122,6 +136,16 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid saved objects permission'); }); + it("should throw permission error if deleting saved object's workspace property is empty", async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.delete('dashboard', 'dashboard-with-empty-workspace-property'); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); it('should call client.delete with arguments if permitted', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const deleteArgs = ['dashboard', 'foo', { force: true }] as const; @@ -157,6 +181,18 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid saved objects permission'); }); + it("should throw permission error if updating saved object's workspace property is empty", async () => { + const { wrapper } = generateWorkspaceSavedObjectsClientWrapper(); + let errorCatched; + try { + await wrapper.update('dashboard', 'dashboard-with-empty-workspace-property', { + bar: 'foo', + }); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toEqual('Invalid saved objects permission'); + }); it('should call client.update with arguments if permitted', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const updateArgs = [ @@ -260,6 +296,26 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ); expect(errorCatched?.message).toEqual('Invalid workspace permission'); }); + it('should throw error if unable to get object when override', async () => { + const { + wrapper, + permissionControlMock, + requestMock, + } = generateWorkspaceSavedObjectsClientWrapper(); + permissionControlMock.validate.mockResolvedValueOnce({ success: true, result: false }); + let errorCatched; + try { + await wrapper.bulkCreate( + [{ type: 'dashboard', id: 'unknown-error-dashboard', attributes: { bar: 'baz' } }], + { + overwrite: true, + } + ); + } catch (e) { + errorCatched = e; + } + expect(errorCatched?.message).toBe('Unknown error'); + }); it('should call client.bulkCreate with arguments if some objects not found', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); const objectsToBulkCreate = [ @@ -268,11 +324,9 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ]; await wrapper.bulkCreate(objectsToBulkCreate, { overwrite: true, - workspaces: ['workspace-1'], }); expect(clientMock.bulkCreate).toHaveBeenCalledWith(objectsToBulkCreate, { overwrite: true, - workspaces: ['workspace-1'], }); }); it('should call client.bulkCreate with arguments if permitted', async () => { @@ -549,6 +603,24 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { ACLSearchParams: {}, }); }); + it('should find permitted workspaces with filtered permission modes', async () => { + const { wrapper, scopedClientMock } = generateWorkspaceSavedObjectsClientWrapper(); + await wrapper.find({ + type: 'dashboard', + ACLSearchParams: { + permissionModes: ['read', 'library_read'], + }, + }); + expect(scopedClientMock.find).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'workspace', + ACLSearchParams: { + permissionModes: ['library_read'], + principals: { users: ['user-1'] }, + }, + }) + ); + }); it('should call client.find with arguments if not workspace type and no options.workspace', async () => { const { wrapper, clientMock } = generateWorkspaceSavedObjectsClientWrapper(); await wrapper.find({ @@ -556,8 +628,6 @@ describe('WorkspaceSavedObjectsClientWrapper', () => { }); expect(clientMock.find).toHaveBeenCalledWith({ type: 'dashboard', - workspaces: ['workspace-1'], - workspacesSearchOperator: 'OR', ACLSearchParams: { permissionModes: ['read', 'write'], principals: { users: ['user-1'] }, diff --git a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts index c515f555fa4b..30c1c91c4223 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts @@ -22,10 +22,10 @@ import { SavedObjectsBulkUpdateResponse, SavedObjectsBulkUpdateOptions, WORKSPACE_TYPE, - SavedObjectsDeleteByWorkspaceOptions, SavedObjectsErrorHelpers, SavedObjectsServiceStart, SavedObjectsClientContract, + SavedObjectsDeleteByWorkspaceOptions, } from '../../../../core/server'; import { SavedObjectsPermissionControlContract } from '../permission_control/client'; import { @@ -68,22 +68,13 @@ const getDefaultValuesForEmpty = (values: T[] | undefined, defaultValues: T[] export class WorkspaceSavedObjectsClientWrapper { private getScopedClient?: SavedObjectsServiceStart['getScopedClient']; - private formatWorkspacePermissionModeToStringArray( - permission: WorkspacePermissionMode | WorkspacePermissionMode[] - ): string[] { - if (Array.isArray(permission)) { - return permission; - } - - return [permission]; - } private async validateObjectsPermissions( objects: Array>, request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) { - // PermissionMode here is an array which is merged by workspace type required permission and other saved object required permission. + // PermissionModes here is an array which is merged by workspace type required permission and other saved object required permission. // So we only need to do one permission check no matter its type. for (const { id, type } of objects) { const validateResult = await this.permissionControl.validate( @@ -92,7 +83,7 @@ export class WorkspaceSavedObjectsClientWrapper { type, id, }, - this.formatWorkspacePermissionModeToStringArray(permissionMode) + permissionModes ); if (!validateResult?.result) { return false; @@ -105,20 +96,20 @@ export class WorkspaceSavedObjectsClientWrapper { private validateMultiWorkspacesPermissions = async ( workspacesIds: string[], request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) => { // for attributes and options passed in this function, the num of workspaces may be 0.This case should not be passed permission check. if (workspacesIds.length === 0) { return false; } const workspaces = workspacesIds.map((id) => ({ id, type: WORKSPACE_TYPE })); - return await this.validateObjectsPermissions(workspaces, request, permissionMode); + return await this.validateObjectsPermissions(workspaces, request, permissionModes); }; private validateAtLeastOnePermittedWorkspaces = async ( workspaces: string[] | undefined, request: OpenSearchDashboardsRequest, - permissionMode: WorkspacePermissionMode | WorkspacePermissionMode[] + permissionModes: WorkspacePermissionMode[] ) => { // for attributes and options passed in this function, the num of workspaces attribute may be 0.This case should not be passed permission check. if (!workspaces || workspaces.length === 0) { @@ -131,7 +122,7 @@ export class WorkspaceSavedObjectsClientWrapper { type: WORKSPACE_TYPE, id: workspaceId, }, - this.formatWorkspacePermissionModeToStringArray(permissionMode) + permissionModes ); if (validateResult?.result) { return true; @@ -495,12 +486,9 @@ export class WorkspaceSavedObjectsClientWrapper { options.workspaces = permittedWorkspaces; } else { /** - * Select all the docs that - * 1. ACL matches read / write / user passed permission OR - * 2. workspaces matches library_read or library_write OR + * If no workspaces present, find all the docs that + * ACL matches read / write / user passed permission */ - options.workspaces = permittedWorkspaceIds; - options.workspacesSearchOperator = 'OR'; options.ACLSearchParams.permissionModes = getDefaultValuesForEmpty( options.ACLSearchParams.permissionModes, [WorkspacePermissionMode.Read, WorkspacePermissionMode.Write] diff --git a/src/plugins/workspace/server/types.ts b/src/plugins/workspace/server/types.ts index ba1ff8a9cd47..b506bb493a4c 100644 --- a/src/plugins/workspace/server/types.ts +++ b/src/plugins/workspace/server/types.ts @@ -12,7 +12,7 @@ import { WorkspaceAttribute, SavedObjectsServiceStart, } from '../../../core/server'; -import { WorkspacePermissionMode } from '../common/constants'; +import { WorkspaceAttributeWithPermission } from '../../../core/types'; export interface WorkspaceFindOptions { page?: number; @@ -53,7 +53,7 @@ export interface IWorkspaceClientImpl { */ create( requestDetail: IRequestDetail, - payload: Omit + payload: Omit ): Promise>; /** * List workspaces @@ -68,7 +68,7 @@ export interface IWorkspaceClientImpl { ): Promise< IResponse< { - workspaces: WorkspaceAttribute[]; + workspaces: WorkspaceAttributeWithPermission[]; } & Pick > >; @@ -76,10 +76,13 @@ export interface IWorkspaceClientImpl { * 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} + * @returns a Promise with the detail of {@link WorkspaceAttributeWithPermission} * @public */ - get(requestDetail: IRequestDetail, id: string): Promise>; + get( + requestDetail: IRequestDetail, + id: string + ): Promise>; /** * Update the detail of a given workspace * @param requestDetail {@link IRequestDetail} @@ -91,7 +94,7 @@ export interface IWorkspaceClientImpl { update( requestDetail: IRequestDetail, id: string, - payload: Omit + payload: Omit ): Promise>; /** * Delete a given workspace @@ -124,11 +127,10 @@ export interface AuthInfo { user_name?: string; } -export type WorkspacePermissionItem = { - modes: Array< - | WorkspacePermissionMode.LibraryRead - | WorkspacePermissionMode.LibraryWrite - | WorkspacePermissionMode.Read - | WorkspacePermissionMode.Write - >; -} & ({ type: 'user'; userId: string } | { type: 'group'; group: string }); +export interface WorkspacePluginSetup { + client: IWorkspaceClientImpl; +} + +export interface WorkspacePluginStart { + client: IWorkspaceClientImpl; +} diff --git a/src/plugins/workspace/server/workspace_client.ts b/src/plugins/workspace/server/workspace_client.ts index e7bdf97b54ec..f9da4130921b 100644 --- a/src/plugins/workspace/server/workspace_client.ts +++ b/src/plugins/workspace/server/workspace_client.ts @@ -4,7 +4,7 @@ */ import { i18n } from '@osd/i18n'; -import type { +import { SavedObject, SavedObjectsClientContract, CoreSetup, @@ -17,14 +17,11 @@ import { WORKSPACE_TYPE, Logger, } from '../../../core/server'; +import { WorkspaceAttributeWithPermission } from '../../../core/types'; import { IWorkspaceClientImpl, WorkspaceFindOptions, IResponse, IRequestDetail } from './types'; import { workspace } from './saved_objects'; import { generateRandomId } from './utils'; -import { - WORKSPACE_OVERVIEW_APP_ID, - WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID, - WORKSPACE_UPDATE_APP_ID, -} from '../common/constants'; +import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants'; const WORKSPACE_ID_SIZE = 6; @@ -65,10 +62,11 @@ export class WorkspaceClient implements IWorkspaceClientImpl { } private getFlattenedResultWithSavedObject( savedObject: SavedObject - ): WorkspaceAttribute { + ): WorkspaceAttributeWithPermission { return { ...savedObject.attributes, id: savedObject.id, + permissions: savedObject.permissions, }; } private formatError(error: Error | any): string { @@ -114,10 +112,10 @@ export class WorkspaceClient implements IWorkspaceClientImpl { } public async create( requestDetail: IRequestDetail, - payload: Omit + payload: Omit ): ReturnType { try { - const attributes = payload; + const { permissions, ...attributes } = payload; const id = generateRandomId(WORKSPACE_ID_SIZE); const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); const existingWorkspaceRes = await this.getScopedClientWithoutPermission(requestDetail)?.find( @@ -135,6 +133,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { attributes, { id, + permissions, } ); return { @@ -214,7 +213,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { public async get( requestDetail: IRequestDetail, id: string - ): Promise> { + ): ReturnType { try { const result = await this.getSavedObjectClientsFromRequestDetail(requestDetail).get< WorkspaceAttribute @@ -233,9 +232,9 @@ export class WorkspaceClient implements IWorkspaceClientImpl { public async update( requestDetail: IRequestDetail, id: string, - payload: Omit + payload: Omit ): Promise> { - const attributes = payload; + const { permissions, ...attributes } = payload; try { const client = this.getSavedObjectClientsFromRequestDetail(requestDetail); const workspaceInDB: SavedObject = await client.get(WORKSPACE_TYPE, id); @@ -255,9 +254,9 @@ export class WorkspaceClient implements IWorkspaceClientImpl { throw new Error(DUPLICATE_WORKSPACE_NAME_ERROR); } } - await client.create>(WORKSPACE_TYPE, attributes, { id, + permissions, overwrite: true, version: workspaceInDB.version, }); From ee245d7b74e57988a492983faa86dbb25dc077fb Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 7 Apr 2024 15:29:16 +0800 Subject: [PATCH 26/36] fix: unit test (#318) Signed-off-by: SuZhou-Joe --- src/plugins/workspace/public/plugin.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/public/plugin.test.ts b/src/plugins/workspace/public/plugin.test.ts index 428d7bbc7d76..fc16bc207345 100644 --- a/src/plugins/workspace/public/plugin.test.ts +++ b/src/plugins/workspace/public/plugin.test.ts @@ -23,7 +23,7 @@ describe('Workspace plugin', () => { await workspacePlugin.setup(setupMock, { savedObjectsManagement: savedObjectManagementSetupMock, }); - expect(setupMock.application.register).toBeCalledTimes(4); + expect(setupMock.application.register).toBeCalledTimes(5); expect(WorkspaceClientMock).toBeCalledTimes(1); expect(workspaceClientMock.enterWorkspace).toBeCalledTimes(0); expect(savedObjectManagementSetupMock.columns.register).toBeCalledTimes(1); @@ -61,7 +61,7 @@ describe('Workspace plugin', () => { await workspacePlugin.setup(setupMock, { savedObjectsManagement: savedObjectsManagementPluginMock.createSetupContract(), }); - expect(setupMock.application.register).toBeCalledTimes(4); + expect(setupMock.application.register).toBeCalledTimes(5); expect(WorkspaceClientMock).toBeCalledTimes(1); expect(workspaceClientMock.enterWorkspace).toBeCalledWith('workspaceId'); expect(setupMock.getStartServices).toBeCalledTimes(1); From 0f34d69d665366345c2a51b5b8a1b5aea940dab4 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 7 Apr 2024 15:29:51 +0800 Subject: [PATCH 27/36] [Workspace] Add APIs to support plugin state in request (#6303) (#315) * feat: add APIs to support plugin state in request (#312) * feat: add APIs to support plugin state in request * feat: add APIs to support plugin state in request --------- * feat: update CHANGELOG * feat: update * feat: use request app to store request workspace id * feat: remove useless if --------- Signed-off-by: SuZhou-Joe --- src/core/server/utils/workspace.test.ts | 4 ++-- src/core/server/utils/workspace.ts | 31 ++++++++++--------------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/core/server/utils/workspace.test.ts b/src/core/server/utils/workspace.test.ts index 49382cfac38f..7dfcff9e5d18 100644 --- a/src/core/server/utils/workspace.test.ts +++ b/src/core/server/utils/workspace.test.ts @@ -10,10 +10,10 @@ describe('updateWorkspaceState', () => { it('update with payload', () => { const requestMock = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(requestMock, { - id: 'foo', + requestWorkspaceId: 'foo', }); expect(getWorkspaceState(requestMock)).toEqual({ - id: 'foo', + requestWorkspaceId: 'foo', }); }); }); diff --git a/src/core/server/utils/workspace.ts b/src/core/server/utils/workspace.ts index c5dcf84b92d9..2003e615d501 100644 --- a/src/core/server/utils/workspace.ts +++ b/src/core/server/utils/workspace.ts @@ -3,14 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -/** - * This file is using {@link PluginsStates} to store workspace info into request. - * The best practice would be using {@link Server.register} to register plugins into the hapi server - * but OSD is wrappering the hapi server and the hapi server instance is hidden as internal implementation. - */ -import { PluginsStates } from '@hapi/hapi'; import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; +export interface WorkspaceState { + requestWorkspaceId?: string; +} + /** * This function will be used as a proxy * because `ensureRequest` is only importable from core module. @@ -20,24 +18,19 @@ import { OpenSearchDashboardsRequest, ensureRawRequest } from '../http/router'; */ export const updateWorkspaceState = ( request: OpenSearchDashboardsRequest, - payload: Partial + payload: Partial ) => { const rawRequest = ensureRawRequest(request); - if (!rawRequest.plugins) { - rawRequest.plugins = {}; - } - - if (!rawRequest.plugins.workspace) { - rawRequest.plugins.workspace = {}; - } - - rawRequest.plugins.workspace = { - ...rawRequest.plugins.workspace, + rawRequest.app = { + ...rawRequest.app, ...payload, }; }; -export const getWorkspaceState = (request: OpenSearchDashboardsRequest) => { - return ensureRawRequest(request).plugins?.workspace; +export const getWorkspaceState = (request: OpenSearchDashboardsRequest): WorkspaceState => { + const { requestWorkspaceId } = ensureRawRequest(request).app as WorkspaceState; + return { + requestWorkspaceId, + }; }; From 878ada6652b160d4aea3db2301728f1ab98e6446 Mon Sep 17 00:00:00 2001 From: Eric Wei Date: Fri, 5 Apr 2024 16:53:55 -0700 Subject: [PATCH 28/36] [Discover - datasource selector] Add extension group title and modal (#5815) * add log explorer re-directon modal Signed-off-by: Eric * adjustments to comments Signed-off-by: Eric * add one missing i18n Signed-off-by: Eric * add redirection text to group title Signed-off-by: Eric * include changes in changelog Signed-off-by: Eric * remove redundent title addition and unnecessary modal toggle functions Signed-off-by: Eric * remove one comment Signed-off-by: Eric * add i18n Signed-off-by: Eric * add unit tests for modal Signed-off-by: Eric * test id change Signed-off-by: Eric * add devDependencies for tests Signed-off-by: Eric * use open confirm api and move mock file to discover mock folder Signed-off-by: Eric * remove unused type Signed-off-by: Eric * remove modal for log explorer redirection Signed-off-by: Eric * modify changelog Signed-off-by: Eric * remove modal test Signed-off-by: Eric * remove one modal related test Signed-off-by: Eric --------- Signed-off-by: Eric --- CHANGELOG.md | 10 +- package.json | 2 + .../datasource_selectable.tsx | 27 ++++- .../public/components/sidebar/index.test.tsx | 110 ++++++++++++++++++ .../public/components/sidebar/index.tsx | 20 +++- .../public/__mock__/index.test.mock.ts | 28 +++++ yarn.lock | 14 +++ 7 files changed, 192 insertions(+), 19 deletions(-) create mode 100644 src/plugins/data_explorer/public/components/sidebar/index.test.tsx create mode 100644 src/plugins/discover/public/__mock__/index.test.mock.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c5c0a7b165..64b89cc0365d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,15 +39,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Discover] Enhanced the data source selector with added sorting functionality ([#5609](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5609)) - [Multiple Datasource] Add datasource picker component and use it in devtools and tutorial page when multiple datasource is enabled ([#5756](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5756)) - [Multiple Datasource] Add datasource picker to import saved object flyout when multiple data source is enabled ([#5781](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5781)) -- [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) -- [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) -- [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895)) -- [Multiple Datasource] Concatenate data source name with index pattern name and change delimiter to double colon ([#5907](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5907)) -- [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881)) -- [Multiple Datasource] Improved error handling for the search API when a null value is passed for the dataSourceId ([#5882](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882)) -- [Multiple Datasource] Hide/Show authentication method in multi data source plugin based on configuration ([#5916](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5916)) -- [[Dynamic Configurations] Add support for dynamic application configurations ([#5855](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5855)) -- [Workspace] Optional workspaces params in repository ([#5949](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5949)) +- [Discover] Add extension group title to non-index data source groups to indicate log explorer redirection in discover data source selector. ([#5815](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5815)) ### 🐛 Bug Fixes diff --git a/package.json b/package.json index cb24289e686f..8bdd92b6371c 100644 --- a/package.json +++ b/package.json @@ -329,6 +329,7 @@ "@types/react-router-dom": "^5.3.2", "@types/react-virtualized": "^9.18.7", "@types/recompose": "^0.30.6", + "@types/redux-mock-store": "^1.0.6", "@types/selenium-webdriver": "^4.0.9", "@types/semver": "^7.5.0", "@types/sinon": "^7.0.13", @@ -446,6 +447,7 @@ "react-test-renderer": "^16.12.0", "reactcss": "1.2.3", "redux": "^4.0.5", + "redux-mock-store": "^1.5.4", "regenerate": "^1.4.0", "reselect": "^4.0.0", "resize-observer-polyfill": "^1.5.1", diff --git a/src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx b/src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx index 1c6876815a0e..682c9c5f5a24 100644 --- a/src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx +++ b/src/plugins/data/public/data_sources/datasource_selector/datasource_selectable.tsx @@ -13,10 +13,17 @@ import { DataSourceGroup, DataSourceSelectableProps } from './types'; type DataSourceTypeKey = 'DEFAULT_INDEX_PATTERNS' | 's3glue' | 'spark'; // Mapping between datasource type and its display name. +// Temporary solution, will be removed along with refactoring of data source APIs const DATASOURCE_TYPE_DISPLAY_NAME_MAP: Record = { - DEFAULT_INDEX_PATTERNS: 'Index patterns', - s3glue: 'Amazon S3', - spark: 'Spark', + DEFAULT_INDEX_PATTERNS: i18n.translate('dataExplorer.dataSourceSelector.indexPatternGroupTitle', { + defaultMessage: 'Index patterns', + }), + s3glue: i18n.translate('dataExplorer.dataSourceSelector.amazonS3GroupTitle', { + defaultMessage: 'Amazon S3', + }), + spark: i18n.translate('dataExplorer.dataSourceSelector.sparkGroupTitle', { + defaultMessage: 'Spark', + }), }; type DataSetType = ISourceDataSet['data_sets'][number]; @@ -66,7 +73,19 @@ const getSourceList = (allDataSets: ISourceDataSet[]) => { const finalList = [] as DataSourceGroup[]; allDataSets.forEach((curDataSet) => { const typeKey = curDataSet.ds.getType() as DataSourceTypeKey; - const groupName = DATASOURCE_TYPE_DISPLAY_NAME_MAP[typeKey] || 'Default Group'; + let groupName = + DATASOURCE_TYPE_DISPLAY_NAME_MAP[typeKey] || + i18n.translate('dataExplorer.dataSourceSelector.defaultGroupTitle', { + defaultMessage: 'Default Group', + }); + + // add '- Opens in Log Explorer' to hint user that selecting these types of data sources + // will lead to redirection to log explorer + if (typeKey !== 'DEFAULT_INDEX_PATTERNS') { + groupName = `${groupName}${i18n.translate('dataExplorer.dataSourceSelector.redirectionHint', { + defaultMessage: ' - Opens in Log Explorer', + })}`; + } const existingGroup = finalList.find((item) => item.label === groupName); const mappedOptions = curDataSet.data_sets.map((dataSet) => diff --git a/src/plugins/data_explorer/public/components/sidebar/index.test.tsx b/src/plugins/data_explorer/public/components/sidebar/index.test.tsx new file mode 100644 index 000000000000..eccb0ffa909e --- /dev/null +++ b/src/plugins/data_explorer/public/components/sidebar/index.test.tsx @@ -0,0 +1,110 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { render, fireEvent, waitFor } from '@testing-library/react'; +import { Sidebar } from './index'; // Adjust the import path as necessary +import { Provider } from 'react-redux'; +import configureMockStore from 'redux-mock-store'; +import { MockS3DataSource } from '../../../../discover/public/__mock__/index.test.mock'; +import { createMemoryHistory } from 'history'; +import { Router } from 'react-router-dom'; + +const mockStore = configureMockStore(); +const initialState = { + metadata: { indexPattern: 'some-index-pattern-id' }, +}; +const store = mockStore(initialState); + +jest.mock('../../../../opensearch_dashboards_react/public', () => { + return { + toMountPoint: jest.fn().mockImplementation((component) => () => component), + useOpenSearchDashboards: jest.fn().mockReturnValue({ + services: { + data: { + indexPatterns: {}, + dataSources: { + dataSourceService: { + dataSources$: { + subscribe: jest.fn((callback) => { + callback({ + 's3-prod-mock': new MockS3DataSource({ + name: 's3-prod-mock', + type: 's3glue', + metadata: {}, + }), + }); + return { unsubscribe: jest.fn() }; + }), + }, + }, + }, + }, + notifications: { + toasts: { + addError: jest.fn(), + }, + }, + application: { + navigateToUrl: jest.fn(), + }, + overlays: { + openConfirm: jest.fn(), + }, + }, + }), + withOpenSearchDashboards: () => (Component: React.ComponentClass) => (props: any) => ( + + ), + }; +}); + +jest.mock('../../../../data_explorer/public', () => ({ + useTypedSelector: jest.fn(), + useTypedDispatch: jest.fn(), +})); + +describe('Sidebar Component', () => { + it('renders without crashing', () => { + const { container, getByTestId } = render( + + + + ); + expect(container).toBeInTheDocument(); + expect(getByTestId('dataExplorerDSSelect')).toBeInTheDocument(); + }); + + it('shows title extensions on the non-index pattern data source', () => { + const { getByText, getByTestId } = render( + + + + ); + + fireEvent.click(getByTestId('comboBoxToggleListButton')); + waitFor(() => { + expect(getByText('Open in Log Explorer')).toBeInTheDocument(); + }); + }); + + it('redirects to log explorer when clicking open-in-log-explorer button', () => { + const history = createMemoryHistory(); + const { getByText, getByTestId } = render( + + + + + + ); + + fireEvent.click(getByTestId('comboBoxToggleListButton')); + waitFor(() => { + expect(getByText('s3-prod-mock')).toBeInTheDocument(); + fireEvent.click(getByText('s3-prod-mock')); + expect(history.location.pathname).toContain('observability-logs#/explorer'); + }); + }); +}); diff --git a/src/plugins/data_explorer/public/components/sidebar/index.tsx b/src/plugins/data_explorer/public/components/sidebar/index.tsx index ee2dd63a6d57..ff07a59ab4b7 100644 --- a/src/plugins/data_explorer/public/components/sidebar/index.tsx +++ b/src/plugins/data_explorer/public/components/sidebar/index.tsx @@ -59,23 +59,31 @@ export const Sidebar: FC = ({ children }) => { } }, [indexPatternId, activeDataSources, dataSourceOptionList]); + const redirectToLogExplorer = useCallback( + (dsName: string, dsType: string) => { + return application.navigateToUrl( + `../observability-logs#/explorer?datasourceName=${dsName}&datasourceType=${dsType}` + ); + }, + [application] + ); + const handleSourceSelection = useCallback( (selectedDataSources: DataSourceOption[]) => { if (selectedDataSources.length === 0) { setSelectedSources(selectedDataSources); return; } - // Temporary redirection solution for 2.11, where clicking non-index-pattern datasource - // will redirect user to Observability event explorer + // Temporary redirection solution for 2.11, where clicking non-index-pattern data sources + // will prompt users with modal explaining they are being redirected to Observability log explorer if (selectedDataSources[0]?.ds?.getType() !== 'DEFAULT_INDEX_PATTERNS') { - return application.navigateToUrl( - `../observability-logs#/explorer?datasourceName=${selectedDataSources[0].label}&datasourceType=${selectedDataSources[0].type}` - ); + redirectToLogExplorer(selectedDataSources[0].label, selectedDataSources[0].type); + return; } setSelectedSources(selectedDataSources); dispatch(setIndexPattern(selectedDataSources[0].value)); }, - [application, dispatch] + [dispatch, redirectToLogExplorer, setSelectedSources] ); const handleGetDataSetError = useCallback( diff --git a/src/plugins/discover/public/__mock__/index.test.mock.ts b/src/plugins/discover/public/__mock__/index.test.mock.ts new file mode 100644 index 000000000000..6b09d1d84253 --- /dev/null +++ b/src/plugins/discover/public/__mock__/index.test.mock.ts @@ -0,0 +1,28 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export class MockS3DataSource { + protected name: string; + protected type: string; + protected metadata: any; + + constructor({ name, type, metadata }: { name: string; type: string; metadata: any }) { + this.name = name; + this.type = type; + this.metadata = metadata; + } + + async getDataSet(dataSetParams?: any) { + return [this.name]; + } + + getName() { + return this.name; + } + + getType() { + return this.type; + } +} diff --git a/yarn.lock b/yarn.lock index e1ac33225f8d..a6bd5836ef6c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3904,6 +3904,13 @@ dependencies: "@types/react" "*" +"@types/redux-mock-store@^1.0.6": + version "1.0.6" + resolved "https://registry.yarnpkg.com/@types/redux-mock-store/-/redux-mock-store-1.0.6.tgz#0a03b2655028b7cf62670d41ac1de5ca1b1f5958" + integrity sha512-eg5RDfhJTXuoJjOMyXiJbaDb1B8tfTaJixscmu+jOusj6adGC0Krntz09Tf4gJgXeCqCrM5bBMd+B7ez0izcAQ== + dependencies: + redux "^4.0.5" + "@types/refractor@^3.0.0": version "3.0.2" resolved "https://registry.yarnpkg.com/@types/refractor/-/refractor-3.0.2.tgz#2d42128d59f78f84d2c799ffc5ab5cadbcba2d82" @@ -15452,6 +15459,13 @@ redent@^3.0.0: indent-string "^4.0.0" strip-indent "^3.0.0" +redux-mock-store@^1.5.4: + version "1.5.4" + resolved "https://registry.yarnpkg.com/redux-mock-store/-/redux-mock-store-1.5.4.tgz#90d02495fd918ddbaa96b83aef626287c9ab5872" + integrity sha512-xmcA0O/tjCLXhh9Fuiq6pMrJCwFRaouA8436zcikdIpYWWCjU76CRk+i2bHx8EeiSiMGnB85/lZdU3wIJVXHTA== + dependencies: + lodash.isplainobject "^4.0.6" + redux-thunk@^2.3.0, redux-thunk@^2.4.1: version "2.4.1" resolved "https://registry.yarnpkg.com/redux-thunk/-/redux-thunk-2.4.1.tgz#0dd8042cf47868f4b29699941de03c9301a75714" From a8ee4b94326c1fe8ba74b648e07d75459256a7e9 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Sun, 7 Apr 2024 15:43:52 +0800 Subject: [PATCH 29/36] feat: remove useless code Signed-off-by: SuZhou-Joe --- CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64b89cc0365d..a2c5c0a7b165 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,15 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Discover] Enhanced the data source selector with added sorting functionality ([#5609](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5609)) - [Multiple Datasource] Add datasource picker component and use it in devtools and tutorial page when multiple datasource is enabled ([#5756](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5756)) - [Multiple Datasource] Add datasource picker to import saved object flyout when multiple data source is enabled ([#5781](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5781)) -- [Discover] Add extension group title to non-index data source groups to indicate log explorer redirection in discover data source selector. ([#5815](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5815)) +- [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851)) +- [Multiple Datasource] Able to Hide "Local Cluster" option from datasource DropDown ([#5827](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5827)) +- [Multiple Datasource] Add api registry and allow it to be added into client config in data source plugin ([#5895](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5895)) +- [Multiple Datasource] Concatenate data source name with index pattern name and change delimiter to double colon ([#5907](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5907)) +- [Multiple Datasource] Refactor client and legacy client to use authentication registry ([#5881](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5881)) +- [Multiple Datasource] Improved error handling for the search API when a null value is passed for the dataSourceId ([#5882](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5882)) +- [Multiple Datasource] Hide/Show authentication method in multi data source plugin based on configuration ([#5916](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5916)) +- [[Dynamic Configurations] Add support for dynamic application configurations ([#5855](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5855)) +- [Workspace] Optional workspaces params in repository ([#5949](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5949)) ### 🐛 Bug Fixes From 9b9fd57c7cbc759d37830d07783816c5682f060a Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Fri, 23 Feb 2024 11:07:37 -0800 Subject: [PATCH 30/36] [Bug][Discover] Allow saved sort from search embeddable to load in Dashboard (#5934) Remove default sort to use saved search sort. Issue Resolve https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5933 Signed-off-by: Anan Zhuang --- CHANGELOG.md | 1 + .../discover/public/embeddable/search_embeddable.tsx | 7 ------- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c5c0a7b165..c3bcd0c78c45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [BUG] Remove duplicate sample data as id 90943e30-9a47-11e8-b64d-95841ca0b247 ([5668](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5668)) - [BUG][Multiple Datasource] Fix datasource testing connection unexpectedly passed with wrong endpoint [#5663](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5663) - [Table Visualization] Fix filter action buttons for split table aggregations ([#5619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5619)) +- [BUG][Discover] Allow saved sort from search embeddable to load in Dashboard ([#5934](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5934)) ### 🚞 Infrastructure diff --git a/src/plugins/discover/public/embeddable/search_embeddable.tsx b/src/plugins/discover/public/embeddable/search_embeddable.tsx index 79080cf8657f..e2c1b1271397 100644 --- a/src/plugins/discover/public/embeddable/search_embeddable.tsx +++ b/src/plugins/discover/public/embeddable/search_embeddable.tsx @@ -47,7 +47,6 @@ import { } from '../../../data/public'; import { Container, Embeddable } from '../../../embeddable/public'; import { ISearchEmbeddable, SearchInput, SearchOutput } from './types'; -import { getDefaultSort } from '../application/view_components/utils/get_default_sort'; import { getSortForSearchSource } from '../application/view_components/utils/get_sort_for_search_source'; import { getRequestInspectorStats, @@ -216,12 +215,6 @@ export class SearchEmbeddable return; } - const sort = getDefaultSort( - indexPattern, - this.services.uiSettings.get(SORT_DEFAULT_ORDER_SETTING, 'desc') - ); - this.savedSearch.sort = sort; - const searchProps: SearchProps = { columns: this.savedSearch.columns, sort: [], From 1eb021f69bf74ba7ba57437dfbfd4b209e87b44b Mon Sep 17 00:00:00 2001 From: Ashwin P Chandran Date: Mon, 26 Feb 2024 16:02:24 -0800 Subject: [PATCH 31/36] [Discover] Fixes safari overflow bug (#5948) * [Discover] Fixes safari overflow bug --------- Signed-off-by: Ashwin P Chandran Signed-off-by: Anan Zhuang Co-authored-by: Anan Zhuang Co-authored-by: Miki --- .../components/default_discover_table/_table_cell.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/application/components/default_discover_table/_table_cell.scss b/src/plugins/discover/public/application/components/default_discover_table/_table_cell.scss index c960e87a9477..e87c3e62ae1f 100644 --- a/src/plugins/discover/public/application/components/default_discover_table/_table_cell.scss +++ b/src/plugins/discover/public/application/components/default_discover_table/_table_cell.scss @@ -85,8 +85,8 @@ .osdDocTableCell__source { .truncate-by-height { - transform: translateY(-1.5px); - margin-bottom: -1.5px; + margin-top: -1.5px; + margin-bottom: -3.5px; } dd, From 9c7de97350eaf87428ad204ebfddeb21b767d33a Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Sun, 3 Mar 2024 18:47:32 -0800 Subject: [PATCH 32/36] [BUG][Discover] Enable 'Back to Top' Feature in Discover for scrolling to top (#6008) * [BUG][Discover] Enable 'Back to Top' Feature in Discover for scrolling to top dscCanvas is the one with scrollable prop. Set window.scrollTo(0, 0) on table will not work. In this PR, we add a ref to EuiPanel directly. Issue Resolve: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/6006 --------- Signed-off-by: Anan Zhuang Co-authored-by: Miki --- .../application/components/data_grid/data_grid_table.tsx | 4 ++++ .../default_discover_table/default_discover_table.tsx | 4 +++- .../view_components/canvas/discover_table.tsx | 5 +++-- .../public/application/view_components/canvas/index.tsx | 9 ++++++++- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx index a0c1851e7716..d4b8de6ad211 100644 --- a/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx +++ b/src/plugins/discover/public/application/components/data_grid/data_grid_table.tsx @@ -46,6 +46,7 @@ export interface DataGridTableProps { isContextView?: boolean; isLoading?: boolean; showPagination?: boolean; + scrollToTop?: () => void; } export const DataGridTable = ({ @@ -67,6 +68,7 @@ export const DataGridTable = ({ isContextView = false, isLoading = false, showPagination, + scrollToTop, }: DataGridTableProps) => { const services = getServices(); const [inspectedHit, setInspectedHit] = useState(); @@ -179,6 +181,7 @@ export const DataGridTable = ({ isShortDots={isShortDots} hideTimeColumn={hideTimeColumn} defaultSortOrder={defaultSortOrder} + scrollToTop={scrollToTop} /> ), [ @@ -197,6 +200,7 @@ export const DataGridTable = ({ defaultSortOrder, hideTimeColumn, isShortDots, + scrollToTop, ] ); diff --git a/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx b/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx index fe8092ed8c9c..d563f1c1d098 100644 --- a/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx +++ b/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx @@ -33,6 +33,7 @@ export interface DefaultDiscoverTableProps { hideTimeColumn: boolean; defaultSortOrder: SortDirection; showPagination?: boolean; + scrollToTop?: () => void; } export const LegacyDiscoverTable = ({ @@ -52,6 +53,7 @@ export const LegacyDiscoverTable = ({ hideTimeColumn, defaultSortOrder, showPagination, + scrollToTop, }: DefaultDiscoverTableProps) => { const displayedColumns = getLegacyDisplayedColumns( columns, @@ -173,7 +175,7 @@ export const LegacyDiscoverTable = ({ values={{ sampleSize }} /> - window.scrollTo(0, 0)}> + diff --git a/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx b/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx index 17f9f26e8b54..ccf82e4ccba0 100644 --- a/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/discover_table.tsx @@ -12,7 +12,6 @@ import { addColumn, moveColumn, removeColumn, - reorderColumn, setColumns, setSort, useDispatch, @@ -27,9 +26,10 @@ import { popularizeField } from '../../helpers/popularize_field'; interface Props { rows?: OpenSearchSearchHit[]; + scrollToTop?: () => void; } -export const DiscoverTable = ({ rows }: Props) => { +export const DiscoverTable = ({ rows, scrollToTop }: Props) => { const { services } = useOpenSearchDashboards(); const { uiSettings, @@ -115,6 +115,7 @@ export const DiscoverTable = ({ rows }: Props) => { displayTimeColumn={displayTimeColumn} title={savedSearch?.id ? savedSearch.title : ''} description={savedSearch?.id ? savedSearch.description : ''} + scrollToTop={scrollToTop} /> ); }; diff --git a/src/plugins/discover/public/application/view_components/canvas/index.tsx b/src/plugins/discover/public/application/view_components/canvas/index.tsx index e3efe878aa83..1c2681995f98 100644 --- a/src/plugins/discover/public/application/view_components/canvas/index.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/index.tsx @@ -24,6 +24,7 @@ import './discover_canvas.scss'; // eslint-disable-next-line import/no-default-export export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewProps) { + const panelRef = useRef(null); const { data$, refetch$, indexPattern } = useDiscoverContext(); const { services: { uiSettings }, @@ -89,9 +90,15 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro }, [dispatch, filteredColumns, indexPattern]); const timeField = indexPattern?.timeFieldName ? indexPattern.timeFieldName : undefined; + const scrollToTop = () => { + if (panelRef.current) { + panelRef.current.scrollTop = 0; + } + }; return ( - + )} From fc1fbe44cf7a00d87214b190334e541aca8902c1 Mon Sep 17 00:00:00 2001 From: Ashwin P Chandran Date: Tue, 5 Mar 2024 13:49:09 -0800 Subject: [PATCH 33/36] [Discover] Fix lazy loading (#6041) * adds callback ref to lazy loading sentinel --------- Signed-off-by: Ashwin P Chandran --- .../default_discover_table.tsx | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx b/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx index d563f1c1d098..21fc1f3670da 100644 --- a/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx +++ b/src/plugins/discover/public/application/components/default_discover_table/default_discover_table.tsx @@ -5,7 +5,7 @@ import './_doc_table.scss'; -import React, { useEffect, useRef, useState } from 'react'; +import React, { useEffect, useRef, useState, useCallback } from 'react'; import { EuiButtonEmpty, EuiCallOut, EuiProgress } from '@elastic/eui'; import { FormattedMessage } from '@osd/i18n/react'; import { TableHeader } from './table_header'; @@ -70,33 +70,34 @@ export const LegacyDiscoverTable = ({ endRow: rows.length < pageSize ? rows.length : pageSize, }); const observerRef = useRef(null); - const sentinelRef = useRef(null); - - const loadMoreRows = () => { - setRenderedRowCount((prevRowCount) => prevRowCount + 50); // Load 50 more rows - }; + const [sentinelEle, setSentinelEle] = useState(); + // Need a callback ref since the element isnt set on the first render. + const sentinelRef = useCallback((node: HTMLDivElement | null) => { + if (node !== null) { + setSentinelEle(node); + } + }, []); useEffect(() => { - const sentinel = sentinelRef.current; - observerRef.current = new IntersectionObserver( - (entries) => { - if (entries[0].isIntersecting) { - loadMoreRows(); - } - }, - { threshold: 1.0 } - ); + if (sentinelEle) { + observerRef.current = new IntersectionObserver( + (entries) => { + if (entries[0].isIntersecting) { + setRenderedRowCount((prevRowCount) => prevRowCount + 50); // Load 50 more rows + } + }, + { threshold: 1.0 } + ); - if (sentinelRef.current) { - observerRef.current.observe(sentinelRef.current); + observerRef.current.observe(sentinelEle); } return () => { - if (observerRef.current && sentinel) { - observerRef.current.unobserve(sentinel); + if (observerRef.current && sentinelEle) { + observerRef.current.unobserve(sentinelEle); } }; - }, []); + }, [sentinelEle]); const [activePage, setActivePage] = useState(0); const pageCount = Math.ceil(rows.length / pageSize); From 1f2cb35b16a5356e65f6b435828497d63f3132f9 Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Mon, 18 Mar 2024 18:52:42 -0700 Subject: [PATCH 34/36] [Discover] options button in canvas to toggle legacy mode (#6170) * [Discover] options button in canvas to toggle legacy mode Removes top nav bar link and provides a button option. Issue: n/a Signed-off-by: Kawika Avilla --- .../components/top_nav/get_top_nav_links.tsx | 57 ------------------- .../components/utils/local_storage.ts | 4 +- .../canvas/discover_canvas.scss | 7 +++ .../view_components/canvas/index.tsx | 50 +++++++++++++++- .../apps/dashboard/dashboard_filter_bar.js | 1 + .../apps/dashboard/dashboard_filtering.js | 1 + .../apps/dashboard/dashboard_state.js | 1 + .../apps/management/_scripted_fields.js | 8 +-- test/functional/page_objects/discover_page.ts | 11 ++-- .../test_suites/doc_views/doc_views.ts | 2 +- 10 files changed, 72 insertions(+), 70 deletions(-) diff --git a/src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx b/src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx index 1b20c444e860..6760416ab8c9 100644 --- a/src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx +++ b/src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx @@ -5,7 +5,6 @@ import { i18n } from '@osd/i18n'; import React from 'react'; -import { EuiText } from '@elastic/eui'; import { DiscoverViewServices } from '../../../build_services'; import { SavedSearch } from '../../../saved_searches'; import { Adapters } from '../../../../../inspector/public'; @@ -25,7 +24,6 @@ import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../ import { getSortForSearchSource } from '../../view_components/utils/get_sort_for_search_source'; import { getRootBreadcrumbs } from '../../helpers/breadcrumbs'; import { syncQueryStateWithUrl } from '../../../../../data/public'; -import { getNewDiscoverSetting, setNewDiscoverSetting } from '../utils/local_storage'; import { OpenSearchPanel } from './open_search_panel'; export const getTopNavLinks = ( @@ -44,7 +42,6 @@ export const getTopNavLinks = ( store, data: { query }, osdUrlStateStorage, - storage, } = services; const newSearch = { @@ -234,61 +231,7 @@ export const getTopNavLinks = ( }, }; - const newDiscoverButtonLabel = i18n.translate('discover.localMenu.discoverButton.label.new', { - defaultMessage: 'Try new Discover', - }); - const oldDiscoverButtonLabel = i18n.translate('discover.localMenu.discoverButton.label.old', { - defaultMessage: 'Use legacy Discover', - }); - const isNewDiscover = getNewDiscoverSetting(storage); - const newTable: TopNavMenuData = { - id: 'table-datagrid', - label: isNewDiscover ? oldDiscoverButtonLabel : newDiscoverButtonLabel, - description: i18n.translate('discover.localMenu.newTableDescription', { - defaultMessage: 'New Discover toggle Experience', - }), - testId: 'datagridTableButton', - run: async () => { - // Read the current state from localStorage - const newDiscoverEnabled = getNewDiscoverSetting(storage); - if (newDiscoverEnabled) { - const confirmed = await services.overlays.openConfirm( - toMountPoint( - -

- Help drive future improvements by{' '} - - providing feedback - {' '} - about your experience. -

- - ), - { - title: i18n.translate('discover.localMenu.newTableConfirmModalTitle', { - defaultMessage: 'Share your thoughts on the latest Discover features', - }), - cancelButtonText: 'Cancel', - confirmButtonText: 'Turn off new features', - defaultFocusedButton: 'confirm', - } - ); - - if (confirmed) { - setNewDiscoverSetting(false, storage); - window.location.reload(); - } - } else { - // Save the new setting to localStorage - setNewDiscoverSetting(true, storage); - window.location.reload(); - } - }, - iconType: isNewDiscover ? 'editorUndo' : 'cheer', - }; - return [ - newTable, newSearch, ...(capabilities.discover?.save ? [saveSearch] : []), openSearch, diff --git a/src/plugins/discover/public/application/components/utils/local_storage.ts b/src/plugins/discover/public/application/components/utils/local_storage.ts index 5e812de8e97d..68bba0aafc99 100644 --- a/src/plugins/discover/public/application/components/utils/local_storage.ts +++ b/src/plugins/discover/public/application/components/utils/local_storage.ts @@ -9,9 +9,9 @@ export const NEW_DISCOVER_KEY = 'discover:newExpereince'; export const getNewDiscoverSetting = (storage: Storage): boolean => { const storedValue = storage.get(NEW_DISCOVER_KEY); - return storedValue !== null ? JSON.parse(storedValue) : false; + return storedValue !== null ? storedValue : false; }; export const setNewDiscoverSetting = (value: boolean, storage: Storage) => { - storage.set(NEW_DISCOVER_KEY, JSON.stringify(value)); + storage.set(NEW_DISCOVER_KEY, value); }; diff --git a/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss b/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss index 36408bd88366..2c2c8dfe8ebb 100644 --- a/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss +++ b/src/plugins/discover/public/application/view_components/canvas/discover_canvas.scss @@ -9,6 +9,13 @@ &_results { margin-left: $euiSizeM; + position: relative; + } + + &_options { + position: absolute; + top: 0; + right: 0; } } diff --git a/src/plugins/discover/public/application/view_components/canvas/index.tsx b/src/plugins/discover/public/application/view_components/canvas/index.tsx index 1c2681995f98..ab34878750a7 100644 --- a/src/plugins/discover/public/application/view_components/canvas/index.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/index.tsx @@ -4,7 +4,7 @@ */ import React, { useEffect, useState, useRef, useCallback } from 'react'; -import { EuiPanel } from '@elastic/eui'; +import { EuiButtonIcon, EuiContextMenu, EuiPanel, EuiPopover, EuiSwitch } from '@elastic/eui'; import { TopNav } from './top_nav'; import { ViewProps } from '../../../../../data_explorer/public'; import { DiscoverTable } from './discover_table'; @@ -21,13 +21,14 @@ import { filterColumns } from '../utils/filter_columns'; import { DEFAULT_COLUMNS_SETTING, MODIFY_COLUMNS_ON_SWITCH } from '../../../../common'; import { OpenSearchSearchHit } from '../../../application/doc_views/doc_views_types'; import './discover_canvas.scss'; +import { getNewDiscoverSetting, setNewDiscoverSetting } from '../../components/utils/local_storage'; // eslint-disable-next-line import/no-default-export export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewProps) { const panelRef = useRef(null); const { data$, refetch$, indexPattern } = useDiscoverContext(); const { - services: { uiSettings }, + services: { uiSettings, storage }, } = useOpenSearchDashboards(); const { columns } = useSelector((state) => state.discover); const filteredColumns = filterColumns( @@ -96,6 +97,50 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro } }; + const [isOptionsOpen, setOptionsOpen] = useState(false); + const [useLegacy, setUseLegacy] = useState(!getNewDiscoverSetting(storage)); + const DiscoverOptions = () => ( + setOptionsOpen(!isOptionsOpen)} + /> + } + closePopover={() => setOptionsOpen(false)} + isOpen={isOptionsOpen} + panelPaddingSize="none" + className="dscCanvas_options" + > + + { + const checked = e.target.checked; + setUseLegacy(checked); + setNewDiscoverSetting(!checked, storage); + window.location.reload(); + }} + /> + + ), + }, + ]} + /> + + ); + return ( + )} diff --git a/test/functional/apps/dashboard/dashboard_filter_bar.js b/test/functional/apps/dashboard/dashboard_filter_bar.js index dde86c697e3c..17e29fa5cf6f 100644 --- a/test/functional/apps/dashboard/dashboard_filter_bar.js +++ b/test/functional/apps/dashboard/dashboard_filter_bar.js @@ -193,6 +193,7 @@ export default function ({ getService, getPageObjects }) { before(async () => { await filterBar.ensureFieldEditorModalIsClosed(); await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setDefaultDataRange(); await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.common.navigateToApp('dashboard'); await PageObjects.dashboard.gotoDashboardLandingPage(); diff --git a/test/functional/apps/dashboard/dashboard_filtering.js b/test/functional/apps/dashboard/dashboard_filtering.js index 1040b87f6168..6be4b4837da0 100644 --- a/test/functional/apps/dashboard/dashboard_filtering.js +++ b/test/functional/apps/dashboard/dashboard_filtering.js @@ -80,6 +80,7 @@ export default function ({ getService, getPageObjects }) { describe('adding a filter that excludes all data', () => { before(async () => { await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setDefaultDataRange(); await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.common.navigateToApp('dashboard'); await PageObjects.dashboard.gotoDashboardLandingPage(); diff --git a/test/functional/apps/dashboard/dashboard_state.js b/test/functional/apps/dashboard/dashboard_state.js index 11196a1b69b9..78f0cdb16184 100644 --- a/test/functional/apps/dashboard/dashboard_state.js +++ b/test/functional/apps/dashboard/dashboard_state.js @@ -57,6 +57,7 @@ export default function ({ getService, getPageObjects }) { describe('dashboard state', function describeIndexTests() { before(async function () { await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setHistoricalDataRange(); await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.common.navigateToApp('dashboard'); await PageObjects.dashboard.initTests(); diff --git a/test/functional/apps/management/_scripted_fields.js b/test/functional/apps/management/_scripted_fields.js index 8a4659630ee1..f3d89138eb77 100644 --- a/test/functional/apps/management/_scripted_fields.js +++ b/test/functional/apps/management/_scripted_fields.js @@ -161,9 +161,9 @@ export default function ({ getService, getPageObjects }) { const fromTime = 'Sep 17, 2015 @ 06:31:44.000'; const toTime = 'Sep 18, 2015 @ 18:31:44.000'; await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.selectIndexPattern('logstash-*'); await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.clickFieldListItem(scriptedPainlessFieldName); await retry.try(async function () { @@ -280,9 +280,9 @@ export default function ({ getService, getPageObjects }) { const fromTime = 'Sep 17, 2015 @ 06:31:44.000'; const toTime = 'Sep 18, 2015 @ 18:31:44.000'; await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.selectIndexPattern('logstash-*'); await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.clickFieldListItem(scriptedPainlessFieldName2); await retry.try(async function () { @@ -377,9 +377,9 @@ export default function ({ getService, getPageObjects }) { const fromTime = 'Sep 17, 2015 @ 06:31:44.000'; const toTime = 'Sep 18, 2015 @ 18:31:44.000'; await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.selectIndexPattern('logstash-*'); await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.clickFieldListItem(scriptedPainlessFieldName2); await retry.try(async function () { @@ -477,9 +477,9 @@ export default function ({ getService, getPageObjects }) { const fromTime = 'Sep 17, 2015 @ 19:22:00.000'; const toTime = 'Sep 18, 2015 @ 07:00:00.000'; await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.selectIndexPattern('logstash-*'); await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + await PageObjects.discover.switchDiscoverTable('new'); await PageObjects.discover.clickFieldListItem(scriptedPainlessFieldName2); await retry.try(async function () { diff --git a/test/functional/page_objects/discover_page.ts b/test/functional/page_objects/discover_page.ts index cf0b03022ab9..8f03ae20ddf7 100644 --- a/test/functional/page_objects/discover_page.ts +++ b/test/functional/page_objects/discover_page.ts @@ -519,12 +519,15 @@ export function DiscoverPageProvider({ getService, getPageObjects }: FtrProvider public async switchDiscoverTable(tableType: string) { await retry.try(async () => { - const switchButton = await testSubjects.find('datagridTableButton'); - const buttonText = await switchButton.getVisibleText(); + const optionsButton = await testSubjects.find('discoverOptionsButton'); + await optionsButton.click(); - if (tableType === 'new' && buttonText.includes('Try new Discover')) { + const switchButton = await testSubjects.find('discoverOptionsLegacySwitch'); + const isLegacyChecked = (await switchButton.getAttribute('aria-checked')) === 'true'; + + if (tableType === 'new' && isLegacyChecked) { await switchButton.click(); - } else if (tableType === 'legacy' && buttonText.includes('Use legacy Discover')) { + } else if (tableType === 'legacy' && !isLegacyChecked) { await switchButton.click(); } }); diff --git a/test/plugin_functional/test_suites/doc_views/doc_views.ts b/test/plugin_functional/test_suites/doc_views/doc_views.ts index b745d6e8a417..d215017132ae 100644 --- a/test/plugin_functional/test_suites/doc_views/doc_views.ts +++ b/test/plugin_functional/test_suites/doc_views/doc_views.ts @@ -39,10 +39,10 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide describe('custom doc views with datagrid table', function () { before(async () => { await PageObjects.common.navigateToApp('discover'); - await PageObjects.discover.switchDiscoverTable('new'); // TODO: change back to setDefaultRange() once we resolve // https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5241 await PageObjects.timePicker.setDefaultRangeForDiscover(); + await PageObjects.discover.switchDiscoverTable('new'); }); it('should show custom doc views', async () => { From 742ec22d5166f432a2d6f3e72dcaeeadbedf8b58 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 8 Apr 2024 10:18:11 +0800 Subject: [PATCH 35/36] refact: update types declaration Signed-off-by: SuZhou-Joe --- src/core/public/saved_objects/saved_objects_client.ts | 3 ++- .../export/get_sorted_objects_for_export.ts | 4 ++-- src/core/server/saved_objects/import/check_conflicts.ts | 3 ++- .../server/saved_objects/import/create_saved_objects.ts | 9 +++++++-- src/core/server/saved_objects/import/types.ts | 6 +++--- src/core/server/saved_objects/types.ts | 2 +- src/core/types/saved_objects.ts | 2 +- 7 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index ce3ebc710ed1..d40b91188cfc 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -38,6 +38,7 @@ import { SavedObjectsClientContract as SavedObjectsApi, SavedObjectsFindOptions as SavedObjectFindOptionsServer, SavedObjectsMigrationVersion, + SavedObjectsBaseOptions, } from '../../server'; import { SimpleSavedObject } from './simple_saved_object'; @@ -65,7 +66,7 @@ export interface SavedObjectsCreateOptions { /** {@inheritDoc SavedObjectsMigrationVersion} */ migrationVersion?: SavedObjectsMigrationVersion; references?: SavedObjectReference[]; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** 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 6c99db6c24af..44595b0c33ff 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 @@ -30,7 +30,7 @@ import Boom from '@hapi/boom'; import { createListStream } from '../../utils/streams'; -import { SavedObjectsClientContract, SavedObject } from '../types'; +import { SavedObjectsClientContract, SavedObject, SavedObjectsBaseOptions } from '../types'; import { fetchNestedDependencies } from './inject_nested_depdendencies'; import { sortObjects } from './sort_objects'; @@ -61,7 +61,7 @@ export interface SavedObjectsExportOptions { /** optional namespace to override the namespace used by the savedObjectsClient. */ namespace?: string; /** optional workspaces to override the workspaces used by the savedObjectsClient. */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** diff --git a/src/core/server/saved_objects/import/check_conflicts.ts b/src/core/server/saved_objects/import/check_conflicts.ts index d173af45f761..7afab618b1ff 100644 --- a/src/core/server/saved_objects/import/check_conflicts.ts +++ b/src/core/server/saved_objects/import/check_conflicts.ts @@ -35,6 +35,7 @@ import { SavedObjectsImportError, SavedObjectError, SavedObjectsImportRetry, + SavedObjectsBaseOptions, } from '../types'; interface CheckConflictsParams { @@ -44,7 +45,7 @@ interface CheckConflictsParams { ignoreRegularConflicts?: boolean; retries?: SavedObjectsImportRetry[]; createNewCopies?: boolean; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } const isUnresolvableConflict = (error: SavedObjectError) => 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 f98e2e816b75..35d0ddd349fa 100644 --- a/src/core/server/saved_objects/import/create_saved_objects.ts +++ b/src/core/server/saved_objects/import/create_saved_objects.ts @@ -28,7 +28,12 @@ * under the License. */ -import { SavedObject, SavedObjectsClientContract, SavedObjectsImportError } from '../types'; +import { + SavedObject, + SavedObjectsBaseOptions, + SavedObjectsClientContract, + SavedObjectsImportError, +} from '../types'; import { extractErrors } from './extract_errors'; import { CreatedObject } from './types'; @@ -41,7 +46,7 @@ interface CreateSavedObjectsParams { overwrite?: boolean; dataSourceId?: string; dataSourceTitle?: string; - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } interface CreateSavedObjectsResult { createdObjects: Array>; diff --git a/src/core/server/saved_objects/import/types.ts b/src/core/server/saved_objects/import/types.ts index 689b8222b510..a243e08f83e0 100644 --- a/src/core/server/saved_objects/import/types.ts +++ b/src/core/server/saved_objects/import/types.ts @@ -29,7 +29,7 @@ */ import { Readable } from 'stream'; -import { SavedObjectsClientContract, SavedObject } from '../types'; +import { SavedObjectsClientContract, SavedObject, SavedObjectsBaseOptions } from '../types'; import { ISavedObjectTypeRegistry } from '..'; /** @@ -190,7 +190,7 @@ export interface SavedObjectsImportOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } /** @@ -215,7 +215,7 @@ export interface SavedObjectsResolveImportErrorsOptions { dataSourceId?: string; dataSourceTitle?: string; /** if specified, will import in given workspaces */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObjectsBaseOptions['workspaces']; } export type CreatedObject = SavedObject & { destinationId?: string }; diff --git a/src/core/server/saved_objects/types.ts b/src/core/server/saved_objects/types.ts index 22093b90d6b3..369cbfd53bf4 100644 --- a/src/core/server/saved_objects/types.ts +++ b/src/core/server/saved_objects/types.ts @@ -132,7 +132,7 @@ export interface SavedObjectsBaseOptions { /** Specify the namespace for this operation */ namespace?: string; /** Specify the workspaces for this operation */ - workspaces?: SavedObject['workspaces']; + workspaces?: SavedObject['workspaces'] | null; } /** diff --git a/src/core/types/saved_objects.ts b/src/core/types/saved_objects.ts index 65ff595fa200..74890bb624a3 100644 --- a/src/core/types/saved_objects.ts +++ b/src/core/types/saved_objects.ts @@ -115,7 +115,7 @@ export interface SavedObject { */ originId?: string; /** Workspaces that this saved object exists in. */ - workspaces?: string[] | null; + workspaces?: string[]; /** Permissions that this saved objects exists in. */ permissions?: Permissions; } From ab0486e5e2811fc86e8c2df043c163c3c6c04e93 Mon Sep 17 00:00:00 2001 From: SuZhou-Joe Date: Mon, 8 Apr 2024 11:32:41 +0800 Subject: [PATCH 36/36] fix: unit test error Signed-off-by: SuZhou-Joe --- src/plugins/workspace/server/plugin.test.ts | 2 +- .../server/saved_objects/workspace_id_consumer_wrapper.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/workspace/server/plugin.test.ts b/src/plugins/workspace/server/plugin.test.ts index a4586fecd3cd..0ad72b51b6dc 100644 --- a/src/plugins/workspace/server/plugin.test.ts +++ b/src/plugins/workspace/server/plugin.test.ts @@ -53,7 +53,7 @@ describe('Workspace server plugin', () => { expect(toolKitMock.rewriteUrl).toBeCalledWith('http://localhost/app'); expect(toolKitMock.next).toBeCalledTimes(0); expect(getWorkspaceState(requestWithWorkspaceInUrl)).toEqual({ - id: 'foo', + requestWorkspaceId: 'foo', }); const requestWithoutWorkspaceInUrl = httpServerMock.createOpenSearchDashboardsRequest({ diff --git a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts index 21ef63c04867..112f31baf562 100644 --- a/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts +++ b/src/plugins/workspace/server/saved_objects/workspace_id_consumer_wrapper.test.ts @@ -14,7 +14,7 @@ describe('WorkspaceIdConsumerWrapper', () => { const mockedClient = savedObjectsClientMock.create(); const workspaceEnabledMockRequest = httpServerMock.createOpenSearchDashboardsRequest(); updateWorkspaceState(workspaceEnabledMockRequest, { - id: 'foo', + requestWorkspaceId: 'foo', }); const wrapperClient = wrapperInstance.wrapperFactory({ client: mockedClient,