From 77e86807178cffee1a873fbaffc93835d732e596 Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Fri, 31 Mar 2023 10:28:15 -0400 Subject: [PATCH] Refactoring frontend to not need singleton --- src/plugins/files/public/mocks.ts | 17 +++- .../test_suites/core_plugins/rendering.ts | 1 + .../plugins/cases/common/constants/owners.ts | 2 +- x-pack/plugins/cases/common/ui/types.ts | 3 +- .../public/common/lib/kibana/services.ts | 13 --- .../plugins/cases/public/files/index.test.ts | 98 +++++++++++++++++++ x-pack/plugins/cases/public/files/index.ts | 29 +++--- x-pack/plugins/cases/public/files/types.ts | 2 +- x-pack/plugins/cases/public/plugin.ts | 16 +-- x-pack/plugins/cases/server/config.test.ts | 1 - x-pack/plugins/cases/server/config.ts | 5 +- .../plugins/cases/server/files/index.test.ts | 54 +++++----- x-pack/plugins/cases/server/files/index.ts | 13 ++- x-pack/plugins/cases/server/index.ts | 2 +- 14 files changed, 177 insertions(+), 79 deletions(-) create mode 100644 x-pack/plugins/cases/public/files/index.test.ts diff --git a/src/plugins/files/public/mocks.ts b/src/plugins/files/public/mocks.ts index c22d9bda24608..447f8c2b85d54 100644 --- a/src/plugins/files/public/mocks.ts +++ b/src/plugins/files/public/mocks.ts @@ -8,10 +8,25 @@ import { createMockFilesClient as createBaseMocksFilesClient } from '@kbn/shared-ux-file-mocks'; import type { DeeplyMockedKeys } from '@kbn/utility-types-jest'; -import type { FilesClient } from './types'; +import { FilesSetup } from '.'; +import type { FilesClient, FilesClientFactory } from './types'; export const createMockFilesClient = (): DeeplyMockedKeys => ({ ...createBaseMocksFilesClient(), getMetrics: jest.fn(), publicDownload: jest.fn(), }); + +export const createMockFilesSetup = (): DeeplyMockedKeys => { + return { + filesClientFactory: createMockFilesClientFactory(), + registerFileKind: jest.fn(), + }; +}; + +export const createMockFilesClientFactory = (): DeeplyMockedKeys => { + return { + asScoped: jest.fn(), + asUnscoped: jest.fn(), + }; +}; diff --git a/test/plugin_functional/test_suites/core_plugins/rendering.ts b/test/plugin_functional/test_suites/core_plugins/rendering.ts index 343877feb7532..2e462dd66bb1b 100644 --- a/test/plugin_functional/test_suites/core_plugins/rendering.ts +++ b/test/plugin_functional/test_suites/core_plugins/rendering.ts @@ -165,6 +165,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) { 'xpack.apm.serviceMapEnabled (boolean)', 'xpack.apm.ui.enabled (boolean)', 'xpack.apm.ui.maxTraceItems (number)', + 'xpack.cases.files.allowedMimeTypes (array)', 'xpack.cases.files.maxSize (number)', 'xpack.cases.markdownPlugins.lens (boolean)', 'xpack.ccr.ui.enabled (boolean)', diff --git a/x-pack/plugins/cases/common/constants/owners.ts b/x-pack/plugins/cases/common/constants/owners.ts index 60463fa57a976..3e799030c7d5b 100644 --- a/x-pack/plugins/cases/common/constants/owners.ts +++ b/x-pack/plugins/cases/common/constants/owners.ts @@ -15,7 +15,7 @@ export const SECURITY_SOLUTION_OWNER = 'securitySolution' as const; export const OBSERVABILITY_OWNER = 'observability' as const; export const GENERAL_CASES_OWNER = APP_ID; -export const OWNERS = [SECURITY_SOLUTION_OWNER, OBSERVABILITY_OWNER, GENERAL_CASES_OWNER] as const; +export const OWNERS = [GENERAL_CASES_OWNER, OBSERVABILITY_OWNER, SECURITY_SOLUTION_OWNER] as const; interface RouteInfo { id: Owner; diff --git a/x-pack/plugins/cases/common/ui/types.ts b/x-pack/plugins/cases/common/ui/types.ts index e6c192e684516..e9bfcd0682f5b 100644 --- a/x-pack/plugins/cases/common/ui/types.ts +++ b/x-pack/plugins/cases/common/ui/types.ts @@ -53,7 +53,8 @@ export interface CasesUiConfigType { lens: boolean; }; files: { - maxSize: number; + maxSize?: number; + allowedMimeTypes: 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 index 5ec5bd8a75e7f..ab06c1be0bf02 100644 --- a/x-pack/plugins/cases/public/common/lib/kibana/services.ts +++ b/x-pack/plugins/cases/public/common/lib/kibana/services.ts @@ -6,7 +6,6 @@ */ import type { CoreStart } from '@kbn/core/public'; -import type { CaseFileKinds } from '../../../files/types'; import type { CasesUiConfigType } from '../../../../common/ui/types'; type GlobalServices = Pick; @@ -15,22 +14,18 @@ export class KibanaServices { private static kibanaVersion?: string; private static services?: GlobalServices; private static config?: CasesUiConfigType; - private static _fileKinds?: CaseFileKinds; public static init({ http, kibanaVersion, config, - fileKinds, }: GlobalServices & { kibanaVersion: string; config: CasesUiConfigType; - fileKinds: CaseFileKinds; }) { this.services = { http }; this.kibanaVersion = kibanaVersion; this.config = config; - this._fileKinds = fileKinds; } public static get(): GlobalServices { @@ -53,14 +48,6 @@ export class KibanaServices { return this.config; } - public static get fileKinds(): CaseFileKinds { - if (!this._fileKinds) { - this.throwUninitializedError(); - } - - return this._fileKinds; - } - private static throwUninitializedError(): never { throw new Error( 'Kibana services not initialized - are you trying to import this module from outside of the Cases app?' diff --git a/x-pack/plugins/cases/public/files/index.test.ts b/x-pack/plugins/cases/public/files/index.test.ts new file mode 100644 index 0000000000000..3db225cf35d6f --- /dev/null +++ b/x-pack/plugins/cases/public/files/index.test.ts @@ -0,0 +1,98 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { MAX_FILE_SIZE } from '../../common/constants'; +import { createMockFilesSetup } from '@kbn/files-plugin/public/mocks'; +import { registerCaseFileKinds } from '.'; +import type { FilesConfig } from './types'; + +describe('ui files index', () => { + describe('registerCaseFileKinds', () => { + const mockFilesSetup = createMockFilesSetup(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('allowedMimeTypes', () => { + const config: FilesConfig = { + allowedMimeTypes: ['abc'], + maxSize: undefined, + }; + + beforeEach(() => { + registerCaseFileKinds(config, mockFilesSetup); + }); + + it('sets cases allowed mime types to abc', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[0][0].allowedMimeTypes).toEqual(['abc']); + }); + + it('sets observability allowed mime types to abc', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[1][0].allowedMimeTypes).toEqual(['abc']); + }); + + it('sets securitySolution allowed mime types to 100 mb', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[2][0].allowedMimeTypes).toEqual(['abc']); + }); + }); + + describe('max file size', () => { + describe('default max file size', () => { + const config: FilesConfig = { + allowedMimeTypes: [], + maxSize: undefined, + }; + + beforeEach(() => { + registerCaseFileKinds(config, mockFilesSetup); + }); + + it('sets cases max file size to 100 mb', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[0][0].maxSizeBytes).toEqual( + MAX_FILE_SIZE + ); + }); + + it('sets observability max file size to 100 mb', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[1][0].maxSizeBytes).toEqual( + MAX_FILE_SIZE + ); + }); + + it('sets securitySolution max file size to 100 mb', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[2][0].maxSizeBytes).toEqual( + MAX_FILE_SIZE + ); + }); + }); + + describe('custom file size', () => { + const config: FilesConfig = { + allowedMimeTypes: [], + maxSize: 5, + }; + + beforeEach(() => { + registerCaseFileKinds(config, mockFilesSetup); + }); + + it('sets cases max file size to 5', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[0][0].maxSizeBytes).toEqual(5); + }); + + it('sets observability max file size to 5', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[1][0].maxSizeBytes).toEqual(5); + }); + + it('sets securitySolution max file size to 5', () => { + expect(mockFilesSetup.registerFileKind.mock.calls[2][0].maxSizeBytes).toEqual(5); + }); + }); + }); + }); +}); diff --git a/x-pack/plugins/cases/public/files/index.ts b/x-pack/plugins/cases/public/files/index.ts index a2b38b92165eb..ba0be399491da 100644 --- a/x-pack/plugins/cases/public/files/index.ts +++ b/x-pack/plugins/cases/public/files/index.ts @@ -7,17 +7,15 @@ import type { FilesSetup } from '@kbn/files-plugin/public'; import type { FileKindBrowser } from '@kbn/shared-ux-file-types'; -import { ALLOWED_MIME_TYPES } from '../../common/constants/mime_types'; -import { constructFileKindIdByOwner, MAX_FILE_SIZE } from '../../common/constants'; +import { constructFileKindIdByOwner, MAX_FILE_SIZE, OWNERS } from '../../common/constants'; import type { Owner } from '../../common/constants/types'; -import { APP_ID, OBSERVABILITY_OWNER, SECURITY_SOLUTION_OWNER } from '../../common'; import type { CaseFileKinds, FilesConfig } from './types'; const buildFileKind = (config: FilesConfig, owner: Owner): FileKindBrowser => { return { id: constructFileKindIdByOwner(owner), - allowedMimeTypes: ALLOWED_MIME_TYPES, - maxSizeBytes: MAX_FILE_SIZE, + allowedMimeTypes: config.allowedMimeTypes, + maxSizeBytes: config.maxSize ?? MAX_FILE_SIZE, }; }; @@ -25,22 +23,19 @@ const buildFileKind = (config: FilesConfig, owner: Owner): FileKindBrowser => { * The file kind definition for interacting with the file service for the UI */ const createFileKinds = (config: FilesConfig): CaseFileKinds => { - return { - [APP_ID]: buildFileKind(config, APP_ID), - [SECURITY_SOLUTION_OWNER]: buildFileKind(config, SECURITY_SOLUTION_OWNER), - [OBSERVABILITY_OWNER]: buildFileKind(config, OBSERVABILITY_OWNER), - }; + const caseFileKinds = new Map(); + + for (const owner of OWNERS) { + caseFileKinds.set(owner, buildFileKind(config, owner)); + } + + return caseFileKinds; }; -export const registerCaseFileKinds = ( - config: FilesConfig, - filesSetupPlugin: FilesSetup -): CaseFileKinds => { +export const registerCaseFileKinds = (config: FilesConfig, filesSetupPlugin: FilesSetup) => { const fileKinds = createFileKinds(config); - for (const fileKind of Object.values(fileKinds)) { + for (const fileKind of fileKinds.values()) { filesSetupPlugin.registerFileKind(fileKind); } - - return fileKinds; }; diff --git a/x-pack/plugins/cases/public/files/types.ts b/x-pack/plugins/cases/public/files/types.ts index c7703414f6b91..d7c4563eb1c06 100644 --- a/x-pack/plugins/cases/public/files/types.ts +++ b/x-pack/plugins/cases/public/files/types.ts @@ -11,4 +11,4 @@ import type { CasesUiConfigType } from '../containers/types'; export type FilesConfig = CasesUiConfigType['files']; -export type CaseFileKinds = Record; +export type CaseFileKinds = Map; diff --git a/x-pack/plugins/cases/public/plugin.ts b/x-pack/plugins/cases/public/plugin.ts index cf5fa10e1bbfb..b1ab9f32700dd 100644 --- a/x-pack/plugins/cases/public/plugin.ts +++ b/x-pack/plugins/cases/public/plugin.ts @@ -28,7 +28,6 @@ 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 { CaseFileKinds } from './files/types'; /** * @public @@ -41,10 +40,6 @@ export class CasesUiPlugin private readonly storage = new Storage(localStorage); private externalReferenceAttachmentTypeRegistry: ExternalReferenceAttachmentTypeRegistry; private persistableStateAttachmentTypeRegistry: PersistableStateAttachmentTypeRegistry; - // The reason this is protected is because we'll get type collisions otherwise because we're using a type guard assert - // to ensure the options member is instantiated before using it in various places - // See for more info: https://stackoverflow.com/questions/66206180/typescript-typeguard-attribut-with-method - protected fileKinds: CaseFileKinds | undefined; constructor(private readonly initializerContext: PluginInitializerContext) { this.kibanaVersion = initializerContext.env.packageInfo.version; @@ -59,7 +54,7 @@ export class CasesUiPlugin const persistableStateAttachmentTypeRegistry = this.persistableStateAttachmentTypeRegistry; const config = this.initializerContext.config.get(); - this.fileKinds = registerCaseFileKinds(config.files, plugins.files); + registerCaseFileKinds(config.files, plugins.files); if (plugins.home) { plugins.home.featureCatalogue.register({ @@ -111,8 +106,6 @@ export class CasesUiPlugin } public start(core: CoreStart, plugins: CasesPluginStart): CasesUiStart { - this.ensureFieldsInitializedInSetup(); - const config = this.initializerContext.config.get(); KibanaServices.init({ @@ -120,7 +113,6 @@ export class CasesUiPlugin ...plugins, kibanaVersion: this.kibanaVersion, config, - fileKinds: this.fileKinds, }); /** @@ -176,11 +168,5 @@ export class CasesUiPlugin }; } - private ensureFieldsInitializedInSetup(): asserts this is this & { fileKinds: CaseFileKinds } { - if (this.fileKinds == null) { - throw new Error('Cases failed to initialize file kinds'); - } - } - public stop() {} } diff --git a/x-pack/plugins/cases/server/config.test.ts b/x-pack/plugins/cases/server/config.test.ts index 7a98cce9b3851..074b124918461 100644 --- a/x-pack/plugins/cases/server/config.test.ts +++ b/x-pack/plugins/cases/server/config.test.ts @@ -102,7 +102,6 @@ describe('config validation', () => { "application/x-tar", "application/pdf", ], - "maxImageSize": 10485760, "maxSize": 104857600, }, "markdownPlugins": Object { diff --git a/x-pack/plugins/cases/server/config.ts b/x-pack/plugins/cases/server/config.ts index c582e325c991f..c2daeb73b03de 100644 --- a/x-pack/plugins/cases/server/config.ts +++ b/x-pack/plugins/cases/server/config.ts @@ -7,7 +7,6 @@ import type { TypeOf } from '@kbn/config-schema'; import { schema } from '@kbn/config-schema'; -import { MAX_FILE_SIZE, MAX_IMAGE_FILE_SIZE } from '../common/constants'; import { ALLOWED_MIME_TYPES } from '../common/constants/mime_types'; export const ConfigSchema = schema.object({ @@ -18,8 +17,8 @@ export const ConfigSchema = schema.object({ allowedMimeTypes: schema.arrayOf(schema.string({ minLength: 1 }), { defaultValue: ALLOWED_MIME_TYPES, }), - maxSize: schema.number({ defaultValue: MAX_FILE_SIZE, min: 0 }), - maxImageSize: schema.number({ defaultValue: MAX_IMAGE_FILE_SIZE, min: 0 }), + // intentionally not setting a default here so that we can determine if the user set it + maxSize: schema.maybe(schema.number({ min: 0 })), }), }); diff --git a/x-pack/plugins/cases/server/files/index.test.ts b/x-pack/plugins/cases/server/files/index.test.ts index 03725322182fc..b07a39f9deec0 100644 --- a/x-pack/plugins/cases/server/files/index.test.ts +++ b/x-pack/plugins/cases/server/files/index.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { MAX_FILE_SIZE, MAX_IMAGE_FILE_SIZE } from '../../common/constants'; import { createFilesSetupMock } from '@kbn/files-plugin/server/mocks'; import type { FileJSON } from '@kbn/shared-ux-file-types'; import { createMaxCallback, registerCaseFileKinds } from '.'; @@ -19,39 +20,50 @@ describe('server files', () => { }); describe('file sizes', () => { - const schema = ConfigSchema.validate({ - files: { - maxImageSize: 0, - maxSize: 1, - }, + it('sets the max image file size to 10 mb', () => { + const schema = ConfigSchema.validate({}); + + const maxFileSizeFn = createMaxCallback(schema.files); + + expect( + maxFileSizeFn({ + mimeType: 'image/png', + } as unknown as FileJSON) + ).toEqual(MAX_IMAGE_FILE_SIZE); }); - const maxFileSizeFn = createMaxCallback(schema.files); + it('sets the max file size to 1 when an image is passed but the configuration was specified', () => { + const schema = ConfigSchema.validate({ + files: { + maxSize: 1, + }, + }); + + const maxFileSizeFn = createMaxCallback(schema.files); - it('sets the max image file size to 0', () => { expect( maxFileSizeFn({ mimeType: 'image/png', } as unknown as FileJSON) - ).toEqual(0); + ).toEqual(1); }); - it('sets the max non-image file size to 1', () => { - registerCaseFileKinds(schema.files, mockFilesSetup); + it('sets the max non-image file size to default 100 mb', () => { + const schema = ConfigSchema.validate({}); + + const maxFileSizeFn = createMaxCallback(schema.files); expect( maxFileSizeFn({ mimeType: 'text/plain', } as unknown as FileJSON) - ).toEqual(1); + ).toEqual(MAX_FILE_SIZE); }); - it('returns the non-image size when images are not allowed in the mime type', () => { + it('returns 100 mb when images are not allowed in the mime type and an image is passed', () => { const schemaNoImages = ConfigSchema.validate({ files: { allowedMimeTypes: ['abc/123'], - maxImageSize: 0, - maxSize: 1, }, }); @@ -61,15 +73,13 @@ describe('server files', () => { maxFn({ mimeType: 'image/png', } as unknown as FileJSON) - ).toEqual(1); + ).toEqual(MAX_FILE_SIZE); }); - it('returns the non-image size when the mime type is not recognized', () => { + it('returns 100 mb when the mime type is not recognized', () => { const schemaNoImages = ConfigSchema.validate({ files: { allowedMimeTypes: ['abc/123'], - maxImageSize: 0, - maxSize: 1, }, }); @@ -79,15 +89,13 @@ describe('server files', () => { maxFn({ mimeType: 'hi/bye', } as unknown as FileJSON) - ).toEqual(1); + ).toEqual(MAX_FILE_SIZE); }); - it('returns the non-image size when the mime type is undefined', () => { + it('returns 100 mb when the mime type is undefined', () => { const schemaNoImages = ConfigSchema.validate({ files: { allowedMimeTypes: ['abc/123'], - maxImageSize: 0, - maxSize: 1, }, }); @@ -97,7 +105,7 @@ describe('server files', () => { maxFn({ mimeType: undefined, } as unknown as FileJSON) - ).toEqual(1); + ).toEqual(MAX_FILE_SIZE); }); }); diff --git a/x-pack/plugins/cases/server/files/index.ts b/x-pack/plugins/cases/server/files/index.ts index 75106854d0def..a6509c84f5c7f 100644 --- a/x-pack/plugins/cases/server/files/index.ts +++ b/x-pack/plugins/cases/server/files/index.ts @@ -11,6 +11,8 @@ import { APP_ID, constructFileKindIdByOwner, constructFilesHttpOperationTag, + MAX_FILE_SIZE, + MAX_IMAGE_FILE_SIZE, OBSERVABILITY_OWNER, SECURITY_SOLUTION_OWNER, } from '../../common/constants'; @@ -49,17 +51,24 @@ const buildTag = (owner: Owner, operation: HttpApiTagOperation) => { export const createMaxCallback = (config: FilesConfig) => (file: FileJSON): number => { + // if the user set a max size, always return that + if (config.maxSize != null) { + return config.maxSize; + } + const allowedMimeTypesSet = new Set(config.allowedMimeTypes); + // if we have the mime type for the file and it exists within the allowed types and it is an image then return the + // image size if ( file.mimeType != null && allowedMimeTypesSet.has(file.mimeType) && IMAGE_MIME_TYPES.has(file.mimeType) ) { - return config.maxImageSize; + return MAX_IMAGE_FILE_SIZE; } - return config.maxSize; + return MAX_FILE_SIZE; }; /** diff --git a/x-pack/plugins/cases/server/index.ts b/x-pack/plugins/cases/server/index.ts index e1173f47d93cc..62748905f6b82 100644 --- a/x-pack/plugins/cases/server/index.ts +++ b/x-pack/plugins/cases/server/index.ts @@ -15,7 +15,7 @@ export const config: PluginConfigDescriptor = { schema: ConfigSchema, exposeToBrowser: { markdownPlugins: true, - files: { maxSize: true }, + files: { maxSize: true, allowedMimeTypes: true }, }, deprecations: ({ renameFromRoot }) => [ renameFromRoot('xpack.case.enabled', 'xpack.cases.enabled', { level: 'critical' }),