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.

Sign up for GitHub

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

Jump to bottom

removing schemas from agg configs #58462

Merged
merged 22 commits into from
Mar 5, 2020
Merged

removing schemas from agg configs #58462

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
1c45299
removing schemas from aggconfigs
ppisljar Feb 25, 2020
03da927
moving to vis_default_editor
ppisljar Feb 25, 2020
d3aecb6
updating tests
ppisljar Feb 25, 2020
36c7bc4
moving initing defaults from schemas to vis
ppisljar Feb 27, 2020
573c7e1
updating editor
ppisljar Mar 2, 2020
c24a818
updating after rebase
ppisljar Mar 2, 2020
2b4fb94
fixes
ppisljar Mar 2, 2020
622b14b
adding utility functions
ppisljar Mar 2, 2020
34aa486
fixing tests
ppisljar Mar 2, 2020
a255bb3
fixes
ppisljar Mar 3, 2020
b14cff4
i18n fixes.
lukeelmers Mar 4, 2020
bf18fee
more fixes
ppisljar Mar 4, 2020
06af6c4
Merge branch 'master' into agg/schemas
ppisljar Mar 4, 2020
dec4ac9
more fixes
ppisljar Mar 4, 2020
338dbf6
schemaTitle
ppisljar Mar 4, 2020
a01f9aa
Merge branch 'master' into agg/schemas
ppisljar Mar 5, 2020
2964295
updating based on review
ppisljar Mar 5, 2020
a3a08dd
updating based on review
ppisljar Mar 5, 2020
7dc487e
merging filter arrays
ppisljar Mar 5, 2020
fc48c65
hidding custom label on pipeline aggs
ppisljar Mar 5, 2020
e35ffbb
fixing
ppisljar Mar 5, 2020
54e0a11
using callback to modify row option
ppisljar Mar 5, 2020
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
View file
Open in desktop
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
3 changes: 1 addition & 2 deletions src/legacy/core_plugins/data/public/search/aggs/agg_config.test.ts
View file
Open in desktop
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
View file
Open in desktop
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 @@ -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,
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 @@ -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;
}
}
82 changes: 3 additions & 79 deletions src/legacy/core_plugins/data/public/search/aggs/agg_configs.test.ts
View file
Open in desktop
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
View file
Open in desktop
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
Loading