Skip to content

Commit

Permalink
[data.search.aggs] Remove service getters from agg types (AggConfig p…
Browse files Browse the repository at this point in the history
…art) (#62548)

* removed getFieldFormats from aggConfig

* made new properties is private

* Fixed ci

* Fixed tests

* Fixes clone method

* move filedFotmats inside opt fro AggConfigs

* Added readonly

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
VladLasitsa and elasticmachine committed Apr 15, 2020
1 parent 8264352 commit 18687a3
Show file tree
Hide file tree
Showing 33 changed files with 156 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,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
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
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
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 readonly fieldFormats: FieldFormatsStart;

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
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

0 comments on commit 18687a3

Please sign in to comment.