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

[data.search.aggs] Remove service getters from agg types #61628

Merged
merged 24 commits into from
Apr 7, 2020
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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
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
43 changes: 43 additions & 0 deletions src/plugins/data/public/field_formats/mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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(),
start: jest.fn(),
};

mocked.setup.mockReturnValue(createSetupContractMock());
mocked.start.mockReturnValue(createStartContractMock());
alexwizp marked this conversation as resolved.
Show resolved Hide resolved
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(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the future, we can remove getters/setters altogether, but now using this approach we can save support of two ways for passing dependencies and refactor all places without any rush.

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 @@ -1094,7 +1094,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 @@ -1307,7 +1307,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 @@ -1533,7 +1533,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