Skip to content

Commit

Permalink
[data.search.aggs] Remove service getters from agg types (elastic#61628)
Browse files Browse the repository at this point in the history
* [data.search.aggs] Remove service getters from agg types

Part of elastic#60333

* new portion of changes

* pass dependencies to MetricAgg Type through constructor

* update docs

* refactoring buckets

* remove unused mockDataServices

* Remove service getters from metrics

* Some fixes

* remove temporary code

* moved notifications to the getInternalStartServices

* fixed karma lock

* update docs

* Fixed tests

* fix broken CI

* fix PR comment

* fix typo

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Uladzislau Lasitsa <[email protected]>
  • Loading branch information
3 people committed Apr 7, 2020
1 parent efc68c2 commit 2b1c217
Show file tree
Hide file tree
Showing 85 changed files with 2,596 additions and 1,862 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ search: {
intervalOptions: ({
display: string;
val: string;
enabled(agg: import("./search/aggs/buckets/_bucket_agg_type").IBucketAggConfig): boolean | "" | undefined;
enabled(agg: import("./search/aggs/buckets/bucket_agg_type").IBucketAggConfig): boolean | "" | undefined;
} | {
display: string;
val: string;
Expand Down
8 changes: 7 additions & 1 deletion src/legacy/ui/public/new_platform/new_platform.karma_mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ const mockCoreStart = {
get: sinon.fake.returns(''),
},
},
notifications: {
toasts: {},
},
i18n: {},
overlays: {},
savedObjects: {
Expand Down Expand Up @@ -164,8 +167,11 @@ const mockAggTypesRegistry = () => {
const registrySetup = registry.setup();
const aggTypes = getAggTypes({
uiSettings: mockCoreSetup.uiSettings,
notifications: mockCoreStart.notifications,
query: querySetup,
getInternalStartServices: () => ({
fieldFormats: getFieldFormatsRegistry(mockCoreStart),
notifications: mockCoreStart.notifications,
}),
});
aggTypes.buckets.forEach(type => registrySetup.registerBucket(type));
aggTypes.metrics.forEach(type => registrySetup.registerMetric(type));
Expand Down
17 changes: 4 additions & 13 deletions src/plugins/data/common/field_formats/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,14 @@
* under the License.
*/

import { FieldFormat, IFieldFormatsRegistry } from '.';

const fieldFormatMock = ({
convert: jest.fn(),
getConverterFor: jest.fn(),
getParamDefaults: jest.fn(),
param: jest.fn(),
params: jest.fn(),
toJSON: jest.fn(),
type: jest.fn(),
setupContentType: jest.fn(),
} as unknown) as FieldFormat;
import { IFieldFormatsRegistry } from '.';

export const fieldFormatsMock: IFieldFormatsRegistry = {
getByFieldType: jest.fn(),
getDefaultConfig: jest.fn(),
getDefaultInstance: jest.fn().mockImplementation(() => fieldFormatMock) as any,
getDefaultInstance: jest.fn().mockImplementation(() => ({
getConverterFor: jest.fn().mockImplementation(() => (t: string) => t),
})) as any,
getDefaultInstanceCacheResolver: jest.fn(),
getDefaultInstancePlain: jest.fn(),
getDefaultType: jest.fn(),
Expand Down
41 changes: 41 additions & 0 deletions src/plugins/data/public/field_formats/mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { FieldFormatsStart, FieldFormatsSetup, FieldFormatsService } from '.';
import { fieldFormatsMock } from '../../common/field_formats/mocks';

type FieldFormatsServiceClientContract = PublicMethodsOf<FieldFormatsService>;

const createSetupContractMock = () => fieldFormatsMock as FieldFormatsSetup;
const createStartContractMock = () => fieldFormatsMock as FieldFormatsStart;

const createMock = () => {
const mocked: jest.Mocked<FieldFormatsServiceClientContract> = {
setup: jest.fn().mockReturnValue(createSetupContractMock()),
start: jest.fn().mockReturnValue(createStartContractMock()),
};

return mocked;
};

export const fieldFormatsServiceMock = {
create: createMock,
createSetupContract: createSetupContractMock,
createStartContract: createStartContractMock,
};
8 changes: 4 additions & 4 deletions src/plugins/data/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
* under the License.
*/

import { Plugin, DataPublicPluginSetup, DataPublicPluginStart, IndexPatternsContract } from '.';
import { fieldFormatsMock } from '../common/field_formats/mocks';
import { Plugin, IndexPatternsContract } from '.';
import { fieldFormatsServiceMock } from './field_formats/mocks';
import { searchSetupMock, searchStartMock } from './search/mocks';
import { queryServiceMock } from './query/mocks';

Expand All @@ -36,7 +36,7 @@ const createSetupContract = (): Setup => {
return {
autocomplete: autocompleteMock,
search: searchSetupMock,
fieldFormats: fieldFormatsMock as DataPublicPluginSetup['fieldFormats'],
fieldFormats: fieldFormatsServiceMock.createSetupContract(),
query: querySetupMock,
};
};
Expand All @@ -49,7 +49,7 @@ const createStartContract = (): Start => {
},
autocomplete: autocompleteMock,
search: searchStartMock,
fieldFormats: fieldFormatsMock as DataPublicPluginStart['fieldFormats'],
fieldFormats: fieldFormatsServiceMock.createStartContract(),
query: queryStartMock,
ui: {
IndexPatternSelect: jest.fn(),
Expand Down
9 changes: 9 additions & 0 deletions src/plugins/data/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
DataPublicPluginStart,
DataSetupDependencies,
DataStartDependencies,
GetInternalStartServicesFn,
} from './types';
import { AutocompleteService } from './autocomplete';
import { SearchService } from './search/search_service';
Expand All @@ -47,6 +48,8 @@ import {
setQueryService,
setSearchService,
setUiSettings,
getFieldFormats,
getNotifications,
} from './services';
import { createSearchBar } from './ui/search_bar/create_search_bar';
import { esaggs } from './search/expressions';
Expand Down Expand Up @@ -100,6 +103,11 @@ export class DataPublicPlugin implements Plugin<DataPublicPluginSetup, DataPubli

expressions.registerFunction(esaggs);

const getInternalStartServices: GetInternalStartServicesFn = () => ({
fieldFormats: getFieldFormats(),
notifications: getNotifications(),
});

const queryService = this.queryService.setup({
uiSettings: core.uiSettings,
storage: this.storage,
Expand All @@ -122,6 +130,7 @@ export class DataPublicPlugin implements Plugin<DataPublicPluginSetup, DataPubli
return {
autocomplete: this.autocomplete.setup(core),
search: this.searchService.setup(core, {
getInternalStartServices,
packageInfo: this.packageInfo,
query: queryService,
}),
Expand Down
20 changes: 10 additions & 10 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { Assign } from '@kbn/utility-types';
import { Breadcrumb } from '@elastic/eui';
import { Component } from 'react';
import { CoreSetup } from 'src/core/public';
import { CoreStart } from 'kibana/public';
import { CoreStart as CoreStart_2 } from 'src/core/public';
import { CoreStart } from 'src/core/public';
import { CoreStart as CoreStart_2 } from 'kibana/public';
import { EuiButtonEmptyProps } from '@elastic/eui';
import { EuiComboBoxProps } from '@elastic/eui';
import { EuiConfirmModalProps } from '@elastic/eui';
Expand Down Expand Up @@ -632,21 +632,21 @@ export type IAggType = AggType;
// Warning: (ae-missing-release-tag) "IDataPluginServices" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
export interface IDataPluginServices extends Partial<CoreStart_2> {
export interface IDataPluginServices extends Partial<CoreStart> {
// (undocumented)
appName: string;
// (undocumented)
data: DataPublicPluginStart;
// (undocumented)
http: CoreStart_2['http'];
http: CoreStart['http'];
// (undocumented)
notifications: CoreStart_2['notifications'];
notifications: CoreStart['notifications'];
// (undocumented)
savedObjects: CoreStart_2['savedObjects'];
savedObjects: CoreStart['savedObjects'];
// (undocumented)
storage: IStorageWrapper;
// (undocumented)
uiSettings: CoreStart_2['uiSettings'];
uiSettings: CoreStart['uiSettings'];
}

// Warning: (ae-missing-release-tag) "IEsSearchRequest" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
Expand Down Expand Up @@ -1104,7 +1104,7 @@ export type ISearch<T extends TStrategyTypes = typeof DEFAULT_SEARCH_STRATEGY> =
// @public (undocumented)
export interface ISearchContext {
// (undocumented)
core: CoreStart;
core: CoreStart_2;
// (undocumented)
getSearchStrategy: <T extends TStrategyTypes>(name: T) => TSearchStrategyProvider<T>;
}
Expand Down Expand Up @@ -1317,7 +1317,7 @@ export class Plugin implements Plugin_2<DataPublicPluginSetup, DataPublicPluginS
// Warning: (ae-forgotten-export) The symbol "DataStartDependencies" needs to be exported by the entry point index.d.ts
//
// (undocumented)
start(core: CoreStart_2, { uiActions }: DataStartDependencies): DataPublicPluginStart;
start(core: CoreStart, { uiActions }: DataStartDependencies): DataPublicPluginStart;
// (undocumented)
stop(): void;
}
Expand Down Expand Up @@ -1543,7 +1543,7 @@ export const search: {
intervalOptions: ({
display: string;
val: string;
enabled(agg: import("./search/aggs/buckets/_bucket_agg_type").IBucketAggConfig): boolean | "" | undefined;
enabled(agg: import("./search/aggs/buckets/bucket_agg_type").IBucketAggConfig): boolean | "" | undefined;
} | {
display: string;
val: string;
Expand Down
15 changes: 0 additions & 15 deletions src/plugins/data/public/search/aggs/agg_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ 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 { dataPluginMock } from '../../../public/mocks';
import { setFieldFormats } from '../../../public/services';

describe('AggConfig', () => {
let indexPattern: IndexPattern;
Expand Down Expand Up @@ -400,13 +398,6 @@ describe('AggConfig', () => {

describe('#fieldFormatter - custom getFormat handler', () => {
it('returns formatter from getFormat handler', () => {
setFieldFormats({
...dataPluginMock.createStartContract().fieldFormats,
getDefaultInstance: jest.fn().mockImplementation(() => ({
getConverterFor: jest.fn().mockImplementation(() => (t: string) => t),
})) as any,
});

const ac = new AggConfigs(indexPattern, [], { typesRegistry });
const configStates = {
enabled: true,
Expand All @@ -429,12 +420,6 @@ describe('AggConfig', () => {
let aggConfig: AggConfig;

beforeEach(() => {
setFieldFormats({
...dataPluginMock.createStartContract().fieldFormats,
getDefaultInstance: jest.fn().mockImplementation(() => ({
getConverterFor: (t?: string) => t || identity,
})) as any,
});
indexPattern.fields.getByName = name =>
({
format: {
Expand Down
18 changes: 14 additions & 4 deletions src/plugins/data/public/search/aggs/agg_params.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,22 @@ import { BaseParamType } from './param_types/base';
import { FieldParamType } from './param_types/field';
import { OptionedParamType } from './param_types/optioned';
import { AggParamType } from '../aggs/param_types/agg';
import { fieldFormatsServiceMock } from '../../field_formats/mocks';
import { notificationServiceMock } from '../../../../../../src/core/public/mocks';
import { AggTypeDependencies } from './agg_type';

describe('AggParams class', () => {
const aggTypesDependencies: AggTypeDependencies = {
getInternalStartServices: () => ({
fieldFormats: fieldFormatsServiceMock.createStartContract(),
notifications: notificationServiceMock.createStartContract(),
}),
};

describe('constructor args', () => {
it('accepts an array of param defs', () => {
const params = [{ name: 'one' }, { name: 'two' }] as AggParamType[];
const aggParams = initParams(params);
const aggParams = initParams(params, aggTypesDependencies);

expect(aggParams).toHaveLength(params.length);
expect(Array.isArray(aggParams)).toBeTruthy();
Expand All @@ -37,7 +47,7 @@ describe('AggParams class', () => {
describe('AggParam creation', () => {
it('Uses the FieldParamType class for params with the name "field"', () => {
const params = [{ name: 'field', type: 'field' }] as AggParamType[];
const aggParams = initParams(params);
const aggParams = initParams(params, aggTypesDependencies);

expect(aggParams).toHaveLength(params.length);
expect(aggParams[0] instanceof FieldParamType).toBeTruthy();
Expand All @@ -50,7 +60,7 @@ describe('AggParams class', () => {
type: 'optioned',
},
] as AggParamType[];
const aggParams = initParams(params);
const aggParams = initParams(params, aggTypesDependencies);

expect(aggParams).toHaveLength(params.length);
expect(aggParams[0] instanceof OptionedParamType).toBeTruthy();
Expand All @@ -72,7 +82,7 @@ describe('AggParams class', () => {
},
] as AggParamType[];

const aggParams = initParams(params);
const aggParams = initParams(params, aggTypesDependencies);

expect(aggParams).toHaveLength(params.length);

Expand Down
6 changes: 4 additions & 2 deletions src/plugins/data/public/search/aggs/agg_params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { BaseParamType } from './param_types/base';

import { AggConfig } from './agg_config';
import { IAggConfigs } from './agg_configs';
import { AggTypeDependencies } from './agg_type';

const paramTypeMap = {
field: FieldParamType,
Expand All @@ -45,12 +46,13 @@ export interface AggParamOption {
}

export const initParams = <TAggParam extends AggParamType = AggParamType>(
params: TAggParam[]
params: TAggParam[],
{ getInternalStartServices }: AggTypeDependencies
): TAggParam[] =>
params.map((config: TAggParam) => {
const Class = paramTypeMap[config.type] || paramTypeMap._default;

return new Class(config);
return new Class(config, { getInternalStartServices });
}) as TAggParam[];

/**
Expand Down
Loading

0 comments on commit 2b1c217

Please sign in to comment.