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 1.x] Add option to configure available bucket agg types + jest tests (#1196) #1309

Merged
Merged
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
27 changes: 26 additions & 1 deletion src/plugins/data/common/search/aggs/agg_types_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,20 @@ describe('AggTypesRegistry', () => {
let registry: AggTypesRegistry;
let setup: AggTypesRegistrySetup;
let start: ReturnType<AggTypesRegistry['start']>;
const uiSettings: any = {
get: jest
.fn()
.mockImplementation((key) => key === 'visualize:disableBucketAgg' && ['significant_terms']),
};

beforeEach(() => {
registry = new AggTypesRegistry();
setup = registry.setup();
start = registry.start();
});

it('registerBucket adds new buckets', () => {
setup.registerBucket('terms', bucketType);
start = registry.start({ uiSettings });
expect(start.getAll().buckets).toEqual([bucketType]);
});

Expand All @@ -69,6 +74,7 @@ describe('AggTypesRegistry', () => {

it('registerMetric adds new metrics', () => {
setup.registerMetric('count', metricType);
start = registry.start({ uiSettings });
expect(start.getAll().metrics).toEqual([metricType]);
});

Expand All @@ -89,16 +95,35 @@ describe('AggTypesRegistry', () => {
it('gets either buckets or metrics by id', () => {
setup.registerBucket('terms', bucketType);
setup.registerMetric('count', metricType);
start = registry.start({ uiSettings });
expect(start.get('terms')).toEqual(bucketType);
expect(start.get('count')).toEqual(metricType);
});

it('getAll returns all buckets and metrics', () => {
setup.registerBucket('terms', bucketType);
setup.registerMetric('count', metricType);
start = registry.start({ uiSettings });
expect(start.getAll()).toEqual({
buckets: [bucketType],
metrics: [metricType],
});
});

it('getAll returns all buckets besides disabled bucket type', () => {
const termsBucket = () => ({ name: 'terms', type: 'buckets' } as BucketAggType<any>);
const histogramBucket = () => ({ name: 'histogram', type: 'buckets' } as BucketAggType<any>);
const significantTermsBucket = () =>
({ name: 'significant_terms', type: 'buckets' } as BucketAggType<any>);
setup.registerBucket('terms', termsBucket);
setup.registerBucket('histogram', histogramBucket);
setup.registerBucket('significant_terms', significantTermsBucket);
start = registry.start({ uiSettings });

expect(start.getAll()).toEqual({
buckets: [termsBucket, histogramBucket],
metrics: [],
});
expect(start.get('significant_terms')).toEqual(undefined);
});
});
17 changes: 16 additions & 1 deletion src/plugins/data/common/search/aggs/agg_types_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
* GitHub history for details.
*/

import { IUiSettingsClient as IUiSettingsClientPublic } from 'src/core/public';
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { IUiSettingsClient as IUiSettingsClientServer } from 'src/core/server';
import { BucketAggType } from './buckets/bucket_agg_type';
import { MetricAggType } from './metrics/metric_agg_type';
import { AggTypesDependencies } from './agg_types';
Expand All @@ -48,6 +51,10 @@ export interface AggTypesRegistryStart {
getAll: () => { buckets: Array<BucketAggType<any>>; metrics: Array<MetricAggType<any>> };
}

export interface AggTypesRegistryStartDependencies {
uiSettings: IUiSettingsClientPublic | IUiSettingsClientServer;
}

export class AggTypesRegistry {
private readonly bucketAggs = new Map();
private readonly metricAggs = new Map();
Expand Down Expand Up @@ -81,7 +88,15 @@ export class AggTypesRegistry {
};
};

start = () => {
start = ({ uiSettings }: AggTypesRegistryStartDependencies) => {
const disabledBucketAgg = uiSettings.get('visualize:disableBucketAgg');

if (disabledBucketAgg !== undefined && Array.isArray(disabledBucketAgg)) {
for (const k of this.bucketAggs.keys()) {
if (disabledBucketAgg.includes(k)) this.bucketAggs.delete(k);
}
}

return {
get: (name: string) => {
return this.bucketAggs.get(name) || this.metricAggs.get(name);
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/common/search/aggs/aggs_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ describe('Aggs service', () => {
};
startDeps = {
getConfig: jest.fn(),
uiSettings: { get: jest.fn().mockImplementation(() => []) } as any,
};
});

Expand Down
8 changes: 6 additions & 2 deletions src/plugins/data/common/search/aggs/aggs_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
*/

import { ExpressionsServiceSetup } from 'src/plugins/expressions/common';
import { IUiSettingsClient as IUiSettingsClientPublic } from 'src/core/public';
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { IUiSettingsClient as IUiSettingsClientServer } from 'src/core/server';
import { UI_SETTINGS } from '../../../common';
import { GetConfigFn } from '../../types';
import {
Expand Down Expand Up @@ -63,6 +66,7 @@ export interface AggsCommonSetupDependencies {
/** @internal */
export interface AggsCommonStartDependencies {
getConfig: GetConfigFn;
uiSettings: IUiSettingsClientPublic | IUiSettingsClientServer;
}

/**
Expand Down Expand Up @@ -90,8 +94,8 @@ export class AggsCommonService {
};
}

public start({ getConfig }: AggsCommonStartDependencies): AggsCommonStart {
const aggTypesStart = this.aggTypesRegistry.start();
public start({ getConfig, uiSettings }: AggsCommonStartDependencies): AggsCommonStart {
const aggTypesStart = this.aggTypesRegistry.start({ uiSettings });

return {
calculateAutoTimeExpression: getCalculateAutoTimeExpression(getConfig),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ export function mockAggTypesRegistry(deps?: AggTypesDependencies): AggTypesRegis
aggTypes.buckets.forEach(({ name, fn }) => registrySetup.registerBucket(name, fn));
aggTypes.metrics.forEach(({ name, fn }) => registrySetup.registerMetric(name, fn));

const registryStart = registry.start();
const registryStart = registry.start({
uiSettings: { get: jest.fn().mockImplementation(() => []) } as any,
});

// initialize each agg type and store in memory
registryStart.getAll().buckets.forEach((type) => {
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/public/search/aggs/aggs_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export class AggsService {

public start({ fieldFormats, uiSettings }: AggsStartDependencies): AggsStart {
const { calculateAutoTimeExpression, types } = this.aggsCommonService.start({
uiSettings,
getConfig: this.getConfig!,
});

Expand Down
5 changes: 4 additions & 1 deletion src/plugins/data/server/search/aggs/aggs_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ export class AggsService {
return uiSettingsCache[key];
};

const { calculateAutoTimeExpression, types } = this.aggsCommonService.start({ getConfig });
const { calculateAutoTimeExpression, types } = this.aggsCommonService.start({
getConfig,
uiSettings: uiSettingsClient,
});

const aggTypesDependencies: AggTypesDependencies = {
calculateBounds: this.calculateBounds,
Expand Down
1 change: 1 addition & 0 deletions src/plugins/visualizations/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@
*/

export const VISUALIZE_ENABLE_LABS_SETTING = 'visualize:enableLabs';
export const VISUALIZE_DISABLE_BUCKET_AGG_SETTING = 'visualize:disableBucketAgg';
5 changes: 4 additions & 1 deletion src/plugins/visualizations/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@
import { PluginInitializerContext } from '../../../core/server';
import { VisualizationsPlugin } from './plugin';

export { VISUALIZE_ENABLE_LABS_SETTING } from '../common/constants';
export {
VISUALIZE_ENABLE_LABS_SETTING,
VISUALIZE_DISABLE_BUCKET_AGG_SETTING,
} from '../common/constants';

// This exports static code and TypeScript types,
// as well as, OpenSearch Dashboards Platform `plugin()` initializer.
Expand Down
17 changes: 16 additions & 1 deletion src/plugins/visualizations/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ import {
Logger,
} from '../../../core/server';

import { VISUALIZE_ENABLE_LABS_SETTING } from '../common/constants';
import {
VISUALIZE_ENABLE_LABS_SETTING,
VISUALIZE_DISABLE_BUCKET_AGG_SETTING,
} from '../common/constants';

import { visualizationSavedObjectType } from './saved_objects';

Expand Down Expand Up @@ -77,6 +80,18 @@ export class VisualizationsPlugin
category: ['visualization'],
schema: schema.boolean(),
},
[VISUALIZE_DISABLE_BUCKET_AGG_SETTING]: {
name: i18n.translate('visualizations.advancedSettings.visualizeDisableBucketAgg', {
defaultMessage: 'Disable visualizations bucket aggregation types',
}),
value: [],
description: i18n.translate('visualizations.advancedSettings.visualizeDisableBucketAgg', {
defaultMessage: `A comma-separated list of bucket aggregations' names. e.g. significant_terms, terms.
Deactivates the specified bucket aggregations from visualizations.`,
}),
category: ['visualization'],
schema: schema.arrayOf(schema.string()),
},
});

if (plugins.usageCollection) {
Expand Down