From 4009edc3ddc86b840e4cc2933b84b9920d788918 Mon Sep 17 00:00:00 2001 From: Matthew Kime Date: Thu, 19 Nov 2020 07:45:45 -0600 Subject: [PATCH] [index patterns] improve index pattern cache (#83368) * cache index pattern promise, not index pattern --- .../index_patterns/_pattern_cache.ts | 4 +- .../index_patterns/index_patterns.test.ts | 49 ++++++++++++++++--- .../index_patterns/index_patterns.ts | 32 +++++++----- 3 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/plugins/data/common/index_patterns/index_patterns/_pattern_cache.ts b/src/plugins/data/common/index_patterns/index_patterns/_pattern_cache.ts index a3653bb529fa3..19fe7c7c26c79 100644 --- a/src/plugins/data/common/index_patterns/index_patterns/_pattern_cache.ts +++ b/src/plugins/data/common/index_patterns/index_patterns/_pattern_cache.ts @@ -20,8 +20,8 @@ import { IndexPattern } from './index_pattern'; export interface PatternCache { - get: (id: string) => IndexPattern; - set: (id: string, value: IndexPattern) => IndexPattern; + get: (id: string) => Promise | undefined; + set: (id: string, value: Promise) => Promise; clear: (id: string) => void; clearAll: () => void; } diff --git a/src/plugins/data/common/index_patterns/index_patterns/index_patterns.test.ts b/src/plugins/data/common/index_patterns/index_patterns/index_patterns.test.ts index b22437ebbdb4e..bf227615f76a1 100644 --- a/src/plugins/data/common/index_patterns/index_patterns/index_patterns.test.ts +++ b/src/plugins/data/common/index_patterns/index_patterns/index_patterns.test.ts @@ -40,6 +40,7 @@ function setDocsourcePayload(id: string | null, providedPayload: any) { describe('IndexPatterns', () => { let indexPatterns: IndexPatternsService; let savedObjectsClient: SavedObjectsClientCommon; + let SOClientGetDelay = 0; beforeEach(() => { const indexPatternObj = { id: 'id', version: 'a', attributes: { title: 'title' } }; @@ -49,11 +50,14 @@ describe('IndexPatterns', () => { ); savedObjectsClient.delete = jest.fn(() => Promise.resolve({}) as Promise); savedObjectsClient.create = jest.fn(); - savedObjectsClient.get = jest.fn().mockImplementation(async (type, id) => ({ - id: object.id, - version: object.version, - attributes: object.attributes, - })); + savedObjectsClient.get = jest.fn().mockImplementation(async (type, id) => { + await new Promise((resolve) => setTimeout(resolve, SOClientGetDelay)); + return { + id: object.id, + version: object.version, + attributes: object.attributes, + }; + }); savedObjectsClient.update = jest .fn() .mockImplementation(async (type, id, body, { version }) => { @@ -87,6 +91,7 @@ describe('IndexPatterns', () => { }); test('does cache gets for the same id', async () => { + SOClientGetDelay = 1000; const id = '1'; setDocsourcePayload(id, { id: 'foo', @@ -96,10 +101,17 @@ describe('IndexPatterns', () => { }, }); - const indexPattern = await indexPatterns.get(id); + // make two requests before first can complete + const indexPatternPromise = indexPatterns.get(id); + indexPatterns.get(id); - expect(indexPattern).toBeDefined(); - expect(indexPattern).toBe(await indexPatterns.get(id)); + indexPatternPromise.then((indexPattern) => { + expect(savedObjectsClient.get).toBeCalledTimes(1); + expect(indexPattern).toBeDefined(); + }); + + expect(await indexPatternPromise).toBe(await indexPatterns.get(id)); + SOClientGetDelay = 0; }); test('savedObjectCache pre-fetches only title', async () => { @@ -211,4 +223,25 @@ describe('IndexPatterns', () => { expect(indexPatterns.savedObjectToSpec(savedObject)).toMatchSnapshot(); }); + + test('failed requests are not cached', async () => { + savedObjectsClient.get = jest + .fn() + .mockImplementation(async (type, id) => { + return { + id: object.id, + version: object.version, + attributes: object.attributes, + }; + }) + .mockRejectedValueOnce({}); + + const id = '1'; + + // failed request! + expect(indexPatterns.get(id)).rejects.toBeDefined(); + + // successful subsequent request + expect(async () => await indexPatterns.get(id)).toBeDefined(); + }); }); diff --git a/src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts b/src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts index 4f91079c1e139..d51de220111e3 100644 --- a/src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts +++ b/src/plugins/data/common/index_patterns/index_patterns/index_patterns.ts @@ -356,17 +356,7 @@ export class IndexPatternsService { }; }; - /** - * Get an index pattern by id. Cache optimized - * @param id - */ - - get = async (id: string): Promise => { - const cache = indexPatternCache.get(id); - if (cache) { - return cache; - } - + private getSavedObjectAndInit = async (id: string): Promise => { const savedObject = await this.savedObjectsClient.get( savedObjectType, id @@ -422,7 +412,6 @@ export class IndexPatternsService { : {}; const indexPattern = await this.create(spec, true); - indexPatternCache.set(id, indexPattern); if (isSaveRequired) { try { this.updateSavedObject(indexPattern); @@ -444,6 +433,23 @@ export class IndexPatternsService { return indexPattern; }; + /** + * Get an index pattern by id. Cache optimized + * @param id + */ + + get = async (id: string): Promise => { + const indexPatternPromise = + indexPatternCache.get(id) || indexPatternCache.set(id, this.getSavedObjectAndInit(id)); + + // don't cache failed requests + indexPatternPromise.catch(() => { + indexPatternCache.clear(id); + }); + + return indexPatternPromise; + }; + /** * Create a new index pattern instance * @param spec @@ -502,7 +508,7 @@ export class IndexPatternsService { id: indexPattern.id, }); indexPattern.id = response.id; - indexPatternCache.set(indexPattern.id, indexPattern); + indexPatternCache.set(indexPattern.id, Promise.resolve(indexPattern)); return indexPattern; }