Skip to content

Commit

Permalink
Cache ad-hoc data views to avoid repeated field list calls (#144465)
Browse files Browse the repository at this point in the history
* Cache ad-hoc data views to avoid repeated field list calls

Closes: #144253

* feat: removed await

* update tests

* update tests

Co-authored-by: Nodir Latipov <nodir.latypov@gmail.com>
  • Loading branch information
alexwizp and nlatipov authored Nov 3, 2022
1 parent e257e8a commit bb67548
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 14 deletions.
48 changes: 42 additions & 6 deletions src/plugins/data_views/common/data_views/data_views.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { defaults } from 'lodash';
import { defaults, get, set } from 'lodash';
import { DataViewsService, DataView } from '.';
import { fieldFormatsMock } from '@kbn/field-formats-plugin/common/mocks';

Expand All @@ -31,6 +31,10 @@ function setDocsourcePayload(id: string | null, providedPayload: any) {
object = defaults(providedPayload || {}, stubbedSavedObjectIndexPattern(id));
}

function doWithTimeout<T = unknown>(fn: () => T, timeout: number = 0): Promise<T> {
return new Promise((resolve) => setTimeout(() => resolve(fn()), timeout));
}

const savedObject = {
id: 'id',
version: 'version',
Expand Down Expand Up @@ -151,13 +155,45 @@ describe('IndexPatterns', () => {

test('does cache ad-hoc data views', async () => {
const id = '1';
const dataView = await indexPatterns.create({ id });
const gettedDataView = await indexPatterns.get(id);

expect(dataView).toBe(gettedDataView);
const createFromSpecOriginal = get(indexPatterns, 'createFromSpec');
let mockedCreateFromSpec: jest.Mock;

set(
indexPatterns,
'createFromSpec',
(mockedCreateFromSpec = jest
.fn()
.mockImplementation((spec: DataViewSpec, skipFetchFields = false, displayErrors = true) =>
doWithTimeout(
() => createFromSpecOriginal.call(indexPatterns, spec, skipFetchFields, displayErrors),
1000
)
))
);

// run creating in parallel
await Promise.all([
indexPatterns.create({ id }),
indexPatterns.create({ id }),
indexPatterns.create({ id }),
doWithTimeout(() => indexPatterns.create({ id }), 1),
doWithTimeout(() => indexPatterns.create({ id }), 10),

doWithTimeout(() => indexPatterns.get(id), 10),
doWithTimeout(() => indexPatterns.get(id), 40),
]).then((results) =>
results.forEach((value) => {
expect(value.id).toBe(id);
})
);

// tests after promise was resolved
expect((await indexPatterns.get(id)).id).toBe(id);
expect((await indexPatterns.create({ id })).id).toBe(id);

const dataView2 = await indexPatterns.create({ id });
expect(dataView2).toBe(gettedDataView);
expect(mockedCreateFromSpec).toHaveBeenCalledTimes(1);
expect(mockedCreateFromSpec).toHaveBeenCalledWith({ id: '1' }, false, true);
});

test('allowNoIndex flag preserves existing fields when index is missing', async () => {
Expand Down
15 changes: 7 additions & 8 deletions src/plugins/data_views/common/data_views/data_views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { i18n } from '@kbn/i18n';
import { PublicMethodsOf } from '@kbn/utility-types';
import type { PublicMethodsOf } from '@kbn/utility-types';
import { castEsToKbnFieldTypeName } from '@kbn/field-types';
import { FieldFormatsStartCommon, FORMATS_UI_SETTINGS } from '@kbn/field-formats-plugin/common';
import { SavedObjectNotFound } from '@kbn/kibana-utils-plugin/common';
Expand Down Expand Up @@ -939,21 +939,20 @@ export class DataViewsService {
skipFetchFields = false,
displayErrors = true
): Promise<DataView> {
const doCreate = () => this.createFromSpec(spec, skipFetchFields, displayErrors);

if (spec.id) {
const cachedDataView = spec.id ? await this.dataViewCache.get(spec.id) : undefined;
const cachedDataView = this.dataViewCache.get(spec.id);

if (cachedDataView) {
return cachedDataView;
}
}

const dataView = await this.createFromSpec(spec, skipFetchFields, displayErrors);

if (dataView.id) {
return this.dataViewCache.set(dataView.id, Promise.resolve(dataView));
return this.dataViewCache.set(spec.id, doCreate());
}

return dataView;
const dataView = await doCreate();
return this.dataViewCache.set(dataView.id!, Promise.resolve(dataView));
}

/**
Expand Down

0 comments on commit bb67548

Please sign in to comment.