[Cases] Fix plugin lifecycle inconsistencies (elastic#177132)

## Summary The script from elastic#171483 can identify inconsistencies and untyped dependencies in Kibana plugins. This PR fixes: - Move `esUiShared`, `kibanaReact`, `kibanaUtils`, and `savedObjectsFinder` to `requiredBundles` - Rename `CasesUiSetup` to `CasesPublicSetup` - Rename `CasesUiStart` to `CasesPublicStart` - Rename `CasesSetup` to `CasesServerSetup` - Rename `CasesStart` to `CasesServerStart` - Rename `CasesPluginSetup` (public) to `CasesPublicSetupDependencies` - Rename `CasesPluginStart` (public) to `CasesPublicStartDependencies` - Moved `PluginsSetup` from `server/plugin.ts` to `server/types.ts` and rename it to `CasesServerSetupDependencies` - Moved `PluginsStart` from `server/plugin.ts` to `server/types.ts` and rename it to `CasesServerStartDependencies` I could not add the `apm` to optional plugins because I get an error due to circular dependencies. Observability and Secuirty solution have `apm` and `cases` as a dependency and probably this cause the following error. <details> <summary>Error</summary> ``` info Running model version mapping addition checks │ info Generating field lists from registry and file │ info Loading core with all plugins enabled so that we can get all savedObject mappings... │ERROR UNHANDLED ERROR: Error: Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies: ["aiAssistantManagementObservability","aiops","apm","assetManager","cases","cloudDefend","datasetQuality","elasticAssistant","enterpriseSearch","exploratoryView","infra","kubernetesSecurity","logsShared","logstash","metricsDataAccess","ml","monitoring","observability","observabilityAIAssistant","observabilityOnboarding","observabilityShared","observabilityLogsExplorer","osquery","profiling","securitySolution","securitySolutionEss","securitySolutionServerless","serverlessObservability","sessionView","synthetics","threatIntelligence","timelines","upgradeAssistant","uptime","ux"] │ERROR at PluginsSystem.getTopologicallySortedPluginNames (plugins_system.ts:354:13) │ERROR at PluginsSystem.uiPlugins (plugins_system.ts:274:36) │ERROR at PluginsService.discover (plugins_service.ts:120:58) │ERROR at Server.preboot (server.ts:172:30) │ERROR at Root.preboot (index.ts:57:14) │ERROR at extract_field_lists_from_plugins_worker.ts:51:3 ERROR UNHANDLED ERROR ERROR Error: worker exited without sending mappings at extractFieldListsFromPlugins (extract_field_lists_from_plugins.ts:62:11) at processTicksAndRejections (node:internal/process/task_queues:95:5) at runModelVersionMappingAdditionsChecks (run_versions_mapping_additions_check.ts:28:66) at run_check_mappings_update_cli.ts:23:9 at tooling_log.ts:84:18 at description (run_check_mappings_update_cli.ts:22:7) at run.ts:73:10 at withProcRunner (with_proc_runner.ts:29:5) at run (run.ts:71:5) ``` </details> Also, the `storage` is passed by the `cases` plugin to the Kibana context and is not a dependency to another plugin. The script assumes that the `storage` is a plugin dependency and reports it. Neverthelss, this demonstrates that we should stop passing `storage` to the Kibana context and use hooks like `useLocalStorage`. ## Results from the script <img width="727" alt="Screenshot 2024-02-17 at 8 54 32 AM" src="https://github.com/elastic/kibana/assets/7871006/1c86f501-72c5-473c-92f5-7a4935da914b"> ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Kibana Machine <[email protected]>
fkanout · Mar 4, 2024 · 0dc6a28 · 0dc6a28
1 parent 136cdba
commit 0dc6a28
Show file tree

Hide file tree

Showing 51 changed files with 203 additions and 170 deletions.
diff --git a/x-pack/plugins/aiops/public/cases/register_change_point_charts_attachment.tsx b/x-pack/plugins/aiops/public/cases/register_change_point_charts_attachment.tsx
@@ -8,7 +8,7 @@
 import React from 'react';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
-import type { CasesUiSetup } from '@kbn/cases-plugin/public';
+import type { CasesPublicSetup } from '@kbn/cases-plugin/public';
 import type { CoreStart } from '@kbn/core/public';
 import {
   CASES_ATTACHMENT_CHANGE_POINT_CHART,
@@ -18,7 +18,7 @@ import { getEmbeddableChangePointChart } from '../embeddable/embeddable_change_p
 import { AiopsPluginStartDeps } from '../types';
 
 export function registerChangePointChartsAttachment(
-  cases: CasesUiSetup,
+  cases: CasesPublicSetup,
   coreStart: CoreStart,
   pluginStart: AiopsPluginStartDeps
 ) {

diff --git a/x-pack/plugins/aiops/public/hooks/use_aiops_app_context.ts b/x-pack/plugins/aiops/public/hooks/use_aiops_app_context.ts
@@ -31,7 +31,7 @@ import type {
 import type { TimeRange as TimeRangeMs } from '@kbn/ml-date-picker';
 import type { PresentationUtilPluginStart } from '@kbn/presentation-util-plugin/public';
 import type { EmbeddableStart } from '@kbn/embeddable-plugin/public';
-import type { CasesUiStart } from '@kbn/cases-plugin/public';
+import type { CasesPublicStart } from '@kbn/cases-plugin/public';
 import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/public';
 import type { UiActionsStart } from '@kbn/ui-actions-plugin/public';
 
@@ -123,7 +123,7 @@ export interface AiopsAppDependencies {
   };
   presentationUtil?: PresentationUtilPluginStart;
   embeddable?: EmbeddableStart;
-  cases?: CasesUiStart;
+  cases?: CasesPublicStart;
   isServerless?: boolean;
   /** Identifier to indicate the plugin utilizing the component */
   embeddingOrigin?: string;

diff --git a/x-pack/plugins/aiops/public/types.ts b/x-pack/plugins/aiops/public/types.ts
@@ -16,13 +16,13 @@ import type { SharePluginStart } from '@kbn/share-plugin/public';
 import type { UiActionsSetup, UiActionsStart } from '@kbn/ui-actions-plugin/public';
 import type { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public';
 import type { EmbeddableSetup, EmbeddableStart } from '@kbn/embeddable-plugin/public';
-import type { CasesUiSetup } from '@kbn/cases-plugin/public';
+import type { CasesPublicSetup } from '@kbn/cases-plugin/public';
 import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/public';
 import type { EmbeddableChangePointChartInput } from './embeddable/embeddable_change_point_chart';
 
 export interface AiopsPluginSetupDeps {
   embeddable: EmbeddableSetup;
-  cases: CasesUiSetup;
+  cases: CasesPublicSetup;
   licensing: LicensingPluginSetup;
 
   uiActions: UiActionsSetup;

diff --git a/x-pack/plugins/aiops/server/register_cases.ts b/x-pack/plugins/aiops/server/register_cases.ts
@@ -6,10 +6,10 @@
  */
 
 import type { Logger } from '@kbn/core/server';
-import type { CasesSetup } from '@kbn/cases-plugin/server';
+import type { CasesServerSetup } from '@kbn/cases-plugin/server';
 import { CASES_ATTACHMENT_CHANGE_POINT_CHART } from '../common/constants';
 
-export function registerCasesPersistableState(cases: CasesSetup | undefined, logger: Logger) {
+export function registerCasesPersistableState(cases: CasesServerSetup | undefined, logger: Logger) {
   if (cases) {
     try {
       cases.attachmentFramework.registerPersistableState({

diff --git a/x-pack/plugins/aiops/server/types.ts b/x-pack/plugins/aiops/server/types.ts
@@ -7,13 +7,13 @@
 
 import type { PluginSetup, PluginStart } from '@kbn/data-plugin/server';
 import type { LicensingPluginSetup } from '@kbn/licensing-plugin/server';
-import type { CasesSetup } from '@kbn/cases-plugin/server';
+import type { CasesServerSetup } from '@kbn/cases-plugin/server';
 import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server';
 
 export interface AiopsPluginSetupDeps {
   data: PluginSetup;
   licensing: LicensingPluginSetup;
-  cases?: CasesSetup;
+  cases?: CasesServerSetup;
   usageCollection?: UsageCollectionSetup;
 }
 

diff --git a/x-pack/plugins/cases/kibana.jsonc b/x-pack/plugins/cases/kibana.jsonc
@@ -15,19 +15,15 @@
       "actions",
       "data",
       "embeddable",
-      "esUiShared",
       "lens",
       "licensing",
       "features",
-      "kibanaReact",
-      "kibanaUtils",
       "triggersActionsUi",
       "management",
       "security",
       "notifications",
       "ruleRegistry",
       "files",
-      "savedObjectsFinder",
       "contentManagement",
       "uiActions",
     ],
@@ -38,7 +34,12 @@
       "spaces",
       "serverless",
     ],
-    "requiredBundles": [],
+    "requiredBundles": [
+      "esUiShared",
+      "kibanaReact",
+      "kibanaUtils",
+      "savedObjectsFinder",
+    ],
     "extraPublicDirs": [
       "common"
     ]

diff --git a/x-pack/plugins/cases/public/client/api/index.ts b/x-pack/plugins/cases/public/client/api/index.ts
@@ -16,9 +16,9 @@ import type {
 import { getCasesFromAlertsUrl } from '../../../common/api';
 import { bulkGetCases, getCases, getCasesMetrics, getCasesStatus } from '../../api';
 import type { CasesFindResponseUI, CasesStatus, CasesMetrics } from '../../../common/ui';
-import type { CasesUiStart } from '../../types';
+import type { CasesPublicStart } from '../../types';
 
-export const createClientAPI = ({ http }: { http: HttpStart }): CasesUiStart['api'] => {
+export const createClientAPI = ({ http }: { http: HttpStart }): CasesPublicStart['api'] => {
   return {
     getRelatedCases: async (
       alertId: string,

diff --git a/x-pack/plugins/cases/public/common/lib/kibana/services.ts b/x-pack/plugins/cases/public/common/lib/kibana/services.ts
@@ -7,10 +7,10 @@
 
 import type { CoreStart } from '@kbn/core/public';
 import type { CasesUiConfigType } from '../../../../common/ui/types';
-import type { CasesPluginStart } from '../../../types';
+import type { CasesPublicStartDependencies } from '../../../types';
 
 type GlobalServices = Pick<CoreStart, 'application' | 'http' | 'theme'> &
-  Pick<CasesPluginStart, 'serverless'>;
+  Pick<CasesPublicStartDependencies, 'serverless'>;
 
 export class KibanaServices {
   private static kibanaVersion?: string;

diff --git a/x-pack/plugins/cases/public/components/utils.ts b/x-pack/plugins/cases/public/components/utils.ts
@@ -14,7 +14,7 @@ import type {
 import type { UserProfileWithAvatar } from '@kbn/user-profile-components';
 import type { ConnectorTypeFields } from '../../common/types/domain';
 import { ConnectorTypes } from '../../common/types/domain';
-import type { CasesPluginStart } from '../types';
+import type { CasesPublicStartDependencies } from '../types';
 import { connectorValidator as swimlaneConnectorValidator } from './connectors/swimlane/validator';
 import type { CaseActionConnector } from './types';
 import type { CaseUser, CaseUsers } from '../../common/ui/types';
@@ -145,7 +145,7 @@ export const getConnectorsFormDeserializer = <T extends { fields: ConnectorTypeF
 };
 
 export const getConnectorIcon = (
-  triggersActionsUi: CasesPluginStart['triggersActionsUi'],
+  triggersActionsUi: CasesPublicStartDependencies['triggersActionsUi'],
   type?: string
 ): IconType => {
   /**

diff --git a/x-pack/plugins/cases/public/components/visualizations/actions/types.ts b/x-pack/plugins/cases/public/components/visualizations/actions/types.ts
@@ -11,7 +11,7 @@ import type * as H from 'history';
 import type { Storage } from '@kbn/kibana-utils-plugin/public';
 
 import type { ActionExecutionContext } from '@kbn/ui-actions-plugin/public';
-import type { CasesPluginStart } from '../../../types';
+import type { CasesPublicStartDependencies } from '../../../types';
 import type { CasesContextProps } from '../../cases_context';
 
 export type CasesUIActionContextProps = Pick<
@@ -23,7 +23,7 @@ export type CasesUIActionContextProps = Pick<
 
 export interface CasesUIActionProps {
   core: CoreStart;
-  plugins: CasesPluginStart;
+  plugins: CasesPublicStartDependencies;
   caseContextProps: CasesUIActionContextProps;
   history: H.History;
   storage: Storage;

diff --git a/x-pack/plugins/cases/public/index.tsx b/x-pack/plugins/cases/public/index.tsx
@@ -14,7 +14,7 @@ export function plugin(initializerContext: PluginInitializerContext) {
 
 export { DRAFT_COMMENT_STORAGE_ID } from './components/markdown_editor/plugins/lens/constants';
 
-export type { CasesUiStart, CasesUiSetup } from './types';
+export type { CasesPublicStart, CasesPublicSetup } from './types';
 export type { GetCreateCaseFlyoutProps } from './client/ui/get_create_case_flyout';
 export type { GetAllCasesSelectorModalProps } from './client/ui/get_all_cases_selector_modal';
 export type { GetRecentCasesProps } from './client/ui/get_recent_cases';

diff --git a/x-pack/plugins/cases/public/mocks.ts b/x-pack/plugins/cases/public/mocks.ts
@@ -6,9 +6,9 @@
  */
 
 import { mockCasesContext } from './mocks/mock_cases_context';
-import type { CasesUiStart } from './types';
+import type { CasesPublicStart } from './types';
 
-const apiMock: jest.Mocked<CasesUiStart['api']> = {
+const apiMock: jest.Mocked<CasesPublicStart['api']> = {
   getRelatedCases: jest.fn(),
   cases: {
     find: jest.fn(),
@@ -18,7 +18,7 @@ const apiMock: jest.Mocked<CasesUiStart['api']> = {
   },
 };
 
-const uiMock: jest.Mocked<CasesUiStart['ui']> = {
+const uiMock: jest.Mocked<CasesPublicStart['ui']> = {
   getCases: jest.fn(),
   getCasesContext: jest.fn().mockImplementation(() => mockCasesContext),
   getAllCasesSelectorModal: jest.fn(),
@@ -28,7 +28,7 @@ const uiMock: jest.Mocked<CasesUiStart['ui']> = {
 export const openAddToExistingCaseModalMock = jest.fn();
 export const openAddToNewCaseFlyoutMock = jest.fn();
 
-const hooksMock: jest.Mocked<CasesUiStart['hooks']> = {
+const hooksMock: jest.Mocked<CasesPublicStart['hooks']> = {
   useCasesAddToNewCaseFlyout: jest.fn().mockImplementation(() => ({
     open: openAddToNewCaseFlyoutMock,
   })),
@@ -37,7 +37,7 @@ const hooksMock: jest.Mocked<CasesUiStart['hooks']> = {
   })),
 };
 
-const helpersMock: jest.Mocked<CasesUiStart['helpers']> = {
+const helpersMock: jest.Mocked<CasesPublicStart['helpers']> = {
   canUseCases: jest.fn(),
   getUICapabilities: jest.fn().mockReturnValue({
     all: false,
@@ -54,10 +54,10 @@ const helpersMock: jest.Mocked<CasesUiStart['helpers']> = {
 };
 
 export interface CaseUiClientMock {
-  api: jest.Mocked<CasesUiStart['api']>;
-  ui: jest.Mocked<CasesUiStart['ui']>;
-  hooks: jest.Mocked<CasesUiStart['hooks']>;
-  helpers: jest.Mocked<CasesUiStart['helpers']>;
+  api: jest.Mocked<CasesPublicStart['api']>;
+  ui: jest.Mocked<CasesPublicStart['ui']>;
+  hooks: jest.Mocked<CasesPublicStart['hooks']>;
+  helpers: jest.Mocked<CasesPublicStart['helpers']>;
 }
 
 export const mockCasesContract = (): CaseUiClientMock => ({

diff --git a/x-pack/plugins/cases/public/plugin.test.ts b/x-pack/plugins/cases/public/plugin.test.ts
@@ -18,7 +18,7 @@ import { lensPluginMock } from '@kbn/lens-plugin/public/mocks';
 import { contentManagementMock } from '@kbn/content-management-plugin/public/mocks';
 import { mockStorage } from '@kbn/kibana-utils-plugin/public/storage/hashed_item_store/mock';
 import { triggersActionsUiMock } from '@kbn/triggers-actions-ui-plugin/public/mocks';
-import type { CasesPluginSetup, CasesPluginStart } from './types';
+import type { CasesPublicStartDependencies, CasesPublicSetupDependencies } from './types';
 import { CasesUiPlugin } from './plugin';
 import { ALLOWED_MIME_TYPES } from '../common/constants/mime_types';
 
@@ -36,8 +36,8 @@ describe('Cases Ui Plugin', () => {
   let plugin: CasesUiPlugin;
   let coreSetup: ReturnType<typeof coreMock.createSetup>;
   let coreStart: ReturnType<typeof coreMock.createStart>;
-  let pluginsSetup: jest.Mocked<CasesPluginSetup>;
-  let pluginsStart: jest.Mocked<CasesPluginStart>;
+  let pluginsSetup: jest.Mocked<CasesPublicSetupDependencies>;
+  let pluginsStart: jest.Mocked<CasesPublicStartDependencies>;
 
   beforeEach(() => {
     context = coreMock.createPluginInitializerContext(getConfig());

diff --git a/x-pack/plugins/cases/public/plugin.ts b/x-pack/plugins/cases/public/plugin.ts
@@ -28,16 +28,27 @@ import { getUICapabilities } from './client/helpers/capabilities';
 import { ExternalReferenceAttachmentTypeRegistry } from './client/attachment_framework/external_reference_registry';
 import { PersistableStateAttachmentTypeRegistry } from './client/attachment_framework/persistable_state_registry';
 import { registerCaseFileKinds } from './files';
-import type { CasesPluginSetup, CasesPluginStart, CasesUiSetup, CasesUiStart } from './types';
 import { registerInternalAttachments } from './internal_attachments';
 import { registerActions } from './components/visualizations/actions';
+import type {
+  CasesPublicSetup,
+  CasesPublicStart,
+  CasesPublicSetupDependencies,
+  CasesPublicStartDependencies,
+} from './types';
 
 /**
  * @public
  * A plugin for retrieving Cases UI components
  */
 export class CasesUiPlugin
-  implements Plugin<void, CasesUiStart, CasesPluginSetup, CasesPluginStart>
+  implements
+    Plugin<
+      CasesPublicSetup,
+      CasesPublicStart,
+      CasesPublicSetupDependencies,
+      CasesPublicStartDependencies
+    >
 {
   private readonly kibanaVersion: string;
   private readonly storage = new Storage(localStorage);
@@ -50,7 +61,7 @@ export class CasesUiPlugin
     this.persistableStateAttachmentTypeRegistry = new PersistableStateAttachmentTypeRegistry();
   }
 
-  public setup(core: CoreSetup, plugins: CasesPluginSetup): CasesUiSetup {
+  public setup(core: CoreSetup, plugins: CasesPublicSetupDependencies): CasesPublicSetup {
     const kibanaVersion = this.kibanaVersion;
     const storage = this.storage;
     const externalReferenceAttachmentTypeRegistry = this.externalReferenceAttachmentTypeRegistry;
@@ -83,7 +94,7 @@ export class CasesUiPlugin
         async mount(params: ManagementAppMountParams) {
           const [coreStart, pluginsStart] = (await core.getStartServices()) as [
             CoreStart,
-            CasesPluginStart,
+            CasesPublicStartDependencies,
             unknown
           ];
 
@@ -114,7 +125,7 @@ export class CasesUiPlugin
     };
   }
 
-  public start(core: CoreStart, plugins: CasesPluginStart): CasesUiStart {
+  public start(core: CoreStart, plugins: CasesPublicStartDependencies): CasesPublicStart {
     const config = this.initializerContext.config.get<CasesUiConfigType>();
 
     KibanaServices.init({

diff --git a/x-pack/plugins/cases/public/types.ts b/x-pack/plugins/cases/public/types.ts
@@ -58,21 +58,26 @@ import type {
   ExternalReferenceSOAttachmentPayload,
 } from '../common/types/domain';
 
-export interface CasesPluginSetup {
+export interface CasesPublicSetupDependencies {
   files: FilesSetup;
   security: SecurityPluginSetup;
   serverless?: ServerlessPluginSetup;
   management: ManagementSetup;
   home?: HomePublicPluginSetup;
 }
 
-export interface CasesPluginStart {
+export interface CasesPublicStartDependencies {
   apm?: ApmBase;
   data: DataPublicPluginStart;
   embeddable: EmbeddableStart;
   features: FeaturesPluginStart;
   files: FilesStart;
   lens: LensPublicStart;
+  /**
+   * Cases in used by other plugins. Plugins pass the
+   * service to their KibanaContext. ML does not pass
+   * the licensing service thus it is optional.
+   */
   licensing?: LicensingPluginStart;
   contentManagement: ContentManagementPublicStart;
   security: SecurityPluginStart;
@@ -89,23 +94,23 @@ export interface CasesPluginStart {
  * Leaving it out currently in lieu of RBAC changes
  */
 
-export type StartServices = CoreStart & CasesPluginStart;
+export type StartServices = CoreStart & CasesPublicStartDependencies;
 
 export interface RenderAppProps {
   mountParams: ManagementAppMountParams;
   coreStart: CoreStart;
-  pluginsStart: CasesPluginStart;
+  pluginsStart: CasesPublicStartDependencies;
   storage: Storage;
   kibanaVersion: string;
   externalReferenceAttachmentTypeRegistry: ExternalReferenceAttachmentTypeRegistry;
   persistableStateAttachmentTypeRegistry: PersistableStateAttachmentTypeRegistry;
 }
 
-export interface CasesUiSetup {
+export interface CasesPublicSetup {
   attachmentFramework: AttachmentFramework;
 }
 
-export interface CasesUiStart {
+export interface CasesPublicStart {
   api: {
     getRelatedCases: (
       alertId: string,

diff --git a/x-pack/plugins/cases/server/index.ts b/x-pack/plugins/cases/server/index.ts
@@ -26,4 +26,4 @@ export const plugin = async (initializerContext: PluginInitializerContext) => {
   return new CasePlugin(initializerContext);
 };
 
-export type { CasesSetup, CasesStart } from './types';
+export type { CasesServerSetup, CasesServerStart } from './types';