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

[Backport 2.x] [augmenter] do not support datasources with no version #8916

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions changelogs/fragments/8915.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Do not support data sources with no version for vis augmenter ([#8915](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8915))
2 changes: 2 additions & 0 deletions src/plugins/vis_augmenter/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
setUiActions,
setEmbeddable,
setQueryService,
setIndexPatterns,
setVisualizations,
setCore,
} from './services';
Expand Down Expand Up @@ -62,6 +63,7 @@
setUiActions(uiActions);
setEmbeddable(embeddable);
setQueryService(data.query);
setIndexPatterns(data.indexPatterns);

Check warning on line 66 in src/plugins/vis_augmenter/public/plugin.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/vis_augmenter/public/plugin.ts#L66

Added line #L66 was not covered by tests
setVisualizations(visualizations);
setCore(core);
setFlyoutState(VIEW_EVENTS_FLYOUT_STATE.CLOSED);
Expand Down
6 changes: 5 additions & 1 deletion src/plugins/vis_augmenter/public/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { IUiSettingsClient } from '../../../core/public';
import { SavedObjectLoaderAugmentVis } from './saved_augment_vis';
import { EmbeddableStart } from '../../embeddable/public';
import { UiActionsStart } from '../../ui_actions/public';
import { DataPublicPluginStart } from '../../../plugins/data/public';
import { DataPublicPluginStart, IndexPatternsContract } from '../../../plugins/data/public';
import { VisualizationsStart } from '../../visualizations/public';
import { CoreStart } from '../../../core/public';

Expand All @@ -26,6 +26,10 @@ export const [getQueryService, setQueryService] = createGetterSetter<
DataPublicPluginStart['query']
>('Query');

export const [getIndexPatterns, setIndexPatterns] = createGetterSetter<IndexPatternsContract>(
'IndexPatterns'
);

export const [getVisualizations, setVisualizations] = createGetterSetter<VisualizationsStart>(
'visualizations'
);
Expand Down
129 changes: 117 additions & 12 deletions src/plugins/vis_augmenter/public/utils/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ import {
PluginResource,
VisLayerErrorTypes,
SavedObjectLoaderAugmentVis,
isEligibleForDataSource,
} from '../';
import { PLUGIN_AUGMENTATION_ENABLE_SETTING } from '../../common/constants';
import { AggConfigs } from '../../../data/common';
import { uiSettingsServiceMock } from '../../../../core/public/mocks';
import { setUISettings } from '../services';
import { setIndexPatterns, setUISettings } from '../services';
import {
STUB_INDEX_PATTERN_WITH_FIELDS,
TYPES_REGISTRY,
Expand All @@ -35,6 +36,7 @@ import {
createPointInTimeEventsVisLayer,
createVisLayer,
} from '../mocks';
import { dataPluginMock } from 'src/plugins/data/public/mocks';

describe('utils', () => {
const uiSettingsMock = uiSettingsServiceMock.createStartContract();
Expand All @@ -60,7 +62,7 @@ describe('utils', () => {
aggs: VALID_AGGS,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
expect(await isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with no date_histogram', async () => {
const invalidConfigStates = [
Expand All @@ -87,7 +89,7 @@ describe('utils', () => {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
expect(await isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with invalid aggs counts', async () => {
const invalidConfigStates = [
Expand All @@ -111,7 +113,7 @@ describe('utils', () => {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
expect(await isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with no metric aggs', async () => {
const invalidConfigStates = [
Expand All @@ -133,7 +135,7 @@ describe('utils', () => {
invalidAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
expect(await isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with series param is not line type', async () => {
const vis = ({
Expand All @@ -154,7 +156,7 @@ describe('utils', () => {
aggs: VALID_AGGS,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
expect(await isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with series param not all being line type', async () => {
const vis = ({
Expand All @@ -178,7 +180,7 @@ describe('utils', () => {
aggs: VALID_AGGS,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(vis)).toEqual(false);
expect(await isEligibleForVisLayers(vis)).toEqual(false);
});
it('vis is ineligible with invalid x-axis due to no segment aggregation', async () => {
const badConfigStates = [
Expand Down Expand Up @@ -216,7 +218,7 @@ describe('utils', () => {
badAggs,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(invalidVis)).toEqual(false);
expect(await isEligibleForVisLayers(invalidVis)).toEqual(false);
});
it('vis is ineligible with xaxis not on bottom', async () => {
const invalidVis = ({
Expand All @@ -237,7 +239,7 @@ describe('utils', () => {
aggs: VALID_AGGS,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(invalidVis)).toEqual(false);
expect(await isEligibleForVisLayers(invalidVis)).toEqual(false);
});
it('vis is ineligible with no seriesParams', async () => {
const invalidVis = ({
Expand All @@ -253,16 +255,16 @@ describe('utils', () => {
aggs: VALID_AGGS,
},
} as unknown) as Vis;
expect(isEligibleForVisLayers(invalidVis)).toEqual(false);
expect(await isEligibleForVisLayers(invalidVis)).toEqual(false);
});
it('vis is ineligible with valid type and disabled setting', async () => {
uiSettingsMock.get.mockImplementation((key: string) => {
return key !== PLUGIN_AUGMENTATION_ENABLE_SETTING;
});
expect(isEligibleForVisLayers(VALID_VIS)).toEqual(false);
expect(await isEligibleForVisLayers(VALID_VIS)).toEqual(false);
});
it('vis is eligible with valid type', async () => {
expect(isEligibleForVisLayers(VALID_VIS)).toEqual(true);
expect(await isEligibleForVisLayers(VALID_VIS)).toEqual(true);
});
});

Expand Down Expand Up @@ -660,4 +662,107 @@ describe('utils', () => {
expect(mockDeleteFn).toHaveBeenCalledTimes(1);
});
});

describe('isEligibleForDataSource', () => {
beforeEach(() => {
jest.clearAllMocks();
});
it('returns true if the Vis indexPattern does not have a dataSourceRef', async () => {
const indexPatternsMock = dataPluginMock.createStartContract().indexPatterns;
indexPatternsMock.getDataSource = jest.fn().mockReturnValue(undefined);
setIndexPatterns(indexPatternsMock);
const vis = {
data: {
indexPattern: {
id: '123',
},
},
} as Vis;
expect(await isEligibleForDataSource(vis)).toEqual(true);
});
it('returns true if the Vis indexPattern has a dataSourceRef with a compatible version', async () => {
const indexPatternsMock = dataPluginMock.createStartContract().indexPatterns;
indexPatternsMock.getDataSource = jest.fn().mockReturnValue({
id: '456',
attributes: {
dataSourceVersion: '1.2.3',
},
});
setIndexPatterns(indexPatternsMock);
const vis = {
data: {
indexPattern: {
id: '123',
dataSourceRef: {
id: '456',
},
},
},
} as Vis;
expect(await isEligibleForDataSource(vis)).toEqual(true);
});
it('returns false if the Vis indexPattern has a dataSourceRef with an incompatible version', async () => {
const indexPatternsMock = dataPluginMock.createStartContract().indexPatterns;
indexPatternsMock.getDataSource = jest.fn().mockReturnValue({
id: '456',
attributes: {
dataSourceVersion: '.0',
},
});
setIndexPatterns(indexPatternsMock);
const vis = {
data: {
indexPattern: {
id: '123',
dataSourceRef: {
id: '456',
},
},
},
} as Vis;
expect(await isEligibleForDataSource(vis)).toEqual(false);
});
it('returns false if the Vis indexPattern has a dataSourceRef with an undefined version', async () => {
const indexPatternsMock = dataPluginMock.createStartContract().indexPatterns;
indexPatternsMock.getDataSource = jest.fn().mockReturnValue({
id: '456',
attributes: {
dataSourceVersion: undefined,
},
});
setIndexPatterns(indexPatternsMock);
const vis = {
data: {
indexPattern: {
id: '123',
dataSourceRef: {
id: '456',
},
},
},
} as Vis;
expect(await isEligibleForDataSource(vis)).toEqual(false);
});
it('returns false if the Vis indexPattern has a dataSourceRef with an empty string version', async () => {
const indexPatternsMock = dataPluginMock.createStartContract().indexPatterns;
indexPatternsMock.getDataSource = jest.fn().mockReturnValue({
id: '456',
attributes: {
dataSourceVersion: '',
},
});
setIndexPatterns(indexPatternsMock);
const vis = {
data: {
indexPattern: {
id: '123',
dataSourceRef: {
id: '456',
},
},
},
} as Vis;
expect(await isEligibleForDataSource(vis)).toEqual(false);
});
});
});
26 changes: 23 additions & 3 deletions src/plugins/vis_augmenter/public/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import { get, isEmpty } from 'lodash';
import semver from 'semver';
import { Vis } from '../../../../plugins/visualizations/public';
import {
formatExpression,
Expand All @@ -20,10 +21,13 @@ import {
VisLayerErrorTypes,
} from '../';
import { PLUGIN_AUGMENTATION_ENABLE_SETTING } from '../../common/constants';
import { getUISettings } from '../services';
import { getUISettings, getIndexPatterns } from '../services';
import { IUiSettingsClient } from '../../../../core/public';

export const isEligibleForVisLayers = (vis: Vis, uiSettingsClient?: IUiSettingsClient): boolean => {
export const isEligibleForVisLayers = async (
vis: Vis,
uiSettingsClient?: IUiSettingsClient
): Promise<boolean> => {
// Only support a date histogram
const dateHistograms = vis.data?.aggs?.byTypeName?.('date_histogram');
if (!Array.isArray(dateHistograms) || dateHistograms.length !== 1) return false;
Expand Down Expand Up @@ -53,6 +57,9 @@ export const isEligibleForVisLayers = (vis: Vis, uiSettingsClient?: IUiSettingsC
)
return false;

// Check if the vis datasource is eligible for the augmentation
if (!(await isEligibleForDataSource(vis))) return false;

// Checks if the augmentation setting is enabled
const config = uiSettingsClient ?? getUISettings();
return config.get(PLUGIN_AUGMENTATION_ENABLE_SETTING);
Expand Down Expand Up @@ -163,7 +170,6 @@ export const getAnyErrors = (visLayers: VisLayer[], visTitle: string): Error | u
* @param visLayers the produced VisLayers containing details if the resource has been deleted
* @param visualizationsLoader the visualizations saved object loader to handle deletion
*/

export const cleanupStaleObjects = (
augmentVisSavedObjs: ISavedAugmentVis[],
visLayers: VisLayer[],
Expand All @@ -187,3 +193,17 @@ export const cleanupStaleObjects = (
loader?.delete(objIdsToDelete);
}
};

/**
* Returns true if the Vis is eligible to be used with the DataSource feature.
* @param vis - The Vis to check
* @returns true if the Vis is eligible for the DataSource feature, false otherwise
*/
export const isEligibleForDataSource = async (vis: Vis) => {
const dataSourceRef = vis.data.indexPattern?.dataSourceRef;
if (!dataSourceRef) return true;
const dataSource = await getIndexPatterns().getDataSource(dataSourceRef.id);
if (!dataSource || !dataSource.attributes) return false;
const version = semver.coerce(dataSource.attributes.dataSourceVersion);
return version ? semver.satisfies(version, '>=1.0.0') : false;
};
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class ViewEventsOptionAction implements Action<EmbeddableContext> {
const vis = (embeddable as VisualizeEmbeddable).vis;
return (
vis !== undefined &&
isEligibleForVisLayers(vis) &&
(await isEligibleForVisLayers(vis)) &&
!isEmpty((embeddable as VisualizeEmbeddable).visLayers)
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_type_vislib/public/line_to_expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const toExpressionAst = async (vis: Vis, params: any) => {
if (
params.visLayers == null ||
Object.keys(params.visLayers).length === 0 ||
!isEligibleForVisLayers(vis)
!(await isEligibleForVisLayers(vis))
) {
// Render using vislib instead of vega-lite
const visConfig = { ...vis.params, dimensions };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ export class VisualizeEmbeddable
this.visAugmenterConfig?.visLayerResourceIds
);

if (!isEmpty(augmentVisSavedObjs) && !aborted && isEligibleForVisLayers(this.vis)) {
if (!isEmpty(augmentVisSavedObjs) && !aborted && (await isEligibleForVisLayers(this.vis))) {
const visLayersPipeline = buildPipelineFromAugmentVisSavedObjs(augmentVisSavedObjs);
// The initial input for the pipeline will just be an empty arr of VisLayers. As plugin
// expression functions are ran, they will incrementally append their generated VisLayers to it.
Expand Down
Loading