From 1f82eda3935dc5b648cb31587662c84cb43f3801 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 19 Jul 2023 21:09:29 -0700 Subject: [PATCH] Optimize `augment-vis` saved obj searching by adding arg to saved obj client (#4595) (#4599) Signed-off-by: Tyler Ohlsen (cherry picked from commit f13e4c32ad56e81232db890fca05c65dab652dd6) Signed-off-by: github-actions[bot] # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] --- .../saved_objects_client.test.ts | 5 +--- .../saved_objects/saved_objects_client.ts | 8 +++++- .../saved_object/saved_object_loader.ts | 8 +++++- .../saved_object_delete_action.test.ts | 14 ++++++++++ .../actions/saved_object_delete_action.ts | 23 ++++++++++------ .../saved_augment_vis/saved_augment_vis.ts | 11 +++++--- .../public/saved_augment_vis/utils/helpers.ts | 27 ++++++++++--------- .../vis_augmenter/public/utils/utils.test.ts | 14 +--------- .../vis_augmenter/public/utils/utils.ts | 17 ++++-------- 9 files changed, 72 insertions(+), 55 deletions(-) diff --git a/src/core/public/saved_objects/saved_objects_client.test.ts b/src/core/public/saved_objects/saved_objects_client.test.ts index 19b140ad3a12..cc3405f246c5 100644 --- a/src/core/public/saved_objects/saved_objects_client.test.ts +++ b/src/core/public/saved_objects/saved_objects_client.test.ts @@ -471,10 +471,7 @@ describe('SavedObjectsClient', () => { "fields": Array [ "title", ], - "has_reference": Object { - "id": "1", - "type": "reference", - }, + "has_reference": "{\\"id\\":\\"1\\",\\"type\\":\\"reference\\"}", "page": 10, "per_page": 100, "search": "what is the meaning of life?|life", diff --git a/src/core/public/saved_objects/saved_objects_client.ts b/src/core/public/saved_objects/saved_objects_client.ts index d43b75b2171d..6e5482614e40 100644 --- a/src/core/public/saved_objects/saved_objects_client.ts +++ b/src/core/public/saved_objects/saved_objects_client.ts @@ -348,7 +348,13 @@ export class SavedObjectsClient { }; const renamedQuery = renameKeys(renameMap, options); - const query = pick.apply(null, [renamedQuery, ...Object.values(renameMap)]); + const query = pick.apply(null, [renamedQuery, ...Object.values(renameMap)]) as Partial< + Record + >; + + // has_reference needs post-processing since it is an object that needs to be read as + // a query param + if (query.has_reference) query.has_reference = JSON.stringify(query.has_reference); const request: ReturnType = this.savedObjectsFetch(path, { method: 'GET', diff --git a/src/plugins/saved_objects/public/saved_object/saved_object_loader.ts b/src/plugins/saved_objects/public/saved_object/saved_object_loader.ts index fa329d9032b8..02a8187d14d9 100644 --- a/src/plugins/saved_objects/public/saved_object/saved_object_loader.ts +++ b/src/plugins/saved_objects/public/saved_object/saved_object_loader.ts @@ -128,7 +128,12 @@ export class SavedObjectLoader { * @param fields * @returns {Promise} */ - findAll(search: string = '', size: number = 100, fields?: string[]) { + findAll( + search: string = '', + size: number = 100, + fields?: string[], + hasReference?: SavedObjectsFindOptions['hasReference'] + ) { return this.savedObjectsClient .find>({ type: this.lowercaseType, @@ -138,6 +143,7 @@ export class SavedObjectLoader { searchFields: ['title^3', 'description'], defaultSearchOperator: 'AND', fields, + hasReference, } as SavedObjectsFindOptions) .then((resp) => { return { diff --git a/src/plugins/vis_augmenter/public/actions/saved_object_delete_action.test.ts b/src/plugins/vis_augmenter/public/actions/saved_object_delete_action.test.ts index cec742a218cc..c8c623fae3a9 100644 --- a/src/plugins/vis_augmenter/public/actions/saved_object_delete_action.test.ts +++ b/src/plugins/vis_augmenter/public/actions/saved_object_delete_action.test.ts @@ -18,6 +18,20 @@ jest.mock('src/plugins/vis_augmenter/public/services.ts', () => { }, }; }, + getUISettings: () => { + return { + get: (config: string) => { + switch (config) { + case 'visualization:enablePluginAugmentation': + return true; + case 'visualization:enablePluginAugmentation.maxPluginObjects': + return 10; + default: + throw new Error(`Accessing ${config} is not supported in the mock.`); + } + }, + }; + }, }; }); diff --git a/src/plugins/vis_augmenter/public/actions/saved_object_delete_action.ts b/src/plugins/vis_augmenter/public/actions/saved_object_delete_action.ts index 56194e281540..5d031359a8d1 100644 --- a/src/plugins/vis_augmenter/public/actions/saved_object_delete_action.ts +++ b/src/plugins/vis_augmenter/public/actions/saved_object_delete_action.ts @@ -7,7 +7,7 @@ import { isEmpty } from 'lodash'; import { i18n } from '@osd/i18n'; import { EuiIconType } from '@elastic/eui/src/components/icon/icon'; import { Action, IncompatibleActionError } from '../../../ui_actions/public'; -import { getAllAugmentVisSavedObjs } from '../utils'; +import { getAugmentVisSavedObjs } from '../utils'; import { getSavedAugmentVisLoader } from '../services'; import { SavedObjectDeleteContext } from '../ui_actions_bootstrap'; @@ -45,12 +45,19 @@ export class SavedObjectDeleteAction implements Action throw new IncompatibleActionError(); } - const loader = getSavedAugmentVisLoader(); - const allAugmentVisObjs = await getAllAugmentVisSavedObjs(loader); - const augmentVisIdsToDelete = allAugmentVisObjs - .filter((augmentVisObj) => augmentVisObj.visId === savedObjectId) - .map((augmentVisObj) => augmentVisObj.id as string); - - if (!isEmpty(augmentVisIdsToDelete)) loader.delete(augmentVisIdsToDelete); + try { + const loader = getSavedAugmentVisLoader(); + const augmentVisObjs = await getAugmentVisSavedObjs(savedObjectId, loader); + const augmentVisIdsToDelete = augmentVisObjs.map( + (augmentVisObj) => augmentVisObj.id as string + ); + + if (!isEmpty(augmentVisIdsToDelete)) loader.delete(augmentVisIdsToDelete); + // silently fail. this is because this is doing extra cleanup on objects unrelated + // to the user flow so we dont want to add confusing errors on UI. + } catch (e) { + // eslint-disable-next-line no-console + console.error(e); + } } } diff --git a/src/plugins/vis_augmenter/public/saved_augment_vis/saved_augment_vis.ts b/src/plugins/vis_augmenter/public/saved_augment_vis/saved_augment_vis.ts index 28b7831d9431..1963dd434336 100644 --- a/src/plugins/vis_augmenter/public/saved_augment_vis/saved_augment_vis.ts +++ b/src/plugins/vis_augmenter/public/saved_augment_vis/saved_augment_vis.ts @@ -4,7 +4,7 @@ */ import { get, isEmpty } from 'lodash'; -import { IUiSettingsClient } from 'opensearch-dashboards/public'; +import { IUiSettingsClient, SavedObjectsFindOptions } from 'opensearch-dashboards/public'; import { SavedObjectLoader, SavedObjectOpenSearchDashboardsServices, @@ -91,9 +91,14 @@ export class SavedObjectLoaderAugmentVis extends SavedObjectLoader { * @param fields * @returns {Promise} */ - findAll(search: string = '', size: number = 100, fields?: string[]) { + findAll( + search: string = '', + size: number = 100, + fields?: string[], + hasReference?: SavedObjectsFindOptions['hasReference'] + ) { this.isAugmentationEnabled(); - return super.findAll(search, size, fields); + return super.findAll(search, size, fields, hasReference); } find(search: string = '', size: number = 100) { diff --git a/src/plugins/vis_augmenter/public/saved_augment_vis/utils/helpers.ts b/src/plugins/vis_augmenter/public/saved_augment_vis/utils/helpers.ts index 0751f1dd330c..36fd15cefa06 100644 --- a/src/plugins/vis_augmenter/public/saved_augment_vis/utils/helpers.ts +++ b/src/plugins/vis_augmenter/public/saved_augment_vis/utils/helpers.ts @@ -33,23 +33,24 @@ export const createAugmentVisSavedObject = async ( } const maxAssociatedCount = config.get(PLUGIN_AUGMENTATION_MAX_OBJECTS_SETTING); - await loader.findAll().then(async (resp) => { - if (resp !== undefined) { - const savedAugmentObjects = get(resp, 'hits', []); - // gets all the saved object for this visualization - const savedObjectsForThisVisualization = savedAugmentObjects.filter( - (savedObj) => get(savedObj, 'visId', '') === AugmentVis.visId - ); + await loader + .findAll('', 100, [], { + type: 'visualization', + id: AugmentVis.visId as string, + }) + .then(async (resp) => { + if (resp !== undefined) { + const savedObjectsForThisVisualization = get(resp, 'hits', []); - if (maxAssociatedCount <= savedObjectsForThisVisualization.length) { - throw new Error( - `Cannot associate the plugin resource to the visualization due to the limit of the max + if (maxAssociatedCount <= savedObjectsForThisVisualization.length) { + throw new Error( + `Cannot associate the plugin resource to the visualization due to the limit of the max amount of associated plugin resources (${maxAssociatedCount}) with ${savedObjectsForThisVisualization.length} associated to the visualization` - ); + ); + } } - } - }); + }); return await loader.get((AugmentVis as any) as Record); }; diff --git a/src/plugins/vis_augmenter/public/utils/utils.test.ts b/src/plugins/vis_augmenter/public/utils/utils.test.ts index 4578a297058a..b7124b9d2b6e 100644 --- a/src/plugins/vis_augmenter/public/utils/utils.test.ts +++ b/src/plugins/vis_augmenter/public/utils/utils.test.ts @@ -304,12 +304,6 @@ describe('utils', () => { pluginResource ); - it('returns no matching saved objs with filtering', async () => { - const loader = createSavedAugmentVisLoader({ - savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2, obj3]), - } as SavedObjectOpenSearchDashboardsServicesWithAugmentVis); - expect((await getAugmentVisSavedObjs(visId3, loader)).length).toEqual(0); - }); it('returns no matching saved objs when client returns empty list', async () => { const loader = createSavedAugmentVisLoader({ savedObjectsClient: getMockAugmentVisSavedObjectClient([]), @@ -349,18 +343,12 @@ describe('utils', () => { } as SavedObjectOpenSearchDashboardsServicesWithAugmentVis); expect((await getAugmentVisSavedObjs(visId1, loader)).length).toEqual(1); }); - it('returns multiple matching saved objs without filtering', async () => { + it('returns multiple matching saved objs', async () => { const loader = createSavedAugmentVisLoader({ savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2]), } as SavedObjectOpenSearchDashboardsServicesWithAugmentVis); expect((await getAugmentVisSavedObjs(visId1, loader)).length).toEqual(2); }); - it('returns multiple matching saved objs with filtering', async () => { - const loader = createSavedAugmentVisLoader({ - savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2, obj3]), - } as SavedObjectOpenSearchDashboardsServicesWithAugmentVis); - expect((await getAugmentVisSavedObjs(visId1, loader)).length).toEqual(2); - }); }); describe('buildPipelineFromAugmentVisSavedObjs', () => { diff --git a/src/plugins/vis_augmenter/public/utils/utils.ts b/src/plugins/vis_augmenter/public/utils/utils.ts index e3f582ae58fb..60bfcbc16a5f 100644 --- a/src/plugins/vis_augmenter/public/utils/utils.ts +++ b/src/plugins/vis_augmenter/public/utils/utils.ts @@ -54,7 +54,7 @@ export const isEligibleForVisLayers = (vis: Vis, uiSettingsClient?: IUiSettingsC /** * Using a SavedAugmentVisLoader, fetch all saved objects that are of 'augment-vis' type. - * Filter by vis ID. + * Filter by vis ID by passing in a 'hasReferences' obj with the vis ID to the findAll() fn call. */ export const getAugmentVisSavedObjs = async ( visId: string | undefined, @@ -69,18 +69,11 @@ export const getAugmentVisSavedObjs = async ( 'Visualization augmentation is disabled, please enable visualization:enablePluginAugmentation.' ); } - const allSavedObjects = await getAllAugmentVisSavedObjs(loader); - return allSavedObjects.filter((hit: ISavedAugmentVis) => hit.visId === visId); -}; - -/** - * Using a SavedAugmentVisLoader, fetch all saved objects that are of 'augment-vis' type. - */ -export const getAllAugmentVisSavedObjs = async ( - loader: SavedAugmentVisLoader | undefined -): Promise => { try { - const resp = await loader?.findAll(); + const resp = await loader?.findAll('', 100, [], { + type: 'visualization', + id: visId as string, + }); return (get(resp, 'hits', []) as any[]) as ISavedAugmentVis[]; } catch (e) { return [] as ISavedAugmentVis[];