[data.search.aggs] Remove service getters from agg types (AggConfig p…

…art) (elastic#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>
VladLasitsa · Apr 15, 2020 · 8b637aa · 8b637aa
1 parent 0da8f78
commit 8b637aa
Show file tree

Hide file tree

Showing 33 changed files with 156 additions and 66 deletions.
diff --git a/src/legacy/ui/public/new_platform/new_platform.karma_mock.js b/src/legacy/ui/public/new_platform/new_platform.karma_mock.js
@@ -453,6 +453,7 @@ export const npStart = {
           createAggConfigs: (indexPattern, configStates = []) => {
             return new AggConfigs(indexPattern, configStates, {
               typesRegistry: aggTypesRegistry.start(),
+              fieldFormats: getFieldFormatsRegistry(mockCoreStart),
             });
           },
           types: aggTypesRegistry.start(),

diff --git a/src/plugins/data/public/plugin.ts b/src/plugins/data/public/plugin.ts
@@ -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));

diff --git a/src/plugins/data/public/search/aggs/agg_config.test.ts b/src/plugins/data/public/search/aggs/agg_config.test.ts
@@ -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();
@@ -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',
@@ -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',
@@ -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',
@@ -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];
@@ -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);
       });
     });
@@ -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);
       });
     });
@@ -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',
@@ -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.
@@ -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);
     });
 
@@ -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',
@@ -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);
     });
 

diff --git a/src/plugins/data/public/search/aggs/agg_config.ts b/src/plugins/data/public/search/aggs/agg_config.ts
@@ -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;
@@ -35,6 +35,10 @@ export interface AggConfigOptions {
   schema?: string;
 }
 
+export interface AggConfigDependencies {
+  fieldFormats: FieldFormatsStart;
+}
+
 /**
  * @name AggConfig
  *
@@ -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;
@@ -115,6 +124,8 @@ export class AggConfig {
 
     // @ts-ignore
     this.__type = this.__type;
+
+    this.fieldFormats = fieldFormats;
   }
 
   /**
@@ -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);
   }
 

diff --git a/src/plugins/data/public/search/aggs/agg_configs.test.ts b/src/plugins/data/public/search/aggs/agg_configs.test.ts
@@ -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();
@@ -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);
     });
 
@@ -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();
@@ -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);
     });
@@ -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({
@@ -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(
@@ -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);
 
@@ -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);
 
@@ -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);
 
@@ -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);
@@ -241,6 +247,7 @@ describe('AggConfigs', () => {
 
       const ac = new AggConfigs(indexPattern, configStates, {
         typesRegistry,
+        fieldFormats,
       });
 
       const aggInfos = ac.aggs.map(aggConfig => {
@@ -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];
@@ -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];
@@ -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');
@@ -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');