Skip to content

Commit

Permalink
Remove schemas from agg configs (#58462)
Browse files Browse the repository at this point in the history
  • Loading branch information
lukeelmers authored Mar 5, 2020
1 parent 8ded7f9 commit 87a3d02
Show file tree
Hide file tree
Showing 64 changed files with 357 additions and 532 deletions.
3 changes: 0 additions & 3 deletions src/legacy/core_plugins/data/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export {
IFieldParamType,
IMetricAggType,
IpRangeKey, // only used in field formatter deserialization, which will live in data
ISchemas,
OptionedParamEditorProps, // only type is used externally
OptionedValueProp, // only type is used externally
} from './search/types';
Expand Down Expand Up @@ -75,8 +74,6 @@ export {
OptionedParamType,
parentPipelineType,
propFilter,
Schema,
Schemas,
siblingPipelineType,
termsAggFilter,
toAbsoluteDates,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,7 @@ describe('AggConfig', () => {
expect(typeof aggConfig.params).toBe('object');
expect(aggConfig.type).toBeInstanceOf(AggType);
expect(aggConfig.type).toHaveProperty('name', 'date_histogram');
expect(typeof aggConfig.schema).toBe('object');
expect(aggConfig.schema).toHaveProperty('name', 'segment');
expect(typeof aggConfig.schema).toBe('string');

const state = aggConfig.toJSON();
expect(state).toHaveProperty('id', '1');
Expand Down
61 changes: 5 additions & 56 deletions src/legacy/core_plugins/data/public/search/aggs/agg_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@
import _ from 'lodash';
import { i18n } from '@kbn/i18n';
import { IAggType } from './agg_type';
import { AggGroupNames } from './agg_groups';
import { writeParams } from './agg_params';
import { IAggConfigs } from './agg_configs';
import { Schema } from './schemas';
import {
ISearchSource,
FetchOptions,
Expand All @@ -38,37 +36,9 @@ export interface AggConfigOptions {
enabled?: boolean;
id?: string;
params?: Record<string, any>;
schema?: string | Schema;
schema?: string;
}

const unknownSchema: Schema = {
name: 'unknown',
title: 'Unknown', // only here for illustrative purposes
hideCustomLabel: true,
aggFilter: [],
min: 1,
max: 1,
params: [],
defaults: {},
editor: false,
group: AggGroupNames.Metrics,
aggSettings: {
top_hits: {
allowStrings: true,
},
},
};

const getSchemaFromRegistry = (schemas: any, schema: string): Schema => {
let registeredSchema = schemas ? schemas.byName[schema] : null;
if (!registeredSchema) {
registeredSchema = Object.assign({}, unknownSchema);
registeredSchema.name = schema;
}

return registeredSchema;
};

/**
* @name AggConfig
*
Expand Down Expand Up @@ -122,8 +92,8 @@ export class AggConfig {
public params: any;
public parent?: IAggConfigs;
public brandNew?: boolean;
public schema?: string;

private __schema: Schema;
private __type: IAggType;
private __typeDecorations: any;
private subAggs: AggConfig[] = [];
Expand All @@ -141,14 +111,12 @@ export class AggConfig {
this.setType(opts.type);

if (opts.schema) {
this.setSchema(opts.schema);
this.schema = opts.schema;
}

// set the params to the values from opts, or just to the defaults
this.setParams(opts.params || {});

// @ts-ignore
this.__schema = this.__schema;
// @ts-ignore
this.__type = this.__type;
}
Expand Down Expand Up @@ -305,16 +273,13 @@ export class AggConfig {
id: this.id,
enabled: this.enabled,
type: this.type && this.type.name,
schema: _.get(this, 'schema.name', this.schema),
schema: this.schema,
params: outParams,
};
}

getAggParams() {
return [
...(_.has(this, 'type.params') ? this.type.params : []),
...(_.has(this, 'schema.params') ? (this.schema as Schema).params : []),
];
return [...(_.has(this, 'type.params') ? this.type.params : [])];
}

getRequestAggs() {
Expand Down Expand Up @@ -434,9 +399,6 @@ export class AggConfig {

// clear out the previous params except for a few special ones
this.setParams({
// split row/columns is "outside" of the agg, so don't reset it
row: this.params.row,

// almost every agg has fields, so we try to persist that when type changes
field: availableFields.find((field: any) => field.name === this.getField()),
});
Expand All @@ -445,17 +407,4 @@ export class AggConfig {
public setType(type: IAggType) {
this.type = type;
}

public get schema() {
return this.__schema;
}

public set schema(schema) {
this.__schema = schema;
}

public setSchema(schema: string | Schema) {
this.schema =
typeof schema === 'string' ? getSchemaFromRegistry(this.aggConfigs.schemas, schema) : schema;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import { indexBy } from 'lodash';
import { AggConfig } from './agg_config';
import { AggConfigs } from './agg_configs';
import { AggTypesRegistryStart } from './agg_types_registry';
import { Schemas } from './schemas';
import { AggGroupNames } from './agg_groups';
import { mockDataServices, mockAggTypesRegistry } from './test_helpers';
import { IndexPatternField, IndexPattern } from '../../../../../../plugins/data/public';
import {
Expand Down Expand Up @@ -81,67 +79,6 @@ describe('AggConfigs', () => {
expect(spy.mock.calls[0]).toEqual([configStates]);
spy.mockRestore();
});

describe('defaults', () => {
const schemas = new Schemas([
{
group: AggGroupNames.Metrics,
name: 'metric',
title: 'Simple',
min: 1,
max: 2,
defaults: [
{ schema: 'metric', type: 'count' },
{ schema: 'metric', type: 'avg' },
{ schema: 'metric', type: 'sum' },
],
},
{
group: AggGroupNames.Buckets,
name: 'segment',
title: 'Example',
min: 0,
max: 1,
defaults: [
{ schema: 'segment', type: 'terms' },
{ schema: 'segment', type: 'filters' },
],
},
]);

it('should only set the number of defaults defined by the max', () => {
const ac = new AggConfigs(indexPattern, [], {
schemas: schemas.all,
typesRegistry,
});
expect(ac.bySchemaName('metric')).toHaveLength(2);
});

it('should set the defaults defined in the schema when none exist', () => {
const ac = new AggConfigs(indexPattern, [], {
schemas: schemas.all,
typesRegistry,
});
expect(ac.aggs).toHaveLength(3);
});

it('should NOT set the defaults defined in the schema when some exist', () => {
const configStates = [
{
enabled: true,
type: 'date_histogram',
params: {},
schema: 'segment',
},
];
const ac = new AggConfigs(indexPattern, configStates, {
schemas: schemas.all,
typesRegistry,
});
expect(ac.aggs).toHaveLength(3);
expect(ac.bySchemaName('segment')[0].type.name).toEqual('date_histogram');
});
});
});

describe('#createAggConfig', () => {
Expand Down Expand Up @@ -285,17 +222,6 @@ describe('AggConfigs', () => {
});

describe('#toDsl', () => {
const schemas = new Schemas([
{
group: AggGroupNames.Buckets,
name: 'segment',
},
{
group: AggGroupNames.Buckets,
name: 'split',
},
]);

beforeEach(() => {
indexPattern = stubIndexPattern as IndexPattern;
indexPattern.fields.getByName = name => (name as unknown) as IndexPatternField;
Expand All @@ -319,7 +245,6 @@ describe('AggConfigs', () => {

const ac = new AggConfigs(indexPattern, configStates, {
typesRegistry,
schemas: schemas.all,
});

const aggInfos = ac.aggs.map(aggConfig => {
Expand Down Expand Up @@ -390,11 +315,10 @@ describe('AggConfigs', () => {

const ac = new AggConfigs(indexPattern, configStates, {
typesRegistry,
schemas: schemas.all,
});
const dsl = ac.toDsl();
const histo = ac.byName('date_histogram')[0];
const metrics = ac.bySchemaGroup('metrics');
const metrics = ac.bySchemaName('metrics');

expect(dsl).toHaveProperty(histo.id);
expect(typeof dsl[histo.id]).toBe('object');
Expand All @@ -418,8 +342,8 @@ describe('AggConfigs', () => {

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const topLevelDsl = ac.toDsl(true);
const buckets = ac.bySchemaGroup('buckets');
const metrics = ac.bySchemaGroup('metrics');
const buckets = ac.bySchemaName('buckets');
const metrics = ac.bySchemaName('metrics');

(function checkLevel(dsl) {
const bucket = buckets.shift();
Expand Down
47 changes: 4 additions & 43 deletions src/legacy/core_plugins/data/public/search/aggs/agg_configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import { Assign } from '@kbn/utility-types';
import { AggConfig, AggConfigOptions, IAggConfig } from './agg_config';
import { IAggType } from './agg_type';
import { AggTypesRegistryStart } from './agg_types_registry';
import { Schema } from './schemas';
import { AggGroupNames } from './agg_groups';
import {
IndexPattern,
Expand All @@ -32,8 +31,6 @@ import {
TimeRange,
} from '../../../../../../plugins/data/public';

type Schemas = Record<string, any>;

function removeParentAggs(obj: any) {
for (const prop in obj) {
if (prop === 'parentAggs') delete obj[prop];
Expand All @@ -51,7 +48,6 @@ function parseParentAggs(dslLvlCursor: any, dsl: any) {
}

export interface AggConfigsOptions {
schemas?: Schemas;
typesRegistry: AggTypesRegistryStart;
}

Expand All @@ -73,7 +69,6 @@ export type IAggConfigs = AggConfigs;

export class AggConfigs {
public indexPattern: IndexPattern;
public schemas: any;
public timeRange?: TimeRange;
private readonly typesRegistry: AggTypesRegistryStart;

Expand All @@ -90,37 +85,8 @@ export class AggConfigs {

this.aggs = [];
this.indexPattern = indexPattern;
this.schemas = opts.schemas;

configStates.forEach((params: any) => this.createAggConfig(params));

if (this.schemas) {
this.initializeDefaultsFromSchemas(this.schemas);
}
}

// do this wherever the schemas were passed in, & pass in state defaults instead
initializeDefaultsFromSchemas(schemas: Schemas) {
// Set the defaults for any schema which has them. If the defaults
// for some reason has more then the max only set the max number
// of defaults (not sure why a someone define more...
// but whatever). Also if a schema.name is already set then don't
// set anything.
_(schemas)
.filter((schema: Schema) => {
return Array.isArray(schema.defaults) && schema.defaults.length > 0;
})
.each((schema: any) => {
if (!this.aggs.find((agg: AggConfig) => agg.schema && agg.schema.name === schema.name)) {
// the result here should be passable as a configState
const defaults = schema.defaults.slice(0, schema.max);
_.each(defaults, defaultState => {
const state = _.defaults({ id: AggConfig.nextId(this.aggs) }, defaultState);
this.createAggConfig(state as AggConfigOptions);
});
}
})
.commit();
}

setTimeRange(timeRange: TimeRange) {
Expand Down Expand Up @@ -148,7 +114,6 @@ export class AggConfigs {
};

const aggConfigs = new AggConfigs(this.indexPattern, this.aggs.filter(filterAggs), {
schemas: this.schemas,
typesRegistry: this.typesRegistry,
});

Expand Down Expand Up @@ -271,23 +236,19 @@ export class AggConfigs {
}

byName(name: string) {
return this.aggs.filter(agg => agg.type.name === name);
return this.aggs.filter(agg => agg.type?.name === name);
}

byType(type: string) {
return this.aggs.filter(agg => agg.type.type === type);
return this.aggs.filter(agg => agg.type?.type === type);
}

byTypeName(type: string) {
return this.aggs.filter(agg => agg.type.name === type);
return this.byName(type);
}

bySchemaName(schema: string) {
return this.aggs.filter(agg => agg.schema && agg.schema.name === schema);
}

bySchemaGroup(group: string) {
return this.aggs.filter(agg => agg.schema && agg.schema.group === group);
return this.aggs.filter(agg => agg.schema === schema);
}

getRequestAggs(): AggConfig[] {
Expand Down
Loading

0 comments on commit 87a3d02

Please sign in to comment.