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

[data.search.aggs] Remove service getters from agg types (AggConfig part) #62548

Merged
merged 10 commits into from
Apr 15, 2020
Merged

[data.search.aggs] Remove service getters from agg types (AggConfig part) #62548

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1f25e16
removed getFieldFormats from aggConfig
VladLasitsa Apr 4, 2020
028bb75
made new properties is private
VladLasitsa Apr 4, 2020
359984e
Fixed ci
VladLasitsa Apr 6, 2020
497dfa2
Fixed tests
VladLasitsa Apr 7, 2020
e574b06
Fixes clone method
VladLasitsa Apr 7, 2020
6a532fa
Merge branch 'master' into #60333_agg_config
VladLasitsa Apr 9, 2020
0d16960
move filedFotmats inside opt fro AggConfigs
VladLasitsa Apr 9, 2020
05ea6f3
Merge branch 'master' into #60333_agg_config
elasticmachine Apr 13, 2020
d2adc78
Added readonly
VladLasitsa Apr 14, 2020
0f15ac3
Merge branch 'master' into #60333_agg_config
elasticmachine Apr 14, 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
1 change: 1 addition & 0 deletions src/legacy/ui/public/new_platform/new_platform.karma_mock.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ export const npStart = {
createAggConfigs: (indexPattern, configStates = []) => {
return new AggConfigs(indexPattern, configStates, {
typesRegistry: aggTypesRegistry.start(),
fieldFormats: getFieldFormatsRegistry(mockCoreStart),
});
},
types: aggTypesRegistry.start(),
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/public/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export class DataPublicPlugin implements Plugin<DataPublicPluginSetup, DataPubli
const query = this.queryService.start(savedObjects);
setQueryService(query);

const search = this.searchService.start(core, indexPatterns);
const search = this.searchService.start(core, { fieldFormats, indexPatterns });
setSearchService(search);

uiActions.attachAction(APPLY_FILTER_TRIGGER, uiActions.getAction(ACTION_GLOBAL_APPLY_FILTER));
Expand Down
34 changes: 18 additions & 16 deletions src/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 @@ -26,10 +26,12 @@ import { AggTypesRegistryStart } from './agg_types_registry';
import { mockDataServices, mockAggTypesRegistry } from './test_helpers';
import { Field as IndexPatternField, IndexPattern } from '../../index_patterns';
import { stubIndexPatternWithFields } from '../../../public/stubs';
import { fieldFormatsServiceMock } from '../../field_formats/mocks';

describe('AggConfig', () => {
let indexPattern: IndexPattern;
let typesRegistry: AggTypesRegistryStart;
const fieldFormats = fieldFormatsServiceMock.createStartContract();

beforeEach(() => {
jest.restoreAllMocks();
Expand All @@ -40,7 +42,7 @@ describe('AggConfig', () => {

describe('#toDsl', () => {
it('calls #write()', () => {
const ac = new AggConfigs(indexPattern, [], { typesRegistry });
const ac = new AggConfigs(indexPattern, [], { typesRegistry, fieldFormats });
const configStates = {
enabled: true,
type: 'date_histogram',
Expand All @@ -55,7 +57,7 @@ describe('AggConfig', () => {
});

it('uses the type name as the agg name', () => {
const ac = new AggConfigs(indexPattern, [], { typesRegistry });
const ac = new AggConfigs(indexPattern, [], { typesRegistry, fieldFormats });
const configStates = {
enabled: true,
type: 'date_histogram',
Expand All @@ -70,7 +72,7 @@ describe('AggConfig', () => {
});

it('uses the params from #write() output as the agg params', () => {
const ac = new AggConfigs(indexPattern, [], { typesRegistry });
const ac = new AggConfigs(indexPattern, [], { typesRegistry, fieldFormats });
const configStates = {
enabled: true,
type: 'date_histogram',
Expand Down Expand Up @@ -100,7 +102,7 @@ describe('AggConfig', () => {
params: {},
},
];
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });

const histoConfig = ac.byName('date_histogram')[0];
const avgConfig = ac.byName('avg')[0];
Expand Down Expand Up @@ -210,8 +212,8 @@ describe('AggConfig', () => {

testsIdentical.forEach((configState, index) => {
it(`identical aggregations (${index})`, () => {
const ac1 = new AggConfigs(indexPattern, configState, { typesRegistry });
const ac2 = new AggConfigs(indexPattern, configState, { typesRegistry });
const ac1 = new AggConfigs(indexPattern, configState, { typesRegistry, fieldFormats });
const ac2 = new AggConfigs(indexPattern, configState, { typesRegistry, fieldFormats });
expect(ac1.jsonDataEquals(ac2.aggs)).toBe(true);
});
});
Expand Down Expand Up @@ -251,8 +253,8 @@ describe('AggConfig', () => {

testsIdenticalDifferentOrder.forEach((test, index) => {
it(`identical aggregations (${index}) - init json is in different order`, () => {
const ac1 = new AggConfigs(indexPattern, test.config1, { typesRegistry });
const ac2 = new AggConfigs(indexPattern, test.config2, { typesRegistry });
const ac1 = new AggConfigs(indexPattern, test.config1, { typesRegistry, fieldFormats });
const ac2 = new AggConfigs(indexPattern, test.config2, { typesRegistry, fieldFormats });
expect(ac1.jsonDataEquals(ac2.aggs)).toBe(true);
});
});
Expand Down Expand Up @@ -316,16 +318,16 @@ describe('AggConfig', () => {

testsDifferent.forEach((test, index) => {
it(`different aggregations (${index})`, () => {
const ac1 = new AggConfigs(indexPattern, test.config1, { typesRegistry });
const ac2 = new AggConfigs(indexPattern, test.config2, { typesRegistry });
const ac1 = new AggConfigs(indexPattern, test.config1, { typesRegistry, fieldFormats });
const ac2 = new AggConfigs(indexPattern, test.config2, { typesRegistry, fieldFormats });
expect(ac1.jsonDataEquals(ac2.aggs)).toBe(false);
});
});
});

describe('#toJSON', () => {
it('includes the aggs id, params, type and schema', () => {
const ac = new AggConfigs(indexPattern, [], { typesRegistry });
const ac = new AggConfigs(indexPattern, [], { typesRegistry, fieldFormats });
const configStates = {
enabled: true,
type: 'date_histogram',
Expand Down Expand Up @@ -356,8 +358,8 @@ describe('AggConfig', () => {
params: {},
},
];
const ac1 = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac2 = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac1 = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
const ac2 = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });

// this relies on the assumption that js-engines consistently loop over properties in insertion order.
// most likely the case, but strictly speaking not guaranteed by the JS and JSON specifications.
Expand All @@ -369,7 +371,7 @@ describe('AggConfig', () => {
let aggConfig: AggConfig;

beforeEach(() => {
const ac = new AggConfigs(indexPattern, [], { typesRegistry });
const ac = new AggConfigs(indexPattern, [], { typesRegistry, fieldFormats });
aggConfig = ac.createAggConfig({ type: 'count' } as CreateAggConfigParams);
});

Expand Down Expand Up @@ -398,7 +400,7 @@ describe('AggConfig', () => {

describe('#fieldFormatter - custom getFormat handler', () => {
it('returns formatter from getFormat handler', () => {
const ac = new AggConfigs(indexPattern, [], { typesRegistry });
const ac = new AggConfigs(indexPattern, [], { typesRegistry, fieldFormats });
const configStates = {
enabled: true,
type: 'count',
Expand Down Expand Up @@ -439,7 +441,7 @@ describe('AggConfig', () => {
},
},
};
const ac = new AggConfigs(indexPattern, [configStates], { typesRegistry });
const ac = new AggConfigs(indexPattern, [configStates], { typesRegistry, fieldFormats });
aggConfig = ac.createAggConfig(configStates);
});

Expand Down
19 changes: 14 additions & 5 deletions src/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 @@ -25,7 +25,7 @@ import { IAggConfigs } from './agg_configs';
import { FetchOptions } from '../fetch';
import { ISearchSource } from '../search_source';
import { FieldFormatsContentType, KBN_FIELD_TYPES } from '../../../common';
import { getFieldFormats } from '../../../public/services';
import { FieldFormatsStart } from '../../field_formats';

export interface AggConfigOptions {
type: IAggType;
Expand All @@ -35,6 +35,10 @@ export interface AggConfigOptions {
schema?: string;
}

export interface AggConfigDependencies {
fieldFormats: FieldFormatsStart;
}

/**
* @name AggConfig
*
Expand Down Expand Up @@ -93,8 +97,13 @@ export class AggConfig {
private __type: IAggType;
private __typeDecorations: any;
private subAggs: AggConfig[] = [];
private fieldFormats: FieldFormatsStart;
VladLasitsa marked this conversation as resolved.
Show resolved Hide resolved

constructor(aggConfigs: IAggConfigs, opts: AggConfigOptions) {
constructor(
aggConfigs: IAggConfigs,
opts: AggConfigOptions,
{ fieldFormats }: AggConfigDependencies
) {
this.aggConfigs = aggConfigs;
this.id = String(opts.id || AggConfig.nextId(aggConfigs.aggs as any));
this.enabled = typeof opts.enabled === 'boolean' ? opts.enabled : true;
Expand All @@ -115,6 +124,8 @@ export class AggConfig {

// @ts-ignore
this.__type = this.__type;

this.fieldFormats = fieldFormats;
}

/**
Expand Down Expand Up @@ -341,12 +352,10 @@ export class AggConfig {
}

fieldOwnFormatter(contentType?: FieldFormatsContentType, defaultFormat?: any) {
const fieldFormatsService = getFieldFormats();

const field = this.getField();
let format = field && field.format;
if (!format) format = defaultFormat;
if (!format) format = fieldFormatsService.getDefaultInstance(KBN_FIELD_TYPES.STRING);
if (!format) format = this.fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING);
return format.getConverterFor(contentType);
}

Expand Down
44 changes: 26 additions & 18 deletions src/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 @@ -24,10 +24,12 @@ import { AggTypesRegistryStart } from './agg_types_registry';
import { mockDataServices, mockAggTypesRegistry } from './test_helpers';
import { Field as IndexPatternField, IndexPattern } from '../../index_patterns';
import { stubIndexPattern, stubIndexPatternWithFields } from '../../../public/stubs';
import { fieldFormatsServiceMock } from '../../field_formats/mocks';

describe('AggConfigs', () => {
let indexPattern: IndexPattern;
let typesRegistry: AggTypesRegistryStart;
const fieldFormats = fieldFormatsServiceMock.createStartContract();

beforeEach(() => {
mockDataServices();
Expand All @@ -45,7 +47,7 @@ describe('AggConfigs', () => {
},
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
expect(ac.aggs).toHaveLength(1);
});

Expand All @@ -70,7 +72,7 @@ describe('AggConfigs', () => {
];

const spy = jest.spyOn(AggConfig, 'ensureIds');
new AggConfigs(indexPattern, configStates, { typesRegistry });
new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
expect(spy).toHaveBeenCalledTimes(1);
expect(spy.mock.calls[0]).toEqual([configStates]);
spy.mockRestore();
Expand All @@ -92,16 +94,20 @@ describe('AggConfigs', () => {
},
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
expect(ac.aggs).toHaveLength(2);

ac.createAggConfig(
new AggConfig(ac, {
enabled: true,
type: typesRegistry.get('terms'),
params: {},
schema: 'split',
})
new AggConfig(
ac,
{
enabled: true,
type: typesRegistry.get('terms'),
params: {},
schema: 'split',
},
{ fieldFormats }
)
);
expect(ac.aggs).toHaveLength(3);
});
Expand All @@ -115,7 +121,7 @@ describe('AggConfigs', () => {
},
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
expect(ac.aggs).toHaveLength(1);

ac.createAggConfig({
Expand All @@ -136,7 +142,7 @@ describe('AggConfigs', () => {
},
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
expect(ac.aggs).toHaveLength(1);

ac.createAggConfig(
Expand Down Expand Up @@ -164,7 +170,7 @@ describe('AggConfigs', () => {
{ type: 'percentiles', enabled: true, params: {}, schema: 'metric' },
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
const sorted = ac.getRequestAggs();
const aggs = indexBy(ac.aggs, agg => agg.type.name);

Expand All @@ -187,7 +193,7 @@ describe('AggConfigs', () => {
{ type: 'count', enabled: true, params: {}, schema: 'metric' },
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
const sorted = ac.getResponseAggs();
const aggs = indexBy(ac.aggs, agg => agg.type.name);

Expand All @@ -204,7 +210,7 @@ describe('AggConfigs', () => {
{ type: 'percentiles', enabled: true, params: { percents: [1, 2, 3] }, schema: 'metric' },
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
const sorted = ac.getResponseAggs();
const aggs = indexBy(ac.aggs, agg => agg.type.name);

Expand All @@ -225,7 +231,7 @@ describe('AggConfigs', () => {

it('uses the sorted aggs', () => {
const configStates = [{ enabled: true, type: 'avg', params: { field: 'bytes' } }];
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
const spy = jest.spyOn(AggConfigs.prototype, 'getRequestAggs');
ac.toDsl();
expect(spy).toHaveBeenCalledTimes(1);
Expand All @@ -241,6 +247,7 @@ describe('AggConfigs', () => {

const ac = new AggConfigs(indexPattern, configStates, {
typesRegistry,
fieldFormats,
});

const aggInfos = ac.aggs.map(aggConfig => {
Expand Down Expand Up @@ -284,7 +291,7 @@ describe('AggConfigs', () => {
},
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
const dsl = ac.toDsl();
const histo = ac.byName('date_histogram')[0];
const count = ac.byName('count')[0];
Expand All @@ -311,6 +318,7 @@ describe('AggConfigs', () => {

const ac = new AggConfigs(indexPattern, configStates, {
typesRegistry,
fieldFormats,
});
const dsl = ac.toDsl();
const histo = ac.byName('date_histogram')[0];
Expand All @@ -336,7 +344,7 @@ describe('AggConfigs', () => {
{ enabled: true, type: 'max', schema: 'metric', params: { field: 'bytes' } },
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
const topLevelDsl = ac.toDsl(true);
const buckets = ac.bySchemaName('buckets');
const metrics = ac.bySchemaName('metrics');
Expand Down Expand Up @@ -406,7 +414,7 @@ describe('AggConfigs', () => {
},
];

const ac = new AggConfigs(indexPattern, configStates, { typesRegistry });
const ac = new AggConfigs(indexPattern, configStates, { typesRegistry, fieldFormats });
const topLevelDsl = ac.toDsl(true)['2'];

expect(Object.keys(topLevelDsl.aggs)).toContain('1');
Expand Down
Loading