Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: move workspace logic from repository to saved objects client wrapper #248

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
4060458
refact: move workspace specific logic to savedObjectWrapper
SuZhou-Joe Oct 18, 2023
41204e2
feat: remove unnecessary changes
SuZhou-Joe Oct 18, 2023
888b77e
fix: unit test
SuZhou-Joe Feb 23, 2024
6560073
fix: some error
SuZhou-Joe Feb 23, 2024
d1c3c23
fix: some error
SuZhou-Joe Feb 23, 2024
c7b0e81
fix: unit test error
SuZhou-Joe Feb 23, 2024
8240051
feat: fix test error
SuZhou-Joe Feb 26, 2024
e79bdd9
refact: move workspace specific logic to savedObjectWrapper
SuZhou-Joe Oct 18, 2023
f21cfe5
fix: some error
SuZhou-Joe Feb 23, 2024
fedb974
feat: fix test error
SuZhou-Joe Feb 26, 2024
f59df29
feat: remove useless config in test
SuZhou-Joe Feb 26, 2024
19f1b8f
feat: add CHANGELOG
SuZhou-Joe Feb 26, 2024
b4e97d5
feat: add more unit test
SuZhou-Joe Sep 26, 2023
c8809f0
fix: unit test
SuZhou-Joe Oct 18, 2023
d8e52fa
feat: revert test in repository.test.js
SuZhou-Joe Feb 27, 2024
6afe452
feat: revert test in import_saved_objects.test.ts
SuZhou-Joe Feb 27, 2024
0fd2229
feat: revert test in repository.test.js
SuZhou-Joe Feb 27, 2024
38071b3
feat: add type
SuZhou-Joe Feb 27, 2024
da391ce
feat: optimize code and add comment
SuZhou-Joe Feb 27, 2024
406da0e
fix: unit test error
SuZhou-Joe Feb 27, 2024
ac0df5f
fix: integration test fail
SuZhou-Joe Feb 28, 2024
1efb8ba
feat: add missing code
SuZhou-Joe Feb 28, 2024
225d64d
feat: fix bootstrap error
SuZhou-Joe Feb 28, 2024
fd447ff
feat: remove useless code
SuZhou-Joe Feb 28, 2024
672b3ee
feat: remove useless code
SuZhou-Joe Feb 28, 2024
ddbeb58
feat: remove useless code
SuZhou-Joe Feb 28, 2024
88ea904
feat: merge
SuZhou-Joe Feb 28, 2024
e59ab71
feat: optimize code
SuZhou-Joe Feb 28, 2024
cbcc035
fix: unit test
SuZhou-Joe Feb 28, 2024
200b204
feat: remove useless code
SuZhou-Joe Feb 28, 2024
21a6b22
feat: optimize code
SuZhou-Joe Feb 29, 2024
25c32e5
feat: remove useless CHANGELOG
SuZhou-Joe Feb 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/server/saved_objects/import/regenerate_ids.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const regenerateIdsWithReference = async (props: {
return acc;
}

const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces(
const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToSourceWorkspaces(
workspaces,
object.workspaces
);
Expand Down
36 changes: 6 additions & 30 deletions src/core/server/saved_objects/service/lib/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,6 @@ describe('SavedObjectsRepository', () => {
};

const bulkCreateSuccess = async (objects, options) => {
const originalObjects = JSON.parse(JSON.stringify(objects));
const multiNamespaceObjects = objects.filter(
({ type, id }) => registry.isMultiNamespace(type) && id
);
Expand All @@ -666,9 +665,7 @@ describe('SavedObjectsRepository', () => {
opensearchClientMock.createSuccessTransportRequestPromise(response)
);
const result = await savedObjectsRepository.bulkCreate(objects, options);
expect(client.mget).toHaveBeenCalledTimes(
multiNamespaceObjects?.length || originalObjects?.some((item) => item.id) ? 1 : 0
);
expect(client.mget).toHaveBeenCalledTimes(multiNamespaceObjects?.length ? 1 : 0);
return result;
};

Expand Down Expand Up @@ -726,10 +723,7 @@ describe('SavedObjectsRepository', () => {
await bulkCreateSuccess(objects);
expect(client.bulk).toHaveBeenCalledTimes(1);
expect(client.mget).toHaveBeenCalledTimes(1);
const docs = [
expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }),
expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` }),
];
const docs = [expect.objectContaining({ _id: `${MULTI_NAMESPACE_TYPE}:${obj2.id}` })];
expect(client.mget.mock.calls[0][0].body).toEqual({ docs });
});

Expand Down Expand Up @@ -1007,12 +1001,6 @@ describe('SavedObjectsRepository', () => {
const response1 = {
status: 200,
docs: [
{
found: true,
_source: {
type: obj1.type,
},
},
{
found: true,
_source: {
Expand All @@ -1036,11 +1024,7 @@ describe('SavedObjectsRepository', () => {
expect(client.mget).toHaveBeenCalled();

const body1 = {
docs: [
expect.objectContaining({ _id: `${obj1.type}:${obj1.id}` }),
expect.objectContaining({ _id: `${obj.type}:${obj.id}` }),
expect.objectContaining({ _id: `${obj2.type}:${obj2.id}` }),
],
docs: [expect.objectContaining({ _id: `${obj.type}:${obj.id}` })],
};
expect(client.mget).toHaveBeenCalledWith(
expect.objectContaining({ body: body1 }),
Expand Down Expand Up @@ -2050,17 +2034,9 @@ describe('SavedObjectsRepository', () => {

const createSuccess = async (type, attributes, options) => {
const result = await savedObjectsRepository.create(type, attributes, options);
let count = 0;
if (options?.overwrite && options?.id) {
/**
* workspace will call extra one to get latest status of current object
*/
count++;
}
if (registry.isMultiNamespace(type) && options.overwrite) {
count++;
}
expect(client.get).toHaveBeenCalledTimes(count);
expect(client.get).toHaveBeenCalledTimes(
registry.isMultiNamespace(type) && options.overwrite ? 1 : 0
);
return result;
};

Expand Down
87 changes: 7 additions & 80 deletions src/core/server/saved_objects/service/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,26 +287,6 @@ export class SavedObjectsRepository {
}
}

let savedObjectWorkspaces = workspaces;

if (id && overwrite) {
try {
const currentItem = await this.get(type, id);
if (
SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces(
workspaces,
currentItem.workspaces
).length
) {
throw SavedObjectsErrorHelpers.createConflictError(type, id);
} else {
savedObjectWorkspaces = currentItem.workspaces;
}
} catch (e) {
// this.get will throw an error when no items can be found
}
}

const migrated = this._migrator.migrateDocument({
id,
type,
Expand All @@ -317,7 +297,7 @@ export class SavedObjectsRepository {
migrationVersion,
updated_at: time,
...(Array.isArray(references) && { references }),
...(Array.isArray(savedObjectWorkspaces) && { workspaces: savedObjectWorkspaces }),
...(Array.isArray(workspaces) && { workspaces }),
...(permissions && { permissions }),
});

Expand Down Expand Up @@ -385,28 +365,15 @@ export class SavedObjectsRepository {

const method = object.id && overwrite ? 'index' : 'create';
const requiresNamespacesCheck = object.id && this._registry.isMultiNamespace(object.type);
/**
* Only when importing an object to a target workspace should we check if the object is workspace-specific.
*/
const requiresWorkspaceCheck = object.id;

if (object.id == null) object.id = uuid.v1();

let opensearchRequestIndexPayload = {};

if (requiresNamespacesCheck || requiresWorkspaceCheck) {
opensearchRequestIndexPayload = {
opensearchRequestIndex: bulkGetRequestIndexCounter,
};
bulkGetRequestIndexCounter++;
}

return {
tag: 'Right' as 'Right',
value: {
method,
object,
...opensearchRequestIndexPayload,
...(requiresNamespacesCheck && { opensearchRequestIndex: bulkGetRequestIndexCounter++ }),
},
};
});
Expand All @@ -417,7 +384,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(
Expand Down Expand Up @@ -495,36 +462,7 @@ export class SavedObjectsRepository {
savedObjectWorkspaces = options.workspaces;

if (expectedBulkGetResult.value.method !== 'create') {
wanglam marked this conversation as resolved.
Show resolved Hide resolved
const rawId = this._serializer.generateRawId(namespace, object.type, object.id);
const findObject =
bulkGetResponse?.statusCode !== 404
? bulkGetResponse?.body.docs?.find((item) => item._id === rawId)
: null;
if (findObject && findObject.found) {
const transformedObject = this._serializer.rawToSavedObject(
findObject as SavedObjectsRawDoc
) as SavedObject;
const filteredWorkspaces = SavedObjectsUtils.filterWorkspacesAccordingToBaseWorkspaces(
options.workspaces,
transformedObject.workspaces
);
if (filteredWorkspaces.length) {
const { id, type } = object;
return {
tag: 'Left' as 'Left',
error: {
id,
type,
error: {
...errorContent(SavedObjectsErrorHelpers.createConflictError(type, id)),
metadata: { isNotOverwritable: true },
},
},
};
} else {
savedObjectWorkspaces = transformedObject.workspaces;
}
}
savedObjectWorkspaces = object.workspaces;
}

const expectedResult = {
Expand All @@ -541,7 +479,7 @@ export class SavedObjectsRepository {
updated_at: time,
references: object.references || [],
originId: object.originId,
workspaces: savedObjectWorkspaces,
...(savedObjectWorkspaces && { workspaces: savedObjectWorkspaces }),
}) as SavedObjectSanitizedDoc
),
};
Expand Down Expand Up @@ -639,7 +577,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(
Expand All @@ -662,24 +600,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 },
}),
},
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/saved_objects/service/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export class SavedObjectsUtils {
saved_objects: [],
});

public static filterWorkspacesAccordingToBaseWorkspaces(
public static filterWorkspacesAccordingToSourceWorkspaces(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this function is only used by src/plugins/workspace/server/saved_objects/saved_objects_wrapper_for_check_workspace_conflict.ts, I think we can move it there so we don't need to bother this util class in core

targetWorkspaces?: string[],
baseWorkspaces?: string[]
): string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ export interface SavedObjectsCreateOptions extends SavedObjectsBaseOptions {
initialNamespaces?: string[];
/** permission control describe by ACL object */
permissions?: Permissions;
/**
* workspaces the new created objects belong to
*/
workspaces?: string[];
SuZhou-Joe marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -94,6 +98,10 @@ export interface SavedObjectsBulkCreateObject<T = unknown> {
* Note: this can only be used for multi-namespace object types.
*/
initialNamespaces?: string[];
/**
* workspaces the objects belong to, will only be used when overwrite is enabled.
*/
workspaces?: string[];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't workspaces already specified in the options? SavedObjectsBaseOptions

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SavedObjectsBulkCreateObject doesn't extends SavedObjectsBaseOptions.

}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/workspace/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ export const PATHS = {
export const WORKSPACE_OP_TYPE_CREATE = 'create';
export const WORKSPACE_OP_TYPE_UPDATE = 'update';
export const WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID = 'workspace';
export const WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID =
'workspace_conflict_control';
15 changes: 14 additions & 1 deletion src/plugins/workspace/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,25 @@ import { IWorkspaceDBImpl } from './types';
import { WorkspaceClientWithSavedObject } from './workspace_client';
import { WorkspaceSavedObjectsClientWrapper } from './saved_objects';
import { registerRoutes } from './routes';
import { WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID } from '../common/constants';
import {
WORKSPACE_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
} from '../common/constants';
import {
SavedObjectsPermissionControl,
SavedObjectsPermissionControlContract,
} from './permission_control/client';
import { registerPermissionCheckRoutes } from './permission_control/routes';
import { WorkspacePluginConfigType } from '../config';
import { cleanWorkspaceId, getWorkspaceIdFromUrl } from '../../../core/server/utils';
import { WorkspaceConflictSavedObjectsClientWrapper } from './saved_objects/saved_objects_wrapper_for_check_workspace_conflict';

export class WorkspacePlugin implements Plugin<{}, {}> {
private readonly logger: Logger;
private client?: IWorkspaceDBImpl;
private permissionControl?: SavedObjectsPermissionControlContract;
private readonly config$: Observable<WorkspacePluginConfigType>;
private workspaceConflictControl?: WorkspaceConflictSavedObjectsClientWrapper;

private proxyWorkspaceTrafficToRealHandler(setupDeps: CoreSetup) {
/**
Expand Down Expand Up @@ -83,6 +88,13 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
}

this.proxyWorkspaceTrafficToRealHandler(core);
this.workspaceConflictControl = new WorkspaceConflictSavedObjectsClientWrapper();

core.savedObjects.addClientWrapper(
-1,
WORKSPACE_CONFLICT_CONTROL_SAVED_OBJECTS_CLIENT_WRAPPER_ID,
this.workspaceConflictControl.wrapperFactory
);

registerRoutes({
http: core.http,
Expand Down Expand Up @@ -110,6 +122,7 @@ export class WorkspacePlugin implements Plugin<{}, {}> {
this.logger.debug('Starting SavedObjects service');
this.permissionControl?.setup(core.savedObjects.getScopedClient);
this.client?.setSavedObjects(core.savedObjects);
this.workspaceConflictControl?.setSerializer(core.savedObjects.createSerializer());

return {
client: this.client as IWorkspaceDBImpl,
Expand Down
Loading
Loading