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

Add resource ID filtering in fetch augment-vis obj queries #4608

Merged
merged 3 commits into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- New management overview page and rename stack management to dashboard management ([#4287](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4287))
- [Vis Augmenter] Update base vis height in view events flyout ([#4535](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4535))
- Optimize `augment-vis` saved obj searching by adding arg to saved obj client ([#4595](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4595))
- Add resource ID filtering in fetch `augment-vis` obj queries ([#4608](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4608))


### 🐛 Bug Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,24 @@ export class SavedObjectLoader {
* @param search
* @param size
* @param fields
* @param hasReference Optional field to specify a reference
* @param searchFields Optional field to specify the search fields in the query
* @returns {Promise}
*/
findAll(
search: string = '',
size: number = 100,
fields?: string[],
hasReference?: SavedObjectsFindOptions['hasReference']
hasReference?: SavedObjectsFindOptions['hasReference'],
searchFields: string[] = ['title^3', 'description']
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
) {
return this.savedObjectsClient
.find<Record<string, unknown>>({
type: this.lowercaseType,
search: search ? `${search}*` : undefined,
perPage: size,
page: 1,
searchFields: ['title^3', 'description'],
searchFields,
defaultSearchOperator: 'AND',
fields,
hasReference,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,19 @@ export class SavedObjectLoaderAugmentVis extends SavedObjectLoader {
* @param search
* @param size
* @param fields
* @param hasReference Optional field to specify a reference
* @param searchFields Optional field to specify the search fields in the query
* @returns {Promise}
*/
findAll(
search: string = '',
size: number = 100,
fields?: string[],
hasReference?: SavedObjectsFindOptions['hasReference']
hasReference?: SavedObjectsFindOptions['hasReference'],
searchFields?: string[]
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
) {
this.isAugmentationEnabled();
return super.findAll(search, size, fields, hasReference);
return super.findAll(search, size, fields, hasReference, searchFields);
}

find(search: string = '', size: number = 100) {
Expand Down
70 changes: 70 additions & 0 deletions src/plugins/vis_augmenter/public/utils/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,76 @@ describe('utils', () => {
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
expect((await getAugmentVisSavedObjs(visId1, loader)).length).toEqual(2);
});
it('undefined plugin resource list has no effect', async () => {
const loader = createSavedAugmentVisLoader({
savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2]),
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
expect((await getAugmentVisSavedObjs(visId1, loader, undefined, undefined)).length).toEqual(
2
);
});
it('empty plugin resource list has no effect', async () => {
const loader = createSavedAugmentVisLoader({
savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2]),
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
expect((await getAugmentVisSavedObjs(visId1, loader, undefined, [])).length).toEqual(2);
});
it('empty / undefined plugin resource list passes correct findAll() params', async () => {
const loader = createSavedAugmentVisLoader({
savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2]),
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
loader.findAll = jest.fn().mockImplementation(loader.findAll);
expect((await getAugmentVisSavedObjs(visId1, loader, undefined, [])).length).toEqual(2);
expect(loader.findAll).toHaveBeenCalledWith(
'',
100,
[],
{
type: 'visualization',
id: visId1 as string,
},
[]
);
});
it('single plugin resource is propagated to findAll()', async () => {
const loader = createSavedAugmentVisLoader({
savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2]),
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
loader.findAll = jest.fn().mockImplementation(loader.findAll);
expect(
(await getAugmentVisSavedObjs(visId1, loader, undefined, ['resource-1'])).length
).toEqual(2);
expect(loader.findAll).toHaveBeenCalledWith(
'resource-1',
100,
[],
{
type: 'visualization',
id: visId1 as string,
},
['pluginResource.id']
);
});
it('multiple plugin resources are propagated to findAll()', async () => {
const loader = createSavedAugmentVisLoader({
savedObjectsClient: getMockAugmentVisSavedObjectClient([obj1, obj2]),
} as SavedObjectOpenSearchDashboardsServicesWithAugmentVis);
loader.findAll = jest.fn().mockImplementation(loader.findAll);
expect(
(await getAugmentVisSavedObjs(visId1, loader, undefined, ['resource-1', 'resource-2']))
.length
).toEqual(2);
expect(loader.findAll).toHaveBeenCalledWith(
'resource-1|resource-2',
100,
[],
{
type: 'visualization',
id: visId1 as string,
},
['pluginResource.id']
);
});
});

describe('buildPipelineFromAugmentVisSavedObjs', () => {
Expand Down
24 changes: 18 additions & 6 deletions src/plugins/vis_augmenter/public/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,14 @@ export const isEligibleForVisLayers = (vis: Vis, uiSettingsClient?: IUiSettingsC

/**
* Using a SavedAugmentVisLoader, fetch all saved objects that are of 'augment-vis' type.
* Filter by vis ID by passing in a 'hasReferences' obj with the vis ID to the findAll() fn call.
* Filter by vis ID by passing in a 'hasReferences' obj with the vis ID to the findAll() fn call,
* and optionally by plugin resource ID list, if specified.
*/
export const getAugmentVisSavedObjs = async (
visId: string | undefined,
loader: SavedAugmentVisLoader | undefined,
uiSettings?: IUiSettingsClient | undefined
uiSettings?: IUiSettingsClient | undefined,
pluginResourceIds?: string[] | undefined
): Promise<ISavedAugmentVis[] | Error> => {
// Using optional services provided, or the built-in services from this plugin
const config = uiSettings !== undefined ? uiSettings : getUISettings();
Expand All @@ -70,10 +72,20 @@ export const getAugmentVisSavedObjs = async (
);
}
try {
const resp = await loader?.findAll('', 100, [], {
type: 'visualization',
id: visId as string,
});
// If there is specified plugin resource IDs, add a search string and search field
// into findAll() fn call
const pluginResourceIdsSpecified =
pluginResourceIds !== undefined && pluginResourceIds.length > 0;
const resp = await loader?.findAll(
pluginResourceIdsSpecified ? pluginResourceIds.join('|') : '',
100,
[],
{
type: 'visualization',
id: visId as string,
},
pluginResourceIdsSpecified ? ['pluginResource.id'] : []
);
return (get(resp, 'hits', []) as any[]) as ISavedAugmentVis[];
} catch (e) {
return [] as ISavedAugmentVis[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,19 +513,14 @@
* e.g., generating other vis embeddables in the view events flyout.
*/
public async populateVisLayers(): Promise<void> {
const visLayers = await this.fetchVisLayers();
this.visLayers =
this.visAugmenterConfig?.visLayerResourceIds === undefined
? visLayers
: visLayers.filter((visLayer) =>
this.visAugmenterConfig.visLayerResourceIds.includes(visLayer.pluginResource.id)
);
this.visLayers = await this.fetchVisLayers();

Check warning on line 516 in src/plugins/visualizations/public/embeddable/visualize_embeddable.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/visualizations/public/embeddable/visualize_embeddable.ts#L516

Added line #L516 was not covered by tests
}

/**
* Collects any VisLayers from plugin expressions functions
* by fetching all AugmentVisSavedObjects that match the vis
* saved object ID.
* by fetching all AugmentVisSavedObjects that meets below criteria:
* - includes a reference to the vis saved object id
* - includes any of the plugin resource IDs, if specified
*/
fetchVisLayers = async (): Promise<VisLayers> => {
try {
Expand All @@ -541,7 +536,9 @@
const aborted = get(this.abortController, 'signal.aborted', false) as boolean;
const augmentVisSavedObjs = await getAugmentVisSavedObjs(
this.vis.id,
this.savedAugmentVisLoader
this.savedAugmentVisLoader,
undefined,
this.visAugmenterConfig?.visLayerResourceIds
);

if (!isEmpty(augmentVisSavedObjs) && !aborted && isEligibleForVisLayers(this.vis)) {
Expand Down
Loading