Skip to content

Commit

Permalink
[augmenter] do not support datasources with no version (opensearch-pr…
Browse files Browse the repository at this point in the history
…oject#8915)

Signed-off-by: Kawika Avilla <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
  • Loading branch information
kavilla and opensearch-changeset-bot[bot] authored Nov 22, 2024
1 parent f110637 commit 539675e
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 19 deletions.
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 @@ import {
setUiActions,
setEmbeddable,
setQueryService,
setIndexPatterns,
setVisualizations,
setCore,
} from './services';
Expand Down Expand Up @@ -62,6 +63,7 @@ export class VisAugmenterPlugin
setUiActions(uiActions);
setEmbeddable(embeddable);
setQueryService(data.query);
setIndexPatterns(data.indexPatterns);
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

0 comments on commit 539675e

Please sign in to comment.