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

removing schemas from agg configs #58462

Merged
merged 22 commits into from
Mar 5, 2020
Merged
Show file tree
Hide file tree
Changes from 14 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
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 @@ -76,8 +75,6 @@ export {
OptionedParamType,
parentPipelineType,
propFilter,
Schema,
Schemas,
siblingPipelineType,
termsAggFilter,
// search_source
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 => {
Copy link
Member Author

Choose a reason for hiding this comment

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

vis.type.schemas.all were passed in to AggConfigs before, when setting the schema we would look it up in there and assign schema instance to AggConfig. N

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;
Copy link
Member Author

Choose a reason for hiding this comment

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

schema is now just a 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 @@ -435,9 +400,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,
Copy link
Member Author

Choose a reason for hiding this comment

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

special parameter used for figuring out if we should do a row or column split (with data table vis). this is not relevant to data querying at all so this setting was moved to vis.params


// 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 @@ -446,17 +408,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 @@ -80,67 +78,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 @@ -284,17 +221,6 @@ describe('AggConfigs', () => {
});

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

beforeEach(() => {
mockDataServices();
indexPattern = stubIndexPattern as IndexPattern;
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);
Copy link
Member Author

Choose a reason for hiding this comment

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

the defaults are no longer initialized on this level, this is moved into vis_impl.js as its visualization specific

}
}

// 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
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,3 @@ export { OptionedParamType } from './param_types/optioned';
export { isValidJson, isValidInterval } from './utils';
export { BUCKET_TYPES } from './buckets/bucket_agg_types';
export { METRIC_TYPES } from './metrics/metric_agg_types';
export { ISchemas, Schema, Schemas } from './schemas';
Loading