Sustainable Kibana Architecture: Remove dependencies between plugins …

…that are related by _App Links_ (elastic#199492) ## Summary This PR introduces a Core API to check whether a given application has been registered. Plugins can use this for their _App Links_, without having to depend on the referenced plugin(s) anymore. This way, we can get rid of some inter-solution dependencies, and categorise plugins more appropriately. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
pheyos · Nov 14, 2024 · ad56ec5 · ad56ec5
1 parent 560ae9a
commit ad56ec5
Show file tree

Hide file tree

Showing 23 changed files with 242 additions and 196 deletions.
diff --git a/packages/core/application/core-application-browser-internal/src/application_service.test.ts b/packages/core/application/core-application-browser-internal/src/application_service.test.ts
@@ -26,14 +26,14 @@ import { themeServiceMock } from '@kbn/core-theme-browser-mocks';
 import { overlayServiceMock } from '@kbn/core-overlays-browser-mocks';
 import { customBrandingServiceMock } from '@kbn/core-custom-branding-browser-mocks';
 import { analyticsServiceMock } from '@kbn/core-analytics-browser-mocks';
-import { MockLifecycle } from './test_helpers/test_types';
+import type { MockLifecycle } from './test_helpers/test_types';
 import { ApplicationService } from './application_service';
 import {
-  App,
-  AppDeepLink,
+  type App,
+  type AppDeepLink,
   AppStatus,
-  AppUpdater,
-  PublicAppInfo,
+  type AppUpdater,
+  type PublicAppInfo,
 } from '@kbn/core-application-browser';
 import { act } from 'react-dom/test-utils';
 import { DEFAULT_APP_VISIBILITY } from './utils';
@@ -618,6 +618,26 @@ describe('#start()', () => {
     });
   });
 
+  describe('isAppRegistered', () => {
+    let isAppRegistered: any;
+    beforeEach(async () => {
+      const { register } = service.setup(setupDeps);
+      register(Symbol(), createApp({ id: 'one_app' }));
+      register(Symbol(), createApp({ id: 'another_app', appRoute: '/custom/path' }));
+
+      const start = await service.start(startDeps);
+      isAppRegistered = start.isAppRegistered;
+    });
+
+    it('returns false for unregistered apps', () => {
+      expect(isAppRegistered('oneApp')).toEqual(false);
+    });
+
+    it('returns true for registered apps', () => {
+      expect(isAppRegistered('another_app')).toEqual(true);
+    });
+  });
+
   describe('getUrlForApp', () => {
     it('creates URL for unregistered appId', async () => {
       service.setup(setupDeps);

diff --git a/packages/core/application/core-application-browser-internal/src/application_service.tsx b/packages/core/application/core-application-browser-internal/src/application_service.tsx
@@ -327,6 +327,9 @@ export class ApplicationService {
         takeUntil(this.stop$)
       ),
       history: this.history!,
+      isAppRegistered: (appId: string): boolean => {
+        return applications$.value.get(appId) !== undefined;
+      },
       getUrlForApp: (
         appId,
         {

diff --git a/packages/core/application/core-application-browser-mocks/src/application_service.mock.ts b/packages/core/application/core-application-browser-mocks/src/application_service.mock.ts
@@ -51,6 +51,7 @@ const createStartContractMock = (): jest.Mocked<ApplicationStart> => {
     navigateToApp: jest.fn(),
     navigateToUrl: jest.fn(),
     getUrlForApp: jest.fn(),
+    isAppRegistered: jest.fn(),
   };
 };
 
@@ -92,6 +93,7 @@ const createInternalStartContractMock = (
     currentActionMenu$: new BehaviorSubject<MountPoint | undefined>(undefined),
     getComponent: jest.fn(),
     getUrlForApp: jest.fn(),
+    isAppRegistered: jest.fn(),
     navigateToApp: jest.fn().mockImplementation((appId) => currentAppId$.next(appId)),
     navigateToUrl: jest.fn(),
     history: createHistoryMock(),

diff --git a/packages/core/application/core-application-browser/src/contracts.ts b/packages/core/application/core-application-browser/src/contracts.ts
@@ -68,9 +68,12 @@ export interface ApplicationStart {
   applications$: Observable<ReadonlyMap<string, PublicAppInfo>>;
 
   /**
-   * Navigate to a given app
+   * Navigate to a given app.
+   * If a plugin is disabled any applications it registers won't be available either.
+   * Before rendering a UI element that a user could use to navigate to another application,
+   * first check if the destination application is actually available using the isAppRegistered API.
    *
-   * @param appId
+   * @param appId - The identifier of the app to navigate to
    * @param options - navigation options
    */
   navigateToApp(appId: string, options?: NavigateToAppOptions): Promise<void>;
@@ -114,6 +117,14 @@ export interface ApplicationStart {
    */
   navigateToUrl(url: string, options?: NavigateToUrlOptions): Promise<void>;
 
+  /**
+   * Checks whether a given application is registered.
+   *
+   * @param appId - The identifier of the app to check
+   * @returns true if the given appId is registered in the system, false otherwise.
+   */
+  isAppRegistered(appId: string): boolean;
+
   /**
    * Returns the absolute path (or URL) to a given app, including the global base path.
    *

diff --git a/packages/core/plugins/core-plugins-browser-internal/src/plugin_context.ts b/packages/core/plugins/core-plugins-browser-internal/src/plugin_context.ts
@@ -143,6 +143,7 @@ export function createPluginStartContext<
       navigateToApp: deps.application.navigateToApp,
       navigateToUrl: deps.application.navigateToUrl,
       getUrlForApp: deps.application.getUrlForApp,
+      isAppRegistered: deps.application.isAppRegistered,
       currentLocation$: deps.application.currentLocation$,
     },
     customBranding: deps.customBranding,

diff --git a/src/plugins/discover/public/__mocks__/start_contract.ts b/src/plugins/discover/public/__mocks__/start_contract.ts
@@ -32,6 +32,7 @@ export const createStartContractMock = (): jest.Mocked<ApplicationStart> => {
     capabilities,
     navigateToApp: jest.fn(),
     navigateToUrl: jest.fn(),
+    isAppRegistered: jest.fn(),
     getUrlForApp: jest.fn(),
   };
 };
diff --git a/...t/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap b/...t/public/management_section/objects_table/__snapshots__/saved_objects_table.test.tsx.snap
diff --git a/x-pack/plugins/enterprise_search/kibana.jsonc b/x-pack/plugins/enterprise_search/kibana.jsonc
@@ -2,9 +2,11 @@
   "type": "plugin",
   "id": "@kbn/enterprise-search-plugin",
   "owner": "@elastic/search-kibana",
-  // Could be categorised as Search in the future, but it currently needs to run in Observability too
-  "group": "platform",
-  "visibility": "shared",
+  // TODO this is currently used from Observability too, must be refactored before solution-specific builds
+  // see x-pack/plugins/observability_solution/observability_ai_assistant_management/public/routes/components/search_connector_tab.tsx
+  // cc sphilipse
+  "group": "search",
+  "visibility": "private",
   "description": "Adds dashboards for discovering and managing Enterprise Search products.",
   "plugin": {
     "id": "enterpriseSearch",

diff --git a/x-pack/plugins/enterprise_search/public/plugin.ts b/x-pack/plugins/enterprise_search/public/plugin.ts
@@ -80,8 +80,8 @@ export type EnterpriseSearchPublicStart = ReturnType<EnterpriseSearchPlugin['sta
 
 interface PluginsSetup {
   cloud?: CloudSetup;
-  licensing: LicensingPluginStart;
   home?: HomePublicPluginSetup;
+  licensing: LicensingPluginStart;
   security?: SecurityPluginSetup;
   share?: SharePluginSetup;
 }
@@ -98,8 +98,8 @@ export interface PluginsStart {
   ml?: MlPluginStart;
   navigation: NavigationPublicPluginStart;
   searchConnectors?: SearchConnectorsPluginStart;
-  searchPlayground?: SearchPlaygroundPluginStart;
   searchInferenceEndpoints?: SearchInferenceEndpointsPluginStart;
+  searchPlayground?: SearchPlaygroundPluginStart;
   security?: SecurityPluginStart;
   share?: SharePluginStart;
 }

diff --git a/x-pack/plugins/fleet/.storybook/context/application.ts b/x-pack/plugins/fleet/.storybook/context/application.ts
@@ -22,6 +22,7 @@ export const getApplication = () => {
     navigateToApp: async (app: string) => {
       action(`Navigate to: ${app}`);
     },
+    isAppRegistered: (appId: string) => true,
     getUrlForApp: (url: string) => url,
     capabilities: {
       catalogue: {},

diff --git a/x-pack/plugins/observability_solution/observability_ai_assistant_management/kibana.jsonc b/x-pack/plugins/observability_solution/observability_ai_assistant_management/kibana.jsonc
@@ -21,7 +21,6 @@
     "optionalPlugins": [
       "home",
       "serverless",
-      "enterpriseSearch"
     ],
     "requiredBundles": [
       "kibanaReact",

diff --git a/x-pack/plugins/observability_solution/observability_ai_assistant_management/public/plugin.ts b/x-pack/plugins/observability_solution/observability_ai_assistant_management/public/plugin.ts
@@ -6,11 +6,10 @@
  */
 
 import { i18n } from '@kbn/i18n';
-import { CoreSetup, Plugin, PluginInitializerContext } from '@kbn/core/public';
-import { ManagementSetup } from '@kbn/management-plugin/public';
-import { HomePublicPluginSetup } from '@kbn/home-plugin/public';
-import { ServerlessPluginStart } from '@kbn/serverless/public';
-import { EnterpriseSearchPublicStart } from '@kbn/enterprise-search-plugin/public';
+import type { CoreSetup, Plugin, PluginInitializerContext } from '@kbn/core/public';
+import type { ManagementSetup } from '@kbn/management-plugin/public';
+import type { HomePublicPluginSetup } from '@kbn/home-plugin/public';
+import type { ServerlessPluginStart } from '@kbn/serverless/public';
 
 import type {
   ObservabilityAIAssistantPublicSetup,
@@ -32,7 +31,6 @@ export interface SetupDependencies {
 export interface StartDependencies {
   observabilityAIAssistant: ObservabilityAIAssistantPublicStart;
   serverless?: ServerlessPluginStart;
-  enterpriseSearch?: EnterpriseSearchPublicStart;
 }
 
 export interface ConfigSchema {

diff --git a/...n/observability_ai_assistant_management/public/routes/components/search_connector_tab.tsx b/...n/observability_ai_assistant_management/public/routes/components/search_connector_tab.tsx
@@ -16,7 +16,7 @@ export const SELECTED_CONNECTOR_LOCAL_STORAGE_KEY =
 
 export function SearchConnectorTab() {
   const { application } = useKibana().services;
-  const url = application.getUrlForApp('enterprise_search', { path: '/content/connectors' });
+  const url = application.getUrlForApp('enterpriseSearch', { path: '/content/connectors' });
 
   return (
     <>

diff --git a/...solution/observability_ai_assistant_management/public/routes/components/settings_page.tsx b/...solution/observability_ai_assistant_management/public/routes/components/settings_page.tsx
@@ -22,9 +22,8 @@ export function SettingsPage() {
   const { setBreadcrumbs } = useAppContext();
   const {
     services: {
-      application: { navigateToApp },
+      application: { navigateToApp, isAppRegistered },
       serverless,
-      enterpriseSearch,
     },
   } = useKibana();
 
@@ -98,7 +97,7 @@ export function SettingsPage() {
         }
       ),
       content: <SearchConnectorTab />,
-      disabled: enterpriseSearch == null,
+      disabled: !isAppRegistered('enterpriseSearch'),
     },
   ];
 

diff --git a/x-pack/plugins/observability_solution/observability_ai_assistant_management/tsconfig.json b/x-pack/plugins/observability_solution/observability_ai_assistant_management/tsconfig.json
@@ -19,7 +19,6 @@
     "@kbn/core-chrome-browser",
     "@kbn/observability-ai-assistant-plugin",
     "@kbn/serverless",
-    "@kbn/enterprise-search-plugin",
     "@kbn/management-settings-components-field-row",
     "@kbn/observability-shared-plugin",
     "@kbn/config-schema",

diff --git a/x-pack/plugins/search_inference_endpoints/kibana.jsonc b/x-pack/plugins/search_inference_endpoints/kibana.jsonc
@@ -2,8 +2,11 @@
   "type": "plugin",
   "id": "@kbn/search-inference-endpoints",
   "owner": "@elastic/search-kibana",
-  "group": "platform",
-  "visibility": "shared",
+  // TODO enterpriseSearch depends on it, and Observability has a menu entry for enterpriseSearch
+  // must be refactored / fixed before solution-specific builds
+  // cc sphilipse
+  "group": "search",
+  "visibility": "private",
   "plugin": {
     "id": "searchInferenceEndpoints",
     "server": true,

diff --git a/x-pack/plugins/search_playground/kibana.jsonc b/x-pack/plugins/search_playground/kibana.jsonc
@@ -2,9 +2,10 @@
   "type": "plugin",
   "id": "@kbn/search-playground",
   "owner": "@elastic/search-kibana",
-  // @kbn/enterprise-search-plugin (platform) and @kbn/serverless-search (search) depend on it
-  "group": "platform",
-  "visibility": "shared",
+  // TODO @kbn/enterprise-search-plugin (platform) and @kbn/serverless-search (search) depend on it
+  // cc sphilipse
+  "group": "search",
+  "visibility": "private",
   "plugin": {
     "id": "searchPlayground",
     "server": true,

diff --git a/x-pack/plugins/serverless_search/kibana.jsonc b/x-pack/plugins/serverless_search/kibana.jsonc
@@ -35,7 +35,6 @@
       "indexManagement",
       "searchConnectors",
       "searchInferenceEndpoints",
-      "searchPlayground",
       "usageCollection"
     ],
     "requiredBundles": ["kibanaReact"]