From 1ed552625c961da85ea42177dc22031ace0311af Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Wed, 18 Dec 2019 11:36:05 +0300 Subject: [PATCH 1/3] Kibana 7.0.0 URL field formatter doesn't render relative hyperlinks properly --- .../stubbed_logstash_index_pattern.js | 11 ++-- .../public/table_vis_controller.test.ts | 2 +- .../ui/public/agg_types/buckets/terms.ts | 9 +--- .../new_platform/new_platform.karma_mock.js | 19 ++++--- .../saved_objects/__tests__/saved_object.js | 6 +-- .../loader/pipeline_helpers/utilities.ts | 53 ++++++------------- .../content_types/html_content_type.ts | 19 +++---- .../common/field_formats/converters/source.ts | 4 +- .../field_formats/converters/url.test.ts | 46 ++++++++-------- .../common/field_formats/converters/url.ts | 14 +++-- .../data/common/field_formats/field_format.ts | 24 +++++++-- .../data/common/field_formats/types.ts | 18 ++++--- .../field_formats_provider/field_formats.ts | 38 ++++++++++--- .../field_formats_service.ts | 14 ++--- .../index_patterns/format_hit.ts | 17 +++--- src/test_utils/public/stub_field_formats.ts | 6 +-- src/test_utils/public/stub_index_pattern.js | 4 +- 17 files changed, 160 insertions(+), 144 deletions(-) diff --git a/src/fixtures/stubbed_logstash_index_pattern.js b/src/fixtures/stubbed_logstash_index_pattern.js index 22fbf0ab5a5f..e20d1b5cd771 100644 --- a/src/fixtures/stubbed_logstash_index_pattern.js +++ b/src/fixtures/stubbed_logstash_index_pattern.js @@ -21,7 +21,7 @@ import StubIndexPattern from 'test_utils/stub_index_pattern'; import stubbedLogstashFields from 'fixtures/logstash_fields'; import { getKbnFieldType } from '../plugins/data/common'; -import { mockUiSettings } from '../legacy/ui/public/new_platform/new_platform.karma_mock'; +import { npSetup } from '../legacy/ui/public/new_platform/new_platform.karma_mock'; export default function stubbedLogstashIndexPatternService() { const mockLogstashFields = stubbedLogstashFields(); @@ -41,13 +41,8 @@ export default function stubbedLogstashIndexPatternService() { }; }); - const indexPattern = new StubIndexPattern( - 'logstash-*', - cfg => cfg, - 'time', - fields, - mockUiSettings - ); + const indexPattern = new StubIndexPattern('logstash-*', cfg => cfg, 'time', fields, npSetup.core); + indexPattern.id = 'logstash-*'; indexPattern.isTimeNanosBased = () => false; diff --git a/src/legacy/core_plugins/vis_type_table/public/table_vis_controller.test.ts b/src/legacy/core_plugins/vis_type_table/public/table_vis_controller.test.ts index a2f98b8c64e5..1f6600ea56a1 100644 --- a/src/legacy/core_plugins/vis_type_table/public/table_vis_controller.test.ts +++ b/src/legacy/core_plugins/vis_type_table/public/table_vis_controller.test.ts @@ -110,7 +110,7 @@ describe('Table Vis - Controller', () => { (cfg: any) => cfg, 'time', stubFields, - npStart.core.uiSettings + npStart.core ); }); diff --git a/src/legacy/ui/public/agg_types/buckets/terms.ts b/src/legacy/ui/public/agg_types/buckets/terms.ts index e38f7ca4cc03..ef9ceb96b005 100644 --- a/src/legacy/ui/public/agg_types/buckets/terms.ts +++ b/src/legacy/ui/public/agg_types/buckets/terms.ts @@ -17,7 +17,6 @@ * under the License. */ -import chrome from 'ui/chrome'; import { noop } from 'lodash'; import { i18n } from '@kbn/i18n'; import { SearchSource, getRequestInspectorStats, getResponseInspectorStats } from '../../courier'; @@ -80,14 +79,8 @@ export const termsBucketAgg = new BucketAggType({ if (val === '__missing__') { return bucket.params.missingBucketLabel; } - const parsedUrl = { - origin: window.location.origin, - pathname: window.location.pathname, - basePath: chrome.getBasePath(), - }; - const converter = bucket.params.field.format.getConverterFor(type); - return converter(val, undefined, undefined, parsedUrl); + return bucket.params.field.format.convert(val, type); }; }, } as FieldFormat; 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 index 3b2a77cefb73..3d4292cef27f 100644 --- a/src/legacy/ui/public/new_platform/new_platform.karma_mock.js +++ b/src/legacy/ui/public/new_platform/new_platform.karma_mock.js @@ -45,11 +45,18 @@ export const mockUiSettings = { 'format:defaultTypeMap': {}, }; -export const npSetup = { - core: { - chrome: {}, - uiSettings: mockUiSettings, +const mockCore = { + chrome: {}, + uiSettings: mockUiSettings, + http: { + basePath: { + get: sinon.fake.returns(''), + }, }, +}; + +export const npSetup = { + core: mockCore, plugins: { usageCollection: { allowTrackUserAgent: sinon.fake(), @@ -95,7 +102,7 @@ export const npSetup = { getSavedQueryCount: sinon.fake(), }, }, - fieldFormats: getFieldFormatsRegistry(mockUiSettings), + fieldFormats: getFieldFormatsRegistry(mockCore), }, share: { register: () => {}, @@ -224,7 +231,7 @@ export const npStart = { history: sinon.fake(), }, }, - fieldFormats: getFieldFormatsRegistry(mockUiSettings), + fieldFormats: getFieldFormatsRegistry(mockCore), }, share: { toggleShareContextMenu: () => {}, diff --git a/src/legacy/ui/public/saved_objects/__tests__/saved_object.js b/src/legacy/ui/public/saved_objects/__tests__/saved_object.js index 30fb0c0c38ae..d9622ac3dc6d 100644 --- a/src/legacy/ui/public/saved_objects/__tests__/saved_object.js +++ b/src/legacy/ui/public/saved_objects/__tests__/saved_object.js @@ -26,7 +26,7 @@ import { createSavedObjectClass } from '../saved_object'; import StubIndexPattern from 'test_utils/stub_index_pattern'; import { npStart } from 'ui/new_platform'; import { InvalidJSONProperty } from '../../../../../plugins/kibana_utils/public'; -import { mockUiSettings } from '../../new_platform/new_platform.karma_mock'; +import { npSetup } from '../../new_platform/new_platform.karma_mock'; const getConfig = cfg => cfg; @@ -310,7 +310,7 @@ describe('Saved Object', function() { getConfig, null, [], - mockUiSettings + npSetup.core ); indexPattern.title = indexPattern.id; savedObject.searchSource.setField('index', indexPattern); @@ -700,7 +700,7 @@ describe('Saved Object', function() { getConfig, null, [], - mockUiSettings + npSetup.core ); indexPattern.title = indexPattern.id; savedObject.searchSource.setField('index', indexPattern); diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts index d754c1d39559..0b99810a85af 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/utilities.ts @@ -23,7 +23,7 @@ import { AggConfig, Vis } from 'ui/vis'; import { npStart } from 'ui/new_platform'; import { SerializedFieldFormat } from 'src/plugins/expressions/public'; -import { IFieldFormatId, FieldFormat } from '../../../../../../plugins/data/public'; +import { IFieldFormatId, FieldFormat, ContentType } from '../../../../../../plugins/data/public'; import { tabifyGetColumns } from '../../../agg_response/tabify/_get_columns'; import { DateRangeKey, convertDateRangeToString } from '../../../agg_types/buckets/date_range'; @@ -129,42 +129,23 @@ export const getFormat: FormatFactory = mapping => { }); return new IpRangeFormat(); } else if (isTermsFieldFormat(mapping) && mapping.params) { - const params = mapping.params; + const { params } = mapping; + const convert = (val: string, type: ContentType) => { + const format = getFieldFormat(params.id, mapping.params); + + if (val === '__other__') { + return params.otherBucketLabel; + } + if (val === '__missing__') { + return params.missingBucketLabel; + } + + return format.convert(val, type); + }; + return { - getConverterFor: (type: string) => { - const format = getFieldFormat(params.id, mapping.params); - return (val: string) => { - if (val === '__other__') { - return params.otherBucketLabel; - } - if (val === '__missing__') { - return params.missingBucketLabel; - } - const parsedUrl = { - origin: window.location.origin, - pathname: window.location.pathname, - basePath: npStart.core.http.basePath, - }; - // @ts-ignore - return format.convert(val, undefined, undefined, parsedUrl); - }; - }, - convert: (val: string, type: string) => { - const format = getFieldFormat(params.id, mapping.params); - if (val === '__other__') { - return params.otherBucketLabel; - } - if (val === '__missing__') { - return params.missingBucketLabel; - } - const parsedUrl = { - origin: window.location.origin, - pathname: window.location.pathname, - basePath: npStart.core.http.basePath, - }; - // @ts-ignore - return format.convert(val, type, undefined, parsedUrl); - }, + convert, + getConverterFor: (type: ContentType) => (val: string) => convert(val, type), } as FieldFormat; } else { return getFieldFormat(id, mapping.params); diff --git a/src/plugins/data/common/field_formats/content_types/html_content_type.ts b/src/plugins/data/common/field_formats/content_types/html_content_type.ts index ba2236c41790..1b6ee9e63fad 100644 --- a/src/plugins/data/common/field_formats/content_types/html_content_type.ts +++ b/src/plugins/data/common/field_formats/content_types/html_content_type.ts @@ -26,7 +26,8 @@ const getConvertFn = ( format: IFieldFormat, convert?: HtmlContextTypeConvert ): HtmlContextTypeConvert => { - const fallbackHtml: HtmlContextTypeConvert = (value, field, hit) => { + const fallbackHtml: HtmlContextTypeConvert = (value, options = {}) => { + const { field, hit } = options; const formatted = escape(format.convert(value, 'text')); return !field || !hit || !hit.highlight || !hit.highlight[field.name] @@ -43,27 +44,23 @@ export const setup = ( ): HtmlContextTypeConvert => { const convert = getConvertFn(format, htmlContextTypeConvert); - const recurse: HtmlContextTypeConvert = (value, field, hit, meta) => { + const recurse: HtmlContextTypeConvert = (value, options = {}) => { if (value == null) { return asPrettyString(value); } if (!value || !isFunction(value.map)) { - return convert.call(format, value, field, hit, meta); + return convert.call(format, value, options); } - const subValues = value.map((v: any) => { - return recurse(v, field, hit, meta); - }); - const useMultiLine = subValues.some((sub: string) => { - return sub.indexOf('\n') > -1; - }); + const subValues = value.map((v: any) => recurse(v, options)); + const useMultiLine = subValues.some((sub: string) => sub.indexOf('\n') > -1); return subValues.join(',' + (useMultiLine ? '\n' : ' ')); }; - const wrap: HtmlContextTypeConvert = (value, field, hit, meta) => { - return `${recurse(value, field, hit, meta)}`; + const wrap: HtmlContextTypeConvert = (value, options) => { + return `${recurse(value, options)}`; }; return wrap; diff --git a/src/plugins/data/common/field_formats/converters/source.ts b/src/plugins/data/common/field_formats/converters/source.ts index c9906fb13605..702e1579e945 100644 --- a/src/plugins/data/common/field_formats/converters/source.ts +++ b/src/plugins/data/common/field_formats/converters/source.ts @@ -42,7 +42,9 @@ export class SourceFormat extends FieldFormat { textConvert: TextContextTypeConvert = value => JSON.stringify(value); - htmlConvert: HtmlContextTypeConvert = (value, field, hit) => { + htmlConvert: HtmlContextTypeConvert = (value, options = {}) => { + const { field, hit } = options; + if (!field) { const converter = this.getConverterFor('text') as Function; diff --git a/src/plugins/data/common/field_formats/converters/url.test.ts b/src/plugins/data/common/field_formats/converters/url.test.ts index 66307cefe08f..b1107d46179b 100644 --- a/src/plugins/data/common/field_formats/converters/url.test.ts +++ b/src/plugins/data/common/field_formats/converters/url.test.ts @@ -169,64 +169,64 @@ describe('UrlFormat', () => { describe('whitelist', () => { test('should assume a relative url if the value is not in the whitelist without a base path', () => { - const url = new UrlFormat({}); const parsedUrl = { origin: 'http://kibana', basePath: '', }; + const url = new UrlFormat({ parsedUrl }); const converter = url.getConverterFor(HTML_CONTEXT_TYPE) as Function; - expect(converter('www.elastic.co', null, null, parsedUrl)).toBe( + expect(converter('www.elastic.co')).toBe( 'www.elastic.co' ); - expect(converter('elastic.co', null, null, parsedUrl)).toBe( + expect(converter('elastic.co')).toBe( 'elastic.co' ); - expect(converter('elastic', null, null, parsedUrl)).toBe( + expect(converter('elastic')).toBe( 'elastic' ); - expect(converter('ftp://elastic.co', null, null, parsedUrl)).toBe( + expect(converter('ftp://elastic.co')).toBe( 'ftp://elastic.co' ); }); test('should assume a relative url if the value is not in the whitelist with a basepath', () => { - const url = new UrlFormat({}); const parsedUrl = { origin: 'http://kibana', basePath: '/xyz', }; + const url = new UrlFormat({ parsedUrl }); const converter = url.getConverterFor(HTML_CONTEXT_TYPE) as Function; - expect(converter('www.elastic.co', null, null, parsedUrl)).toBe( + expect(converter('www.elastic.co')).toBe( 'www.elastic.co' ); - expect(converter('elastic.co', null, null, parsedUrl)).toBe( + expect(converter('elastic.co')).toBe( 'elastic.co' ); - expect(converter('elastic', null, null, parsedUrl)).toBe( + expect(converter('elastic')).toBe( 'elastic' ); - expect(converter('ftp://elastic.co', null, null, parsedUrl)).toBe( + expect(converter('ftp://elastic.co')).toBe( 'ftp://elastic.co' ); }); test('should rely on parsedUrl', () => { - const url = new UrlFormat({}); const parsedUrl = { origin: 'http://kibana.host.com', basePath: '/abc', }; + const url = new UrlFormat({ parsedUrl }); const converter = url.getConverterFor(HTML_CONTEXT_TYPE) as Function; - expect(converter('../app/kibana', null, null, parsedUrl)).toBe( + expect(converter('../app/kibana')).toBe( '../app/kibana' ); }); @@ -244,54 +244,52 @@ describe('UrlFormat', () => { }); test('should support multiple types of relative urls', () => { - const url = new UrlFormat({}); const parsedUrl = { origin: 'http://kibana.host.com', pathname: '/nbc/app/kibana#/discover', basePath: '/nbc', }; + const url = new UrlFormat({ parsedUrl }); const converter = url.getConverterFor(HTML_CONTEXT_TYPE) as Function; - expect(converter('#/foo', null, null, parsedUrl)).toBe( + expect(converter('#/foo')).toBe( '#/foo' ); - expect(converter('/nbc/app/kibana#/discover', null, null, parsedUrl)).toBe( + expect(converter('/nbc/app/kibana#/discover')).toBe( '/nbc/app/kibana#/discover' ); - expect(converter('../foo/bar', null, null, parsedUrl)).toBe( + expect(converter('../foo/bar')).toBe( '../foo/bar' ); }); test('should support multiple types of urls w/o basePath', () => { - const url = new UrlFormat({}); const parsedUrl = { origin: 'http://kibana.host.com', pathname: '/app/kibana', }; + const url = new UrlFormat({ parsedUrl }); const converter = url.getConverterFor(HTML_CONTEXT_TYPE) as Function; - expect(converter('10.22.55.66', null, null, parsedUrl)).toBe( + expect(converter('10.22.55.66')).toBe( '10.22.55.66' ); - expect( - converter('http://www.domain.name/app/kibana#/dashboard/', null, null, parsedUrl) - ).toBe( + expect(converter('http://www.domain.name/app/kibana#/dashboard/')).toBe( 'http://www.domain.name/app/kibana#/dashboard/' ); - expect(converter('/app/kibana', null, null, parsedUrl)).toBe( + expect(converter('/app/kibana')).toBe( '/app/kibana' ); - expect(converter('kibana#/dashboard/', null, null, parsedUrl)).toBe( + expect(converter('kibana#/dashboard/')).toBe( 'kibana#/dashboard/' ); - expect(converter('#/dashboard/', null, null, parsedUrl)).toBe( + expect(converter('#/dashboard/')).toBe( '#/dashboard/' ); }); diff --git a/src/plugins/data/common/field_formats/converters/url.ts b/src/plugins/data/common/field_formats/converters/url.ts index bd68dedf38a6..3c88511a4c63 100644 --- a/src/plugins/data/common/field_formats/converters/url.ts +++ b/src/plugins/data/common/field_formats/converters/url.ts @@ -134,7 +134,11 @@ export class UrlFormat extends FieldFormat { textConvert: TextContextTypeConvert = value => this.formatLabel(value); - htmlConvert: HtmlContextTypeConvert = (rawValue, field, hit, parsedUrl) => { + htmlConvert: HtmlContextTypeConvert = (rawValue, options = {}) => { + const { field, hit } = options; + const { parsedUrl } = this._params; + const { basePath, pathname, origin } = parsedUrl || {}; + const url = escape(this.formatUrl(rawValue)); const label = escape(this.formatLabel(rawValue, url)); @@ -170,17 +174,17 @@ export class UrlFormat extends FieldFormat { if (!inWhitelist) { // Handles urls like: `#/discover` if (url[0] === '#') { - prefix = `${parsedUrl.origin}${parsedUrl.pathname}`; + prefix = `${origin}${pathname}`; } // Handle urls like: `/app/kibana` or `/xyz/app/kibana` - else if (url.indexOf(parsedUrl.basePath || '/') === 0) { - prefix = `${parsedUrl.origin}`; + else if (url.indexOf(basePath || '/') === 0) { + prefix = `${origin}`; } // Handle urls like: `../app/kibana` else { const prefixEnd = url[0] === '/' ? '' : '/'; - prefix = `${parsedUrl.origin}${parsedUrl.basePath || ''}/app${prefixEnd}`; + prefix = `${origin}${basePath || ''}/app${prefixEnd}`; } } diff --git a/src/plugins/data/common/field_formats/field_format.ts b/src/plugins/data/common/field_formats/field_format.ts index 85d276767b5a..50f07846a3ce 100644 --- a/src/plugins/data/common/field_formats/field_format.ts +++ b/src/plugins/data/common/field_formats/field_format.ts @@ -24,6 +24,8 @@ import { FIELD_FORMAT_IDS, FieldFormatConvert, FieldFormatConvertFunction, + HtmlContextTypeOptions, + TextContextTypeOptions, } from './types'; import { htmlContentTypeSetup, @@ -63,8 +65,20 @@ export abstract class FieldFormat { */ convertObject: FieldFormatConvert | undefined; + /** + * @property {htmlConvert} + * @protected + * have to remove the protected because of + * https://github.com/Microsoft/TypeScript/issues/17293 + */ htmlConvert: HtmlContextTypeConvert | undefined; + /** + * @property {textConvert} + * @protected + * have to remove the protected because of + * https://github.com/Microsoft/TypeScript/issues/17293 + */ textConvert: TextContextTypeConvert | undefined; /** @@ -76,7 +90,7 @@ export abstract class FieldFormat { protected readonly _params: any; protected getConfig: Function | undefined; - constructor(_params: any = {}, getConfig?: Function) { + constructor(_params: Record = {}, getConfig?: Function) { this._params = _params; if (getConfig) { @@ -94,11 +108,15 @@ export abstract class FieldFormat { * injecting into the DOM or a DOM attribute * @public */ - convert(value: any, contentType: ContentType = DEFAULT_CONTEXT_TYPE): string { + convert( + value: any, + contentType: ContentType = DEFAULT_CONTEXT_TYPE, + options?: HtmlContextTypeOptions | TextContextTypeOptions + ): string { const converter = this.getConverterFor(contentType); if (converter) { - return converter.call(this, value); + return converter.call(this, value, options); } return value; diff --git a/src/plugins/data/common/field_formats/types.ts b/src/plugins/data/common/field_formats/types.ts index fc8e6e20a1a9..dce3c66b0f88 100644 --- a/src/plugins/data/common/field_formats/types.ts +++ b/src/plugins/data/common/field_formats/types.ts @@ -24,15 +24,19 @@ export type ContentType = 'html' | 'text'; export { IFieldFormat } from './field_format'; /** @internal **/ -export type HtmlContextTypeConvert = ( - value: any, - field?: any, - hit?: Record, - meta?: any -) => string; +export interface HtmlContextTypeOptions { + field?: any; + hit?: Record; +} + +/** @internal **/ +export type HtmlContextTypeConvert = (value: any, options?: HtmlContextTypeOptions) => string; + +/** @internal **/ +export type TextContextTypeOptions = Record; /** @internal **/ -export type TextContextTypeConvert = (value: any) => string; +export type TextContextTypeConvert = (value: any, options?: TextContextTypeOptions) => string; /** @internal **/ export type FieldFormatConvertFunction = HtmlContextTypeConvert | TextContextTypeConvert; diff --git a/src/plugins/data/public/field_formats_provider/field_formats.ts b/src/plugins/data/public/field_formats_provider/field_formats.ts index 20e90b8e4a54..de25731c187a 100644 --- a/src/plugins/data/public/field_formats_provider/field_formats.ts +++ b/src/plugins/data/public/field_formats_provider/field_formats.ts @@ -18,7 +18,7 @@ */ import { forOwn, isFunction, memoize } from 'lodash'; -import { IUiSettingsClient } from 'kibana/public'; +import { IUiSettingsClient, CoreSetup } from 'kibana/public'; import { ES_FIELD_TYPES, KBN_FIELD_TYPES, @@ -33,6 +33,7 @@ export class FieldFormatRegisty { private fieldFormats: Map; private uiSettings!: IUiSettingsClient; private defaultMap: Record; + private basePath?: string; constructor() { this.fieldFormats = new Map(); @@ -41,8 +42,9 @@ export class FieldFormatRegisty { getConfig = (key: string, override?: any) => this.uiSettings.get(key, override); - init(uiSettings: IUiSettingsClient) { + init({ uiSettings, http }: CoreSetup) { this.uiSettings = uiSettings; + this.basePath = http.basePath.get(); this.parseDefaultTypeMap(this.uiSettings.get('format:defaultTypeMap')); @@ -76,7 +78,11 @@ export class FieldFormatRegisty { * @return {FieldFormat} */ getType = (formatId: IFieldFormatId): IFieldFormatType | undefined => { - return this.fieldFormats.get(formatId); + const decoratedFieldFormat: any = this.decorateFieldFormatWithParams(formatId); + + if (decoratedFieldFormat) { + return decoratedFieldFormat as IFieldFormatType; + } }; /** @@ -136,14 +142,14 @@ export class FieldFormatRegisty { * @return {FIELD_FORMATS_INSTANCES[number]} */ getInstance = memoize( - (formatId: IFieldFormatId): FieldFormat => { + (formatId: IFieldFormatId, params: Record = {}): FieldFormat => { const DerivedFieldFormat = this.getType(formatId); if (!DerivedFieldFormat) { throw new Error(`Field Format '${formatId}' not found!`); } - return new DerivedFieldFormat({}, this.getConfig); + return new DerivedFieldFormat(params, this.getConfig); } ); @@ -217,10 +223,26 @@ export class FieldFormatRegisty { } register = (fieldFormats: IFieldFormatType[]) => { - fieldFormats.forEach(fieldFormat => { - this.fieldFormats.set(fieldFormat.id, fieldFormat); - }); + fieldFormats.forEach(fieldFormat => this.fieldFormats.set(fieldFormat.id, fieldFormat)); return this; }; + + private decorateFieldFormatWithParams = (formatId: IFieldFormatId): Function | undefined => { + const concreteFieldFormat = this.fieldFormats.get(formatId); + const decorateMetaParams = (customOptions: Record = {}) => ({ + parsedUrl: { + origin: window.location.origin, + pathname: window.location.pathname, + basePath: this.basePath, + }, + ...customOptions, + }); + + if (concreteFieldFormat) { + return function(params: Record = {}, getConfig?: Function) { + return new concreteFieldFormat(decorateMetaParams(params), getConfig); + }; + } + }; } diff --git a/src/plugins/data/public/field_formats_provider/field_formats_service.ts b/src/plugins/data/public/field_formats_provider/field_formats_service.ts index ea1a8af2930b..42abeecc6fda 100644 --- a/src/plugins/data/public/field_formats_provider/field_formats_service.ts +++ b/src/plugins/data/public/field_formats_provider/field_formats_service.ts @@ -17,7 +17,7 @@ * under the License. */ -import { IUiSettingsClient } from 'src/core/public'; +import { CoreSetup } from 'src/core/public'; import { FieldFormatRegisty } from './field_formats'; import { @@ -38,19 +38,11 @@ import { UrlFormat, } from '../../common/'; -/** - * Field Format Service - * @internal - */ -interface FieldFormatsServiceDependencies { - uiSettings: IUiSettingsClient; -} - export class FieldFormatsService { private readonly fieldFormats: FieldFormatRegisty = new FieldFormatRegisty(); - public setup({ uiSettings }: FieldFormatsServiceDependencies) { - this.fieldFormats.init(uiSettings); + public setup(core: CoreSetup) { + this.fieldFormats.init(core); this.fieldFormats.register([ BoolFormat, diff --git a/src/plugins/data/public/index_patterns/index_patterns/format_hit.ts b/src/plugins/data/public/index_patterns/index_patterns/format_hit.ts index 02d61f8b32c8..59ee18b3dcb5 100644 --- a/src/plugins/data/public/index_patterns/index_patterns/format_hit.ts +++ b/src/plugins/data/public/index_patterns/index_patterns/format_hit.ts @@ -19,6 +19,7 @@ import _ from 'lodash'; import { IndexPattern } from './index_pattern'; +import { ContentType } from '../../../common'; const formattedCache = new WeakMap(); const partialFormattedCache = new WeakMap(); @@ -26,14 +27,16 @@ const partialFormattedCache = new WeakMap(); // Takes a hit, merges it with any stored/scripted fields, and with the metaFields // returns a formatted version export function formatHitProvider(indexPattern: IndexPattern, defaultFormat: any) { - function convert(hit: Record, val: any, fieldName: string, type: string = 'html') { + function convert( + hit: Record, + val: any, + fieldName: string, + type: ContentType = 'html' + ) { const field = indexPattern.fields.getByName(fieldName); - if (!field) return defaultFormat.convert(val, type); - const parsedUrl = { - origin: window.location.origin, - pathname: window.location.pathname, - }; - return field.format.getConverterFor(type)(val, field, hit, parsedUrl); + const format = field ? field.format : defaultFormat; + + return format.convert(val, type, { field, hit }); } function formatHit(hit: Record, type: string = 'html') { diff --git a/src/test_utils/public/stub_field_formats.ts b/src/test_utils/public/stub_field_formats.ts index da1a31f1cc7a..c4da5ab338e2 100644 --- a/src/test_utils/public/stub_field_formats.ts +++ b/src/test_utils/public/stub_field_formats.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { IUiSettingsClient } from 'kibana/public'; +import { CoreSetup } from 'kibana/public'; import { FieldFormatRegisty, @@ -37,7 +37,7 @@ import { UrlFormat, } from '../../plugins/data/public/'; -export const getFieldFormatsRegistry = (uiSettings: IUiSettingsClient) => { +export const getFieldFormatsRegistry = (npSetup: CoreSetup) => { const fieldFormats = new FieldFormatRegisty(); fieldFormats.register([ @@ -58,7 +58,7 @@ export const getFieldFormatsRegistry = (uiSettings: IUiSettingsClient) => { UrlFormat, ]); - fieldFormats.init(uiSettings); + fieldFormats.init(npSetup); return fieldFormats; }; diff --git a/src/test_utils/public/stub_index_pattern.js b/src/test_utils/public/stub_index_pattern.js index 14931b938732..0f1bc422e777 100644 --- a/src/test_utils/public/stub_index_pattern.js +++ b/src/test_utils/public/stub_index_pattern.js @@ -40,8 +40,8 @@ setFieldFormats({ import { getFieldFormatsRegistry } from './stub_field_formats'; -export default function StubIndexPattern(pattern, getConfig, timeField, fields, uiSettings) { - const registeredFieldFormats = getFieldFormatsRegistry(uiSettings); +export default function StubIndexPattern(pattern, getConfig, timeField, fields, npSetup) { + const registeredFieldFormats = getFieldFormatsRegistry(npSetup); this.id = pattern; this.title = pattern; From 14e9e1dfe913341289efbdccb07c33d978882ee3 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Wed, 18 Dec 2019 15:15:55 +0300 Subject: [PATCH 2/3] fix CI --- .../angular/doc_table/__tests__/lib/rows_headers.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/angular/doc_table/__tests__/lib/rows_headers.js b/src/legacy/core_plugins/kibana/public/discover/angular/doc_table/__tests__/lib/rows_headers.js index 677a067e7b3f..012f2b6061ee 100644 --- a/src/legacy/core_plugins/kibana/public/discover/angular/doc_table/__tests__/lib/rows_headers.js +++ b/src/legacy/core_plugins/kibana/public/discover/angular/doc_table/__tests__/lib/rows_headers.js @@ -50,14 +50,18 @@ describe('Doc Table', function() { // Stub `getConverterFor` for a field in the indexPattern to return mock data. // Returns `val` if provided, otherwise generates fake data for the field. fakeRowVals = getFakeRowVals('formatted', 0, mapping); - stubFieldFormatConverter = function($root, field, val = null) { - $root.indexPattern.fields.getByName(field).format.getConverterFor = () => (...args) => { + stubFieldFormatConverter = function($root, field, val) { + const convertFn = (value, type, options) => { if (val) { return val; } - const fieldName = _.get(args, '[1].name', null); + const fieldName = _.get(options, 'field.name', null); + return fakeRowVals[fieldName] || ''; }; + + $root.indexPattern.fields.getByName(field).format.convert = convertFn; + $root.indexPattern.fields.getByName(field).format.getConverterFor = () => convertFn; }; }) ); From f16f2346a2388df75bdd96feac3a9fe3eec45155 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Thu, 19 Dec 2019 14:08:22 +0300 Subject: [PATCH 3/3] fix PR comment / add tests --- .../field_formats.test.ts | 136 ++++++++++++++++++ .../field_formats_provider/field_formats.ts | 23 +-- src/test_utils/public/stub_field_formats.ts | 4 +- src/test_utils/public/stub_index_pattern.js | 4 +- 4 files changed, 155 insertions(+), 12 deletions(-) create mode 100644 src/plugins/data/public/field_formats_provider/field_formats.test.ts diff --git a/src/plugins/data/public/field_formats_provider/field_formats.test.ts b/src/plugins/data/public/field_formats_provider/field_formats.test.ts new file mode 100644 index 000000000000..e58435fc7141 --- /dev/null +++ b/src/plugins/data/public/field_formats_provider/field_formats.test.ts @@ -0,0 +1,136 @@ +/* + * 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 { CoreSetup, IUiSettingsClient } from 'kibana/public'; + +import { FieldFormatRegisty } from './field_formats'; +import { + IFieldFormatType, + PercentFormat, + BoolFormat, + StringFormat, +} from '../../common/field_formats'; +import { coreMock } from '../../../../core/public/mocks'; + +const getValueOfPrivateField = (instance: any, field: string) => instance[field]; +const getUiSettingsMock = (data: any): IUiSettingsClient['get'] => () => data; + +describe('FieldFormatRegisty', () => { + let mockCoreSetup: CoreSetup; + let fieldFormatRegisty: FieldFormatRegisty; + + beforeEach(() => { + mockCoreSetup = coreMock.createSetup(); + fieldFormatRegisty = new FieldFormatRegisty(); + }); + + test('should allows to create an instance of "FieldFormatRegisty"', () => { + expect(fieldFormatRegisty).toBeDefined(); + expect(getValueOfPrivateField(fieldFormatRegisty, 'fieldFormats')).toBeDefined(); + expect(getValueOfPrivateField(fieldFormatRegisty, 'defaultMap')).toEqual({}); + }); + + describe('init', () => { + test('should provide an public "init" method', () => { + expect(fieldFormatRegisty.init).toBeDefined(); + expect(typeof fieldFormatRegisty.init).toBe('function'); + }); + + test('should set basePath value from "init" method', () => { + fieldFormatRegisty.init(mockCoreSetup); + + expect(getValueOfPrivateField(fieldFormatRegisty, 'basePath')).toBe( + mockCoreSetup.http.basePath.get() + ); + }); + + test('should populate the "defaultMap" object', () => { + const defaultMap = { + number: { id: 'number', params: {} }, + }; + + mockCoreSetup.uiSettings.get = getUiSettingsMock(defaultMap); + fieldFormatRegisty.init(mockCoreSetup); + expect(getValueOfPrivateField(fieldFormatRegisty, 'defaultMap')).toEqual(defaultMap); + }); + }); + + describe('register', () => { + test('should provide an public "register" method', () => { + expect(fieldFormatRegisty.register).toBeDefined(); + expect(typeof fieldFormatRegisty.register).toBe('function'); + }); + + test('should register field formats', () => { + fieldFormatRegisty.register([StringFormat, BoolFormat]); + + const registeredFieldFormatters: Map = getValueOfPrivateField( + fieldFormatRegisty, + 'fieldFormats' + ); + + expect(registeredFieldFormatters.size).toBe(2); + + expect(registeredFieldFormatters.get(BoolFormat.id)).toBe(BoolFormat); + expect(registeredFieldFormatters.get(StringFormat.id)).toBe(StringFormat); + expect(registeredFieldFormatters.get(PercentFormat.id)).toBeUndefined(); + }); + }); + + describe('getType', () => { + test('should provide an public "getType" method', () => { + expect(fieldFormatRegisty.getType).toBeDefined(); + expect(typeof fieldFormatRegisty.getType).toBe('function'); + }); + + test('should return the registered type of the field format by identifier', () => { + fieldFormatRegisty.register([StringFormat]); + + expect(fieldFormatRegisty.getType(StringFormat.id)).toBeDefined(); + }); + + test('should return void if the field format type has not been registered', () => { + fieldFormatRegisty.register([BoolFormat]); + + expect(fieldFormatRegisty.getType(StringFormat.id)).toBeUndefined(); + }); + }); + + describe('fieldFormatMetaParamsDecorator', () => { + test('should set meta params for all instances of FieldFormats', () => { + fieldFormatRegisty.register([StringFormat]); + + const ConcreteFormat = fieldFormatRegisty.getType(StringFormat.id); + + expect(ConcreteFormat).toBeDefined(); + + if (ConcreteFormat) { + const stringFormat = new ConcreteFormat({ + foo: 'foo', + }); + const params = getValueOfPrivateField(stringFormat, '_params'); + + expect(params).toHaveProperty('foo'); + expect(params).toHaveProperty('parsedUrl'); + expect(params.parsedUrl).toHaveProperty('origin'); + expect(params.parsedUrl).toHaveProperty('pathname'); + expect(params.parsedUrl).toHaveProperty('basePath'); + } + }); + }); +}); diff --git a/src/plugins/data/public/field_formats_provider/field_formats.ts b/src/plugins/data/public/field_formats_provider/field_formats.ts index de25731c187a..3d60965f2e53 100644 --- a/src/plugins/data/public/field_formats_provider/field_formats.ts +++ b/src/plugins/data/public/field_formats_provider/field_formats.ts @@ -75,10 +75,10 @@ export class FieldFormatRegisty { * Get a derived FieldFormat class by its id. * * @param {IFieldFormatId} formatId - the format id - * @return {FieldFormat} + * @return {FieldFormat | void} */ - getType = (formatId: IFieldFormatId): IFieldFormatType | undefined => { - const decoratedFieldFormat: any = this.decorateFieldFormatWithParams(formatId); + getType = (formatId: IFieldFormatId): IFieldFormatType | void => { + const decoratedFieldFormat: any = this.fieldFormatMetaParamsDecorator(formatId); if (decoratedFieldFormat) { return decoratedFieldFormat as IFieldFormatType; @@ -92,12 +92,12 @@ export class FieldFormatRegisty { * * @param {KBN_FIELD_TYPES} fieldType * @param {ES_FIELD_TYPES[]} esTypes - Array of ES data types - * @return {FieldFormat} + * @return {FieldFormat | void} */ getDefaultType = ( fieldType: KBN_FIELD_TYPES, esTypes: ES_FIELD_TYPES[] - ): IFieldFormatType | undefined => { + ): IFieldFormatType | void => { const config = this.getDefaultConfig(fieldType, esTypes); return this.getType(config.id); @@ -108,9 +108,9 @@ export class FieldFormatRegisty { * using the format:defaultTypeMap config map * * @param {ES_FIELD_TYPES[]} esTypes - Array of ES data types - * @return {ES_FIELD_TYPES} + * @return {ES_FIELD_TYPES | void} */ - getTypeNameByEsTypes = (esTypes: ES_FIELD_TYPES[] | undefined): ES_FIELD_TYPES | undefined => { + getTypeNameByEsTypes = (esTypes: ES_FIELD_TYPES[] | undefined): ES_FIELD_TYPES | void => { if (!Array.isArray(esTypes)) { return; } @@ -228,7 +228,14 @@ export class FieldFormatRegisty { return this; }; - private decorateFieldFormatWithParams = (formatId: IFieldFormatId): Function | undefined => { + /** + * FieldFormat decorator - provide a one way to add meta-params for all field formatters + * + * @private + * @param {IFieldFormatId} formatId - the format id + * @return {FieldFormat | void} + */ + private fieldFormatMetaParamsDecorator = (formatId: IFieldFormatId): Function | void => { const concreteFieldFormat = this.fieldFormats.get(formatId); const decorateMetaParams = (customOptions: Record = {}) => ({ parsedUrl: { diff --git a/src/test_utils/public/stub_field_formats.ts b/src/test_utils/public/stub_field_formats.ts index c4da5ab338e2..ea46710c0dc8 100644 --- a/src/test_utils/public/stub_field_formats.ts +++ b/src/test_utils/public/stub_field_formats.ts @@ -37,7 +37,7 @@ import { UrlFormat, } from '../../plugins/data/public/'; -export const getFieldFormatsRegistry = (npSetup: CoreSetup) => { +export const getFieldFormatsRegistry = (core: CoreSetup) => { const fieldFormats = new FieldFormatRegisty(); fieldFormats.register([ @@ -58,7 +58,7 @@ export const getFieldFormatsRegistry = (npSetup: CoreSetup) => { UrlFormat, ]); - fieldFormats.init(npSetup); + fieldFormats.init(core); return fieldFormats; }; diff --git a/src/test_utils/public/stub_index_pattern.js b/src/test_utils/public/stub_index_pattern.js index 0f1bc422e777..7807821ab09b 100644 --- a/src/test_utils/public/stub_index_pattern.js +++ b/src/test_utils/public/stub_index_pattern.js @@ -40,8 +40,8 @@ setFieldFormats({ import { getFieldFormatsRegistry } from './stub_field_formats'; -export default function StubIndexPattern(pattern, getConfig, timeField, fields, npSetup) { - const registeredFieldFormats = getFieldFormatsRegistry(npSetup); +export default function StubIndexPattern(pattern, getConfig, timeField, fields, core) { + const registeredFieldFormats = getFieldFormatsRegistry(core); this.id = pattern; this.title = pattern;