-
Notifications
You must be signed in to change notification settings - Fork 8.6k
Sourcerer Improvements #224781
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
Sourcerer Improvements #224781
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -434,6 +434,25 @@ describe('IndexPatterns', () => { | |
| expect(savedObjectsClient.find).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| test('caches fields toSpec calls', async () => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Built on top of this PR: #224767 to test performance |
||
| const indexPatternSpec: DataViewSpec = { | ||
| runtimeFieldMap: { | ||
| a: { | ||
| type: 'keyword', | ||
| script: { | ||
| source: "emit('a');", | ||
| }, | ||
| }, | ||
| }, | ||
| title: 'test', | ||
| }; | ||
|
|
||
| const indexPattern = await indexPatterns.create(indexPatternSpec); | ||
| const firstSpec = indexPattern.fields.toSpec(); | ||
| const secondSpec = indexPattern.fields.toSpec(); | ||
| expect(firstSpec).toEqual(secondSpec); | ||
| }); | ||
|
|
||
| test('deletes the index pattern', async () => { | ||
| const id = '1'; | ||
| const indexPattern = await indexPatterns.get(id); | ||
|
|
@@ -936,5 +955,25 @@ describe('IndexPatterns', () => { | |
| // @ts-expect-error | ||
| expect(apiClient.getFieldsForWildcard.mock.calls[0][0].allowNoIndex).toBe(true); | ||
| }); | ||
|
|
||
| test('refreshFields should return a new fields spec instance', async () => { | ||
| const indexPatternSpec: DataViewSpec = { | ||
| runtimeFieldMap: { | ||
| a: { | ||
| type: 'keyword', | ||
| script: { | ||
| source: "emit('a');", | ||
| }, | ||
| }, | ||
| }, | ||
| title: 'test', | ||
| }; | ||
|
|
||
| const indexPattern = await indexPatterns.create(indexPatternSpec); | ||
| const originalFieldsSpec = indexPattern.fields.toSpec(); | ||
|
|
||
| await indexPatterns.refreshFields(indexPattern); | ||
| expect(indexPattern.fields.toSpec()).not.toBe(originalFieldsSpec); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,245 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| import { fieldList, IIndexPatternFieldList } from './field_list'; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Built on top of this PR: #224767 to test performance |
||
| import { DataViewField } from './data_view_field'; | ||
| import type { FieldSpec } from '../types'; | ||
|
|
||
| const baseField: FieldSpec = { | ||
| name: 'foo', | ||
| type: 'string', | ||
| esTypes: ['keyword'], | ||
| aggregatable: true, | ||
| searchable: true, | ||
| scripted: false, | ||
| }; | ||
|
|
||
| const anotherField: FieldSpec = { | ||
| name: 'bar', | ||
| type: 'number', | ||
| esTypes: ['long'], | ||
| aggregatable: false, | ||
| searchable: false, | ||
| scripted: false, | ||
| count: 5, | ||
| customLabel: 'Bar Label', | ||
| customDescription: 'Bar Description', | ||
| conflictDescriptions: { idx1: ['type1', 'type2'] }, | ||
| readFromDocValues: true, | ||
| indexed: true, | ||
| isMapped: true, | ||
| }; | ||
|
|
||
| const scriptedField: FieldSpec = { | ||
| name: 'scripted', | ||
| type: 'number', | ||
| esTypes: ['long'], | ||
| aggregatable: false, | ||
| searchable: false, | ||
| scripted: true, | ||
| script: 'doc["bar"].value + 1', | ||
| lang: 'painless', | ||
| }; | ||
|
|
||
| describe('fieldList', () => { | ||
| let list: IIndexPatternFieldList; | ||
|
|
||
| beforeEach(() => { | ||
| list = fieldList([baseField], false); | ||
| }); | ||
|
|
||
| it('creates a field list with initial fields', () => { | ||
| expect(list.length).toBe(1); | ||
| expect(list[0]).toBeInstanceOf(DataViewField); | ||
| expect(list[0].name).toBe('foo'); | ||
| expect(list[0].type).toBe('string'); | ||
| expect(list[0].esTypes).toEqual(['keyword']); | ||
| expect(list[0].aggregatable).toBe(true); | ||
| expect(list[0].searchable).toBe(true); | ||
| expect(list[0].scripted).toBe(false); | ||
| }); | ||
|
|
||
| it('creates a DataViewField instance with create()', () => { | ||
| const field = list.create(anotherField); | ||
| expect(field).toBeInstanceOf(DataViewField); | ||
| expect(field.name).toBe('bar'); | ||
| expect(field.type).toBe('number'); | ||
| expect(field.customLabel).toBe('Bar Label'); | ||
| expect(field.customDescription).toBe('Bar Description'); | ||
| expect(field.conflictDescriptions).toEqual({ idx1: ['type1', 'type2'] }); | ||
| expect(field.count).toBe(5); | ||
| expect(field.readFromDocValues).toBe(true); | ||
| expect(field.isMapped).toBe(true); | ||
| }); | ||
|
|
||
| it('adds a field with add()', () => { | ||
| const added = list.add(anotherField); | ||
| expect(list.length).toBe(2); | ||
| expect(list.getByName('bar')).toBe(added); | ||
| expect(list.getByType('number')).toContain(added); | ||
| expect(list.getByType('string')[0].name).toBe('foo'); | ||
| }); | ||
|
|
||
| it('gets all fields with getAll()', () => { | ||
| list.add(anotherField); | ||
| const all = list.getAll(); | ||
| expect(all.length).toBe(2); | ||
| expect(all[0]).toBeInstanceOf(DataViewField); | ||
| expect(all[1]).toBeInstanceOf(DataViewField); | ||
| expect(all.map((f) => f.name)).toEqual(['foo', 'bar']); | ||
| }); | ||
|
|
||
| it('gets a field by name with getByName()', () => { | ||
| expect(list.getByName('foo')).toBeInstanceOf(DataViewField); | ||
| expect(list.getByName('foo')!.name).toBe('foo'); | ||
| expect(list.getByName('notfound')).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('gets fields by type with getByType()', () => { | ||
| list.add(anotherField); | ||
| const stringFields = list.getByType('string'); | ||
| expect(stringFields.length).toBe(1); | ||
| expect(stringFields[0].name).toBe('foo'); | ||
| const numberFields = list.getByType('number'); | ||
| expect(numberFields.length).toBe(1); | ||
| expect(numberFields[0].name).toBe('bar'); | ||
| const notFound = list.getByType('boolean'); | ||
| expect(notFound.length).toBe(0); | ||
| }); | ||
|
|
||
| it('removes a field with remove()', () => { | ||
| const field = list.getByName('foo')!; | ||
| list.remove(field); | ||
| expect(list.length).toBe(0); | ||
| expect(list.getByName('foo')).toBeUndefined(); | ||
| expect(list.getByType('string').length).toBe(0); | ||
| }); | ||
|
|
||
| it('updates a field with update()', () => { | ||
| list.add(anotherField); | ||
| const updatedField: FieldSpec = { | ||
| ...anotherField, | ||
| aggregatable: true, | ||
| name: 'bar', | ||
| customLabel: 'Updated Label', | ||
| }; | ||
| list.update(updatedField); | ||
| const field = list.getByName('bar'); | ||
| expect(field).toBeInstanceOf(DataViewField); | ||
| expect(field!.aggregatable).toBe(true); | ||
| expect(field!.customLabel).toBe('Updated Label'); | ||
| }); | ||
|
|
||
| it('removes all fields with removeAll()', () => { | ||
| list.add(anotherField); | ||
| list.removeAll(); | ||
| expect(list.length).toBe(0); | ||
| expect(list.getAll().length).toBe(0); | ||
| expect(list.getByName('foo')).toBeUndefined(); | ||
| expect(list.getByType('string').length).toBe(0); | ||
| }); | ||
|
|
||
| it('replaces all fields with replaceAll()', () => { | ||
| list.replaceAll([anotherField, scriptedField]); | ||
| expect(list.length).toBe(2); | ||
| expect(list[0].name).toBe('bar'); | ||
| expect(list[1].name).toBe('scripted'); | ||
| expect(list.getByName('foo')).toBeUndefined(); | ||
| expect(list.getByName('bar')).toBeInstanceOf(DataViewField); | ||
| expect(list.getByName('scripted')).toBeInstanceOf(DataViewField); | ||
| expect(list.getByName('scripted')?.scripted).toBe(true); | ||
| expect(list.getByName('scripted')?.script).toBe('doc["bar"].value + 1'); | ||
| expect(list.getByName('scripted')?.lang).toBe('painless'); | ||
| }); | ||
|
|
||
| it('caches and clears toSpec()', () => { | ||
| const spy = jest.spyOn(list[0], 'toSpec'); | ||
| // First call, not cached | ||
| const spec1 = list.toSpec(); | ||
| expect(spec1.foo).toBeDefined(); | ||
| expect(spy).toHaveBeenCalled(); | ||
|
|
||
| // Second call, should be cached (no new calls to toSpec) | ||
| spy.mockClear(); | ||
| const spec2 = list.toSpec(); | ||
| expect(spec2.foo).toBeDefined(); | ||
| expect(spy).not.toHaveBeenCalled(); | ||
|
|
||
| // After add, cache should be cleared | ||
| list.add(anotherField); | ||
| const spec3 = list.toSpec(); | ||
| expect(spec3.bar).toBeDefined(); | ||
| }); | ||
|
|
||
| it('calls toSpec with getFormatterForField', () => { | ||
| const getFormatterForField = jest.fn().mockReturnValue({ toJSON: () => ({ id: 'test' }) }); | ||
| list.toSpec({ getFormatterForField }); | ||
| expect(getFormatterForField).toHaveBeenCalledWith(list[0]); | ||
| }); | ||
|
|
||
| it('handles remove on non-existent field gracefully', () => { | ||
| expect(list.length).toBe(1); | ||
| const fakeField = new DataViewField({ ...baseField, name: 'notfound' }); | ||
| expect(() => list.remove(fakeField)).not.toThrow(); | ||
| expect(list.length).toBe(1); | ||
| }); | ||
|
|
||
| it('handles update on non-existent field gracefully', () => { | ||
| const fakeField: FieldSpec = { ...baseField, name: 'notfound' }; | ||
| expect(() => list.update(fakeField)).not.toThrow(); | ||
| expect(list.length).toBe(1); | ||
| }); | ||
|
|
||
| it('handles replaceAll with empty array', () => { | ||
| list.replaceAll([]); | ||
| expect(list.length).toBe(0); | ||
| }); | ||
|
|
||
| it('supports adding and updating a scripted field', () => { | ||
| list.add(scriptedField); | ||
| const field = list.getByName('scripted'); | ||
| expect(field).toBeInstanceOf(DataViewField); | ||
| expect(field?.scripted).toBe(true); | ||
| expect(field?.script).toBe('doc["bar"].value + 1'); | ||
| expect(field?.lang).toBe('painless'); | ||
|
|
||
| // Update scripted field | ||
| const updatedScripted: FieldSpec = { | ||
| ...scriptedField, | ||
| script: 'doc["bar"].value + 2', | ||
| lang: 'expression', | ||
| }; | ||
| list.update(updatedScripted); | ||
| const updated = list.getByName('scripted'); | ||
| expect(updated?.script).toBe('doc["bar"].value + 2'); | ||
| expect(updated?.lang).toBe('expression'); | ||
| }); | ||
|
|
||
| it('does not throw if remove is called on a field not in the list', () => { | ||
| const field = new DataViewField({ ...baseField, name: 'notfound' }); | ||
| expect(() => list.remove(field)).not.toThrow(); | ||
| }); | ||
|
|
||
| it('does not throw if update is called on a field not in the list', () => { | ||
| const field: FieldSpec = { ...baseField, name: 'notfound' }; | ||
| expect(() => list.update(field)).not.toThrow(); | ||
| }); | ||
|
|
||
| it('preserves shortDotsEnable option', () => { | ||
| const shortDotsList = fieldList([baseField], true); | ||
| expect(shortDotsList[0].spec.shortDotsEnable).toBe(true); | ||
| }); | ||
|
|
||
| it('does not fail if add is called with a field with the same name', () => { | ||
| list.add(baseField); | ||
| expect(list.length).toBe(2); | ||
| expect(list[0].name).toBe('foo'); | ||
| expect(list[1].name).toBe('foo'); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,12 @@ export interface IIndexPatternFieldList extends Array<DataViewField> { | |
| * @returns a new data view field instance | ||
| */ | ||
| create(field: FieldSpec): DataViewField; | ||
|
|
||
| /** | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Built on top of this PR: #224767 to test performance |
||
| * Clear the cached field spec map. | ||
| * This is used to avoid recalculating the spec every time `toSpec` is called. | ||
| */ | ||
| clearSpecCache(): void; | ||
| /** | ||
| * Add field to field list. | ||
| * @param field field spec to add field to list | ||
|
|
@@ -97,6 +103,16 @@ export const fieldList = ( | |
| private removeByGroup = (field: DataViewField) => | ||
| this.groups.get(field.type)?.delete(field.name); | ||
|
|
||
| /** | ||
| * @private | ||
| * @description | ||
| * This cache is used to store the result of the `toSpec` method, which converts the field list | ||
| * to a map of field specs by name. It is initialized to null and is cleared every time a field | ||
| * is added, removed, or updated. This is done to avoid recalculating the spec every time `toSpec` | ||
| * is called, which can be expensive if the field list is large. | ||
| */ | ||
| private cachedFieldSpec: DataViewFieldMap | null = null; | ||
|
|
||
| constructor() { | ||
| super(); | ||
| specs.map((field) => this.add(field)); | ||
|
|
@@ -112,11 +128,16 @@ export const fieldList = ( | |
| return new DataViewField({ ...field, shortDotsEnable }); | ||
| }; | ||
|
|
||
| public clearSpecCache = () => { | ||
| this.cachedFieldSpec = null; | ||
| }; | ||
|
|
||
| public readonly add = (field: FieldSpec): DataViewField => { | ||
| const newField = this.create(field); | ||
| this.push(newField); | ||
| this.setByName(newField); | ||
| this.setByGroup(newField); | ||
| this.clearSpecCache(); | ||
| return newField; | ||
| }; | ||
|
|
||
|
|
@@ -125,7 +146,9 @@ export const fieldList = ( | |
| this.byName.delete(field.name); | ||
|
|
||
| const fieldIndex = findIndex(this, { name: field.name }); | ||
| if (fieldIndex === -1) return; | ||
| this.splice(fieldIndex, 1); | ||
| this.clearSpecCache(); | ||
| }; | ||
|
|
||
| public readonly update = (field: FieldSpec) => { | ||
|
|
@@ -135,12 +158,14 @@ export const fieldList = ( | |
| this.setByName(newField); | ||
| this.removeByGroup(newField); | ||
| this.setByGroup(newField); | ||
| this.clearSpecCache(); | ||
| }; | ||
|
|
||
| public readonly removeAll = () => { | ||
| this.length = 0; | ||
| this.byName.clear(); | ||
| this.groups.clear(); | ||
| this.clearSpecCache(); | ||
| }; | ||
|
|
||
| public readonly replaceAll = (spcs: FieldSpec[] = []) => { | ||
|
|
@@ -153,12 +178,17 @@ export const fieldList = ( | |
| }: { | ||
| getFormatterForField?: DataView['getFormatterForField']; | ||
| } = {}) { | ||
| return { | ||
| if (this.cachedFieldSpec) { | ||
| return this.cachedFieldSpec; | ||
| } | ||
| const toSpecResult = { | ||
| ...this.reduce<DataViewFieldMap>((collector, field) => { | ||
| collector[field.name] = field.toSpec({ getFormatterForField }); | ||
| return collector; | ||
| }, {}), | ||
| }; | ||
| this.cachedFieldSpec = toSpecResult; | ||
| return toSpecResult; | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Built on top of this PR: #224767 to test performance