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.

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

Optimize augment-vis saved obj searching by adding arg to saved obj client #4595

Merged
merged 4 commits into from
Jul 20, 2023
Merged
Changes from 1 commit
Commits
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
Next Next commit
Optimize augment-vis saved obj searching
Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
ohltyler committed Jul 19, 2023
commit eaa403a83f5d73c7eb352ccedea1672feebe3b1a
Original file line number Diff line number Diff line change
@@ -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",
4 changes: 4 additions & 0 deletions src/core/public/saved_objects/saved_objects_client.ts
Original file line number Diff line number Diff line change
@@ -350,6 +350,10 @@ export class SavedObjectsClient {
const renamedQuery = renameKeys<SavedObjectsFindOptions, any>(renameMap, options);
const query = pick.apply(null, [renamedQuery, ...Object.values<string>(renameMap)]);

// 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<SavedObjectsApi['find']> = this.savedObjectsFetch(path, {
method: 'GET',
query,
Original file line number Diff line number Diff line change
@@ -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<Record<string, unknown>>({
type: this.lowercaseType,
@@ -138,6 +143,7 @@ export class SavedObjectLoader {
searchFields: ['title^3', 'description'],
defaultSearchOperator: 'AND',
fields,
hasReference,
} as SavedObjectsFindOptions)
.then((resp) => {
return {
Original file line number Diff line number Diff line change
@@ -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.`);
}
},
};
},
};
});

Original file line number Diff line number Diff line change
@@ -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,16 @@ export class SavedObjectDeleteAction implements Action<SavedObjectDeleteContext>
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 log lines or error toasts
} catch (e) {} // eslint-disable-line no-empty
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
@@ -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) {
Original file line number Diff line number Diff line change
@@ -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<string, unknown>);
};
14 changes: 1 addition & 13 deletions src/plugins/vis_augmenter/public/utils/utils.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
17 changes: 5 additions & 12 deletions src/plugins/vis_augmenter/public/utils/utils.ts
Original file line number Diff line number Diff line change
@@ -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<ISavedAugmentVis[]> => {
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[];