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

[Workspace]Optional workspaces params in repository #5162

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
53d84f2
[Workspace] Add workspaces parameters to all saved objects API
gaobinlong Sep 20, 2023
c841390
feat: update snapshot
SuZhou-Joe Sep 22, 2023
fb43df1
feat: optimize logic when checkConflict and bulkCreate (#189)
SuZhou-Joe Sep 23, 2023
7e18485
feat: call get when create with override
SuZhou-Joe Sep 23, 2023
aa69695
feat: update test according to count
SuZhou-Joe Sep 24, 2023
674bd09
feat: add integration test
SuZhou-Joe Sep 25, 2023
2fb66b7
fix: unit test
SuZhou-Joe Sep 25, 2023
952f13e
feat: regenerate ids when import
SuZhou-Joe Sep 25, 2023
23481a1
feat: add more unit test
SuZhou-Joe Sep 26, 2023
e740165
feat: minor changes logic on repository
SuZhou-Joe Sep 26, 2023
1997c73
feat: update unit test
SuZhou-Joe Sep 26, 2023
cae196e
feat: update test
SuZhou-Joe Sep 26, 2023
fd24685
feat: optimization according to comments
SuZhou-Joe Sep 28, 2023
ab92370
feat: update test
SuZhou-Joe Sep 28, 2023
c0cfd17
feat: optimize code
SuZhou-Joe Sep 28, 2023
f4b86f0
feat: add changelog
SuZhou-Joe Sep 30, 2023
8002f1c
Merge branch 'main' into feature/optional-workspaces-params-in-reposi…
joshuarrrr Oct 3, 2023
48f7ccf
Merge branch 'main' into feature/optional-workspaces-params-in-reposi…
SuZhou-Joe Oct 4, 2023
eb6d98f
feat: modify CHANGELOG
SuZhou-Joe Oct 4, 2023
2bf837a
feat: increase unit test coverage
SuZhou-Joe Oct 12, 2023
dbb1248
feat: add comment
SuZhou-Joe Oct 13, 2023
005d694
Merge branch 'main' into feature/optional-workspaces-params-in-reposi…
SuZhou-Joe Oct 17, 2023
0ba8df5
feat: remove useless generateId method
SuZhou-Joe Oct 17, 2023
743bf33
feat: remove flaky test
SuZhou-Joe Oct 17, 2023
285adad
feat: remove useless code
SuZhou-Joe Oct 17, 2023
142e9c0
fix: unit test
SuZhou-Joe Oct 18, 2023
e8aa3a4
feat: update snapshot
SuZhou-Joe Oct 18, 2023
3288961
feat: increase code coverage
SuZhou-Joe Oct 18, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854))
- [Discover] Update embeddable for saved searches ([#5081](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5081))
- [Workspace] Add core workspace service module to enable the implementation of workspace features within OSD plugins ([#5092](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5092))
- [Workspace] Optional workspaces params in repository ([#5162](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5162))

### 🐛 Bug Fixes

Expand Down
1 change: 1 addition & 0 deletions src/core/public/saved_objects/saved_objects_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ export class SavedObjectsClient {
filter: 'filter',
namespaces: 'namespaces',
preference: 'preference',
workspaces: 'workspaces',
};

const renamedQuery = renameKeys<SavedObjectsFindOptions, any>(renameMap, options);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -857,4 +857,17 @@ describe('getSortedObjectsForExport()', () => {
`Can't specify both "search" and "objects" properties when exporting`
);
});

test('rejects when both types and objecys are passed in', () => {
const exportOpts = {
exportSizeLimit: 1,
savedObjectsClient,
objects: [{ type: 'index-pattern', id: '1' }],
types: ['foo'],
};

expect(exportSavedObjectsToStream(exportOpts)).rejects.toThrowErrorMatchingInlineSnapshot(
`Can't specify both "types" and "objects" properties when exporting`
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export interface SavedObjectsExportOptions {
excludeExportDetails?: boolean;
/** optional namespace to override the namespace used by the savedObjectsClient. */
namespace?: string;
/** optional workspaces to override the workspaces used by the savedObjectsClient. */
workspaces?: string[];
}

/**
Expand Down Expand Up @@ -87,13 +89,15 @@ async function fetchObjectsToExport({
exportSizeLimit,
savedObjectsClient,
namespace,
workspaces,
}: {
objects?: SavedObjectsExportOptions['objects'];
types?: string[];
search?: string;
exportSizeLimit: number;
savedObjectsClient: SavedObjectsClientContract;
namespace?: string;
workspaces?: string[];
}) {
if ((types?.length ?? 0) > 0 && (objects?.length ?? 0) > 0) {
throw Boom.badRequest(`Can't specify both "types" and "objects" properties when exporting`);
Expand All @@ -105,7 +109,9 @@ async function fetchObjectsToExport({
if (typeof search === 'string') {
throw Boom.badRequest(`Can't specify both "search" and "objects" properties when exporting`);
}
const bulkGetResult = await savedObjectsClient.bulkGet(objects, { namespace });
const bulkGetResult = await savedObjectsClient.bulkGet(objects, {
namespace,
});
const erroredObjects = bulkGetResult.saved_objects.filter((obj) => !!obj.error);
if (erroredObjects.length) {
const err = Boom.badRequest();
Expand All @@ -121,6 +127,7 @@ async function fetchObjectsToExport({
search,
perPage: exportSizeLimit,
namespaces: namespace ? [namespace] : undefined,
...(workspaces ? { workspaces } : {}),
Copy link
Member

Choose a reason for hiding this comment

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

are we passing an object instead of an array?

Copy link
Member Author

@SuZhou-Joe SuZhou-Joe Oct 17, 2023

Choose a reason for hiding this comment

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

Workspaces is an array, in here if workspaces is null, ...({}) will have no impact on the params that passed to find, if we use workspaces: workspaces ? workspaces : undefined, there will be an extra { ...., workspaces: undefined } in find params.

});
if (findResponse.total > exportSizeLimit) {
throw Boom.badRequest(`Can't export more than ${exportSizeLimit} objects`);
Expand Down Expand Up @@ -153,6 +160,7 @@ export async function exportSavedObjectsToStream({
includeReferencesDeep = false,
excludeExportDetails = false,
namespace,
workspaces,
}: SavedObjectsExportOptions) {
const rootObjects = await fetchObjectsToExport({
types,
Expand All @@ -161,6 +169,7 @@ export async function exportSavedObjectsToStream({
savedObjectsClient,
exportSizeLimit,
namespace,
workspaces,
});
let exportedObjects: Array<SavedObject<unknown>> = [];
let missingReferences: SavedObjectsExportResultDetails['missingReferences'] = [];
Expand Down
3 changes: 3 additions & 0 deletions src/core/server/saved_objects/import/check_conflicts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ interface CheckConflictsParams {
ignoreRegularConflicts?: boolean;
retries?: SavedObjectsImportRetry[];
createNewCopies?: boolean;
workspaces?: string[];
}

const isUnresolvableConflict = (error: SavedObjectError) =>
Expand All @@ -56,6 +57,7 @@ export async function checkConflicts({
ignoreRegularConflicts,
retries = [],
createNewCopies,
workspaces,
}: CheckConflictsParams) {
const filteredObjects: Array<SavedObject<{ title?: string }>> = [];
const errors: SavedObjectsImportError[] = [];
Expand All @@ -77,6 +79,7 @@ export async function checkConflicts({
});
const checkConflictsResult = await savedObjectsClient.checkConflicts(objectsToCheck, {
namespace,
workspaces,
});
const errorMap = checkConflictsResult.errors.reduce(
(acc, { type, id, error }) => acc.set(`${type}:${id}`, error),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ interface CreateSavedObjectsParams<T> {
importIdMap: Map<string, { id?: string; omitOriginId?: boolean }>;
namespace?: string;
overwrite?: boolean;
workspaces?: string[];
}
interface CreateSavedObjectsResult<T> {
createdObjects: Array<CreatedObject<T>>;
Expand All @@ -56,6 +57,7 @@ export const createSavedObjects = async <T>({
importIdMap,
namespace,
overwrite,
workspaces,
}: CreateSavedObjectsParams<T>): Promise<CreateSavedObjectsResult<T>> => {
// filter out any objects that resulted in errors
const errorSet = accumulatedErrors.reduce(
Expand Down Expand Up @@ -103,6 +105,7 @@ export const createSavedObjects = async <T>({
const bulkCreateResponse = await savedObjectsClient.bulkCreate(objectsToCreate, {
namespace,
overwrite,
workspaces,
});
expectedResults = bulkCreateResponse.saved_objects;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ describe('#importSavedObjectsFromStream', () => {
expect(validateReferences).toHaveBeenCalledWith(
collectedObjects,
savedObjectsClient,
namespace
namespace,
undefined,
undefined
);
});

Expand Down Expand Up @@ -268,6 +270,52 @@ describe('#importSavedObjectsFromStream', () => {
};
expect(createSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams);
});

test('creates saved objects when workspaces param is provided', async () => {
const options = setupOptions();
options.workspaces = ['foo'];
const collectedObjects = [createObject()];
const filteredObjects = [createObject()];
const errors = [createError(), createError(), createError(), createError()];
getMockFn(collectSavedObjects).mockResolvedValue({
errors: [errors[0]],
collectedObjects,
importIdMap: new Map([
['foo', {}],
['bar', {}],
['baz', {}],
]),
});
getMockFn(validateReferences).mockResolvedValue([errors[1]]);
getMockFn(checkConflicts).mockResolvedValue({
errors: [errors[2]],
filteredObjects,
importIdMap: new Map([['bar', { id: 'newId1' }]]),
pendingOverwrites: new Set(),
});
getMockFn(checkOriginConflicts).mockResolvedValue({
errors: [errors[3]],
importIdMap: new Map([['baz', { id: 'newId2' }]]),
pendingOverwrites: new Set(),
});

await importSavedObjectsFromStream(options);
const importIdMap = new Map([
['foo', {}],
['bar', { id: 'newId1' }],
['baz', { id: 'newId2' }],
]);
const createSavedObjectsParams = {
objects: collectedObjects,
accumulatedErrors: errors,
savedObjectsClient,
importIdMap,
overwrite,
namespace,
workspaces: options.workspaces,
};
expect(createSavedObjects).toHaveBeenCalledWith(createSavedObjectsParams);
});
});

describe('with createNewCopies enabled', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export async function importSavedObjectsFromStream({
savedObjectsClient,
typeRegistry,
namespace,
workspaces,
}: SavedObjectsImportOptions): Promise<SavedObjectsImportResponse> {
let errorAccumulator: SavedObjectsImportError[] = [];
const supportedTypes = typeRegistry.getImportableAndExportableTypes().map((type) => type.name);
Expand All @@ -73,19 +74,23 @@ export async function importSavedObjectsFromStream({
const validateReferencesResult = await validateReferences(
collectSavedObjectsResult.collectedObjects,
savedObjectsClient,
namespace
namespace,
undefined,
workspaces
);
errorAccumulator = [...errorAccumulator, ...validateReferencesResult];

if (createNewCopies) {
importIdMap = regenerateIds(collectSavedObjectsResult.collectedObjects);
} else {
// Check single-namespace objects for conflicts in this namespace, and check multi-namespace objects for conflicts across all namespaces
// Check for conflicts across all workspaces
const checkConflictsParams = {
objects: collectSavedObjectsResult.collectedObjects,
savedObjectsClient,
namespace,
ignoreRegularConflicts: overwrite,
workspaces,
};
const checkConflictsResult = await checkConflicts(checkConflictsParams);
errorAccumulator = [...errorAccumulator, ...checkConflictsResult.errors];
Expand Down Expand Up @@ -118,6 +123,7 @@ export async function importSavedObjectsFromStream({
importIdMap,
overwrite,
namespace,
...(workspaces ? { workspaces } : {}),
};
const createSavedObjectsResult = await createSavedObjects(createSavedObjectsParams);
errorAccumulator = [...errorAccumulator, ...createSavedObjectsResult.errors];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export async function resolveSavedObjectsImportErrors({
typeRegistry,
namespace,
createNewCopies,
workspaces,
}: SavedObjectsResolveImportErrorsOptions): Promise<SavedObjectsImportResponse> {
// throw a BadRequest error if we see invalid retries
validateRetries(retries);
Expand Down Expand Up @@ -157,6 +158,7 @@ export async function resolveSavedObjectsImportErrors({
importIdMap,
namespace,
overwrite,
workspaces,
};
const { createdObjects, errors: bulkCreateErrors } = await createSavedObjects(
createSavedObjectsParams
Expand Down
4 changes: 4 additions & 0 deletions src/core/server/saved_objects/import/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ export interface SavedObjectsImportOptions {
namespace?: string;
/** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */
createNewCopies: boolean;
/** if specified, will import in given workspaces, else will import as global object */
workspaces?: string[];
}

/**
Expand All @@ -208,6 +210,8 @@ export interface SavedObjectsResolveImportErrorsOptions {
namespace?: string;
/** If true, will create new copies of import objects, each with a random `id` and undefined `originId`. */
createNewCopies: boolean;
/** if specified, will import in given workspaces, else will import as global object */
workspaces?: string[];
}

export type CreatedObject<T> = SavedObject<T> & { destinationId?: string };
Original file line number Diff line number Diff line change
Expand Up @@ -601,4 +601,96 @@ describe('validateReferences()', () => {
validateReferences(savedObjects, savedObjectsClient)
).rejects.toThrowErrorMatchingInlineSnapshot(`Bad Request`);
});

test('returns errors when references have conflict on workspaces', async () => {
savedObjectsClient.bulkGet.mockResolvedValue({
saved_objects: [
{
type: 'index-pattern',
id: '3',
attributes: {},
references: [],
workspaces: ['foo'],
},
{
type: 'index-pattern',
id: '5',
attributes: {},
references: [],
workspaces: ['bar'],
},
],
});
const savedObjects = [
{
id: '5',
type: 'index-pattern',
attributes: {
title: 'My Visualization 2',
},
references: [
{
name: 'ref_0',
type: 'index-pattern',
id: '3',
},
],
},
];
const result = await validateReferences(
savedObjects,
savedObjectsClient,
undefined,
undefined,
['bar']
);
expect(result).toMatchInlineSnapshot(`
Array [
Object {
error: Object {
references: Array [
Object {
id: 3,
type: index-pattern,
},
],
type: missing_references,
},
id: 5,
meta: Object {
title: My Visualization 2,
},
title: My Visualization 2,
type: index-pattern,
},
]
`);
expect(savedObjectsClient.bulkGet).toMatchInlineSnapshot(`
[MockFunction] {
"calls": Array [
Array [
Array [
Object {
fields: Array [
id,
workspaces,
],
id: 3,
type: index-pattern,
},
],
Object {
namespace: undefined,
},
],
],
"results": Array [
Object {
type: return,
value: Promise {},
},
],
}
`);
});
});
Loading
Loading