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

[MDS] Add Vega support for importing saved objects #6123

Merged
merged 31 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
efe5116
Add MDS support for Vega
huyaboo Feb 26, 2024
eb9c9ef
Refactor field to data_source_id
huyaboo Feb 27, 2024
ddba426
Add to CHANGELOG.md
huyaboo Feb 28, 2024
76aa282
Added test cases and renamed field to use data_source_name
huyaboo Mar 6, 2024
5658eb5
Add prefix datasource name test case and add example in default hjson
huyaboo Mar 7, 2024
3c4a03b
Merge branch 'main' into vega-mds-support
huyaboo Mar 7, 2024
ef3fd46
Move CHANGELOG to appropriate section
huyaboo Mar 7, 2024
be65384
Increased test coverage of search() method
huyaboo Mar 8, 2024
a5aa803
Merge branch 'main' into vega-mds-support
huyaboo Mar 8, 2024
fb8fe84
Merge branch 'main' into vega-mds-support
huyaboo Mar 8, 2024
f9747f3
Merge branch 'main' into vega-mds-support
huyaboo Mar 8, 2024
796551b
Add test cases for util function
huyaboo Mar 7, 2024
c22ce7c
Add util function to modify Vega Spec
huyaboo Mar 7, 2024
24a34a5
Add method to verify Vega saved object type
huyaboo Mar 8, 2024
d8b58e6
Add import saved object support for Vega
huyaboo Mar 11, 2024
d3f4c9d
Add unit tests for Vega objects in create and conflict modes
huyaboo Mar 12, 2024
9808989
Refactored utils test file
huyaboo Mar 12, 2024
89b8ccc
Merge branch 'main' into import-vega
huyaboo Mar 12, 2024
f93caca
Add to CHANGELOG
huyaboo Mar 12, 2024
bbfe01e
Merge branch 'main' into import-vega
huyaboo Mar 12, 2024
27fb804
Use bulkget instead of single get
huyaboo Mar 14, 2024
21e4273
Add datasource references to the specs
huyaboo Mar 16, 2024
612154c
Fix bootstrap errors
huyaboo Mar 16, 2024
437bae6
Merge branch 'main' into import-vega
huyaboo Mar 16, 2024
f1b1b51
Add edge case where title is potentially undefined
huyaboo Mar 16, 2024
c1e0406
Address PR comments
huyaboo Mar 18, 2024
85aa317
Add more test coverage for checking conflict
huyaboo Mar 19, 2024
e6a3449
Merge branch 'main' into import-vega
huyaboo Mar 19, 2024
6238bc5
Merge branch 'main' into import-vega
huyaboo Mar 19, 2024
c392de4
Fix unit test
huyaboo Mar 19, 2024
18f412c
Merge branch 'main' into import-vega
huyaboo Mar 19, 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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Workspace] Add workspace id in basePath ([#6060](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6060))
- Implement new home page ([#6065](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6065))
- Add sidecar service ([#5920](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5920))

- [Multiple Datasource] Add import support for Vega when specifying a datasource ([#6123](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6123))

### 🐛 Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { mockUuidv4 } from './__mocks__';
import { SavedObjectReference, SavedObjectsImportRetry } from 'opensearch-dashboards/public';
import { SavedObject } from '../types';
import { SavedObject, SavedObjectsClientContract } from '../types';
import { SavedObjectsErrorHelpers } from '..';
import {
checkConflictsForDataSource,
Expand All @@ -24,6 +24,45 @@ const createObject = (type: string, id: string): SavedObjectType => ({
references: (Symbol() as unknown) as SavedObjectReference[],
});

const createVegaVisualizationObject = (id: string): SavedObjectType => {
const visState =
id.split('_').length > 1
? '{"title":"some-title","type":"vega","aggs":[],"params":{"spec":"{\\n data: {\\n url: {\\n index: example_index\\n data_source_name: old-datasource-title\\n }\\n }\\n}"}}'
: '{"title":"some-title","type":"vega","aggs":[],"params":{"spec":"{\\n data: {\\n url: {\\n index: example_index\\n }\\n }\\n}"}}';
return {
type: 'visualization',
id,
attributes: { title: 'some-title', visState },
references:
id.split('_').length > 1
? [{ id: id.split('_')[0], type: 'data-source', name: 'dataSource' }]
: [],
} as SavedObjectType;
};

const getSavedObjectClient = (): SavedObjectsClientContract => {
const savedObject = {} as SavedObjectsClientContract;
savedObject.get = jest.fn().mockImplementation((type, id) => {
if (type === 'data-source' && id === 'old-datasource-id') {
return Promise.resolve({
attributes: {
title: 'old-datasource-title',
},
});
} else if (type === 'data-source') {
return Promise.resolve({
attributes: {
title: 'some-datasource-title',
},
});
}

return Promise.resolve(undefined);
});

return savedObject;
};

const getResultMock = {
conflict: (type: string, id: string) => {
const error = SavedObjectsErrorHelpers.createConflictError(type, id).output.payload;
Expand Down Expand Up @@ -56,6 +95,7 @@ describe('#checkConflictsForDataSource', () => {
retries?: SavedObjectsImportRetry[];
createNewCopies?: boolean;
dataSourceId?: string;
savedObjectsClient?: SavedObjectsClientContract;
}): ConflictsForDataSourceParams => {
return { ...partial };
};
Expand Down Expand Up @@ -140,4 +180,82 @@ describe('#checkConflictsForDataSource', () => {
importIdMap: new Map(),
});
});

/*
Vega test cases
*/
it('will attach datasource name to Vega spec when importing from local to datasource', async () => {
const vegaSavedObject = createVegaVisualizationObject('some-object-id');
const params = setupParams({
objects: [vegaSavedObject],
ignoreRegularConflicts: true,
dataSourceId: 'some-datasource-id',
savedObjectsClient: getSavedObjectClient(),
});
const checkConflictsForDataSourceResult = await checkConflictsForDataSource(params);

expect(checkConflictsForDataSourceResult).toEqual(
expect.objectContaining({
filteredObjects: [
{
...vegaSavedObject,
attributes: {
title: 'some-title',
visState:
'{"title":"some-title","type":"vega","aggs":[],"params":{"spec":"{\\n data: {\\n url: {\\n index: example_index\\n data_source_name: some-datasource-title\\n }\\n }\\n}"}}',
},
id: 'some-datasource-id_some-object-id',
references: [
{
id: 'some-datasource-id',
type: 'data-source',
name: 'dataSource',
},
],
},
],
errors: [],
importIdMap: new Map([
[
`visualization:some-object-id`,
{ id: 'some-datasource-id_some-object-id', omitOriginId: true },
],
]),
})
);
});
Copy link
Member

Choose a reason for hiding this comment

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

minor: for better coverage, maybe we want to test that savedobjectclient.get have been called? And also test called with expected attributes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, added a test to ensure get() was called.


it('will not change Vega spec when importing from datasource to different datasource', async () => {
const vegaSavedObject = createVegaVisualizationObject('old-datasource-id_some-object-id');
const params = setupParams({
objects: [vegaSavedObject],
ignoreRegularConflicts: true,
dataSourceId: 'some-datasource-id',
savedObjectsClient: getSavedObjectClient(),
});
const checkConflictsForDataSourceResult = await checkConflictsForDataSource(params);

expect(checkConflictsForDataSourceResult).toEqual(
expect.objectContaining({
filteredObjects: [
{
...vegaSavedObject,
attributes: {
title: 'some-title',
visState:
'{"title":"some-title","type":"vega","aggs":[],"params":{"spec":"{\\n data: {\\n url: {\\n index: example_index\\n data_source_name: old-datasource-title\\n }\\n }\\n}"}}',
},
id: 'some-datasource-id_some-object-id',
},
],
errors: [],
importIdMap: new Map([
[
`visualization:some-object-id`,
{ id: 'some-datasource-id_some-object-id', omitOriginId: true },
],
]),
})
);
});
Copy link
Member

Choose a reason for hiding this comment

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

same as above

Copy link
Member

Choose a reason for hiding this comment

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

also can we add test for duplicate and undefined data source title?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since this makes a get by id, I don't think we need to check for duplicate titles here since there is no ambiguity. The duplicate title check will be done when the visualization is saved and when the visualization is rendered.

});
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,24 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { SavedObject, SavedObjectsImportError, SavedObjectsImportRetry } from '../types';
import {
SavedObject,
SavedObjectsClientContract,
SavedObjectsImportError,
SavedObjectsImportRetry,
} from '../types';
import {
extractVegaSpecFromSavedObject,
getDataSourceTitleFromId,
updateDataSourceNameInVegaSpec,
} from './utils';

export interface ConflictsForDataSourceParams {
objects: Array<SavedObject<{ title?: string }>>;
ignoreRegularConflicts?: boolean;
retries?: SavedObjectsImportRetry[];
dataSourceId?: string;
savedObjectsClient?: SavedObjectsClientContract;
}

interface ImportIdMapEntry {
Expand All @@ -31,6 +42,7 @@ export async function checkConflictsForDataSource({
ignoreRegularConflicts,
retries = [],
dataSourceId,
savedObjectsClient,
}: ConflictsForDataSourceParams) {
const filteredObjects: Array<SavedObject<{ title?: string }>> = [];
const errors: SavedObjectsImportError[] = [];
Expand All @@ -43,6 +55,12 @@ export async function checkConflictsForDataSource({
(acc, cur) => acc.set(`${cur.type}:${cur.id}`, cur),
new Map<string, SavedObjectsImportRetry>()
);

const dataSourceTitle =
!!dataSourceId && !!savedObjectsClient
? await getDataSourceTitleFromId(dataSourceId, savedObjectsClient)
: undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

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

how do we handle undefined data source title here?


objects.forEach((object) => {
const {
type,
Expand Down Expand Up @@ -74,6 +92,33 @@ export async function checkConflictsForDataSource({
/**
* Only update importIdMap and filtered objects
*/

// Some visualization types will need special modifications, like Vega visualizations
if (object.type === 'visualization') {
const vegaSpec = extractVegaSpecFromSavedObject(object);

if (!!vegaSpec && !!dataSourceTitle) {
const updatedVegaSpec = updateDataSourceNameInVegaSpec({
spec: vegaSpec,
newDataSourceName: dataSourceTitle,
});

// @ts-expect-error
const visStateObject = JSON.parse(object.attributes?.visState);
visStateObject.params.spec = updatedVegaSpec;

// @ts-expect-error
object.attributes.visState = JSON.stringify(visStateObject);
if (!!dataSourceId) {
object.references.push({
id: dataSourceId,
name: 'dataSource',
type: 'data-source',
});
}
}
}

const omitOriginId = ignoreRegularConflicts;
importIdMap.set(`${type}:${id}`, { id: `${dataSourceId}_${rawId}`, omitOriginId });
filteredObjects.push({ ...object, id: `${dataSourceId}_${rawId}` });
Expand Down
116 changes: 115 additions & 1 deletion src/core/server/saved_objects/import/create_saved_objects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,37 @@ const visualizationObj = {
},
},
};

const getVegaVisualizationObj = (id: string) => ({
type: 'visualization',
id,
attributes: {
title: 'some-title',
visState:
'{"title":"some-title","type":"vega","aggs":[],"params":{"spec":"{\\n data: {\\n url: {\\n index: example_index\\n }\\n }\\n}"}}',
},
references: [],
namespaces: ['default'],
version: 'some-version',
updated_at: 'some-date',
});

const getVegaMDSVisualizationObj = (id: string, dataSourceId: string) => ({
type: 'visualization',
id: dataSourceId ? `${dataSourceId}_${id}` : id,
attributes: {
title: 'some-other-title',
visState:
'{"title":"some-other-title","type":"vega","aggs":[],"params":{"spec":"{\\n data: {\\n url: {\\n index: example_index\\n data_source_name: old-datasource-title\\n }\\n }\\n}"}}',
},
references: [
{
id: dataSourceId,
name: 'dataSource',
type: 'data-source',
},
],
});
// non-multi-namespace types shouldn't have origin IDs, but we include test cases to ensure it's handled gracefully
// non-multi-namespace types by definition cannot result in an unresolvable conflict, so we don't include test cases for those
const importId3 = 'id-foo';
Expand Down Expand Up @@ -142,8 +173,11 @@ describe('#createSavedObjects', () => {
overwrite?: boolean;
dataSourceId?: string;
dataSourceTitle?: string;
savedObjectsCustomClient?: jest.Mocked<SavedObjectsClientContract>;
}): CreateSavedObjectsParams => {
savedObjectsClient = savedObjectsClientMock.create();
savedObjectsClient = !!partial.savedObjectsCustomClient
? partial.savedObjectsCustomClient
: savedObjectsClientMock.create();
bulkCreate = savedObjectsClient.bulkCreate;
return { accumulatedErrors: [], ...partial, savedObjectsClient, importIdMap };
};
Expand Down Expand Up @@ -490,6 +524,29 @@ describe('#createSavedObjects', () => {
expect(results).toEqual(expectedResultsWithDataSource);
};

const testVegaVisualizationsWithDataSources = async (params: {
objects: SavedObject[];
expectedFilteredObjects: Array<Record<string, unknown>>;
dataSourceId?: string;
dataSourceTitle?: string;
}) => {
const savedObjectsCustomClient = savedObjectsClientMock.create();

const options = setupParams({
...params,
savedObjectsCustomClient,
});
savedObjectsCustomClient.bulkCreate = jest.fn().mockResolvedValue({
saved_objects: params.objects.map((obj) => {
return getResultMock.success(obj, options);
}),
});

const results = await createSavedObjects(options);

expect(results.createdObjects).toMatchObject(params.expectedFilteredObjects);
};

describe('with an undefined namespace', () => {
test('calls bulkCreate once with input objects', async () => {
await testBulkCreateObjects();
Expand Down Expand Up @@ -546,4 +603,61 @@ describe('#createSavedObjects', () => {
);
});
});

describe('with a data source for Vega saved objects', () => {
test('can attach a data source name to the Vega spec if there is a local query', async () => {
const objects = [getVegaVisualizationObj('some-vega-id')];
const expectedObject = getVegaVisualizationObj('some-vega-id');
const expectedFilteredObjects = [
{
...expectedObject,
attributes: {
title: 'some-title_dataSourceName',
visState:
'{"title":"some-title","type":"vega","aggs":[],"params":{"spec":"{\\n data: {\\n url: {\\n index: example_index\\n data_source_name: dataSourceName\\n }\\n }\\n}"}}',
},
id: 'some-vega-id',
references: [
{
id: 'some-datasource-id',
type: 'data-source',
name: 'dataSource',
},
],
},
];
await testVegaVisualizationsWithDataSources({
objects,
expectedFilteredObjects,
dataSourceId: 'some-datasource-id',
dataSourceTitle: 'dataSourceName',
});
});

test('will not update the data source name in the Vega spec if no local cluster queries', async () => {
const objects = [getVegaMDSVisualizationObj('some-vega-id', 'old-datasource-id')];
const expectedObject = getVegaMDSVisualizationObj('some-vega-id', 'old-datasource-id');
expectedObject.references.push({
id: 'some-datasource-id',
name: 'dataSource',
type: 'data-source',
});
const expectedFilteredObjects = [
{
...expectedObject,
attributes: {
title: 'some-other-title_dataSourceName',
visState:
'{"title":"some-other-title","type":"vega","aggs":[],"params":{"spec":"{\\n data: {\\n url: {\\n index: example_index\\n data_source_name: old-datasource-title\\n }\\n }\\n}"}}',
},
},
];
await testVegaVisualizationsWithDataSources({
objects,
expectedFilteredObjects,
dataSourceId: 'some-datasource-id',
dataSourceTitle: 'dataSourceName',
});
});
});
});
Loading
Loading