Skip to content
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.

Sign up for GitHub

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

[Lens] Load indexpatterns list from indexPattern Service #91984

Merged
merged 4 commits into from
Feb 22, 2021
Merged

[Lens] Load indexpatterns list from indexPattern Service #91984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e2f39a5
[Lens] Load lists from indexPattern Service
stratoula Feb 19, 2021
2d4f030
test that indexpattern service has not been called
stratoula Feb 19, 2021
36b56ff
Use mock data
stratoula Feb 19, 2021
018da76
Merge branch 'master' into lens-load-list-indexpattern-service
kibanamachine Feb 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ export function getIndexPatternDatasource({
data: DataPublicPluginStart;
charts: ChartsPluginSetup;
}) {
const savedObjectsClient = core.savedObjects.client;
const uiSettings = core.uiSettings;
const onIndexPatternLoadError = (err: Error) =>
core.notifications.toasts.addError(err, {
Expand Down Expand Up @@ -121,7 +120,6 @@ export function getIndexPatternDatasource({
return loadInitialState({
persistedState,
references,
savedObjectsClient: await savedObjectsClient,
defaultIndexPatternId: core.uiSettings.get('defaultIndex'),
storage,
indexPatternsService,
Expand Down
58 changes: 31 additions & 27 deletions x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { HttpHandler, SavedObjectsClientContract } from 'kibana/public';
import { HttpHandler } from 'kibana/public';
import _ from 'lodash';
import {
loadInitialState,
Expand Down Expand Up @@ -183,23 +183,24 @@ const sampleIndexPatterns = {
'2': indexPattern2,
};

function mockClient() {
return ({
find: jest.fn(async () => ({
savedObjects: [
{ id: '1', attributes: { title: sampleIndexPatterns[1].title } },
{ id: '2', attributes: { title: sampleIndexPatterns[2].title } },
],
})),
} as unknown) as Pick<SavedObjectsClientContract, 'find'>;
}

function mockIndexPatternsService() {
return ({
get: jest.fn(async (id: '1' | '2') => {
return { ...sampleIndexPatternsFromService[id], metaFields: [] };
}),
} as unknown) as Pick<IndexPatternsContract, 'get'>;
getIdsWithTitle: jest.fn(async () => {
return [
{
id: sampleIndexPatterns[1].id,
title: sampleIndexPatterns[1].title,
},
{
id: sampleIndexPatterns[2].id,
title: sampleIndexPatterns[2].title,
},
];
}),
} as unknown) as Pick<IndexPatternsContract, 'get' | 'getIdsWithTitle'>;
}

describe('loader', () => {
Expand All @@ -212,7 +213,8 @@ describe('loader', () => {
get: jest.fn(() =>
Promise.reject('mockIndexPatternService.get should not have been called')
),
} as unknown) as Pick<IndexPatternsContract, 'get'>,
getIdsWithTitle: jest.fn(),
} as unknown) as Pick<IndexPatternsContract, 'get' | 'getIdsWithTitle'>,
});

expect(cache).toEqual(sampleIndexPatterns);
Expand Down Expand Up @@ -281,7 +283,11 @@ describe('loader', () => {
},
],
})),
} as unknown) as Pick<IndexPatternsContract, 'get'>,
getIdsWithTitle: jest.fn(async () => ({
id: 'foo',
title: 'Foo index',
})),
} as unknown) as Pick<IndexPatternsContract, 'get' | 'getIdsWithTitle'>,
});

expect(cache.foo.getFieldByName('bytes')!.aggregationRestrictions).toEqual({
Expand Down Expand Up @@ -333,7 +339,11 @@ describe('loader', () => {
},
],
})),
} as unknown) as Pick<IndexPatternsContract, 'get'>,
getIdsWithTitle: jest.fn(async () => ({
id: 'foo',
title: 'Foo index',
})),
} as unknown) as Pick<IndexPatternsContract, 'get' | 'getIdsWithTitle'>,
});

expect(cache.foo.getFieldByName('timestamp')!.meta).toEqual(true);
Expand All @@ -344,7 +354,6 @@ describe('loader', () => {
it('should load a default state', async () => {
const storage = createMockStorage();
const state = await loadInitialState({
savedObjectsClient: mockClient(),
indexPatternsService: mockIndexPatternsService(),
storage,
options: { isFullEditor: true },
Expand All @@ -368,10 +377,9 @@ describe('loader', () => {

it('should load a default state without loading the indexPatterns when embedded', async () => {
const storage = createMockStorage();
const savedObjectsClient = mockClient();
const indexPatternsService = mockIndexPatternsService();
const state = await loadInitialState({
savedObjectsClient,
indexPatternsService: mockIndexPatternsService(),
indexPatternsService,
storage,
options: { isFullEditor: false },
});
Expand All @@ -384,14 +392,12 @@ describe('loader', () => {
});

expect(storage.set).not.toHaveBeenCalled();

expect(savedObjectsClient.find).not.toHaveBeenCalled();
expect(indexPatternsService.getIdsWithTitle).not.toHaveBeenCalled();
});

it('should load a default state when lastUsedIndexPatternId is not found in indexPatternRefs', async () => {
const storage = createMockStorage({ indexPatternId: 'c' });
const state = await loadInitialState({
savedObjectsClient: mockClient(),
indexPatternsService: mockIndexPatternsService(),
storage,
options: { isFullEditor: true },
Expand All @@ -415,7 +421,6 @@ describe('loader', () => {

it('should load lastUsedIndexPatternId if in localStorage', async () => {
const state = await loadInitialState({
savedObjectsClient: mockClient(),
indexPatternsService: mockIndexPatternsService(),
storage: createMockStorage({ indexPatternId: '2' }),
options: { isFullEditor: true },
Expand All @@ -438,7 +443,6 @@ describe('loader', () => {
const storage = createMockStorage();
const state = await loadInitialState({
defaultIndexPatternId: '2',
savedObjectsClient: mockClient(),
indexPatternsService: mockIndexPatternsService(),
storage,
options: { isFullEditor: true },
Expand All @@ -463,7 +467,6 @@ describe('loader', () => {
it('should use the indexPatternId of the visualize trigger field, if provided', async () => {
const storage = createMockStorage();
const state = await loadInitialState({
savedObjectsClient: mockClient(),
indexPatternsService: mockIndexPatternsService(),
storage,
initialContext: {
Expand Down Expand Up @@ -524,7 +527,6 @@ describe('loader', () => {
{ name: 'indexpattern-datasource-layer-layerb', id: '2', type: 'index-pattern' },
{ name: 'another-reference', id: 'c', type: 'index-pattern' },
],
savedObjectsClient: mockClient(),
indexPatternsService: mockIndexPatternsService(),
storage,
options: { isFullEditor: true },
Expand Down Expand Up @@ -681,6 +683,7 @@ describe('loader', () => {
get: jest.fn(async () => {
throw err;
}),
getIdsWithTitle: jest.fn(),
},
onError,
storage,
Expand Down Expand Up @@ -808,6 +811,7 @@ describe('loader', () => {
get: jest.fn(async () => {
throw err;
}),
getIdsWithTitle: jest.fn(),
},
onError,
storage,
Expand Down
28 changes: 8 additions & 20 deletions x-pack/plugins/lens/public/indexpattern_datasource/loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import _ from 'lodash';
import { IStorageWrapper } from 'src/plugins/kibana_utils/public';
import { SavedObjectsClientContract, HttpSetup, SavedObjectReference } from 'kibana/public';
import { HttpSetup, SavedObjectReference } from 'kibana/public';
import { InitializationOptions, StateSetter } from '../types';
import {
IndexPattern,
Expand All @@ -30,8 +30,7 @@ import { readFromStorage, writeToStorage } from '../settings_storage';
import { getFieldByNameFactory } from './pure_helpers';

type SetState = StateSetter<IndexPatternPrivateState>;
type SavedObjectsClient = Pick<SavedObjectsClientContract, 'find'>;
type IndexPatternsService = Pick<IndexPatternsContract, 'get'>;
type IndexPatternsService = Pick<IndexPatternsContract, 'get' | 'getIdsWithTitle'>;
type ErrorHandler = (err: Error) => void;

export async function loadIndexPatterns({
Expand Down Expand Up @@ -186,7 +185,6 @@ export function injectReferences(
export async function loadInitialState({
persistedState,
references,
savedObjectsClient,
defaultIndexPatternId,
storage,
indexPatternsService,
Expand All @@ -195,15 +193,14 @@ export async function loadInitialState({
}: {
persistedState?: IndexPatternPersistedState;
references?: SavedObjectReference[];
savedObjectsClient: SavedObjectsClient;
defaultIndexPatternId?: string;
storage: IStorageWrapper;
indexPatternsService: IndexPatternsService;
initialContext?: VisualizeFieldContext;
options?: InitializationOptions;
}): Promise<IndexPatternPrivateState> {
const { isFullEditor } = options ?? {};
const indexPatternRefs = await (isFullEditor ? loadIndexPatternRefs(savedObjectsClient) : []);
const indexPatternRefs = await (isFullEditor ? loadIndexPatternRefs(indexPatternsService) : []);
const lastUsedIndexPatternId = getLastUsedIndexPatternId(storage, indexPatternRefs);

const state =
Expand Down Expand Up @@ -334,22 +331,13 @@ export async function changeLayerIndexPattern({
}

async function loadIndexPatternRefs(
savedObjectsClient: SavedObjectsClient
indexPatternsService: IndexPatternsService
): Promise<IndexPatternRef[]> {
const result = await savedObjectsClient.find<{ title: string }>({
type: 'index-pattern',
fields: ['title'],
perPage: 10000,
});
const indexPatterns = await indexPatternsService.getIdsWithTitle();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we should pass in the refresh flag here (because that's the current behavior). I'm not sure whether the cache will be in a good state in all cases (like navigating to management, importing an index pattern, then coming back to Lens).

We already have the isFullEditor flag to prevent this on dashboards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moving here what we discussed offline. I did some tests, created an index pattern, removed one etc and it seems that it works fine so we decided to not do it for now. If we discover problems with the cache, I will enable it then


return result.savedObjects
.map((o) => ({
id: String(o.id),
title: (o.attributes as { title: string }).title,
}))
.sort((a, b) => {
return a.title.localeCompare(b.title);
});
return indexPatterns.sort((a, b) => {
return a.title.localeCompare(b.title);
});
}

export async function syncExistingFields({
Expand Down