Skip to content

Commit

Permalink
[SOM] Fix UI and export when handling more than 10k objects (elastic#…
Browse files Browse the repository at this point in the history
…118335) (elastic#118669)

* delete the unused `scroll/export` endpoint

* adapt the scroll/counts endpoint to use PIT search

* improve export error message

* only display 10k first items

* fix findAll unit tests

* fix FTR test

* remove unused comment

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Pierre Gayvallet <pierre.gayvallet@elastic.co>
  • Loading branch information
kibanamachine and pgayvallet committed Nov 16, 2021
1 parent 368fe55 commit 24f8717
Show file tree
Hide file tree
Showing 11 changed files with 197 additions and 182 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ interface TableState {
activeAction?: SavedObjectsManagementAction;
}

const MAX_PAGINATED_ITEM = 10000;

export class Table extends PureComponent<TableProps, TableState> {
state: TableState = {
isSearchTextValid: true,
Expand Down Expand Up @@ -150,10 +152,12 @@ export class Table extends PureComponent<TableProps, TableState> {
allowedTypes,
} = this.props;

const cappedTotalItemCount = Math.min(totalItemCount, MAX_PAGINATED_ITEM);

const pagination = {
pageIndex,
pageSize,
totalItemCount,
totalItemCount: cappedTotalItemCount,
pageSizeOptions: [5, 10, 20, 50],
};

Expand Down Expand Up @@ -321,6 +325,7 @@ export class Table extends PureComponent<TableProps, TableState> {
);

const activeActionContents = this.state.activeAction?.render() ?? null;
const exceededResultCount = totalItemCount > MAX_PAGINATED_ITEM;

return (
<Fragment>
Expand Down Expand Up @@ -392,6 +397,18 @@ export class Table extends PureComponent<TableProps, TableState> {
/>
{queryParseError}
<EuiSpacer size="s" />
{exceededResultCount && (
<>
<EuiText color="subdued" size="s" data-test-subj="savedObjectsTableTooManyResultsLabel">
<FormattedMessage
id="savedObjectsManagement.objectsTable.table.tooManyResultsLabel"
defaultMessage="Showing {limit} of {totalItemCount, plural, one {# object} other {# objects}}"
values={{ totalItemCount, limit: MAX_PAGINATED_ITEM }}
/>
</EuiText>
<EuiSpacer size="s" />
</>
)}
<div data-test-subj="savedObjectsTable">
<EuiBasicTable
loading={isSearching}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,11 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
blob = await fetchExportObjects(http, objectsToExport, includeReferencesDeep);
} catch (e) {
notifications.toasts.addDanger({
title: i18n.translate('savedObjectsManagement.objectsTable.export.dangerNotification', {
defaultMessage: 'Unable to generate export',
title: i18n.translate('savedObjectsManagement.objectsTable.export.toastErrorMessage', {
defaultMessage: 'Unable to generate export: {error}',
values: {
error: e.body?.message ?? e,
},
}),
});
throw e;
Expand Down Expand Up @@ -423,8 +426,11 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb
});
} catch (e) {
notifications.toasts.addDanger({
title: i18n.translate('savedObjectsManagement.objectsTable.export.dangerNotification', {
defaultMessage: 'Unable to generate export',
title: i18n.translate('savedObjectsManagement.objectsTable.export.toastErrorMessage', {
defaultMessage: 'Unable to generate export: {error}',
values: {
error: e.body?.message ?? e,
},
}),
});
throw e;
Expand Down
61 changes: 37 additions & 24 deletions src/plugins/saved_objects_management/server/lib/find_all.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,25 @@ describe('findAll', () => {
savedObjectsClient = savedObjectsClientMock.create();
});

it('calls the saved object client with the correct parameters', async () => {
it('calls `client.createPointInTimeFinder` with the correct parameters', async () => {
const query: SavedObjectsFindOptions = {
type: ['some-type', 'another-type'],
};

savedObjectsClient.find.mockResolvedValue({
saved_objects: [],
total: 1,
per_page: 20,
page: 1,
});

await findAll(savedObjectsClient, query);

expect(savedObjectsClient.createPointInTimeFinder).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.createPointInTimeFinder).toHaveBeenCalledWith(query);
});

it('returns the results from the PIT search', async () => {
const query: SavedObjectsFindOptions = {
type: ['some-type', 'another-type'],
};
Expand All @@ -41,45 +59,40 @@ describe('findAll', () => {
const results = await findAll(savedObjectsClient, query);

expect(savedObjectsClient.find).toHaveBeenCalledTimes(1);
expect(savedObjectsClient.find).toHaveBeenCalledWith({
...query,
page: 1,
});
expect(savedObjectsClient.find).toHaveBeenCalledWith(
expect.objectContaining({
...query,
})
);

expect(results).toEqual([createObj(1), createObj(2)]);
});

it('recursively call find until all objects are fetched', async () => {
it('works when the PIT search returns multiple batches', async () => {
const query: SavedObjectsFindOptions = {
type: ['some-type', 'another-type'],
perPage: 2,
};
const objPerPage = 2;

savedObjectsClient.find.mockImplementation(({ page }) => {
const firstInPage = (page! - 1) * objPerPage + 1;
let callCount = 0;
savedObjectsClient.find.mockImplementation(({}) => {
callCount++;
const firstInPage = (callCount - 1) * objPerPage + 1;
return Promise.resolve({
saved_objects: [createObj(firstInPage), createObj(firstInPage + 1)],
saved_objects:
callCount > 3
? [createObj(firstInPage)]
: [createObj(firstInPage), createObj(firstInPage + 1)],
total: objPerPage * 3,
per_page: objPerPage,
page: page!,
page: callCount!,
});
});

const results = await findAll(savedObjectsClient, query);
expect(savedObjectsClient.find).toHaveBeenCalledTimes(3);
expect(savedObjectsClient.find).toHaveBeenCalledWith({
...query,
page: 1,
});
expect(savedObjectsClient.find).toHaveBeenCalledWith({
...query,
page: 2,
});
expect(savedObjectsClient.find).toHaveBeenCalledWith({
...query,
page: 3,
});

expect(results).toEqual(times(6, (num) => createObj(num + 1)));
expect(savedObjectsClient.find).toHaveBeenCalledTimes(4);
expect(results).toEqual(times(7, (num) => createObj(num + 1)));
});
});
32 changes: 11 additions & 21 deletions src/plugins/saved_objects_management/server/lib/find_all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,20 @@
* Side Public License, v 1.
*/

import { SavedObjectsClientContract, SavedObject, SavedObjectsFindOptions } from 'src/core/server';
import {
SavedObjectsClientContract,
SavedObject,
SavedObjectsCreatePointInTimeFinderOptions,
} from 'src/core/server';

export const findAll = async (
client: SavedObjectsClientContract,
findOptions: SavedObjectsFindOptions
findOptions: SavedObjectsCreatePointInTimeFinderOptions
): Promise<SavedObject[]> => {
return recursiveFind(client, findOptions, 1, []);
};

const recursiveFind = async (
client: SavedObjectsClientContract,
findOptions: SavedObjectsFindOptions,
page: number,
allObjects: SavedObject[]
): Promise<SavedObject[]> => {
const objects = await client.find({
...findOptions,
page,
});

allObjects.push(...objects.saved_objects);
if (allObjects.length < objects.total) {
return recursiveFind(client, findOptions, page + 1, allObjects);
const finder = client.createPointInTimeFinder(findOptions);
const results: SavedObject[] = [];
for await (const result of finder.find()) {
results.push(...result.saved_objects);
}

return allObjects;
return results;
};
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('registerRoutes', () => {

expect(httpSetup.createRouter).toHaveBeenCalledTimes(1);
expect(router.get).toHaveBeenCalledTimes(3);
expect(router.post).toHaveBeenCalledTimes(3);
expect(router.post).toHaveBeenCalledTimes(2);

expect(router.get).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down Expand Up @@ -56,11 +56,5 @@ describe('registerRoutes', () => {
}),
expect.any(Function)
);
expect(router.post).toHaveBeenCalledWith(
expect.objectContaining({
path: '/api/kibana/management/saved_objects/scroll/export',
}),
expect.any(Function)
);
});
});
2 changes: 0 additions & 2 deletions src/plugins/saved_objects_management/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { ISavedObjectsManagement } from '../services';
import { registerFindRoute } from './find';
import { registerBulkGetRoute } from './bulk_get';
import { registerScrollForCountRoute } from './scroll_count';
import { registerScrollForExportRoute } from './scroll_export';
import { registerRelationshipsRoute } from './relationships';
import { registerGetAllowedTypesRoute } from './get_allowed_types';

Expand All @@ -25,7 +24,6 @@ export function registerRoutes({ http, managementServicePromise }: RegisterRoute
registerFindRoute(router, managementServicePromise);
registerBulkGetRoute(router, managementServicePromise);
registerScrollForCountRoute(router);
registerScrollForExportRoute(router);
registerRelationshipsRoute(router, managementServicePromise);
registerGetAllowedTypesRoute(router);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { schema } from '@kbn/config-schema';
import { IRouter, SavedObjectsFindOptions } from 'src/core/server';
import { IRouter, SavedObjectsCreatePointInTimeFinderOptions } from 'src/core/server';
import { chain } from 'lodash';
import { findAll } from '../lib';

Expand Down Expand Up @@ -42,7 +42,7 @@ export const registerScrollForCountRoute = (router: IRouter) => {
.value();

const client = getClient({ includedHiddenTypes });
const findOptions: SavedObjectsFindOptions = {
const findOptions: SavedObjectsCreatePointInTimeFinderOptions = {
type: typesToInclude,
perPage: 1000,
};
Expand Down

This file was deleted.

Loading

0 comments on commit 24f8717

Please sign in to comment.