Skip to content

Commit 7775fbe

Browse files
Optimize augment-vis saved obj searching by adding arg to saved obj client (#4595)
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com> (cherry picked from commit f13e4c3) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
1 parent 25ad2be commit 7775fbe

File tree

9 files changed

+72
-55
lines changed

9 files changed

+72
-55
lines changed

src/core/public/saved_objects/saved_objects_client.test.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -471,10 +471,7 @@ describe('SavedObjectsClient', () => {
471471
"fields": Array [
472472
"title",
473473
],
474-
"has_reference": Object {
475-
"id": "1",
476-
"type": "reference",
477-
},
474+
"has_reference": "{\\"id\\":\\"1\\",\\"type\\":\\"reference\\"}",
478475
"page": 10,
479476
"per_page": 100,
480477
"search": "what is the meaning of life?|life",

src/core/public/saved_objects/saved_objects_client.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,13 @@ export class SavedObjectsClient {
348348
};
349349

350350
const renamedQuery = renameKeys<SavedObjectsFindOptions, any>(renameMap, options);
351-
const query = pick.apply(null, [renamedQuery, ...Object.values<string>(renameMap)]);
351+
const query = pick.apply(null, [renamedQuery, ...Object.values<string>(renameMap)]) as Partial<
352+
Record<string, any>
353+
>;
354+
355+
// has_reference needs post-processing since it is an object that needs to be read as
356+
// a query param
357+
if (query.has_reference) query.has_reference = JSON.stringify(query.has_reference);
352358

353359
const request: ReturnType<SavedObjectsApi['find']> = this.savedObjectsFetch(path, {
354360
method: 'GET',

src/plugins/saved_objects/public/saved_object/saved_object_loader.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,12 @@ export class SavedObjectLoader {
128128
* @param fields
129129
* @returns {Promise}
130130
*/
131-
findAll(search: string = '', size: number = 100, fields?: string[]) {
131+
findAll(
132+
search: string = '',
133+
size: number = 100,
134+
fields?: string[],
135+
hasReference?: SavedObjectsFindOptions['hasReference']
136+
) {
132137
return this.savedObjectsClient
133138
.find<Record<string, unknown>>({
134139
type: this.lowercaseType,
@@ -138,6 +143,7 @@ export class SavedObjectLoader {
138143
searchFields: ['title^3', 'description'],
139144
defaultSearchOperator: 'AND',
140145
fields,
146+
hasReference,
141147
} as SavedObjectsFindOptions)
142148
.then((resp) => {
143149
return {

src/plugins/vis_augmenter/public/actions/saved_object_delete_action.test.ts

+14
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,20 @@ jest.mock('src/plugins/vis_augmenter/public/services.ts', () => {
1818
},
1919
};
2020
},
21+
getUISettings: () => {
22+
return {
23+
get: (config: string) => {
24+
switch (config) {
25+
case 'visualization:enablePluginAugmentation':
26+
return true;
27+
case 'visualization:enablePluginAugmentation.maxPluginObjects':
28+
return 10;
29+
default:
30+
throw new Error(`Accessing ${config} is not supported in the mock.`);
31+
}
32+
},
33+
};
34+
},
2135
};
2236
});
2337

src/plugins/vis_augmenter/public/actions/saved_object_delete_action.ts

+15-8
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { isEmpty } from 'lodash';
77
import { i18n } from '@osd/i18n';
88
import { EuiIconType } from '@elastic/eui/src/components/icon/icon';
99
import { Action, IncompatibleActionError } from '../../../ui_actions/public';
10-
import { getAllAugmentVisSavedObjs } from '../utils';
10+
import { getAugmentVisSavedObjs } from '../utils';
1111
import { getSavedAugmentVisLoader } from '../services';
1212
import { SavedObjectDeleteContext } from '../ui_actions_bootstrap';
1313

@@ -45,12 +45,19 @@ export class SavedObjectDeleteAction implements Action<SavedObjectDeleteContext>
4545
throw new IncompatibleActionError();
4646
}
4747

48-
const loader = getSavedAugmentVisLoader();
49-
const allAugmentVisObjs = await getAllAugmentVisSavedObjs(loader);
50-
const augmentVisIdsToDelete = allAugmentVisObjs
51-
.filter((augmentVisObj) => augmentVisObj.visId === savedObjectId)
52-
.map((augmentVisObj) => augmentVisObj.id as string);
53-
54-
if (!isEmpty(augmentVisIdsToDelete)) loader.delete(augmentVisIdsToDelete);
48+
try {
49+
const loader = getSavedAugmentVisLoader();
50+
const augmentVisObjs = await getAugmentVisSavedObjs(savedObjectId, loader);
51+
const augmentVisIdsToDelete = augmentVisObjs.map(
52+
(augmentVisObj) => augmentVisObj.id as string
53+
);
54+
55+
if (!isEmpty(augmentVisIdsToDelete)) loader.delete(augmentVisIdsToDelete);
56+
// silently fail. this is because this is doing extra cleanup on objects unrelated
57+
// to the user flow so we dont want to add confusing errors on UI.
58+
} catch (e) {
59+
// eslint-disable-next-line no-console
60+
console.error(e);
61+
}
5562
}
5663
}

src/plugins/vis_augmenter/public/saved_augment_vis/saved_augment_vis.ts

+8-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
import { get, isEmpty } from 'lodash';
7-
import { IUiSettingsClient } from 'opensearch-dashboards/public';
7+
import { IUiSettingsClient, SavedObjectsFindOptions } from 'opensearch-dashboards/public';
88
import {
99
SavedObjectLoader,
1010
SavedObjectOpenSearchDashboardsServices,
@@ -91,9 +91,14 @@ export class SavedObjectLoaderAugmentVis extends SavedObjectLoader {
9191
* @param fields
9292
* @returns {Promise}
9393
*/
94-
findAll(search: string = '', size: number = 100, fields?: string[]) {
94+
findAll(
95+
search: string = '',
96+
size: number = 100,
97+
fields?: string[],
98+
hasReference?: SavedObjectsFindOptions['hasReference']
99+
) {
95100
this.isAugmentationEnabled();
96-
return super.findAll(search, size, fields);
101+
return super.findAll(search, size, fields, hasReference);
97102
}
98103

99104
find(search: string = '', size: number = 100) {

src/plugins/vis_augmenter/public/saved_augment_vis/utils/helpers.ts

+14-13
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,24 @@ export const createAugmentVisSavedObject = async (
3333
}
3434
const maxAssociatedCount = config.get(PLUGIN_AUGMENTATION_MAX_OBJECTS_SETTING);
3535

36-
await loader.findAll().then(async (resp) => {
37-
if (resp !== undefined) {
38-
const savedAugmentObjects = get(resp, 'hits', []);
39-
// gets all the saved object for this visualization
40-
const savedObjectsForThisVisualization = savedAugmentObjects.filter(
41-
(savedObj) => get(savedObj, 'visId', '') === AugmentVis.visId
42-
);
36+
await loader
37+
.findAll('', 100, [], {
38+
type: 'visualization',
39+
id: AugmentVis.visId as string,
40+
})
41+
.then(async (resp) => {
42+
if (resp !== undefined) {
43+
const savedObjectsForThisVisualization = get(resp, 'hits', []);
4344

44-
if (maxAssociatedCount <= savedObjectsForThisVisualization.length) {
45-
throw new Error(
46-
`Cannot associate the plugin resource to the visualization due to the limit of the max
45+
if (maxAssociatedCount <= savedObjectsForThisVisualization.length) {
46+
throw new Error(
47+
`Cannot associate the plugin resource to the visualization due to the limit of the max
4748
amount of associated plugin resources (${maxAssociatedCount}) with
4849
${savedObjectsForThisVisualization.length} associated to the visualization`
49-
);
50+
);
51+
}
5052
}
51-
}
52-
});
53+
});
5354

5455
return await loader.get((AugmentVis as any) as Record<string, unknown>);
5556
};

src/plugins/vis_augmenter/public/utils/utils.test.ts

+1-13
Original file line numberDiff line numberDiff line change
@@ -304,12 +304,6 @@ describe('utils', () => {
304304
pluginResource
305305
);
306306

307-
it('returns no matching saved objs with filtering', async () => {
308-
const loader = createSavedAugmentVisLoader({
309-
savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2, obj3]),
310-
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
311-
expect((await getAugmentVisSavedObjs(visId3, loader)).length).toEqual(0);
312-
});
313307
it('returns no matching saved objs when client returns empty list', async () => {
314308
const loader = createSavedAugmentVisLoader({
315309
savedObjectsClient: getMockAugmentVisSavedObjectClient([]),
@@ -349,18 +343,12 @@ describe('utils', () => {
349343
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
350344
expect((await getAugmentVisSavedObjs(visId1, loader)).length).toEqual(1);
351345
});
352-
it('returns multiple matching saved objs without filtering', async () => {
346+
it('returns multiple matching saved objs', async () => {
353347
const loader = createSavedAugmentVisLoader({
354348
savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2]),
355349
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
356350
expect((await getAugmentVisSavedObjs(visId1, loader)).length).toEqual(2);
357351
});
358-
it('returns multiple matching saved objs with filtering', async () => {
359-
const loader = createSavedAugmentVisLoader({
360-
savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2, obj3]),
361-
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
362-
expect((await getAugmentVisSavedObjs(visId1, loader)).length).toEqual(2);
363-
});
364352
});
365353

366354
describe('buildPipelineFromAugmentVisSavedObjs', () => {

src/plugins/vis_augmenter/public/utils/utils.ts

+5-12
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export const isEligibleForVisLayers = (vis: Vis, uiSettingsClient?: IUiSettingsC
5454

5555
/**
5656
* Using a SavedAugmentVisLoader, fetch all saved objects that are of 'augment-vis' type.
57-
* Filter by vis ID.
57+
* Filter by vis ID by passing in a 'hasReferences' obj with the vis ID to the findAll() fn call.
5858
*/
5959
export const getAugmentVisSavedObjs = async (
6060
visId: string | undefined,
@@ -69,18 +69,11 @@ export const getAugmentVisSavedObjs = async (
6969
'Visualization augmentation is disabled, please enable visualization:enablePluginAugmentation.'
7070
);
7171
}
72-
const allSavedObjects = await getAllAugmentVisSavedObjs(loader);
73-
return allSavedObjects.filter((hit: ISavedAugmentVis) => hit.visId === visId);
74-
};
75-
76-
/**
77-
* Using a SavedAugmentVisLoader, fetch all saved objects that are of 'augment-vis' type.
78-
*/
79-
export const getAllAugmentVisSavedObjs = async (
80-
loader: SavedAugmentVisLoader | undefined
81-
): Promise<ISavedAugmentVis[]> => {
8272
try {
83-
const resp = await loader?.findAll();
73+
const resp = await loader?.findAll('', 100, [], {
74+
type: 'visualization',
75+
id: visId as string,
76+
});
8477
return (get(resp, 'hits', []) as any[]) as ISavedAugmentVis[];
8578
} catch (e) {
8679
return [] as ISavedAugmentVis[];

0 commit comments

Comments
 (0)