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

Clean up stale augment-vis saved objs #4059

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"version": "opensearchDashboards",
"server": true,
"ui": true,
"requiredPlugins": ["management", "data"],
"requiredPlugins": ["management", "data", "uiActions"],
"optionalPlugins": [
"dashboard",
"visualizations",
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/saved_objects_management/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ export {
} from './services';
export { ProcessedImportResponse, processImportResponse, FailedImport } from './lib';
export { SavedObjectRelation, SavedObjectWithMetadata, SavedObjectMetadata } from './types';
export { SAVED_OBJECT_DELETE_TRIGGER, savedObjectDeleteTrigger } from './triggers';
export { SavedObjectDeleteContext } from './ui_actions_bootstrap';

export function plugin(initializerContext: PluginInitializerContext) {
return new SavedObjectsManagementPlugin();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import { ISavedObjectsManagementServiceRegistry } from '../../services';
import { Header, NotFoundErrors, Intro, Form } from './components';
import { canViewInApp } from '../../lib';
import { SubmittedFormData } from '../types';
import { UiActionsStart } from '../../../../ui_actions/public';
import { SAVED_OBJECT_DELETE_TRIGGER } from '../../triggers';

interface SavedObjectEditionProps {
id: string;
Expand Down Expand Up @@ -168,7 +170,10 @@ export class SavedObjectEdition extends Component<
}
);
if (confirmed) {
const uiActions = this.props.serviceRegistry.get('uiActions')?.service as UiActionsStart;
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
uiActions.getTrigger(SAVED_OBJECT_DELETE_TRIGGER).exec({ type, savedObjectId: id });
await savedObjectsClient.delete(type, id);

notifications.toasts.addSuccess(`Deleted ${this.formatTitle(object)} ${type} object`);
this.redirectToListing();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { coreMock } from '../../../core/public/mocks';
import { homePluginMock } from '../../home/public/mocks';
import { managementPluginMock } from '../../management/public/mocks';
import { dataPluginMock } from '../../data/public/mocks';
import { uiActionsPluginMock } from '../../ui_actions/public/mocks';
import { SavedObjectsManagementPlugin } from './plugin';

describe('SavedObjectsManagementPlugin', () => {
Expand All @@ -48,8 +49,13 @@ describe('SavedObjectsManagementPlugin', () => {
});
const homeSetup = homePluginMock.createSetupContract();
const managementSetup = managementPluginMock.createSetupContract();
const uiActionsSetup = uiActionsPluginMock.createSetupContract();

await plugin.setup(coreSetup, { home: homeSetup, management: managementSetup });
await plugin.setup(coreSetup, {
home: homeSetup,
management: managementSetup,
uiActions: uiActionsSetup,
});

expect(homeSetup.featureCatalogue.register).toHaveBeenCalledTimes(1);
expect(homeSetup.featureCatalogue.register).toHaveBeenCalledWith(
Expand Down
13 changes: 11 additions & 2 deletions src/plugins/saved_objects_management/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { CoreSetup, CoreStart, Plugin } from 'src/core/public';

import { VisBuilderStart } from '../../vis_builder/public';
import { ManagementSetup } from '../../management/public';
import { UiActionsSetup, UiActionsStart } from '../../ui_actions/public';
import { DataPublicPluginStart } from '../../data/public';
import { DashboardStart } from '../../dashboard/public';
import { DiscoverStart } from '../../discover/public';
Expand All @@ -53,6 +54,7 @@ import {
ISavedObjectsManagementServiceRegistry,
} from './services';
import { registerServices } from './register_services';
import { bootstrap } from './ui_actions_bootstrap';

export interface SavedObjectsManagementPluginSetup {
actions: SavedObjectsManagementActionServiceSetup;
Expand All @@ -65,11 +67,13 @@ export interface SavedObjectsManagementPluginStart {
actions: SavedObjectsManagementActionServiceStart;
columns: SavedObjectsManagementColumnServiceStart;
namespaces: SavedObjectsManagementNamespaceServiceStart;
uiActions: UiActionsStart;
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
}

export interface SetupDependencies {
management: ManagementSetup;
home?: HomePublicPluginSetup;
uiActions: UiActionsSetup;
}

export interface StartDependencies {
Expand All @@ -79,6 +83,7 @@ export interface StartDependencies {
visAugmenter?: VisAugmenterStart;
discover?: DiscoverStart;
visBuilder?: VisBuilderStart;
uiActions: UiActionsStart;
}

export class SavedObjectsManagementPlugin
Expand All @@ -96,7 +101,7 @@ export class SavedObjectsManagementPlugin

public setup(
core: CoreSetup<StartDependencies, SavedObjectsManagementPluginStart>,
{ home, management }: SetupDependencies
{ home, management, uiActions }: SetupDependencies
): SavedObjectsManagementPluginSetup {
const actionSetup = this.actionService.setup();
const columnSetup = this.columnService.setup();
Expand Down Expand Up @@ -136,6 +141,9 @@ export class SavedObjectsManagementPlugin
},
});

// sets up the context mappings and registers any triggers/actions for the plugin
bootstrap(uiActions);

// depends on `getStartServices`, should not be awaited
registerServices(this.serviceRegistry, core.getStartServices);

Expand All @@ -147,7 +155,7 @@ export class SavedObjectsManagementPlugin
};
}

public start(core: CoreStart, { data }: StartDependencies) {
public start(core: CoreStart, { data, uiActions }: StartDependencies) {
const actionStart = this.actionService.start();
const columnStart = this.columnService.start();
const namespaceStart = this.namespaceService.start();
Expand All @@ -156,6 +164,7 @@ export class SavedObjectsManagementPlugin
actions: actionStart,
columns: columnStart,
namespaces: namespaceStart,
uiActions,
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const registerServices = async (
) => {
const [
,
{ dashboard, visualizations, visAugmenter, discover, visBuilder },
{ dashboard, visualizations, visAugmenter, discover, visBuilder, uiActions },
] = await getStartServices();

if (dashboard) {
Expand Down Expand Up @@ -80,4 +80,12 @@ export const registerServices = async (
service: visBuilder.savedVisBuilderLoader,
});
}

if (uiActions) {
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
registry.register({
id: 'uiActions',
title: 'uiActions',
service: uiActions,
});
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { SavedObjectLoader } from '../../../saved_objects/public';

export interface SavedObjectsManagementServiceRegistryEntry {
id: string;
service: SavedObjectLoader;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, Why do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

service: SavedObjectLoader | unknown;
title: string;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

export {
SAVED_OBJECT_DELETE_TRIGGER,
savedObjectDeleteTrigger,
} from './saved_object_delete_trigger';
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { i18n } from '@osd/i18n';
import { Trigger } from '../../../ui_actions/public';

export const SAVED_OBJECT_DELETE_TRIGGER = 'SAVED_OBJECT_DELETE_TRIGGER';
export const savedObjectDeleteTrigger: Trigger<'SAVED_OBJECT_DELETE_TRIGGER'> = {
id: SAVED_OBJECT_DELETE_TRIGGER,
title: i18n.translate('savedObjectsManagement.triggers.savedObjectDeleteTitle', {
defaultMessage: 'Saved object delete',
}),
description: i18n.translate('savedObjectsManagement.triggers.savedObjectDeleteDescription', {
defaultMessage: 'Perform additional actions after deleting a saved object',
}),
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { UiActionsSetup } from '../../ui_actions/public';
import { SAVED_OBJECT_DELETE_TRIGGER, savedObjectDeleteTrigger } from './triggers';

export interface SavedObjectDeleteContext {
type: string;
savedObjectId: string;
}

declare module '../../ui_actions/public' {
export interface TriggerContextMapping {
[SAVED_OBJECT_DELETE_TRIGGER]: SavedObjectDeleteContext;
}
}

export const bootstrap = (uiActions: UiActionsSetup) => {
uiActions.registerTrigger(savedObjectDeleteTrigger);
};
2 changes: 1 addition & 1 deletion src/plugins/vis_augmenter/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@
"uiActions",
"embeddable"
],
"requiredBundles": ["opensearchDashboardsReact"]
"requiredBundles": ["opensearchDashboardsReact", "savedObjectsManagement"]
}
10 changes: 10 additions & 0 deletions src/plugins/vis_augmenter/public/actions/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

export {
PLUGIN_RESOURCE_DELETE_ACTION,
PluginResourceDeleteAction,
} from './plugin_resource_delete_action';
export { SAVED_OBJECT_DELETE_ACTION, SavedObjectDeleteAction } from './saved_object_delete_action';
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { generateAugmentVisSavedObject } from '../saved_augment_vis';
import { VisLayerTypes } from '../types';
import { generateVisLayer } from '../utils';
import { PluginResourceDeleteAction } from './plugin_resource_delete_action';

const sampleSavedObj = generateAugmentVisSavedObject(
'test-id',
{
type: 'PointInTimeEvents',
name: 'test-fn-name',
args: {},
},
'test-vis-id',
'test-origin-plugin',
{
type: 'test-resource-type',
id: 'test-resource-id',
}
);

const sampleVisLayer = generateVisLayer(VisLayerTypes.PointInTimeEvents);

describe('SavedObjectDeleteAction', () => {
it('is incompatible with invalid saved obj list', async () => {
const action = new PluginResourceDeleteAction();
const visLayers = [sampleVisLayer];
// @ts-ignore
expect(await action.isCompatible({ savedObjs: null, visLayers })).toBe(false);
// @ts-ignore
expect(await action.isCompatible({ savedObjs: undefined, visLayers })).toBe(false);
expect(await action.isCompatible({ savedObjs: [], visLayers })).toBe(false);
});

it('is incompatible with invalid vislayer list', async () => {
const action = new PluginResourceDeleteAction();
const savedObjs = [sampleSavedObj];
// @ts-ignore
expect(await action.isCompatible({ savedObjs, visLayers: null })).toBe(false);
// @ts-ignore
expect(await action.isCompatible({ savedObjs, visLayers: undefined })).toBe(false);
expect(await action.isCompatible({ savedObjs, visLayers: [] })).toBe(false);
});

it('execute throws error if incompatible saved objs list', async () => {
const action = new PluginResourceDeleteAction();
async function check(savedObjs: any, visLayers: any) {
await action.execute({ savedObjs, visLayers });
}
await expect(check(null, [sampleVisLayer])).rejects.toThrow(Error);
});

it('execute throws error if incompatible vis layer list', async () => {
const action = new PluginResourceDeleteAction();
async function check(savedObjs: any, visLayers: any) {
await action.execute({ savedObjs, visLayers });
}
await expect(check([sampleSavedObj], null)).rejects.toThrow(Error);
});

it('execute is successful if valid saved obj and vis layer lists', async () => {
const action = new PluginResourceDeleteAction();
async function check(savedObjs: any, visLayers: any) {
await action.execute({ savedObjs, visLayers });
}
await expect(check([sampleSavedObj], [sampleVisLayer])).resolves;
});

it('Returns display name', async () => {
const action = new PluginResourceDeleteAction();
expect(action.getDisplayName()).toBeDefined();
});

it('Returns icon type', async () => {
const action = new PluginResourceDeleteAction();
expect(action.getIconType()).toBeDefined();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

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 { getSavedAugmentVisLoader } from '../services';
import { PluginResourceDeleteContext } from '../ui_actions_bootstrap';
import { cleanupStaleObjects } from '../utils';

export const PLUGIN_RESOURCE_DELETE_ACTION = 'PLUGIN_RESOURCE_DELETE_ACTION';

export class PluginResourceDeleteAction implements Action<PluginResourceDeleteContext> {
public readonly type = PLUGIN_RESOURCE_DELETE_ACTION;
public readonly id = PLUGIN_RESOURCE_DELETE_ACTION;
public order = 1;

public getIconType(): EuiIconType {
return `trash`;
}

public getDisplayName() {
return i18n.translate('dashboard.actions.deleteSavedObject.name', {
defaultMessage:
'Clean up all augment-vis saved objects associated to the deleted visualization',
});
}

public async isCompatible({ savedObjs, visLayers }: PluginResourceDeleteContext) {
return !isEmpty(savedObjs) && !isEmpty(visLayers);
}

/**
* If we have just collected all of the saved objects and generated vis layers,
* sweep through them all and if any of the resources are deleted, delete those
* corresponding saved objects
*/
public async execute({ savedObjs, visLayers }: PluginResourceDeleteContext) {
if (!(await this.isCompatible({ savedObjs, visLayers }))) {
throw new IncompatibleActionError();
}
cleanupStaleObjects(savedObjs, visLayers, getSavedAugmentVisLoader());
}
}
Loading