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

Move ui/agg_types in to shim data plugin #56353

Merged
merged 10 commits into from
Feb 5, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import _ from 'lodash';
import moment from 'moment';
import expect from '@kbn/expect';

jest.mock('../../../../../ui/public/agg_types/agg_configs', () => ({
jest.mock('../../search/aggs', () => ({
AggConfigs: function AggConfigs() {
return {
createAggConfig: ({ params }) => ({
Expand Down
50 changes: 47 additions & 3 deletions src/legacy/core_plugins/data/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

// /// Define plugin function
import { DataPlugin as Plugin, DataStart } from './plugin';
import { DataPlugin as Plugin } from './plugin';

export function plugin() {
return new Plugin();
Expand All @@ -27,14 +27,58 @@ export function plugin() {
// /// Export types & static code

/** @public types */
export { DataStart };
export { DataSetup, DataStart } from './plugin';
export {
SavedQueryAttributes,
SavedQuery,
SavedQueryTimeFilter,
} from '../../../../plugins/data/public';
export {
// agg_types
AggParam,
Copy link
Member

Choose a reason for hiding this comment

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

where is this used externally ?

Copy link
Member Author

Choose a reason for hiding this comment

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

just in default editor i believe, and just used as a type.

AggParamOption,
Copy link
Member

Choose a reason for hiding this comment

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

only type is used externally

DateRangeKey,
IAggConfig,
IAggConfigs,
IAggType,
IFieldParamType,
IMetricAggType,
IpRangeKey,
Copy link
Member

Choose a reason for hiding this comment

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

this and DateRangeKey are only used by field formater deserialization, which i am moving inside data plugin

ISchemas,
OptionedParamEditorProps,
Copy link
Member

Choose a reason for hiding this comment

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

only type is needed

Copy link
Member

Choose a reason for hiding this comment

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

same for the one below

OptionedValueProp,
} from './search/types';

/** @public static code */
export * from '../common';
export { FilterStateManager } from './filter/filter_manager';
export { getRequestInspectorStats, getResponseInspectorStats } from './search';
export {
// agg_types TODO need to group these under a namespace or prefix
AggParamType,
AggTypeFilters, // TODO convert to interface
aggTypeFilters,
AggTypeFieldFilters, // TODO convert to interface
AggGroupNames,
aggGroupNamesMap,
BUCKET_TYPES,
CidrMask,
convertDateRangeToString,
convertIPRangeToString,
intervalOptions, // only used in Discover
isDateHistogramBucketAggConfig,
isStringType,
isType,
isValidInterval,
isValidJson,
METRIC_TYPES,
OptionedParamType,
parentPipelineType,
propFilter,
Schema,
Schemas,
siblingPipelineType,
Copy link
Member

Choose a reason for hiding this comment

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

most (if not all) of theese are just types needed by default editor to build their agg type option to editor control map

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, in the next pass I want to go through and strip away more of this stuff. For this first PR I kept things focused on just re-exporting absolutely anything that was needed outside of the data plugin, but there will be plenty more cleanup to follow.

termsAggFilter,
// search_source
getRequestInspectorStats,
getResponseInspectorStats,
} from './search';
28 changes: 25 additions & 3 deletions src/legacy/core_plugins/data/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import {
} from '../../../../plugins/embeddable/public/lib/triggers';
import { IUiActionsSetup, IUiActionsStart } from '../../../../plugins/ui_actions/public';

import { SearchSetup, SearchStart, SearchService } from './search/search_service';

export interface DataPluginSetupDependencies {
data: DataPublicPluginSetup;
expressions: ExpressionsSetup;
Expand All @@ -56,12 +58,23 @@ export interface DataPluginStartDependencies {
uiActions: IUiActionsStart;
}

/**
* Interface for this plugin's returned `setup` contract.
*
* @public
*/
export interface DataSetup {
search: SearchSetup;
}

/**
* Interface for this plugin's returned `start` contract.
*
* @public
*/
export interface DataStart {} // eslint-disable-line @typescript-eslint/no-empty-interface
export interface DataStart {
search: SearchStart;
}

/**
* Data Plugin - public
Expand All @@ -76,7 +89,10 @@ export interface DataStart {} // eslint-disable-line @typescript-eslint/no-empty
*/

export class DataPlugin
implements Plugin<void, DataStart, DataPluginSetupDependencies, DataPluginStartDependencies> {
implements
Plugin<DataSetup, DataStart, DataPluginSetupDependencies, DataPluginStartDependencies> {
private readonly search = new SearchService();

public setup(core: CoreSetup, { data, uiActions }: DataPluginSetupDependencies) {
setInjectedMetadata(core.injectedMetadata);

Expand All @@ -89,6 +105,10 @@ export class DataPlugin
uiActions.registerAction(
valueClickAction(data.query.filterManager, data.query.timefilter.timefilter)
);

return {
search: this.search.setup(core),
};
}

public start(core: CoreStart, { data, uiActions }: DataPluginStartDependencies): DataStart {
Expand All @@ -102,7 +122,9 @@ export class DataPlugin
uiActions.attachAction(SELECT_RANGE_TRIGGER, SELECT_RANGE_ACTION);
uiActions.attachAction(VALUE_CLICK_TRIGGER, VALUE_CLICK_ACTION);

return {};
return {
search: this.search.start(core),
};
}

public stop() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
mergeOtherBucketAggResponse,
updateMissingBucket,
} from '../../buckets/_terms_other_bucket_helper';
import { Vis } from '../../../../../core_plugins/visualizations/public';
import { Vis } from '../../../../../../../core_plugins/visualizations/public';
import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';

const visConfigSingleTerm = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@
import _ from 'lodash';
import { i18n } from '@kbn/i18n';
import { npStart } from 'ui/new_platform';
import { AggType } from './agg_type';
import { IAggType } from './agg_type';
import { AggGroupNames } from './agg_groups';
import { writeParams } from './agg_params';
import { AggConfigs } from './agg_configs';
import { IAggConfigs } from './agg_configs';
import { Schema } from './schemas';
import {
ISearchSource,
FetchOptions,
fieldFormats,
KBN_FIELD_TYPES,
} from '../../../../plugins/data/public';
} from '../../../../../../plugins/data/public';

export interface AggConfigOptions {
enabled: boolean;
Expand All @@ -60,13 +60,13 @@ const unknownSchema: Schema = {
group: AggGroupNames.Metrics,
};

const getTypeFromRegistry = (type: string): AggType => {
const getTypeFromRegistry = (type: string): IAggType => {
// We need to inline require here, since we're having a cyclic dependency
// from somewhere inside agg_types back to AggConfig.
const aggTypes = require('./agg_types').aggTypes;
const aggTypes = require('../aggs').aggTypes;
const registeredType =
aggTypes.metrics.find((agg: AggType) => agg.name === type) ||
aggTypes.buckets.find((agg: AggType) => agg.name === type);
aggTypes.metrics.find((agg: IAggType) => agg.name === type) ||
aggTypes.buckets.find((agg: IAggType) => agg.name === type);

if (!registeredType) {
throw new Error('unknown type');
Expand All @@ -85,6 +85,9 @@ const getSchemaFromRegistry = (schemas: any, schema: string): Schema => {
return registeredSchema;
};

// TODO need to make a more explicit interface for this
export type IAggConfig = AggConfig;

export class AggConfig {
/**
* Ensure that all of the objects in the list have ids, the objects
Expand Down Expand Up @@ -122,19 +125,19 @@ export class AggConfig {
);
}

public aggConfigs: AggConfigs;
public aggConfigs: IAggConfigs;
public id: string;
public enabled: boolean;
public params: any;
public parent?: AggConfigs;
public parent?: IAggConfigs;
public brandNew?: boolean;

private __schema: Schema;
private __type: AggType;
private __type: IAggType;
private __typeDecorations: any;
private subAggs: AggConfig[] = [];

constructor(aggConfigs: AggConfigs, opts: AggConfigOptions) {
constructor(aggConfigs: IAggConfigs, opts: AggConfigOptions) {
this.aggConfigs = aggConfigs;
this.id = String(opts.id || AggConfig.nextId(aggConfigs.aggs as any));
this.enabled = typeof opts.enabled === 'boolean' ? opts.enabled : true;
Expand Down Expand Up @@ -207,7 +210,7 @@ export class AggConfig {
return _.get(this.params, key);
}

write(aggs?: AggConfigs) {
write(aggs?: IAggConfigs) {
return writeParams<AggConfig>(this.type.params, this, aggs);
}

Expand Down Expand Up @@ -262,7 +265,7 @@ export class AggConfig {
* @return {void|Object} - if the config has a dsl representation, it is
* returned, else undefined is returned
*/
toDsl(aggConfigs?: AggConfigs) {
toDsl(aggConfigs?: IAggConfigs) {
if (this.type.hasNoDsl) return;
const output = this.write(aggConfigs) as any;

Expand Down Expand Up @@ -360,7 +363,7 @@ export class AggConfig {

if (!this.type) return '';
return percentageMode
? i18n.translate('common.ui.vis.aggConfig.percentageOfLabel', {
? i18n.translate('data.search.aggs.percentageOfLabel', {
defaultMessage: 'Percentage of {label}',
values: { label: this.type.makeLabel(this) },
})
Expand Down Expand Up @@ -448,7 +451,7 @@ export class AggConfig {
});
}

public setType(type: string | AggType) {
public setType(type: string | IAggType) {
this.type = typeof type === 'string' ? getTypeFromRegistry(type) : type;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
ISearchSource,
FetchOptions,
TimeRange,
} from '../../../../plugins/data/public';
} from '../../../../../../plugins/data/public';

type Schemas = Record<string, any>;

Expand All @@ -55,6 +55,9 @@ function parseParentAggs(dslLvlCursor: any, dsl: any) {
}
}

// TODO need to make a more explicit interface for this
export type IAggConfigs = AggConfigs;

export class AggConfigs {
public indexPattern: IndexPattern;
public schemas: any;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ export const AggGroupNames = Object.freeze({
export type AggGroupNames = $Values<typeof AggGroupNames>;

export const aggGroupNamesMap = () => ({
[AggGroupNames.Metrics]: i18n.translate('common.ui.aggTypes.aggGroups.metricsText', {
[AggGroupNames.Metrics]: i18n.translate('data.search.aggs.aggGroups.metricsText', {
defaultMessage: 'Metrics',
}),
[AggGroupNames.Buckets]: i18n.translate('common.ui.aggTypes.aggGroups.bucketsText', {
[AggGroupNames.Buckets]: i18n.translate('data.search.aggs.aggGroups.bucketsText', {
defaultMessage: 'Buckets',
}),
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { initParams } from './agg_params';
import { BaseParamType } from './param_types/base';
import { FieldParamType } from './param_types/field';
import { OptionedParamType } from './param_types/optioned';
import { AggParamType } from '../agg_types/param_types/agg';
import { AggParamType } from '../aggs/param_types/agg';

jest.mock('ui/new_platform');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { JsonParamType } from './param_types/json';
import { BaseParamType } from './param_types/base';

import { AggConfig } from './agg_config';
import { AggConfigs } from './agg_configs';
import { IAggConfigs } from './agg_configs';

const paramTypeMap = {
field: FieldParamType,
Expand Down Expand Up @@ -73,7 +73,7 @@ export const writeParams = <
>(
params: Array<Partial<TAggParam>> = [],
aggConfig: TAggConfig,
aggs?: AggConfigs,
aggs?: IAggConfigs,
locals?: Record<string, any>
) => {
const output = { params: {} as Record<string, any> };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import { AggType, AggTypeConfig } from './agg_type';
import { AggConfig } from './agg_config';
import { IAggConfig } from './agg_config';
import { npStart } from 'ui/new_platform';

jest.mock('ui/new_platform');
Expand Down Expand Up @@ -48,7 +48,7 @@ describe('AggType Class', () => {
describe('makeLabel', () => {
it('makes a function when the makeLabel config is not specified', () => {
const makeLabel = () => 'label';
const aggConfig = {} as AggConfig;
const aggConfig = {} as IAggConfig;
const config: AggTypeConfig = {
name: 'name',
title: 'title',
Expand All @@ -64,7 +64,7 @@ describe('AggType Class', () => {

describe('getResponseAggs/getRequestAggs', () => {
it('copies the value', () => {
const testConfig = (aggConfig: AggConfig) => [aggConfig];
const testConfig = (aggConfig: IAggConfig) => [aggConfig];

const aggType = new AggType({
name: 'name',
Expand All @@ -78,7 +78,7 @@ describe('AggType Class', () => {
});

it('defaults to noop', () => {
const aggConfig = {} as AggConfig;
const aggConfig = {} as IAggConfig;
const aggType = new AggType({
name: 'name',
title: 'title',
Expand Down Expand Up @@ -130,13 +130,13 @@ describe('AggType Class', () => {
});

describe('getFormat', function() {
let aggConfig: AggConfig;
let aggConfig: IAggConfig;
let field: any;

beforeEach(() => {
aggConfig = ({
getField: jest.fn(() => field),
} as unknown) as AggConfig;
} as unknown) as IAggConfig;
});

it('returns the formatter for the aggConfig', () => {
Expand Down
Loading