From d2a5ebd1fc17326daad585c54a1649a1807d634e Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Tue, 27 Oct 2020 17:31:36 +0100 Subject: [PATCH 1/9] [docs] Resolving failed Kibana upgrade migrations (#80999) * Resolving failed Kibana upgrade migrations * Move warning against rolling upgrades into upgrade-standard and call out stopping all instances in specific upgrade steps * Add preventing migration failures section * Add incompatible xpack.tasks.index: .tasks setting to preventing migration failures * Fix link Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- docs/setup/upgrade.asciidoc | 21 ++-- .../setup/upgrade/upgrade-migrations.asciidoc | 119 ++++++++++++++---- docs/setup/upgrade/upgrade-standard.asciidoc | 16 ++- 3 files changed, 116 insertions(+), 40 deletions(-) diff --git a/docs/setup/upgrade.asciidoc b/docs/setup/upgrade.asciidoc index 6d69b6921b612..92cd6e9ead5a1 100644 --- a/docs/setup/upgrade.asciidoc +++ b/docs/setup/upgrade.asciidoc @@ -4,29 +4,24 @@ Depending on the {kib} version you're upgrading from, the upgrade process to 7.0 varies. -NOTE: {kib} upgrades automatically when starting a new version, as described in -<>. -Although you do not need to manually back up {kib} before upgrading, we recommend -that you have a backup on hand. You can use -<> to back up {kib} -data by targeting `.kibana*` indices. If you are using the Reporting plugin, -you can also target `.reporting*` indices. - [float] [[upgrade-before-you-begin]] === Before you begin +WARNING: {kib} automatically runs upgrade migrations when required. To roll back to an earlier version in case of an upgrade failure, you **must** have a backup snapshot available. Use <> to back up {kib} data by targeting the `.kibana*` indices. For more information see <>. + Before you upgrade {kib}: * Consult the <>. +* Back up your data with <>. To roll back to an earlier version, you **must** have a snapshot of the `.kibana*` indices. +* Although not a requirement for rollbacks, we recommend taking a snapshot of all {kib} indices created by the plugins you use such as the `.reporting*` indices created by the reporting plugin. * Before you upgrade production servers, test the upgrades in a dev environment. -* Back up your data with {es} {ref}/modules-snapshots.html[snapshots]. - To roll back to an earlier version, you **must** have a backup of your data. +* See <> for common reasons upgrades fail and how to prevent these. * If you are using custom plugins, check that a compatible version is available. -* Shut down all {kib} nodes. Running more than one {kib} version against the - same Elasticseach index is unsupported. If you upgrade while older {kib} nodes are - running, the upgrade can fail. +* Shut down all {kib} instances. Running more than one {kib} version against + the same Elasticseach index is unsupported. Upgrading while older {kib} + instances are running can cause data loss or upgrade failures. To identify the changes you need to make to upgrade, and to enable you to perform an Elasticsearch rolling upgrade with no downtime, you must upgrade to diff --git a/docs/setup/upgrade/upgrade-migrations.asciidoc b/docs/setup/upgrade/upgrade-migrations.asciidoc index cbe56b9e65894..74d097164c4a7 100644 --- a/docs/setup/upgrade/upgrade-migrations.asciidoc +++ b/docs/setup/upgrade/upgrade-migrations.asciidoc @@ -1,54 +1,127 @@ [[upgrade-migrations]] -=== Migrate saved objects +=== Upgrade migrations -Every time {kib} is upgraded it checks to see if all saved objects, such as dashboards, visualizations, and index patterns, are compatible with the new version. If any objects need to be updated, then the automatic saved object migration process is kicked off. +Every time {kib} is upgraded it checks to see if all saved objects, such as dashboards, visualizations, and index patterns, are compatible with the new version. If any saved objects need to be updated, then the automatic saved object migration process is kicked off. NOTE: 6.7 includes an https://www.elastic.co/guide/en/kibana/6.7/upgrade-assistant.html[Upgrade Assistant] to help you prepare for your upgrade to 7.0. To access the assistant, go to *Management > 7.0 Upgrade Assistant*. +WARNING: The following instructions assumes {kib} is using the default index names. If the `kibana.index` or `xpack.tasks.index` configuration settings were changed these instructions will have to be adapted accordingly. + [float] [[upgrade-migrations-process]] -==== How the process works +==== Background -Saved objects are stored in an index named `.kibana_N`, where `N` is a number that increments over time as {kib} is upgraded. The index alias `.kibana` points to the latest up-to-date index for a given install. +Saved objects are stored in two indices: -NOTE: Prior to 6.5.0, saved objects were stored directly in an index named `.kibana`, so the first time you upgrade to {kib} version 6.5+, {kib} will migrate into `.kibana_1` and set `.kibana` up as an index alias. +* `.kibana_N`, or if set, the `kibana.index` configuration setting +* `.kibana_task_manager_N`, or if set, the `xpack.tasks.index` configuration setting + +For each of these indices, `N` is a number that increments every time {kib} runs an upgrade migration on that index. The index aliases `.kibana` and `.kibana_task_manager` point to the most up-to-date index. While {kib} is starting up and before serving any HTTP traffic, it checks to see if any internal mapping changes or data transformations for existing saved objects are required. -When changes are necessary, a new incremental `.kibana_N` index is created with updated mappings, then the saved objects are loaded in batches from the existing index, transformed to whatever extent necessary, and added to this new index. +When changes are necessary, a new migration is started. To ensure that only one {kib} instance performs the migration, each instance will attempt to obtain a migration lock by creating a new `.kibana_N+1` index. The instance that succeeds in creating the index will then read batches of documents from the existing index, migrate them, and write them to the new index. Once the objects are migrated, the lock is released by pointing the `.kibana` index alias the new upgraded `.kibana_N+1` index. + +Instances that failed to acquire a lock will log `Another Kibana instance appears to be migrating the index. Waiting for that migration to complete`. The instance will then wait until `.kibana` points to an upgraded index before starting up and serving HTTP traffic. -Once the objects are migrated, the `.kibana` index alias is updated to point to the new index, and {kib} finishes starting up and serving HTTP traffic. +NOTE: Prior to 6.5.0, saved objects were stored directly in an index named `.kibana`. After upgrading to version 6.5+, {kib} will migrate this index into `.kibana_N` and set `.kibana` up as an index alias. + +Prior to 7.4.0, task manager tasks were stored directly in an index name `.kibana_task_manager`. After upgrading to version 7.4+, {kib} will migrate this index into `.kibana_task_manager_N` and set `.kibana_task_manager` up as an index alias. [float] -[[upgrade-migrations-old-indices]] -==== Handling old `.kibana` indices +[[preventing-migration-failures]] +==== Preventing migration failures +This section highlights common causes of {kib} upgrade failures and how to prevent them. + +[float] +===== Corrupt saved objects +We highly recommend testing your {kib} upgrade in a development cluster to discover and remedy problems caused by corrupt documents, especially when there are custom integrations creating saved objects in your environment. Saved objects that were corrupted through manual editing or integrations will cause migration failures with a log message like `Failed to transform document. Transform: index-pattern:7.0.0\n Doc: {...}` or `Unable to migrate the corrupt Saved Object document ...`. Corrupt documents will have to be fixed or deleted before an upgrade migration can succeed. + +[float] +===== User defined index templates that causes new `.kibana*` indices to have incompatible settings or mappings +Matching index templates which specify `settings.refresh_interval` or `mappings` are known to interfere with {kib} upgrades. + +Prevention: narrow down the index patterns of any user-defined index templates to ensure that these won't apply to new `.kibana*` indices. -After migrations have run, there will be multiple {kib} indices in {es}: (`.kibana_1`, `.kibana_2`, etc). {kib} only uses the index that the `.kibana` alias points to. The other {kib} indices can be safely deleted, but are left around as a matter of historical record, and to facilitate rolling {kib} back to a previous version. +Note: {kib} < 6.5 creates it's own index template called `kibana_index_template:.kibana` and index pattern `.kibana`. This index template will not interfere and does not need to be changed or removed. + +[float] +===== An unhealthy {es} cluster +Problems with your {es} cluster can prevent {kib} upgrades from succeeding. Ensure that your cluster has: + + * enough free disk space, at least twice the amount of storage taken up by the `.kibana` and `.kibana_task_manager` indices + * sufficient heap size + * a "green" cluster status + +[float] +===== Running different versions of {kib} connected to the same {es} index +Kibana does not support rolling upgrades. Stop all {kib} instances before starting a newer version to prevent upgrade failures and data loss. + +[float] +===== Incompatible `xpack.tasks.index` configuration setting +For {kib} < 7.5.1, if the task manager index is set to `.tasks` with the configuration setting `xpack.tasks.index: ".tasks"`, upgrade migrations will fail. {kib} 7.5.1 and later prevents this by refusing to start with an incompatible configuration setting. [float] -[[upgrade-migrations-errors]] -==== Handling errors during saved object migrations +[[resolve-migrations-failures]] +==== Resolving migration failures -If {kib} terminates unexpectedly while migrating a saved object index, some additional work may be required in order to get {kib} to re-attempt the migration. +If {kib} terminates unexpectedly while migrating a saved object index, manual intervention is required before {kib} will attempt to perform the migration again. Follow the advice in (preventing migration failures)[preventing-migration-failures] before retrying a migration upgrade. -For example, if the `.kibana` alias is pointing to `.kibana_4`, and there is a `.kibana_5` index in {es}, the `.kibana_5` index will need to be deleted. {kib} will never attempt to overwrite an existing index. +As mentioned above, {kib} will create a migration lock for each index that requires a migration by creating a new `.kibana_N+1` index. For example: if the `.kibana_task_manager` alias is pointing to `.kibana_task_manager_5` then the first {kib} that succeeds in creating `.kibana_task_manager_6` will obtain the lock to start migrations. + +However, if the instance that obtained the lock fails to migrate the index, all other {kib} instances will be blocked from performing this migration. This includes the instance that originally obtained the lock, it will be blocked from retrying the migration even when restarted. [float] -[[upgrade-migrations-multiple-instances]] -==== Support for multiple {kib} instances +===== Retry a migration by restoring a backup snapshot: + +1. Before proceeding ensure that you have a recent and successful backup snapshot of all `.kibana*` indices. +2. Shutdown all {kib} instances to be 100% sure that there are no instances currently performing a migration. +3. Delete all saved object indices with `DELETE /.kibana*` +4. Restore the `.kibana* indices and their aliases from the backup snapshot. See {es} {ref}/modules-snapshots.html[snapshots] +5. Start up all {kib} instances to retry the upgrade migration. -If you're running multiple {kib} instances for a single index behind a load balancer, it's important that you stop all instances before upgrading, so you do not have multiple different versions of {kib} trying to perform saved object migrations. +[float] +===== (Not recommended) Retry a migration without a backup snapshot: -The first instance that triggers saved object migrations will run the entire process. Any other instances started up while a migration is running will log a message and then wait until saved object migration has completed before they start serving HTTP traffic. +1. Shutdown all {kib} instances to be 100% sure that there are no instances currently performing a migration. +2. Identify any migration locks by comparing the output of `GET /_cat/aliases` and `GET /_cat/indices`. If e.g. `.kibana` is pointing to `.kibana_4` and there is a `.kibana_5` index, the `.kibana_5` index will act like a migration lock blocking further attempts. Be sure to check both the `.kibana` and `.kibana_task_manager` aliases and their indices. +3. Remove any migration locks e.g. `DELETE /.kibana_5`. +4. Start up all {kib} instances. [float] [[upgrade-migrations-rolling-back]] ==== Rolling back to a previous version of {kib} -When rolling {kib} back to a previous version, point the `.kibana` alias to -the appropriate {kib} index. When you have the previous version running again, -delete the more recent `.kibana_N` index or indices so that future upgrades are -based on the current {kib} index. You must restart {kib} to re-trigger the migration. +If you've followed the advice in (preventing migration failures)[preventing-migration-failures] and (resolving migration failures)[resolve-migrations-failures] and {kib} is still not able to upgrade successfully, you might choose to rollback {kib} until you're able to identify the root cause. + +WARNING: Before rolling back {kib}, ensure that the version you wish to rollback to is compatible with your {es} cluster. If the version you're rolling back to is not compatible, you will have to also rollback {es}. + +Any changes made after an upgrade will be lost when rolling back to a previous version. + +In order to rollback after a failed upgrade migration, the saved object indices might also have to be rolled back to be compatible with the previous {kibana} version. + +[float] +===== Rollback by restoring a backup snapshot: + +1. Before proceeding ensure that you have a recent and successful backup snapshot of all `.kibana*` indices. +2. Shutdown all {kib} instances to be 100% sure that there are no instances currently performing a migration. +3. Delete all saved object indices with `DELETE /.kibana*` +4. Restore the `.kibana* indices and their aliases from the backup snapshot. See {es} {ref}/modules-snapshots.html[snapshots] +5. Start up all {kib} instances on the older version you wish to rollback to. + +[float] +===== (Not recommended) Rollback without a backup snapshot: + +WARNING: {kib} does not run a migration for every saved object index on every upgrade. A {kib} version upgrade can cause no migrations, migrate only the `.kibana` or the `.kibana_task_manager` index or both. Carefully read the logs to ensure that you're only deleting indices created by a later version of {kib} to avoid data loss. + +1. Shutdown all {kib} instances to be 100% sure that there are no {kib} instances currently performing a migration. +2. Create a backup snapshot of the `.kibana*` indices. +3. Use the logs from the upgraded instances to identify which indices {kib} attempted to upgrade. The server logs will contain an entry like `[savedobjects-service] Creating index .kibana_4.` and/or `[savedobjects-service] Creating index .kibana_task_manager_2.` If no indices were created after upgrading {kib} then no further action is required to perform a rollback, skip ahead to step (5). If you're running multiple {kib} instances, be sure to inspect all instances' logs. +4. Delete each of the indices identified in step (2). e.g. `DELETE /.kibana_task_manager_2` +5. Inspect the output of `GET /_cat/aliases`. If either the `.kibana` and/or `.kibana_task_manager` alias is missing, these will have to be created manually. Find the latest index from the output of `GET /_cat/indices` and create the missing alias to point to the latest index. E.g. if the `.kibana` alias was missing and the latest index is `.kibana_3` create a new alias with `POST /.kibana_3/_aliases/.kibana`. +6. Start up {kib} on the older version you wish to rollback to. + +[float] +[[upgrade-migrations-old-indices]] +==== Handling old `.kibana_N` indices -WARNING: Rolling back to a previous {kib} version can result in saved object data loss if you had successfully upgraded and made changes to saved objects before rolling back. +After migrations have completed, there will be multiple {kib} indices in {es}: (`.kibana_1`, `.kibana_2`, etc). {kib} only uses the index that the `.kibana` alias points to. The other {kib} indices can be safely deleted, but are left around as a matter of historical record, and to facilitate rolling {kib} back to a previous version. \ No newline at end of file diff --git a/docs/setup/upgrade/upgrade-standard.asciidoc b/docs/setup/upgrade/upgrade-standard.asciidoc index df38427881d65..b27bb8867e624 100644 --- a/docs/setup/upgrade/upgrade-standard.asciidoc +++ b/docs/setup/upgrade/upgrade-standard.asciidoc @@ -12,11 +12,20 @@ If you've saved and/or exported objects in {kib} that rely on the necessary remediation steps as per those instructions. =========================================== +[float] +==== Upgrading multiple {kib} instances + +WARNING: Kibana does not support rolling upgrades. If you're running multiple {kib} instances, all instances should be stopped before upgrading. + +Different versions of {kib} running against the same {es} index, such as during a rolling upgrade, can cause upgrade migration failures and data loss. This is because acknowledged writes from the older instances could be written into the _old_ index while the migration is in progress. To prevent this from happening ensure that all old {kib} instances are shutdown before starting up instances on a newer version. + +The first instance that triggers saved object migrations will run the entire process. Any other instances started up while a migration is running will log a message and then wait until saved object migrations has completed before they start serving HTTP traffic. + [float] ==== Upgrade using a `deb` or `rpm` package . Stop the existing {kib} process using the appropriate command for your - system. + system. If you have multiple {kib} instances connecting to the same {es} cluster ensure that all instances are stopped before proceeding to the next step to avoid data loss. . Use `rpm` or `dpkg` to install the new package. All files should be placed in their proper locations and config files should not be overwritten. + @@ -43,8 +52,7 @@ otherwise {kib} will fail to start. don't overwrite the `config` or `data` directories. + + -- -IMPORTANT: If you use {monitor-features}, you must re-use the data directory when you -upgrade {kib}. Otherwise, the {kib} instance is assigned a new persistent UUID +IMPORTANT: If you use {monitor-features}, you must re-use the data directory when you upgrade {kib}. Otherwise, the {kib} instance is assigned a new persistent UUID and becomes a new instance in the monitoring data. -- @@ -57,5 +65,5 @@ and becomes a new instance in the monitoring data. . Install the appropriate versions of all your plugins for your new installation using the `kibana-plugin` script. Check out the <> documentation for more information. -. Stop the old {kib} process. +. Stop the old {kib} process. If you have multiple {kib} instances connecting to the same {es} cluster ensure that all instances are stopped before proceeding to the next step to avoid data loss. . Start the new {kib} process. From 6b1409bc98fb7425c1838a087d9c1eaa486c26fd Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Tue, 27 Oct 2020 12:39:30 -0400 Subject: [PATCH 2/9] [Time to Visualize] Update Library Text with Call to Action (#81667) * Added call to action to unlink message --- .../public/application/actions/library_notification_popover.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/dashboard/public/application/actions/library_notification_popover.tsx b/src/plugins/dashboard/public/application/actions/library_notification_popover.tsx index 8bc81b3296c3d..6ec5b0d637556 100644 --- a/src/plugins/dashboard/public/application/actions/library_notification_popover.tsx +++ b/src/plugins/dashboard/public/application/actions/library_notification_popover.tsx @@ -72,7 +72,7 @@ export function LibraryNotificationPopover({

{i18n.translate('dashboard.panel.libraryNotification.toolTip', { defaultMessage: - 'This panel is linked to a library item. Editing the panel might affect other dashboards.', + 'Editing this panel might affect other dashboards. To change to this panel only, unlink it from the library.', })}

From 161972ef2cc319873e71467a213abc8c825a6820 Mon Sep 17 00:00:00 2001 From: Robert Austin Date: Tue, 27 Oct 2020 12:41:02 -0400 Subject: [PATCH 3/9] [Resolver] `SideEffectContext` changes, remove `@testing-library` uses (#81077) * `getBoundingClientRect` is now accessed through `SideEffectContext`. * `writeText` from the `Clipboard` API is now accessed through the `SideEffectContext` * No longer using `@testing-library/react` and `@testing-library/react-hooks` * No longer using `jest.spyOn` (mostly) or `jest.clearAllMocks` The motivation for this PR: * We already have `SideEffectContext`, which is meant to be an alternative to using `jest.spyOn`. This PR uses the `SideEffectContext` for `getBoundingClientRect` and `navigator.clipboard.writeText`. * We have been using `enzyme` lately. This removes uses of `@testing-library/react` and `@testing-library/react-hooks` in favor of `enzyme`. --- ..._children_with_related_events_on_origin.ts | 3 +- .../{get_ui_settings.ts => ui_setting.ts} | 5 +- .../public/resolver/store/actions.ts | 9 + .../resolver/test_utilities/extend_jest.ts | 6 +- .../test_utilities/simulator/index.tsx | 58 ++++- .../public/resolver/types.ts | 27 ++ .../resolver/view/clickthrough.test.tsx | 5 +- ...ntent_utilities.tsx => generated_text.tsx} | 36 +-- .../public/resolver/view/panel.test.tsx | 174 +++++++++---- .../view/panels/copyable_panel_field.tsx | 57 +++-- .../resolver/view/panels/event_detail.tsx | 43 ++-- .../resolver/view/panels/node_detail.tsx | 2 +- .../view/panels/node_events_of_type.tsx | 12 +- .../public/resolver/view/panels/node_list.tsx | 24 +- .../public/resolver/view/panels/styles.tsx | 20 ++ .../view/panels/use_formatted_date.test.tsx | 115 +++------ .../resolver/view/side_effect_context.ts | 6 + .../view/side_effect_simulator_factory.ts | 49 ++-- .../public/resolver/view/use_camera.test.tsx | 238 ++++++++++++------ .../public/resolver/view/use_camera.ts | 17 +- 20 files changed, 550 insertions(+), 356 deletions(-) rename x-pack/plugins/security_solution/public/resolver/mocks/{get_ui_settings.ts => ui_setting.ts} (79%) rename x-pack/plugins/security_solution/public/resolver/view/{panels/panel_content_utilities.tsx => generated_text.tsx} (59%) diff --git a/x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/no_ancestors_two_children_with_related_events_on_origin.ts b/x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/no_ancestors_two_children_with_related_events_on_origin.ts index 6fb84eaf7fda6..837d824db8748 100644 --- a/x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/no_ancestors_two_children_with_related_events_on_origin.ts +++ b/x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/no_ancestors_two_children_with_related_events_on_origin.ts @@ -78,8 +78,7 @@ export function noAncestorsTwoChildrenWithRelatedEventsOnOrigin(): { */ async eventsWithEntityIDAndCategory( entityID: string, - category: string, - after?: string + category: string ): Promise<{ events: SafeResolverEvent[]; nextEvent: string | null }> { const events = entityID === metadata.entityIDs.origin diff --git a/x-pack/plugins/security_solution/public/resolver/mocks/get_ui_settings.ts b/x-pack/plugins/security_solution/public/resolver/mocks/ui_setting.ts similarity index 79% rename from x-pack/plugins/security_solution/public/resolver/mocks/get_ui_settings.ts rename to x-pack/plugins/security_solution/public/resolver/mocks/ui_setting.ts index ab1a5c86859ac..4d173cd270cb8 100644 --- a/x-pack/plugins/security_solution/public/resolver/mocks/get_ui_settings.ts +++ b/x-pack/plugins/security_solution/public/resolver/mocks/ui_setting.ts @@ -4,7 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -export function getUiSettings(key: string): string | undefined { +/** + * A mock for Kibana UI settings. + */ +export function uiSetting(key: string): string | undefined { if (key === 'dateFormat') { return 'MMM D, YYYY @ HH:mm:ss.SSS'; } diff --git a/x-pack/plugins/security_solution/public/resolver/store/actions.ts b/x-pack/plugins/security_solution/public/resolver/store/actions.ts index 66a32ba29cd74..26a5f8555a81b 100644 --- a/x-pack/plugins/security_solution/public/resolver/store/actions.ts +++ b/x-pack/plugins/security_solution/public/resolver/store/actions.ts @@ -8,16 +8,25 @@ import { DataAction } from './data/action'; /** * When the user wants to bring a node front-and-center on the map. + * @deprecated Nodes are brought into view upon selection instead. See `appReceivedNewExternalProperties` */ interface UserBroughtNodeIntoView { + /** + * @deprecated Nodes are brought into view upon selection instead. See `appReceivedNewExternalProperties` + */ readonly type: 'userBroughtNodeIntoView'; + /** + * @deprecated Nodes are brought into view upon selection instead. See `appReceivedNewExternalProperties` + */ readonly payload: { /** * Used to identify the node that should be brought into view. + * @deprecated Nodes are brought into view upon selection instead. See `appReceivedNewExternalProperties` */ readonly nodeID: string; /** * The time (since epoch in milliseconds) when the action was dispatched. + * @deprecated Nodes are brought into view upon selection instead. See `appReceivedNewExternalProperties` */ readonly time: number; }; diff --git a/x-pack/plugins/security_solution/public/resolver/test_utilities/extend_jest.ts b/x-pack/plugins/security_solution/public/resolver/test_utilities/extend_jest.ts index aa04221361de0..23b651c262cba 100644 --- a/x-pack/plugins/security_solution/public/resolver/test_utilities/extend_jest.ts +++ b/x-pack/plugins/security_solution/public/resolver/test_utilities/extend_jest.ts @@ -34,7 +34,7 @@ expect.extend({ expected: T ): Promise<{ pass: boolean; message: () => string }> { // Used in printing out the pass or fail message - const matcherName = 'toSometimesYieldEqualTo'; + const matcherName = 'toYieldEqualTo'; const options: jest.MatcherHintOptions = { comment: 'deep equality with any yielded value', isNot: this.isNot, @@ -100,9 +100,9 @@ expect.extend({ expected: T ): Promise<{ pass: boolean; message: () => string }> { // Used in printing out the pass or fail message - const matcherName = 'toSometimesYieldEqualTo'; + const matcherName = 'toYieldObjectEqualTo'; const options: jest.MatcherHintOptions = { - comment: 'deep equality with any yielded value', + comment: 'subset equality with any yielded value', isNot: this.isNot, promise: this.promise, }; diff --git a/x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx b/x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx index 2a399b6844bd7..2a538620dce0b 100644 --- a/x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx +++ b/x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx @@ -16,7 +16,7 @@ import { MockResolver } from './mock_resolver'; import { ResolverState, DataAccessLayer, SpyMiddleware, SideEffectSimulator } from '../../types'; import { ResolverAction } from '../../store/actions'; import { sideEffectSimulatorFactory } from '../../view/side_effect_simulator_factory'; -import { getUiSettings } from '../../mocks/get_ui_settings'; +import { uiSetting } from '../../mocks/ui_setting'; /** * Test a Resolver instance using jest, enzyme, and a mock data layer. @@ -62,6 +62,13 @@ export class Simulator { return selector; } + /** + * The simulator returns enzyme `ReactWrapper`s from various methods. Use this predicate to determine if they are DOM nodes. + */ + public static isDOM(wrapper: ReactWrapper): boolean { + return typeof wrapper.type() === 'string'; + } + constructor({ dataAccessLayer, resolverComponentInstanceID, @@ -110,7 +117,7 @@ export class Simulator { // Used for `KibanaContextProvider` const coreStart = coreMock.createStart(); - coreStart.uiSettings.get.mockImplementation(getUiSettings); + coreStart.uiSettings.get.mockImplementation(uiSetting); this.sideEffectSimulator = sideEffectSimulatorFactory(); @@ -190,7 +197,7 @@ export class Simulator { * After 10 times, quit. * Use this to continually check a value. See `toYieldEqualTo`. */ - public async *map(mapper: () => R): AsyncIterable { + public async *map(mapper: (() => Promise) | (() => R)): AsyncIterable { let timeoutCount = 0; while (timeoutCount < 10) { timeoutCount++; @@ -267,6 +274,20 @@ export class Simulator { this.sideEffectSimulator.controls.provideAnimationFrame(); } + /** + * The last value written to the clipboard via the `SideEffectors`. + */ + public get clipboardText(): string { + return this.sideEffectSimulator.controls.clipboardText; + } + + /** + * Call this to resolve the promise returned by the `SideEffectors` `writeText` method (which in production points to `navigator.clipboard.writeText`. + */ + confirmTextWrittenToClipboard(): void { + this.sideEffectSimulator.controls.confirmTextWrittenToClipboard(); + } + /** * The 'search' part of the URL. */ @@ -296,13 +317,36 @@ export class Simulator { return this.domNodes(`[data-test-subj="${selector}"]`); } + /** + * Given a `ReactWrapper`, returns a wrapper containing immediately following `dd` siblings. + * `subject` must contain just 1 element. + */ + public descriptionDetails(subject: ReactWrapper): ReactWrapper { + // find the associated DOM nodes, then return an enzyme wrapper that only contains those. + const subjectNode = subject.getDOMNode(); + let current = subjectNode.nextElementSibling; + const associated: Set = new Set(); + // Multiple `dt`s can be associated with a set of `dd`s. Skip immediately following `dt`s. + while (current !== null && current.nodeName === 'DT') { + current = current.nextElementSibling; + } + while (current !== null && current.nodeName === 'DD') { + associated.add(current); + current = current.nextElementSibling; + } + return subject + .closest('dl') + .find('dd') + .filterWhere((candidate) => { + return associated.has(candidate.getDOMNode()); + }); + } + /** * Return DOM nodes that match `enzymeSelector`. */ private domNodes(enzymeSelector: string): ReactWrapper { - return this.wrapper - .find(enzymeSelector) - .filterWhere((wrapper) => typeof wrapper.type() === 'string'); + return this.wrapper.find(enzymeSelector).filterWhere(Simulator.isDOM); } /** @@ -331,7 +375,7 @@ export class Simulator { * Resolve the wrapper returned by `wrapperFactory` only once it has at least 1 element in it. */ public async resolveWrapper( - wrapperFactory: () => ReactWrapper, + wrapperFactory: (() => Promise) | (() => ReactWrapper), predicate: (wrapper: ReactWrapper) => boolean = (wrapper) => wrapper.length > 0 ): Promise { for await (const wrapper of this.map(wrapperFactory)) { diff --git a/x-pack/plugins/security_solution/public/resolver/types.ts b/x-pack/plugins/security_solution/public/resolver/types.ts index fb57f85639e33..7129e3a47120a 100644 --- a/x-pack/plugins/security_solution/public/resolver/types.ts +++ b/x-pack/plugins/security_solution/public/resolver/types.ts @@ -490,9 +490,26 @@ export interface SideEffectors { * A function which returns the time since epoch in milliseconds. Injected because mocking Date is tedious. */ timestamp: () => number; + /** + * Use instead of `window.requestAnimationFrame` + **/ requestAnimationFrame: typeof window.requestAnimationFrame; + /** + * Use instead of `window.cancelAnimationFrame` + **/ cancelAnimationFrame: typeof window.cancelAnimationFrame; + /** + * Use instead of the `ResizeObserver` global. + */ ResizeObserver: ResizeObserverConstructor; + /** + * Use this instead of the Clipboard API's `writeText` method. + */ + writeTextToClipboard(text: string): Promise; + /** + * Use this instead of `Element.prototype.getBoundingClientRect` . + */ + getBoundingClientRect(element: Element): DOMRect; } export interface SideEffectSimulator { @@ -512,6 +529,16 @@ export interface SideEffectSimulator { * Trigger `ResizeObserver` callbacks for `element` and update the mocked value for `getBoundingClientRect`. */ simulateElementResize: (element: Element, contentRect: DOMRect) => void; + + /** + * Get the most recently written clipboard text. This is only updated when `confirmTextWrittenToClipboard` is called. + */ + clipboardText: string; + + /** + * Call this to resolve the promise returned by `writeText`. + */ + confirmTextWrittenToClipboard: () => void; }; /** * Mocked `SideEffectors`. diff --git a/x-pack/plugins/security_solution/public/resolver/view/clickthrough.test.tsx b/x-pack/plugins/security_solution/public/resolver/view/clickthrough.test.tsx index c781832dc8a3b..7739d81269180 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/clickthrough.test.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/clickthrough.test.tsx @@ -165,10 +165,7 @@ describe('Resolver, when analyzing a tree that has no ancestors and 2 children', ).toYieldEqualTo({ treeCount: 1, nodesOwnedByTrees: 3 }); }); - it(`should show links to the 3 nodes (with icons) in the node list.`, async () => { - await expect( - simulator.map(() => simulator.testSubject('resolver:node-list:node-link:title').length) - ).toYieldEqualTo(3); + it(`should show links to the 3 nodes in the node list.`, async () => { await expect( simulator.map(() => simulator.testSubject('resolver:node-list:node-link:title').length) ).toYieldEqualTo(3); diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/panel_content_utilities.tsx b/x-pack/plugins/security_solution/public/resolver/view/generated_text.tsx similarity index 59% rename from x-pack/plugins/security_solution/public/resolver/view/panels/panel_content_utilities.tsx rename to x-pack/plugins/security_solution/public/resolver/view/generated_text.tsx index a20498cbfb67b..61a12fa33cc9d 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/panel_content_utilities.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/generated_text.tsx @@ -4,33 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ +import React from 'react'; /* eslint-disable react/display-name */ - -import { EuiCode } from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; -import styled from 'styled-components'; -import React, { memo } from 'react'; - -/** - * Text to use in place of an undefined timestamp value - */ - -export const noTimestampRetrievedText = i18n.translate( - 'xpack.securitySolution.enpdoint.resolver.panelutils.noTimestampRetrieved', - { - defaultMessage: 'No timestamp retrieved', - } -); - -/** - * A bold version of EuiCode to display certain titles with - */ -export const BoldCode = styled(EuiCode)` - &.euiCodeBlock code.euiCodeBlock__code { - font-weight: 900; - } -`; - /** * A component that renders an element with breaking opportunities (``s) * spliced into text children at word boundaries. @@ -61,12 +36,3 @@ export const GeneratedText = React.memo(function ({ children }) { }); } }); - -/** - * A component to keep time representations in blocks so they don't wrap - * and look bad. - */ -export const StyledTime = memo(styled('time')` - display: inline-block; - text-align: start; -`); diff --git a/x-pack/plugins/security_solution/public/resolver/view/panel.test.tsx b/x-pack/plugins/security_solution/public/resolver/view/panel.test.tsx index 9d72af3109564..3b3651ec2558a 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panel.test.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panel.test.tsx @@ -29,6 +29,20 @@ describe(`Resolver: when analyzing a tree with no ancestors and two children and secondChild: string; }; + /** + * These are the details we expect to see in the node detail view when the origin is selected. + */ + const originEventDetailEntries: ReadonlyMap = new Map([ + ['@timestamp', 'Sep 23, 2020 @ 08:25:32.316'], + ['process.executable', 'executable'], + ['process.pid', '0'], + ['user.name', 'user.name'], + ['user.domain', 'user.domain'], + ['process.parent.pid', '0'], + ['process.hash.md5', 'hash.md5'], + ['process.args', 'args'], + ]); + beforeEach(() => { // create a mock data access layer const { @@ -86,16 +100,7 @@ describe(`Resolver: when analyzing a tree with no ancestors and two children and ).toYieldEqualTo({ title: 'c.ext', titleIcon: 'Running Process', - detailEntries: [ - ['@timestamp', 'Sep 23, 2020 @ 08:25:32.316'], - ['process.executable', 'executable'], - ['process.pid', '0'], - ['user.name', 'user.name'], - ['user.domain', 'user.domain'], - ['process.parent.pid', '0'], - ['process.hash.md5', 'hash.md5'], - ['process.args', 'args'], - ], + detailEntries: [...originEventDetailEntries], }); }); it('should have breaking opportunities (s) in node titles to allow wrapping', async () => { @@ -111,16 +116,46 @@ describe(`Resolver: when analyzing a tree with no ancestors and two children and wordBreaks: 2, }); }); - it('should allow all node details to be copied', async () => { - const copyableFields = await simulator().resolve('resolver:panel:copyable-field'); - copyableFields?.map((copyableField) => { - copyableField.simulate('mouseenter'); - simulator().testSubject('resolver:panel:clipboard').last().simulate('click'); - expect(navigator.clipboard.writeText).toHaveBeenCalledWith(copyableField.text()); - copyableField.simulate('mouseleave'); - }); - }); + /** + * These tests use a statically defined map of fields and expected values. The test finds the `dt` for each field and then finds the related `dd`s. From there it finds a special 'hover area' (via `data-test-subj`) and simulates a `mouseenter` on it. This is because the feature work by adding event listeners to `div`s. There is no way for the user to know that the `div`s are interactable. + + * Finally the test clicks a button and checks that the clipboard was written to. + */ + describe.each([...originEventDetailEntries])( + 'when the user hovers over the description for the field (%p) with their mouse', + (fieldTitleText, value) => { + beforeEach(async () => { + const dt = await simulator().resolveWrapper(() => { + return simulator() + .testSubject('resolver:node-detail:entry-title') + .filterWhere((title) => title.text() === fieldTitleText); + }); + + expect(dt).toHaveLength(1); + + const copyableFieldHoverArea = simulator() + .descriptionDetails(dt!) + // The copyable field popup does not use a button as a trigger. It is instead triggered by mouse interaction with this `div`. + .find(`[data-test-subj="resolver:panel:copyable-field-hover-area"]`) + .filterWhere(Simulator.isDOM); + + expect(copyableFieldHoverArea).toHaveLength(1); + copyableFieldHoverArea!.simulate('mouseenter'); + }); + describe('and when they click the copy-to-clipboard button', () => { + beforeEach(async () => { + const copyButton = await simulator().resolve('resolver:panel:clipboard'); + expect(copyButton).toHaveLength(1); + copyButton!.simulate('click'); + simulator().confirmTextWrittenToClipboard(); + }); + it(`should write ${value} to the clipboard`, async () => { + await expect(simulator().map(() => simulator().clipboardText)).toYieldEqualTo(value); + }); + }); + } + ); }); const queryStringWithFirstChildSelected = urlSearch(resolverComponentInstanceID, { @@ -160,23 +195,43 @@ describe(`Resolver: when analyzing a tree with no ancestors and two children and it('should have 3 nodes (with icons) in the node list', async () => { await expect( - simulator().map(() => simulator().testSubject('resolver:node-list:node-link:title').length) - ).toYieldEqualTo(3); - await expect( - simulator().map(() => simulator().testSubject('resolver:node-list:node-link:icon').length) - ).toYieldEqualTo(3); + simulator().map(() => { + return { + titleCount: simulator().testSubject('resolver:node-list:node-link:title').length, + iconCount: simulator().testSubject('resolver:node-list:node-link:icon').length, + }; + }) + ).toYieldEqualTo({ titleCount: 3, iconCount: 3 }); }); - it('should be able to copy the timestamps for all 3 nodes', async () => { - const copyableFields = await simulator().resolve('resolver:panel:copyable-field'); + describe('when the user hovers over the timestamp for "c.ext" with their mouse', () => { + beforeEach(async () => { + const cExtHoverArea = await simulator().resolveWrapper(async () => { + const nodeLinkTitles = await simulator().resolve('resolver:node-list:node-link:title'); - expect(copyableFields?.length).toBe(3); + expect(nodeLinkTitles).toHaveLength(3); - copyableFields?.map((copyableField) => { - copyableField.simulate('mouseenter'); - simulator().testSubject('resolver:panel:clipboard').last().simulate('click'); - expect(navigator.clipboard.writeText).toHaveBeenCalledWith(copyableField.text()); - copyableField.simulate('mouseleave'); + return ( + nodeLinkTitles! + .filterWhere((linkTitle) => linkTitle.text() === 'c.ext') + // Find the parent `tr` and the find all hover areas in that TR. The test assumes that all cells in a row are associated. + .closest('tr') + // The copyable field popup does not use a button as a trigger. It is instead triggered by mouse interaction with this `div`. + .find('[data-test-subj="resolver:panel:copyable-field-hover-area"]') + .filterWhere(Simulator.isDOM) + ); + }); + cExtHoverArea!.simulate('mouseenter'); + }); + describe('and when the user clicks the copy-to-clipboard button', () => { + beforeEach(async () => { + (await simulator().resolve('resolver:panel:clipboard'))!.simulate('click'); + simulator().confirmTextWrittenToClipboard(); + }); + const expected = 'Sep 23, 2020 @ 08:25:32.316'; + it(`should write "${expected}" to the clipboard`, async () => { + await expect(simulator().map(() => simulator().clipboardText)).toYieldEqualTo(expected); + }); }); }); @@ -191,16 +246,7 @@ describe(`Resolver: when analyzing a tree with no ancestors and two children and it('should show the details for the first node', async () => { await expect( simulator().map(() => simulator().nodeDetailDescriptionListEntries()) - ).toYieldEqualTo([ - ['@timestamp', 'Sep 23, 2020 @ 08:25:32.316'], - ['process.executable', 'executable'], - ['process.pid', '0'], - ['user.name', 'user.name'], - ['user.domain', 'user.domain'], - ['process.parent.pid', '0'], - ['process.hash.md5', 'hash.md5'], - ['process.args', 'args'], - ]); + ).toYieldEqualTo([...originEventDetailEntries]); }); it("should have the first node's ID in the query string", async () => { await expect(simulator().map(() => simulator().historyLocationSearch)).toYieldEqualTo( @@ -278,16 +324,40 @@ describe(`Resolver: when analyzing a tree with no ancestors and two children and simulator().map(() => simulator().testSubject('resolver:panel:event-detail').length) ).toYieldEqualTo(1); }); - it('should allow all fields to be copied', async () => { - const copyableFields = await simulator().resolve('resolver:panel:copyable-field'); - - copyableFields?.map((copyableField) => { - copyableField.simulate('mouseenter'); - simulator().testSubject('resolver:panel:clipboard').last().simulate('click'); - expect(navigator.clipboard.writeText).toHaveBeenCalledWith(copyableField.text()); - copyableField.simulate('mouseleave'); - }); - }); + describe.each([['user.domain', 'user.domain']])( + 'when the user hovers over the description for the field "%p"', + (fieldName, expectedValue) => { + beforeEach(async () => { + const fieldHoverArea = await simulator().resolveWrapper(async () => { + const dt = ( + await simulator().resolve('resolver:panel:event-detail:event-field-title') + )?.filterWhere((title) => title.text() === fieldName); + return ( + simulator() + .descriptionDetails(dt!) + // The copyable field popup does not use a button as a trigger. It is instead triggered by mouse interaction with this `div`. + .find(`[data-test-subj="resolver:panel:copyable-field-hover-area"]`) + .filterWhere(Simulator.isDOM) + ); + }); + expect(fieldHoverArea).toBeTruthy(); + fieldHoverArea?.simulate('mouseenter'); + }); + describe('when the user clicks on the clipboard button', () => { + beforeEach(async () => { + const button = await simulator().resolve('resolver:panel:clipboard'); + expect(button).toBeTruthy(); + button!.simulate('click'); + simulator().confirmTextWrittenToClipboard(); + }); + it(`should write ${expectedValue} to the clipboard`, async () => { + await expect(simulator().map(() => simulator().clipboardText)).toYieldEqualTo( + expectedValue + ); + }); + }); + } + ); }); }); }); diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/copyable_panel_field.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/copyable_panel_field.tsx index f6a585ea566bb..6a1667a839548 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/copyable_panel_field.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/copyable_panel_field.tsx @@ -9,10 +9,11 @@ import { EuiToolTip, EuiButtonIcon, EuiPopover } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import styled from 'styled-components'; -import React, { memo, useState, useCallback } from 'react'; +import React, { memo, useState, useCallback, useContext, useMemo } from 'react'; import { useKibana } from '../../../../../../../src/plugins/kibana_react/public'; import { useColors } from '../use_colors'; import { StyledPanel } from '../styles'; +import { SideEffectContext } from '../side_effect_context'; interface StyledCopyableField { readonly backgroundColor: string; @@ -48,39 +49,41 @@ export const CopyablePanelField = memo( const onMouseEnter = () => setIsOpen(true); const onMouseLeave = () => setIsOpen(false); - const ButtonContent = memo(() => ( - - {content} - - )); + const hoverArea = useMemo( + () => ( + + {content} + + ), + [content, copyableFieldBackground, linkColor] + ); - const onClick = useCallback( - async (event: React.MouseEvent) => { - try { - await navigator.clipboard.writeText(textToCopy); - } catch (error) { - if (toasts) { - toasts.addError(error, { - title: i18n.translate('xpack.securitySolution.resolver.panel.copyFailureTitle', { - defaultMessage: 'Copy Failure', - }), - }); - } + const { writeTextToClipboard } = useContext(SideEffectContext); + + const onClick = useCallback(async () => { + try { + await writeTextToClipboard(textToCopy); + } catch (error) { + if (toasts) { + toasts.addError(error, { + title: i18n.translate('xpack.securitySolution.resolver.panel.copyFailureTitle', { + defaultMessage: 'Copy Failure', + }), + }); } - }, - [textToCopy, toasts] - ); + } + }, [textToCopy, toasts, writeTextToClipboard]); return (
} + button={hoverArea} closePopover={onMouseLeave} hasArrow={false} isOpen={isOpen} diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/event_detail.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/event_detail.tsx index e5569b30abb9d..4936cf0cbb80e 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/event_detail.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/event_detail.tsx @@ -15,12 +15,8 @@ import { EuiSpacer, EuiText, EuiDescriptionList, EuiTextColor, EuiTitle } from ' import styled from 'styled-components'; import { useSelector } from 'react-redux'; import { StyledPanel } from '../styles'; -import { - BoldCode, - StyledTime, - GeneratedText, - noTimestampRetrievedText, -} from './panel_content_utilities'; +import { BoldCode, StyledTime } from './styles'; +import { GeneratedText } from '../generated_text'; import { CopyablePanelField } from './copyable_panel_field'; import { Breadcrumbs } from './breadcrumbs'; import * as eventModel from '../../../../common/endpoint/models/event'; @@ -97,7 +93,12 @@ const EventDetailContents = memo(function ({ processEvent: SafeResolverEvent | null; }) { const timestamp = eventModel.timestampSafeVersion(event); - const formattedDate = useFormattedDate(timestamp) || noTimestampRetrievedText; + const formattedDate = + useFormattedDate(timestamp) || + i18n.translate('xpack.securitySolution.enpdoint.resolver.panelutils.noTimestampRetrieved', { + defaultMessage: 'No timestamp retrieved', + }); + const nodeName = processEvent ? eventModel.processNameSafeVersion(processEvent) : null; return ( @@ -155,15 +156,20 @@ function EventDetailFields({ event }: { event: SafeResolverEvent }) { const section = { // Group the fields by their top-level namespace namespace: {key}, - descriptions: deepObjectEntries(value).map(([path, fieldValue]) => ({ - title: {path.join('.')}, - description: ( - {String(fieldValue)}} - /> - ), - })), + descriptions: deepObjectEntries(value).map(([path, fieldValue]) => { + // The field name is the 'namespace' key as well as the rest of the path, joined with '.' + const fieldName = [key, ...path].join('.'); + + return { + title: {fieldName}, + description: ( + {String(fieldValue)}} + /> + ), + }; + }), }; returnValue.push(section); } @@ -187,7 +193,10 @@ function EventDetailFields({ event }: { event: SafeResolverEvent }) { diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/node_detail.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/node_detail.tsx index c7d4f8632659b..5675e29fc2bc1 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/node_detail.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/node_detail.tsx @@ -16,7 +16,7 @@ import { EuiDescriptionListProps } from '@elastic/eui/src/components/description import { StyledDescriptionList, StyledTitle } from './styles'; import * as selectors from '../../store/selectors'; import * as eventModel from '../../../../common/endpoint/models/event'; -import { GeneratedText } from './panel_content_utilities'; +import { GeneratedText } from '../generated_text'; import { CopyablePanelField } from './copyable_panel_field'; import { Breadcrumbs } from './breadcrumbs'; import { processPath, processPID } from '../../models/process_event'; diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx index 17e91902d0c96..c9648c6f562e5 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx @@ -4,6 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ +/* eslint-disable react/display-name */ + import React, { memo, useCallback, Fragment } from 'react'; import { i18n } from '@kbn/i18n'; import { @@ -18,7 +20,7 @@ import { import { useSelector } from 'react-redux'; import { FormattedMessage } from '@kbn/i18n/react'; import { StyledPanel } from '../styles'; -import { BoldCode, noTimestampRetrievedText, StyledTime } from './panel_content_utilities'; +import { BoldCode, StyledTime } from './styles'; import { Breadcrumbs } from './breadcrumbs'; import * as eventModel from '../../../../common/endpoint/models/event'; import { SafeResolverEvent } from '../../../../common/endpoint/types'; @@ -99,8 +101,6 @@ export const NodeEventsInCategory = memo(function ({ ); }); -NodeEventsInCategory.displayName = 'NodeEventsInCategory'; - /** * Rendered for each event in the list. */ @@ -114,7 +114,11 @@ const NodeEventsListItem = memo(function ({ eventCategory: string; }) { const timestamp = eventModel.eventTimestamp(event); - const date = useFormattedDate(timestamp) || noTimestampRetrievedText; + const date = + useFormattedDate(timestamp) || + i18n.translate('xpack.securitySolution.enpdoint.resolver.panelutils.noTimestampRetrieved', { + defaultMessage: 'No timestamp retrieved', + }); const linkProps = useLinkProps({ panelView: 'eventDetail', panelParameters: { diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/node_list.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/node_list.tsx index 9ef72c414bb63..e53cd2cc0860d 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/node_list.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/node_list.tsx @@ -38,17 +38,14 @@ import { LimitWarning } from '../limit_warnings'; import { ResolverState } from '../../types'; import { useLinkProps } from '../use_link_props'; import { useColors } from '../use_colors'; -import { SafeResolverEvent } from '../../../../common/endpoint/types'; import { ResolverAction } from '../../store/actions'; import { useFormattedDate } from './use_formatted_date'; -import { getEmptyTagValue } from '../../../common/components/empty_value'; import { CopyablePanelField } from './copyable_panel_field'; interface ProcessTableView { name?: string; timestamp?: Date; nodeID: string; - event: SafeResolverEvent; } /** @@ -68,7 +65,7 @@ export const NodeList = memo(() => { sortable: true, truncateText: true, render(name: string | undefined, item: ProcessTableView) { - return ; + return ; }, }, { @@ -101,7 +98,6 @@ export const NodeList = memo(() => { name, timestamp: eventModel.timestampAsDateSafeVersion(processEvent), nodeID, - event: processEvent, }); } } @@ -111,7 +107,7 @@ export const NodeList = memo(() => { const numberOfProcesses = processTableView.length; - const crumbs = useMemo(() => { + const breadcrumbs = useMemo(() => { return [ { text: i18n.translate('xpack.securitySolution.resolver.panel.nodeList.title', { @@ -127,7 +123,7 @@ export const NodeList = memo(() => { const rowProps = useMemo(() => ({ 'data-test-subj': 'resolver:node-list:item' }), []); return ( - + {showWarning && } @@ -141,15 +137,7 @@ export const NodeList = memo(() => { ); }); -function NodeDetailLink({ - name, - nodeID, - event, -}: { - name?: string; - nodeID: string; - event: SafeResolverEvent; -}) { +function NodeDetailLink({ name, nodeID }: { name?: string; nodeID: string }) { const isOrigin = useSelector((state: ResolverState) => { return selectors.originID(state) === nodeID; }); @@ -175,7 +163,7 @@ function NodeDetailLink({ ); return ( - {name === '' ? ( + {name === undefined ? ( {i18n.translate( 'xpack.securitySolution.endpoint.resolver.panel.table.row.valueMissingDescription', @@ -218,6 +206,6 @@ const NodeDetailTimestamp = memo(({ eventDate }: { eventDate: Date | undefined } return formattedDate ? ( ) : ( - getEmptyTagValue() + {'—'} ); }); diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/styles.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/styles.tsx index 03826dd38397b..6f9d4bb600fde 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/styles.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/styles.tsx @@ -4,6 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ +import { EuiCode } from '@elastic/eui'; + /* eslint-disable no-duplicate-imports */ import { EuiBreadcrumbs } from '@elastic/eui'; @@ -89,3 +91,21 @@ export const StyledLabelContainer = styled.div` white-space: nowrap; } `; + +/** + * A bold version of EuiCode to display certain titles with + */ +export const BoldCode = styled(EuiCode)` + &.euiCodeBlock code.euiCodeBlock__code { + font-weight: 900; + } +`; + +/** + * A component to keep time representations in blocks so they don't wrap + * and look bad. + */ +export const StyledTime = styled('time')` + display: inline-block; + text-align: start; +`; diff --git a/x-pack/plugins/security_solution/public/resolver/view/panels/use_formatted_date.test.tsx b/x-pack/plugins/security_solution/public/resolver/view/panels/use_formatted_date.test.tsx index c08c3b370558b..647f7c75d0298 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/panels/use_formatted_date.test.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/panels/use_formatted_date.test.tsx @@ -4,103 +4,52 @@ * you may not use this file except in compliance with the Elastic License. */ +import { mount } from 'enzyme'; + import React from 'react'; -import { render, RenderResult } from '@testing-library/react'; import { useFormattedDate } from './use_formatted_date'; import { coreMock } from '../../../../../../../src/core/public/mocks'; import { KibanaContextProvider } from '../../../../../../../src/plugins/kibana_react/public'; -import { getUiSettings } from '../../mocks/get_ui_settings'; +import { uiSetting } from '../../mocks/ui_setting'; -describe('useFormattedDate', () => { - let element: HTMLElement; - const testID = 'formattedDate'; - let reactRenderResult: ( - date: ConstructorParameters[0] | Date | undefined - ) => RenderResult; +describe(`useFormattedDate, when the "dateFormat" UI setting is "${uiSetting( + 'dateFormat' +)}" and the "dateFormat:tz" setting is "${uiSetting('dateFormat:tz')}"`, () => { + let formattedDate: (date: ConstructorParameters[0] | Date | undefined) => string; beforeEach(async () => { const mockCoreStart = coreMock.createStart(); - mockCoreStart.uiSettings.get.mockImplementation(getUiSettings); + mockCoreStart.uiSettings.get.mockImplementation(uiSetting); function Test({ date }: { date: ConstructorParameters[0] | Date | undefined }) { - const formattedDate = useFormattedDate(date); - return
{formattedDate}
; + return <>{useFormattedDate(date)}; } - reactRenderResult = ( - date: ConstructorParameters[0] | Date | undefined - ): RenderResult => - render( + formattedDate = (date: ConstructorParameters[0] | Date | undefined): string => + mount( - ); - }); - afterEach(() => { - jest.clearAllMocks(); - }); - - describe('when the provided date is undefined', () => { - it('should return undefined', async () => { - const { findByTestId } = reactRenderResult(undefined); - element = await findByTestId(testID); - - expect(element).toBeEmptyDOMElement(); - }); - }); - - describe('when the provided date is empty', () => { - it('should return undefined', async () => { - const { findByTestId } = reactRenderResult(''); - element = await findByTestId(testID); - - expect(element).toBeEmptyDOMElement(); - }); - }); - - describe('when the provided date is an invalid date', () => { - it('should return the string invalid date', async () => { - const { findByTestId } = reactRenderResult('randomString'); - element = await findByTestId(testID); - - expect(element).toHaveTextContent('Invalid Date'); - }); - }); - - describe('when the provided date is a stringified unix timestamp', () => { - it('should return the string invalid date', async () => { - const { findByTestId } = reactRenderResult('1600863932316'); - element = await findByTestId(testID); - - expect(element).toHaveTextContent('Invalid Date'); - }); - }); - - describe('when the provided date is a valid numerical timestamp', () => { - it('should return the string invalid date', async () => { - const { findByTestId } = reactRenderResult(1600863932316); - element = await findByTestId(testID); - - expect(element).toHaveTextContent('Sep 23, 2020 @ 08:25:32.316'); - }); - }); - - describe('when the provided date is a date string', () => { - it('should return the string invalid date', async () => { - const { findByTestId } = reactRenderResult('2020-09-23T12:25:32Z'); - element = await findByTestId(testID); - - expect(element).toHaveTextContent('Sep 23, 2020 @ 08:25:32.000'); - }); - }); - - describe('when the provided date is a valid date', () => { - it('should return the string invalid date', async () => { - const validDate = new Date(1600863932316); - const { findByTestId } = reactRenderResult(validDate); - element = await findByTestId(testID); - - expect(element).toHaveTextContent('Sep 23, 2020 @ 08:25:32.316'); - }); + ).text(); + }); + + it.each([ + ['randomString', 'an invalid string', 'Invalid Date'], + [ + '1600863932316', + "a string that does't match the configured time format settings", + 'Invalid Date', + ], + [1600863932316, 'a valid unix timestamp', 'Sep 23, 2020 @ 08:25:32.316'], + [undefined, 'undefined', ''], + ['', 'an empty string', ''], + [ + '2020-09-23T12:25:32Z', + 'a string that conforms to the specified format', + 'Sep 23, 2020 @ 08:25:32.000', + ], + [new Date(1600863932316), 'a defined Date object', 'Sep 23, 2020 @ 08:25:32.316'], + ])('when the provided date is %p (%s) it should return %p', (value, _explanation, expected) => { + expect(formattedDate(value)).toBe(expected); }); }); diff --git a/x-pack/plugins/security_solution/public/resolver/view/side_effect_context.ts b/x-pack/plugins/security_solution/public/resolver/view/side_effect_context.ts index ab7f41d815026..71b054948160e 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/side_effect_context.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/side_effect_context.ts @@ -19,6 +19,12 @@ const sideEffectors: SideEffectors = { return window.cancelAnimationFrame(...args); }, ResizeObserver, + writeTextToClipboard(text: string): Promise { + return navigator.clipboard.writeText(text); + }, + getBoundingClientRect(element: Element): DOMRect { + return element.getBoundingClientRect(); + }, }; /** diff --git a/x-pack/plugins/security_solution/public/resolver/view/side_effect_simulator_factory.ts b/x-pack/plugins/security_solution/public/resolver/view/side_effect_simulator_factory.ts index 8517459b8aba3..84da2824962d6 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/side_effect_simulator_factory.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/side_effect_simulator_factory.ts @@ -37,7 +37,7 @@ export const sideEffectSimulatorFactory: () => SideEffectSimulator = () => { /** * Get the simulate `DOMRect` for `element`. */ - const contentRectForElement: (target: Element) => DOMRect = (target) => { + const getBoundingClientRect: (target: Element) => DOMRect = (target) => { if (contentRects.has(target)) { return contentRects.get(target)!; } @@ -58,26 +58,33 @@ export const sideEffectSimulatorFactory: () => SideEffectSimulator = () => { }; /** - * Change `Element.prototype.getBoundingClientRect` to return our faked values. + * Last value written to the clipboard, of '' if no text has been written. Returned by the `controls`. */ - jest - .spyOn(Element.prototype, 'getBoundingClientRect') - .mockImplementation(function (this: Element) { - return contentRectForElement(this); - }); + let clipboardText: string = ''; // the `readText` method of the Clipboard API returns an empty string if the clipboard is empty. + + function confirmTextWrittenToClipboard() { + const next = clipboardWriteTextQueue.shift(); + if (next) { + const [text, resolve] = next; + clipboardText = text; + resolve(); + } + } /** - * Mock the global writeText method as it is not available in jsDOM and alows us to track what was copied + * Queue of `text` waiting to be written to the clipboard. Calling `resolve` will resolve the promise returned by the mock `writeTextToClipboard` method. */ - const MockClipboard: Clipboard = { - writeText: jest.fn(), - readText: jest.fn(), - addEventListener: jest.fn(), - dispatchEvent: jest.fn(), - removeEventListener: jest.fn(), - }; - // @ts-ignore navigator doesn't natively exist on global - global.navigator.clipboard = MockClipboard; + const clipboardWriteTextQueue: Array<[text: string, resolve: () => void]> = []; + + /** + * Mock `writeText` method of the `Clipboard` API. + */ + function writeTextToClipboard(text: string): Promise { + return new Promise((resolve) => { + clipboardWriteTextQueue.push([text, resolve]); + }); + } + /** * A mock implementation of `ResizeObserver` that works with our fake `getBoundingClientRect` and `simulateElementResize` */ @@ -171,12 +178,20 @@ export const sideEffectSimulatorFactory: () => SideEffectSimulator = () => { }, simulateElementResize, + + get clipboardText() { + return clipboardText; + }, + + confirmTextWrittenToClipboard, }, mock: { requestAnimationFrame, cancelAnimationFrame, timestamp, ResizeObserver: MockResizeObserver, + writeTextToClipboard, + getBoundingClientRect, }, }; return retval; diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_camera.test.tsx b/x-pack/plugins/security_solution/public/resolver/view/use_camera.test.tsx index bf72a52559cbd..35cf2c36d6627 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_camera.test.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/use_camera.test.tsx @@ -4,13 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { FunctionComponent } from 'react'; -import { render, waitFor, RenderResult, fireEvent } from '@testing-library/react'; -import { renderHook, act } from '@testing-library/react-hooks'; -import { useCamera, useAutoUpdatingClientRect } from './use_camera'; +// Extend jest with a custom matcher +import '../test_utilities/extend_jest'; + +import { mount, ReactWrapper } from 'enzyme'; +import React from 'react'; +import { useCamera } from './use_camera'; import { Provider } from 'react-redux'; import * as selectors from '../store/selectors'; -import { Matrix3, ResolverStore, SideEffectSimulator } from '../types'; +import { Matrix3, ResolverStore, SideEffectors, SideEffectSimulator } from '../types'; import { SafeResolverEvent } from '../../../common/endpoint/types'; import { SideEffectContext } from './side_effect_context'; import { applyMatrix3 } from '../models/vector2'; @@ -22,45 +24,120 @@ import { createStore } from 'redux'; import { resolverReducer } from '../store/reducer'; import { mockTreeFetcherParameters } from '../mocks/tree_fetcher_parameters'; import { entityIDSafeVersion } from '../../../common/endpoint/models/event'; +import { act } from 'react-dom/test-utils'; describe('useCamera on an unpainted element', () => { - let element: HTMLElement; + /** Enzyme full DOM wrapper for the element the camera is attached to. */ + let element: ReactWrapper; + /** + * Enzyme full DOM wrapper for the alternate element that the camera can be attached to. Used for testing that the `ResizeObserver` attaches itself to the latest `ref`. + */ + let alternateElement: ReactWrapper; + /** + * projection matrix returned by camera on last render. + */ let projectionMatrix: Matrix3; + /** + * A `data-test-subj` ID used to identify the element the camera normally attaches to. + */ const testID = 'camera'; - let reactRenderResult: RenderResult; + /** + * A `data-test-subj` ID used to identify the element the camera alternatively attaches to. + */ + const alternateTestID = 'alternate'; + /** + * Returned by the legacy framework's render/mount function. + */ + let wrapper: ReactWrapper; let store: ResolverStore; let simulator: SideEffectSimulator; - beforeEach(async () => { - store = createStore(resolverReducer); - - const Test = function () { - const camera = useCamera(); - const { ref, onMouseDown } = camera; - projectionMatrix = camera.projectionMatrix; - return
; - }; + /** Used to find an element by the data-test-subj attribute. + */ + let domElementByTestSubj: (testSubj: string) => ReactWrapper; - simulator = sideEffectSimulatorFactory(); + /** + * Yield the result of `mapper` over and over, once per event-loop cycle. + * After 10 times, quit. + * Use this to continually check a value. See `toYieldEqualTo`. + */ + async function* map(mapper: () => R): AsyncIterable { + let timeoutCount = 0; + while (timeoutCount < 10) { + timeoutCount++; + yield mapper(); + await new Promise((resolve) => { + setTimeout(() => { + wrapper.update(); + resolve(); + }, 0); + }); + } + } - reactRenderResult = render( - - - + function TestWrapper({ + useSecondElement: useAlternateElement = false, + resolverStore, + sideEffectors, + }: { + /** + * Pass `true`, to attach the camera to an alternate element. Used to test that the `ResizeObserver` attaches itself to the latest `ref`. + */ + useSecondElement?: boolean; + resolverStore: ResolverStore; + sideEffectors: SideEffectors; + }) { + return ( + + + ); + } - const { findByTestId } = reactRenderResult; - element = await findByTestId(testID); - }); - afterEach(() => { - jest.clearAllMocks(); - }); - it('should be usable in React', async () => { - expect(element).toBeInTheDocument(); + function Test({ + useAlternateElement = false, + }: { + /** + * Pass `true`, to attach the camera to an alternate element. Used to test that the `ResizeObserver` attaches itself to the latest `ref`. + */ + useAlternateElement?: boolean; + }) { + const camera = useCamera(); + const { ref, onMouseDown } = camera; + projectionMatrix = camera.projectionMatrix; + return useAlternateElement ? ( + <> +
+
+ + ) : ( + <> +
+
+ + ); + } + + beforeEach(async () => { + store = createStore(resolverReducer); + + simulator = sideEffectSimulatorFactory(); + + wrapper = mount(); + + domElementByTestSubj = (testSubj: string) => + wrapper + .find(`[data-test-subj="${testSubj}"]`) + // Omit React components that may be returned. + .filterWhere((item) => typeof item.type() === 'string'); + + element = domElementByTestSubj(testID); + + alternateElement = domElementByTestSubj(alternateTestID); }); - test('returns a projectionMatrix that changes everything to 0', () => { + it('returns a projectionMatrix that changes everything to 0', () => { expect(applyMatrix3([0, 0], projectionMatrix)).toEqual([0, 0]); }); describe('which has been resized to 800x600', () => { @@ -71,8 +148,8 @@ describe('useCamera on an unpainted element', () => { const centerX = width / 2 + leftMargin; const centerY = height / 2 + topMargin; beforeEach(async () => { - await waitFor(() => { - simulator.controls.simulateElementResize(element, { + act(() => { + simulator.controls.simulateElementResize(element.getDOMNode(), { width, height, left: leftMargin, @@ -87,73 +164,82 @@ describe('useCamera on an unpainted element', () => { }); }); }); - test('should observe all resize reference changes', async () => { - const wrapper: FunctionComponent = ({ children }) => ( - - {children} - - ); - - const { result } = renderHook(() => useAutoUpdatingClientRect(), { wrapper }); - const resizeObserverSpy = jest.spyOn(simulator.mock.ResizeObserver.prototype, 'observe'); - - let [rect, ref] = result.current; - act(() => ref(element)); - expect(resizeObserverSpy).toHaveBeenCalledWith(element); - - const div = document.createElement('div'); - act(() => ref(div)); - expect(resizeObserverSpy).toHaveBeenCalledWith(div); - - [rect, ref] = result.current; - expect(rect?.width).toBe(0); - }); - test('provides a projection matrix that inverts the y axis and translates 400,300 (center of the element)', () => { - expect(applyMatrix3([0, 0], projectionMatrix)).toEqual([400, 300]); + it('provides a projection matrix that inverts the y axis and translates 400,300 (center of the element)', () => { + expect(map(() => applyMatrix3([0, 0], projectionMatrix))).toYieldEqualTo([400, 300]); }); describe('when the user presses the mousedown button in the middle of the element', () => { beforeEach(() => { - fireEvent.mouseDown(element, { + element.simulate('mousedown', { clientX: centerX, clientY: centerY, }); }); describe('when the user moves the mouse 50 pixels to the right', () => { beforeEach(() => { - fireEvent.mouseMove(element, { + element.simulate('mousemove', { clientX: centerX + 50, clientY: centerY, }); }); it('should project [0, 0] in world corrdinates 50 pixels to the right of the center of the element', () => { - expect(applyMatrix3([0, 0], projectionMatrix)).toEqual([450, 300]); + expect(map(() => applyMatrix3([0, 0], projectionMatrix))).toYieldEqualTo([450, 300]); }); }); }); describe('when the user uses the mousewheel w/ ctrl held down', () => { beforeEach(() => { - fireEvent.wheel(element, { + element.simulate('wheel', { ctrlKey: true, deltaY: -10, deltaMode: 0, }); }); it('should zoom in', () => { - expect(projectionMatrix).toMatchInlineSnapshot(` - Array [ - 1.0292841801261479, - 0, - 400, - 0, - -1.0292841801261479, - 300, - 0, - 0, - 0, - ] - `); + expect(map(() => projectionMatrix)).toYieldEqualTo([ + 1.0292841801261479, + 0, + 400, + 0, + -1.0292841801261479, + 300, + 0, + 0, + 0, + ]); + }); + }); + + describe('when the element the camera is attached to is switched', () => { + beforeEach(() => { + wrapper.setProps({ + useAlternateElement: true, + }); + }); + describe('and when that element changes size to 1200x800', () => { + beforeEach(() => { + act(() => { + const alternateElementWidth = 1200; + const alternateElementHeight = 800; + simulator.controls.simulateElementResize(alternateElement.getDOMNode(), { + width: alternateElementWidth, + height: alternateElementHeight, + left: leftMargin, + top: topMargin, + right: leftMargin + alternateElementWidth, + bottom: topMargin + alternateElementHeight, + x: leftMargin, + y: topMargin, + toJSON() { + return this; + }, + }); + }); + }); + it('provides a projection matrix that inverts the y axis and translates 600,400', () => { + expect(map(() => applyMatrix3([0, 0], projectionMatrix))).toYieldEqualTo([600, 400]); + }); }); }); @@ -185,9 +271,7 @@ describe('useCamera on an unpainted element', () => { type: 'serverReturnedResolverData', payload: { result: tree, parameters: mockTreeFetcherParameters() }, }; - await waitFor(() => { - store.dispatch(serverResponseAction); - }); + store.dispatch(serverResponseAction); } else { throw new Error('failed to create tree'); } @@ -210,9 +294,7 @@ describe('useCamera on an unpainted element', () => { nodeID, }, }; - await waitFor(() => { - store.dispatch(cameraAction); - }); + store.dispatch(cameraAction); }); it('should request animation frames in a loop', () => { diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_camera.ts b/x-pack/plugins/security_solution/public/resolver/view/use_camera.ts index 661e038d04e32..c58b9f77d097d 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_camera.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/use_camera.ts @@ -280,7 +280,10 @@ export function useCamera(): { * tracked. So if the element's position moves for some reason, be sure to * handle that. */ -export function useAutoUpdatingClientRect(): [DOMRect | null, (node: Element | null) => void] { +function useAutoUpdatingClientRect(): [DOMRect | null, (node: Element | null) => void] { + // Access `getBoundingClientRect` via the `SideEffectContext` (for testing.) + const { getBoundingClientRect } = useContext(SideEffectContext); + // This hooks returns `rect`. const [rect, setRect] = useState(null); @@ -302,9 +305,9 @@ export function useAutoUpdatingClientRect(): [DOMRect | null, (node: Element | n useEffect(() => { if (currentNode !== null) { // When the DOM node is received, immedaiately calculate its DOM Rect and return that - setRect(currentNode.getBoundingClientRect()); + setRect(getBoundingClientRect(currentNode)); } - }, [currentNode]); + }, [currentNode, getBoundingClientRect]); /** * When scroll events occur, recalculate the DOMRect. DOMRect represents the position of an element relative to the viewport, so that may change during scroll (depending on the layout.) @@ -322,7 +325,7 @@ export function useAutoUpdatingClientRect(): [DOMRect | null, (node: Element | n const currentY = window.scrollY; if (currentNode !== null && (previousX !== currentX || previousY !== currentY)) { - setRect(currentNode.getBoundingClientRect()); + setRect(getBoundingClientRect(currentNode)); } previousX = currentX; @@ -334,13 +337,13 @@ export function useAutoUpdatingClientRect(): [DOMRect | null, (node: Element | n return () => { window.removeEventListener('scroll', handleScroll); }; - }, [currentNode, requestAnimationFrame]); + }, [currentNode, requestAnimationFrame, getBoundingClientRect]); useEffect(() => { if (currentNode !== null) { const resizeObserver = new ResizeObserver((entries) => { if (currentNode !== null && currentNode === entries[0].target) { - setRect(currentNode.getBoundingClientRect()); + setRect(getBoundingClientRect(currentNode)); } }); resizeObserver.observe(currentNode); @@ -348,6 +351,6 @@ export function useAutoUpdatingClientRect(): [DOMRect | null, (node: Element | n resizeObserver.disconnect(); }; } - }, [ResizeObserver, currentNode]); + }, [ResizeObserver, currentNode, getBoundingClientRect]); return [rect, ref]; } From 8f92de860c676d11d98ff8ff3b69e8c22d8d7973 Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Tue, 27 Oct 2020 18:08:01 +0100 Subject: [PATCH 4/9] [Lens] Improved range formatter (#80132) Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Wylie Conlon Co-authored-by: Joe Reuter --- .../aggs/utils/get_format_with_aggs.test.ts | 27 ++++ .../search/aggs/utils/get_format_with_aggs.ts | 24 ++- .../editor_frame_service/format_column.ts | 100 ------------- .../public/editor_frame_service/service.tsx | 2 - .../dimension_panel/dimension_editor.tsx | 3 +- .../indexpattern_datasource/format_column.ts | 137 ++++++++++++++++++ .../public/indexpattern_datasource/index.ts | 5 +- .../indexpattern_datasource/indexpattern.tsx | 1 + .../indexpattern_datasource/loader.test.ts | 1 + .../public/indexpattern_datasource/loader.ts | 9 +- .../public/indexpattern_datasource/mocks.ts | 1 + .../definitions/ranges/advanced_editor.scss | 6 +- .../definitions/ranges/advanced_editor.tsx | 13 +- .../definitions/ranges/ranges.test.tsx | 93 +++++++++++- .../operations/definitions/ranges/ranges.tsx | 47 +++++- .../shared_components/label_input.tsx | 3 + .../indexpattern_datasource/to_expression.ts | 56 ++++--- 17 files changed, 384 insertions(+), 144 deletions(-) delete mode 100644 x-pack/plugins/lens/public/editor_frame_service/format_column.ts create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/format_column.ts diff --git a/src/plugins/data/common/search/aggs/utils/get_format_with_aggs.test.ts b/src/plugins/data/common/search/aggs/utils/get_format_with_aggs.test.ts index 28646c092c01c..826e402b14682 100644 --- a/src/plugins/data/common/search/aggs/utils/get_format_with_aggs.test.ts +++ b/src/plugins/data/common/search/aggs/utils/get_format_with_aggs.test.ts @@ -79,6 +79,33 @@ describe('getFormatWithAggs', () => { expect(getFormat).toHaveBeenCalledTimes(1); }); + test('creates alternative format for range using the template parameter', () => { + const mapping = { id: 'range', params: { template: 'arrow_right' } }; + const getFieldFormat = getFormatWithAggs(getFormat); + const format = getFieldFormat(mapping); + + expect(format.convert({ gte: 1, lt: 20 })).toBe('1 → 20'); + expect(getFormat).toHaveBeenCalledTimes(1); + }); + + test('handles Infinity values internally when no nestedFormatter is passed', () => { + const mapping = { id: 'range', params: { replaceInfinity: true } }; + const getFieldFormat = getFormatWithAggs(getFormat); + const format = getFieldFormat(mapping); + + expect(format.convert({ gte: -Infinity, lt: Infinity })).toBe('≥ −∞ and < +∞'); + expect(getFormat).toHaveBeenCalledTimes(1); + }); + + test('lets Infinity values handling to nestedFormatter even when flag is on', () => { + const mapping = { id: 'range', params: { replaceInfinity: true, id: 'any' } }; + const getFieldFormat = getFormatWithAggs(getFormat); + const format = getFieldFormat(mapping); + + expect(format.convert({ gte: -Infinity, lt: Infinity })).toBe('≥ -Infinity and < Infinity'); + expect(getFormat).toHaveBeenCalledTimes(1); + }); + test('returns custom label for range if provided', () => { const mapping = { id: 'range', params: {} }; const getFieldFormat = getFormatWithAggs(getFormat); diff --git a/src/plugins/data/common/search/aggs/utils/get_format_with_aggs.ts b/src/plugins/data/common/search/aggs/utils/get_format_with_aggs.ts index a8134619fec0d..6b03dc5f70edc 100644 --- a/src/plugins/data/common/search/aggs/utils/get_format_with_aggs.ts +++ b/src/plugins/data/common/search/aggs/utils/get_format_with_aggs.ts @@ -56,15 +56,35 @@ export function getFormatWithAggs(getFieldFormat: GetFieldFormat): GetFieldForma id: nestedFormatter.id, params: nestedFormatter.params, }); + const gte = '\u2265'; const lt = '\u003c'; + let fromValue = format.convert(range.gte); + let toValue = format.convert(range.lt); + // In case of identity formatter and a specific flag, replace Infinity values by specific strings + if (params.replaceInfinity && nestedFormatter.id == null) { + const FROM_PLACEHOLDER = '\u2212\u221E'; + const TO_PLACEHOLDER = '+\u221E'; + fromValue = isFinite(range.gte) ? fromValue : FROM_PLACEHOLDER; + toValue = isFinite(range.lt) ? toValue : TO_PLACEHOLDER; + } + + if (params.template === 'arrow_right') { + return i18n.translate('data.aggTypes.buckets.ranges.rangesFormatMessageArrowRight', { + defaultMessage: '{from} → {to}', + values: { + from: fromValue, + to: toValue, + }, + }); + } return i18n.translate('data.aggTypes.buckets.ranges.rangesFormatMessage', { defaultMessage: '{gte} {from} and {lt} {to}', values: { gte, - from: format.convert(range.gte), + from: fromValue, lt, - to: format.convert(range.lt), + to: toValue, }, }); }); diff --git a/x-pack/plugins/lens/public/editor_frame_service/format_column.ts b/x-pack/plugins/lens/public/editor_frame_service/format_column.ts deleted file mode 100644 index 2da6e7195a5e1..0000000000000 --- a/x-pack/plugins/lens/public/editor_frame_service/format_column.ts +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { ExpressionFunctionDefinition, Datatable } from 'src/plugins/expressions/public'; - -interface FormatColumn { - format: string; - columnId: string; - decimals?: number; -} - -const supportedFormats: Record string }> = { - number: { - decimalsToPattern: (decimals = 2) => { - if (decimals === 0) { - return `0,0`; - } - return `0,0.${'0'.repeat(decimals)}`; - }, - }, - percent: { - decimalsToPattern: (decimals = 2) => { - if (decimals === 0) { - return `0,0%`; - } - return `0,0.${'0'.repeat(decimals)}%`; - }, - }, - bytes: { - decimalsToPattern: (decimals = 2) => { - if (decimals === 0) { - return `0,0b`; - } - return `0,0.${'0'.repeat(decimals)}b`; - }, - }, -}; - -export const formatColumn: ExpressionFunctionDefinition< - 'lens_format_column', - Datatable, - FormatColumn, - Datatable -> = { - name: 'lens_format_column', - type: 'datatable', - help: '', - args: { - format: { - types: ['string'], - help: '', - required: true, - }, - columnId: { - types: ['string'], - help: '', - required: true, - }, - decimals: { - types: ['number'], - help: '', - }, - }, - inputTypes: ['datatable'], - fn(input, { format, columnId, decimals }: FormatColumn) { - return { - ...input, - columns: input.columns.map((col) => { - if (col.id === columnId) { - if (supportedFormats[format]) { - return { - ...col, - meta: { - ...col.meta, - params: { - id: format, - params: { pattern: supportedFormats[format].decimalsToPattern(decimals) }, - }, - }, - }; - } else { - return { - ...col, - meta: { - ...col.meta, - params: { - id: format, - }, - }, - }; - } - } - return col; - }), - }; - }, -}; diff --git a/x-pack/plugins/lens/public/editor_frame_service/service.tsx b/x-pack/plugins/lens/public/editor_frame_service/service.tsx index e2a382133cb3c..d1df63780594e 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/service.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/service.tsx @@ -23,7 +23,6 @@ import { } from '../types'; import { Document } from '../persistence/saved_object_store'; import { mergeTables } from './merge_tables'; -import { formatColumn } from './format_column'; import { EmbeddableFactory, LensEmbeddableStartServices } from './embeddable/embeddable_factory'; import { UiActionsStart } from '../../../../../src/plugins/ui_actions/public'; import { DashboardStart } from '../../../../../src/plugins/dashboard/public'; @@ -86,7 +85,6 @@ export class EditorFrameService { getAttributeService: () => Promise ): EditorFrameSetup { plugins.expressions.registerFunction(() => mergeTables); - plugins.expressions.registerFunction(() => formatColumn); const getStartServices = async (): Promise => { const [coreStart, deps] = await core.getStartServices(); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 310548e5ab817..9500d4b44b79e 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -435,7 +435,8 @@ export function DimensionEditor(props: DimensionEditorProps) { /> )} - {selectedColumn && selectedColumn.dataType === 'number' ? ( + {selectedColumn && + (selectedColumn.dataType === 'number' || selectedColumn.operationType === 'range') ? ( { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/format_column.ts b/x-pack/plugins/lens/public/indexpattern_datasource/format_column.ts new file mode 100644 index 0000000000000..3666528f43166 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/format_column.ts @@ -0,0 +1,137 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { + ExpressionFunctionDefinition, + Datatable, + DatatableColumn, +} from 'src/plugins/expressions/public'; + +interface FormatColumn { + format: string; + columnId: string; + decimals?: number; + parentFormat?: string; +} + +export const supportedFormats: Record< + string, + { decimalsToPattern: (decimals?: number) => string } +> = { + number: { + decimalsToPattern: (decimals = 2) => { + if (decimals === 0) { + return `0,0`; + } + return `0,0.${'0'.repeat(decimals)}`; + }, + }, + percent: { + decimalsToPattern: (decimals = 2) => { + if (decimals === 0) { + return `0,0%`; + } + return `0,0.${'0'.repeat(decimals)}%`; + }, + }, + bytes: { + decimalsToPattern: (decimals = 2) => { + if (decimals === 0) { + return `0,0b`; + } + return `0,0.${'0'.repeat(decimals)}b`; + }, + }, +}; + +export const formatColumn: ExpressionFunctionDefinition< + 'lens_format_column', + Datatable, + FormatColumn, + Datatable +> = { + name: 'lens_format_column', + type: 'datatable', + help: '', + args: { + format: { + types: ['string'], + help: '', + required: true, + }, + columnId: { + types: ['string'], + help: '', + required: true, + }, + decimals: { + types: ['number'], + help: '', + }, + parentFormat: { + types: ['string'], + help: '', + }, + }, + inputTypes: ['datatable'], + fn(input, { format, columnId, decimals, parentFormat }: FormatColumn) { + return { + ...input, + columns: input.columns.map((col) => { + if (col.id === columnId) { + if (!parentFormat) { + if (supportedFormats[format]) { + return withParams(col, { + id: format, + params: { pattern: supportedFormats[format].decimalsToPattern(decimals) }, + }); + } else if (format) { + return withParams(col, { id: format }); + } else { + return col; + } + } + + const parsedParentFormat = JSON.parse(parentFormat); + const parentFormatId = parsedParentFormat.id; + const parentFormatParams = parsedParentFormat.params ?? {}; + + if (!parentFormatId) { + return col; + } + + if (format && supportedFormats[format]) { + return withParams(col, { + id: parentFormatId, + params: { + id: format, + params: { + pattern: supportedFormats[format].decimalsToPattern(decimals), + }, + ...parentFormatParams, + }, + }); + } + if (parentFormatParams) { + const innerParams = (col.meta.params?.params as Record) ?? {}; + return withParams(col, { + ...col.meta.params, + params: { + ...innerParams, + ...parentFormatParams, + }, + }); + } + } + return col; + }), + }; + }, +}; + +function withParams(col: DatatableColumn, params: Record) { + return { ...col, meta: { ...col.meta, params } }; +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/index.ts index 4fbed04112632..35987656f6670 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/index.ts @@ -33,8 +33,11 @@ export class IndexPatternDatasource { { expressions, editorFrame, charts }: IndexPatternDatasourceSetupPlugins ) { editorFrame.registerDatasource(async () => { - const { getIndexPatternDatasource, renameColumns } = await import('../async_services'); + const { getIndexPatternDatasource, renameColumns, formatColumn } = await import( + '../async_services' + ); expressions.registerFunction(renameColumns); + expressions.registerFunction(formatColumn); return core.getStartServices().then(([coreStart, { data }]) => getIndexPatternDatasource({ core: coreStart, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index 28aeac223e4a6..a6edfd71c93ca 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -107,6 +107,7 @@ export function uniqueLabels(layers: Record) { } export * from './rename_columns'; +export * from './format_column'; export function getIndexPatternDatasource({ core, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts index 06cfdf7e03481..4222c02388433 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts @@ -117,6 +117,7 @@ const indexPattern2 = ({ title: 'my-fake-restricted-pattern', timeFieldName: 'timestamp', hasRestrictions: true, + fieldFormatMap: { bytes: { id: 'bytes', params: { pattern: '0.0' } } }, fields: [ { name: 'timestamp', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts b/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts index fd8e071d524ee..70079cce6cc46 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts @@ -103,7 +103,14 @@ export async function loadIndexPatterns({ id: indexPattern.id!, // id exists for sure because we got index patterns by id title, timeFieldName, - fieldFormatMap, + fieldFormatMap: + fieldFormatMap && + Object.fromEntries( + Object.entries(fieldFormatMap).map(([id, format]) => [ + id, + 'toJSON' in format ? format.toJSON() : format, + ]) + ), fields: newFields, hasRestrictions: !!typeMeta?.aggs, }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts b/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts index 21ed23321cf57..744a9f6743d09 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/mocks.ts @@ -82,6 +82,7 @@ export const createMockedRestrictedIndexPattern = () => ({ title: 'my-fake-restricted-pattern', timeFieldName: 'timestamp', hasRestrictions: true, + fieldFormatMap: { bytes: { id: 'bytes', params: { pattern: '0.0' } } }, fields: [ { name: 'timestamp', diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.scss b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.scss index b1658043f3204..4af490e7479da 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.scss +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.scss @@ -3,4 +3,8 @@ @include euiFontSizeS; min-height: $euiSizeXL; width: 100%; -} \ No newline at end of file +} + +.lnsRangesOperation__popoverNumberField { + width: 14ch; // Roughly 10 characters plus extra for the padding +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx index 96f4120e3df78..c6773e806aef8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/advanced_editor.tsx @@ -20,8 +20,8 @@ import { EuiPopover, EuiToolTip, htmlIdGenerator, + keys, } from '@elastic/eui'; -import { keys } from '@elastic/eui'; import { IFieldFormat } from '../../../../../../../../src/plugins/data/common'; import { RangeTypeLens, isValidRange, isValidNumber } from './ranges'; import { FROM_PLACEHOLDER, TO_PLACEHOLDER, TYPING_DEBOUNCE_TIME } from './constants'; @@ -39,8 +39,8 @@ type LocalRangeType = RangeTypeLens & { id: string }; const getBetterLabel = (range: RangeTypeLens, formatter: IFieldFormat) => range.label || formatter.convert({ - gte: isValidNumber(range.from) ? range.from : FROM_PLACEHOLDER, - lt: isValidNumber(range.to) ? range.to : TO_PLACEHOLDER, + gte: isValidNumber(range.from) ? range.from : -Infinity, + lt: isValidNumber(range.to) ? range.to : Infinity, }); export const RangePopover = ({ @@ -55,7 +55,6 @@ export const RangePopover = ({ Button: React.FunctionComponent<{ onClick: MouseEventHandler }>; isOpenByCreation: boolean; setIsOpenByCreation: (open: boolean) => void; - formatter: IFieldFormat; }) => { const [isPopoverOpen, setIsPopoverOpen] = useState(false); const [tempRange, setTempRange] = useState(range); @@ -112,6 +111,7 @@ export const RangePopover = ({ { const newRange = { @@ -126,7 +126,6 @@ export const RangePopover = ({ {lteAppendLabel} } - fullWidth compressed placeholder={FROM_PLACEHOLDER} isInvalid={!isValidRange(tempRange)} @@ -137,6 +136,7 @@ export const RangePopover = ({ { const newRange = { @@ -151,7 +151,6 @@ export const RangePopover = ({ {ltPrependLabel} } - fullWidth compressed placeholder={TO_PLACEHOLDER} isInvalid={!isValidRange(tempRange)} @@ -180,6 +179,7 @@ export const RangePopover = ({ { defaultMessage: 'Custom label' } )} onSubmit={onSubmit} + compressed dataTestSubj="indexPattern-ranges-label" /> @@ -284,7 +284,6 @@ export const AdvancedRangeEditor = ({ } setLocalRanges(newRanges); }} - formatter={formatter} Button={({ onClick }: { onClick: MouseEventHandler }) => ( { - return { convert: ({ gte, lt }: { gte: string; lt: string }) => `${gte} - ${lt}` }; +dataPluginMockValue.fieldFormats.deserialize = jest.fn().mockImplementation(({ params }) => { + return { + convert: ({ gte, lt }: { gte: string; lt: string }) => { + if (params?.id === 'custom') { + return `Custom format: ${gte} - ${lt}`; + } + if (params?.id === 'bytes') { + return `Bytes format: ${gte} - ${lt}`; + } + return `${gte} - ${lt}`; + }, + }; }); type ReactMouseEvent = React.MouseEvent & @@ -74,7 +90,14 @@ describe('ranges', () => { function getDefaultState(): IndexPatternPrivateState { return { indexPatternRefs: [], - indexPatterns: {}, + indexPatterns: { + '1': { + id: '1', + title: 'my_index_pattern', + hasRestrictions: false, + fields: [{ name: sourceField, type: 'number', displayName: sourceField }], + }, + }, existingFields: {}, currentIndexPatternId: '1', isFirstExistenceFetch: false, @@ -396,7 +419,7 @@ describe('ranges', () => { /> ); - // This series of act clojures are made to make it work properly the update flush + // This series of act closures are made to make it work properly the update flush act(() => { instance.find(EuiButtonEmpty).prop('onClick')!({} as ReactMouseEvent); }); @@ -453,7 +476,7 @@ describe('ranges', () => { /> ); - // This series of act clojures are made to make it work properly the update flush + // This series of act closures are made to make it work properly the update flush act(() => { instance.find(EuiButtonEmpty).prop('onClick')!({} as ReactMouseEvent); }); @@ -510,7 +533,7 @@ describe('ranges', () => { /> ); - // This series of act clojures are made to make it work properly the update flush + // This series of act closures are made to make it work properly the update flush act(() => { instance.find(RangePopover).find(EuiLink).prop('onClick')!({} as ReactMouseEvent); }); @@ -667,6 +690,60 @@ describe('ranges', () => { ); }); }); + + it('should correctly handle the default formatter for the field', () => { + const setStateSpy = jest.fn(); + + // set a default formatter for the sourceField used + state.indexPatterns['1'].fieldFormatMap = { + MyField: { id: 'custom', params: {} }, + }; + + const instance = mount( + + ); + + expect(instance.find(RangePopover).find(EuiText).prop('children')).toMatch( + /^Custom format:/ + ); + }); + + it('should correctly pick the dimension formatter for the field', () => { + const setStateSpy = jest.fn(); + + // set a default formatter for the sourceField used + state.indexPatterns['1'].fieldFormatMap = { + MyField: { id: 'custom', params: {} }, + }; + + // now set a format on the range operation + (state.layers.first.columns.col1 as RangeIndexPatternColumn).params.format = { + id: 'bytes', + params: { decimals: 0 }, + }; + + const instance = mount( + + ); + + expect(instance.find(RangePopover).find(EuiText).prop('children')).toMatch( + /^Bytes format:/ + ); + }); }); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx index a256f5e4ecfa1..1050eef45a71c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx @@ -13,7 +13,9 @@ import { RangeEditor } from './range_editor'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; import { updateColumnParam, changeColumn } from '../../../state_helpers'; +import { supportedFormats } from '../../../format_column'; import { MODES, AUTO_BARS, DEFAULT_INTERVAL, MIN_HISTOGRAM_BARS, SLICES } from './constants'; +import { IndexPattern, IndexPatternField } from '../../../types'; type RangeType = Omit; // Try to cover all possible serialized states for ranges @@ -32,6 +34,11 @@ export interface RangeIndexPatternColumn extends FieldBasedIndexPatternColumn { type: MODES_TYPES; maxBars: typeof AUTO_BARS | number; ranges: RangeTypeLens[]; + format?: { id: string; params?: { decimals: number } }; + parentFormat?: { + id: string; + params?: { id?: string; template?: string; replaceInfinity?: boolean }; + }; }; } @@ -55,6 +62,15 @@ export const isValidRange = (range: RangeTypeLens): boolean => { return true; }; +function getFieldDefaultFormat(indexPattern: IndexPattern, field: IndexPatternField | undefined) { + if (field) { + if (indexPattern.fieldFormatMap && indexPattern.fieldFormatMap[field.name]) { + return indexPattern.fieldFormatMap[field.name]; + } + } + return undefined; +} + function getEsAggsParams({ sourceField, params }: RangeIndexPatternColumn) { if (params.type === MODES.Range) { return { @@ -105,7 +121,7 @@ export const rangeOperation: OperationDefinition { - const rangeFormatter = data.fieldFormats.deserialize({ id: 'range' }); + const indexPattern = state.indexPatterns[state.layers[layerId].indexPatternId]; + const currentField = indexPattern.fields.find( + (field) => field.name === currentColumn.sourceField + ); + const numberFormat = currentColumn.params.format; + const numberFormatterPattern = + numberFormat && + supportedFormats[numberFormat.id] && + supportedFormats[numberFormat.id].decimalsToPattern(numberFormat.params?.decimals || 0); + + const rangeFormatter = data.fieldFormats.deserialize({ + ...currentColumn.params.parentFormat, + params: { + ...currentColumn.params.parentFormat?.params, + ...(numberFormat + ? { id: numberFormat.id, params: { pattern: numberFormatterPattern } } + : getFieldDefaultFormat(indexPattern, currentField)), + }, + }); + const MAX_HISTOGRAM_BARS = uiSettings.get(UI_SETTINGS.HISTOGRAM_MAX_BARS); const granularityStep = (MAX_HISTOGRAM_BARS - MIN_HISTOGRAM_BARS) / SLICES; const maxBarsDefaultValue = (MAX_HISTOGRAM_BARS - MIN_HISTOGRAM_BARS) / 2; @@ -171,6 +208,10 @@ export const rangeOperation: OperationDefinition { const scale = newMode === MODES.Range ? 'ordinal' : 'interval'; const dataType = newMode === MODES.Range ? 'string' : 'number'; + const parentFormat = + newMode === MODES.Range + ? { id: 'range', params: { template: 'arrow_right', replaceInfinity: true } } + : undefined; setState( changeColumn({ state, @@ -184,6 +225,8 @@ export const rangeOperation: OperationDefinition void; @@ -23,6 +24,7 @@ export const LabelInput = ({ inputRef?: React.MutableRefObject; onSubmit?: () => void; dataTestSubj?: string; + compressed?: boolean; }) => { const [inputValue, setInputValue] = useState(value); @@ -57,6 +59,7 @@ export const LabelInput = ({ prepend={i18n.translate('xpack.lens.labelInput.label', { defaultMessage: 'Label', })} + compressed={compressed} /> ); }; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts index 1b87c48dc7193..e2c4323b56c2a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts @@ -63,32 +63,50 @@ function getExpressionForLayer( }; }, {} as Record); - type FormattedColumn = Required>; + type FormattedColumn = Required< + Extract< + IndexPatternColumn, + | { + params?: { + format: unknown; + }; + } + // when formatters are nested there's a slightly different format + | { + params: { + format?: unknown; + parentFormat?: unknown; + }; + } + > + >; const columnsWithFormatters = columnEntries.filter( - ([, col]) => col.params && 'format' in col.params && col.params.format + ([, col]) => + col.params && + (('format' in col.params && col.params.format) || + ('parentFormat' in col.params && col.params.parentFormat)) ) as Array<[string, FormattedColumn]>; - const formatterOverrides: ExpressionFunctionAST[] = columnsWithFormatters.map(([id, col]) => { - const format = (col as FormattedColumn).params!.format; - const base: ExpressionFunctionAST = { - type: 'function', - function: 'lens_format_column', - arguments: { - format: [format.id], - columnId: [id], - }, - }; - if (typeof format.params?.decimals === 'number') { - return { - ...base, + const formatterOverrides: ExpressionFunctionAST[] = columnsWithFormatters.map( + ([id, col]: [string, FormattedColumn]) => { + // TODO: improve the type handling here + const parentFormat = 'parentFormat' in col.params ? col.params!.parentFormat! : undefined; + const format = (col as FormattedColumn).params!.format; + + const base: ExpressionFunctionAST = { + type: 'function', + function: 'lens_format_column', arguments: { - ...base.arguments, - decimals: [format.params.decimals], + format: format ? [format.id] : [''], + columnId: [id], + decimals: typeof format?.params?.decimals === 'number' ? [format.params.decimals] : [], + parentFormat: parentFormat ? [JSON.stringify(parentFormat)] : [], }, }; + + return base; } - return base; - }); + ); const allDateHistogramFields = Object.values(columns) .map((column) => From c0fea3abd9e05edccbf06f2f9b82e9fa3777122c Mon Sep 17 00:00:00 2001 From: Shahzad Date: Tue, 27 Oct 2020 18:23:30 +0100 Subject: [PATCH 5/9] [UX] Fix search term reset from url (#81654) --- .../URLFilter/URLSearch/SelectableUrlList.tsx | 20 +++++++- .../__tests__/SelectableUrlList.test.tsx | 41 ++++++++++++++++ .../URLFilter/URLSearch/index.tsx | 17 ++++--- .../app/RumDashboard/utils/test_helper.tsx | 47 +++++++++++++++++++ 4 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/__tests__/SelectableUrlList.test.tsx create mode 100644 x-pack/plugins/apm/public/components/app/RumDashboard/utils/test_helper.tsx diff --git a/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/SelectableUrlList.tsx b/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/SelectableUrlList.tsx index d9d3d23299371..7bd9b2c87814b 100644 --- a/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/SelectableUrlList.tsx +++ b/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/SelectableUrlList.tsx @@ -10,6 +10,7 @@ import React, { useRef, useState, KeyboardEvent, + useEffect, } from 'react'; import { EuiFlexGroup, @@ -67,6 +68,7 @@ interface Props { searchValue: string; onClose: () => void; popoverIsOpen: boolean; + initialValue?: string; setPopoverIsOpen: React.Dispatch>; } @@ -80,6 +82,7 @@ export function SelectableUrlList({ onClose, popoverIsOpen, setPopoverIsOpen, + initialValue, }: Props) { const [darkMode] = useUiSetting$('theme:darkMode'); @@ -92,6 +95,9 @@ export function SelectableUrlList({ if (evt.key.toLowerCase() === 'enter') { onTermChange(); setPopoverIsOpen(false); + if (searchRef) { + searchRef.blur(); + } } }; @@ -126,6 +132,16 @@ export function SelectableUrlList({ } }; + useEffect(() => { + if (searchRef && initialValue) { + searchRef.value = initialValue; + } + + // only want to call it at initial render to set value + // coming from initial value/url + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [searchRef]); + const loadingMessage = ( @@ -165,12 +181,12 @@ export function SelectableUrlList({ renderOption={selectableRenderOptions} singleSelection={false} searchProps={{ - placeholder: I18LABELS.searchByUrl, isClearable: true, onFocus: searchOnFocus, onBlur: searchOnBlur, onInput: onSearchInput, inputRef: setSearchRef, + placeholder: I18LABELS.searchByUrl, }} listProps={{ rowHeight: 68, @@ -197,7 +213,7 @@ export function SelectableUrlList({ {searchValue}, icon: ( diff --git a/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/__tests__/SelectableUrlList.test.tsx b/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/__tests__/SelectableUrlList.test.tsx new file mode 100644 index 0000000000000..abafdf089748b --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/__tests__/SelectableUrlList.test.tsx @@ -0,0 +1,41 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import React from 'react'; +import { createMemoryHistory } from 'history'; +import * as fetcherHook from '../../../../../../hooks/useFetcher'; +import { SelectableUrlList } from '../SelectableUrlList'; +import { render } from '../../../utils/test_helper'; + +describe('SelectableUrlList', () => { + it('it uses search term value from url', () => { + jest.spyOn(fetcherHook, 'useFetcher').mockReturnValue({ + data: {}, + status: fetcherHook.FETCH_STATUS.SUCCESS, + refetch: jest.fn(), + }); + + const customHistory = createMemoryHistory({ + initialEntries: ['/?searchTerm=blog'], + }); + + const { getByDisplayValue } = render( + , + { customHistory } + ); + expect(getByDisplayValue('blog')).toBeInTheDocument(); + }); +}); diff --git a/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/index.tsx b/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/index.tsx index 661f4406990f6..02334be5f722e 100644 --- a/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/index.tsx +++ b/x-pack/plugins/apm/public/components/app/RumDashboard/URLFilter/URLSearch/index.tsx @@ -30,9 +30,9 @@ export function URLSearch({ onChange: onFilterChange }: Props) { const [popoverIsOpen, setPopoverIsOpen] = useState(false); - const [searchValue, setSearchValue] = useState(''); + const [searchValue, setSearchValue] = useState(searchTerm ?? ''); - const [debouncedValue, setDebouncedValue] = useState(''); + const [debouncedValue, setDebouncedValue] = useState(searchTerm ?? ''); useDebounce( () => { @@ -44,12 +44,16 @@ export function URLSearch({ onChange: onFilterChange }: Props) { const updateSearchTerm = useCallback( (searchTermN: string) => { + const newQuery = { + ...toQuery(history.location.search), + searchTerm: searchTermN || undefined, + }; + if (!searchTermN) { + delete newQuery.searchTerm; + } const newLocation = { ...history.location, - search: fromQuery({ - ...toQuery(history.location.search), - searchTerm: searchTermN, - }), + search: fromQuery(newQuery), }; history.push(newLocation); }, @@ -133,6 +137,7 @@ export function URLSearch({ onChange: onFilterChange }: Props) {

{I18LABELS.url}

true, + get$: (key: string) => of(true), + }, +} as unknown) as CoreStart; + +export const render = ( + component: React.ReactNode, + options: { customHistory: MemoryHistory } +) => { + const history = options?.customHistory ?? createMemoryHistory(); + + history.location.key = 'TestKeyForTesting'; + + return testLibRender( + + + + {component} + + + + ); +}; From dab3d394d487b304b5f25efe59aab26eb265bd86 Mon Sep 17 00:00:00 2001 From: Ahmad Bamieh Date: Tue, 27 Oct 2020 20:31:20 +0300 Subject: [PATCH 6/9] [i18n] add get_kibana_translation_paths tests (#81624) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../i18n/get_kibana_translation_paths.test.ts | 58 +++++++++++++++++ .../i18n/get_kibana_translation_paths.ts | 42 +++++++++++++ ...tions_path.ts => get_translation_paths.ts} | 0 src/legacy/server/i18n/i18n_mixin.ts | 63 +++++++++++++++++++ src/legacy/server/i18n/index.ts | 58 +---------------- 5 files changed, 164 insertions(+), 57 deletions(-) create mode 100644 src/legacy/server/i18n/get_kibana_translation_paths.test.ts create mode 100644 src/legacy/server/i18n/get_kibana_translation_paths.ts rename src/legacy/server/i18n/{get_translations_path.ts => get_translation_paths.ts} (100%) create mode 100644 src/legacy/server/i18n/i18n_mixin.ts diff --git a/src/legacy/server/i18n/get_kibana_translation_paths.test.ts b/src/legacy/server/i18n/get_kibana_translation_paths.test.ts new file mode 100644 index 0000000000000..0f202c4d433c0 --- /dev/null +++ b/src/legacy/server/i18n/get_kibana_translation_paths.test.ts @@ -0,0 +1,58 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { I18N_RC } from './constants'; +import { fromRoot } from '../../../core/server/utils'; + +jest.mock('./get_translation_paths', () => ({ getTranslationPaths: jest.fn() })); +import { getKibanaTranslationPaths } from './get_kibana_translation_paths'; +import { getTranslationPaths as mockGetTranslationPaths } from './get_translation_paths'; + +describe('getKibanaTranslationPaths', () => { + const mockConfig = { get: jest.fn() }; + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('calls getTranslationPaths against kibana root and kibana-extra', async () => { + mockConfig.get.mockReturnValue([]); + await getKibanaTranslationPaths(mockConfig); + expect(mockGetTranslationPaths).toHaveBeenNthCalledWith(1, { + cwd: fromRoot('.'), + glob: `*/${I18N_RC}`, + }); + + expect(mockGetTranslationPaths).toHaveBeenNthCalledWith(2, { + cwd: fromRoot('../kibana-extra'), + glob: `*/${I18N_RC}`, + }); + }); + + it('calls getTranslationPaths for each config returned in plugin.paths and plugins.scanDirs', async () => { + mockConfig.get.mockReturnValueOnce(['a', 'b']).mockReturnValueOnce(['c']); + await getKibanaTranslationPaths(mockConfig); + expect(mockConfig.get).toHaveBeenNthCalledWith(1, 'plugins.paths'); + expect(mockConfig.get).toHaveBeenNthCalledWith(2, 'plugins.scanDirs'); + + expect(mockGetTranslationPaths).toHaveBeenNthCalledWith(2, { cwd: 'a', glob: I18N_RC }); + expect(mockGetTranslationPaths).toHaveBeenNthCalledWith(3, { cwd: 'b', glob: I18N_RC }); + expect(mockGetTranslationPaths).toHaveBeenNthCalledWith(4, { cwd: 'c', glob: `*/${I18N_RC}` }); + }); +}); diff --git a/src/legacy/server/i18n/get_kibana_translation_paths.ts b/src/legacy/server/i18n/get_kibana_translation_paths.ts new file mode 100644 index 0000000000000..d7f77d3185ba4 --- /dev/null +++ b/src/legacy/server/i18n/get_kibana_translation_paths.ts @@ -0,0 +1,42 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { KibanaConfig } from '../kbn_server'; +import { fromRoot } from '../../../core/server/utils'; +import { I18N_RC } from './constants'; +import { getTranslationPaths } from './get_translation_paths'; + +export async function getKibanaTranslationPaths(config: Pick) { + return await Promise.all([ + getTranslationPaths({ + cwd: fromRoot('.'), + glob: `*/${I18N_RC}`, + }), + ...(config.get('plugins.paths') as string[]).map((cwd) => + getTranslationPaths({ cwd, glob: I18N_RC }) + ), + ...(config.get('plugins.scanDirs') as string[]).map((cwd) => + getTranslationPaths({ cwd, glob: `*/${I18N_RC}` }) + ), + getTranslationPaths({ + cwd: fromRoot('../kibana-extra'), + glob: `*/${I18N_RC}`, + }), + ]); +} diff --git a/src/legacy/server/i18n/get_translations_path.ts b/src/legacy/server/i18n/get_translation_paths.ts similarity index 100% rename from src/legacy/server/i18n/get_translations_path.ts rename to src/legacy/server/i18n/get_translation_paths.ts diff --git a/src/legacy/server/i18n/i18n_mixin.ts b/src/legacy/server/i18n/i18n_mixin.ts new file mode 100644 index 0000000000000..4f77fa8df96cd --- /dev/null +++ b/src/legacy/server/i18n/i18n_mixin.ts @@ -0,0 +1,63 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { i18n, i18nLoader } from '@kbn/i18n'; +import { basename } from 'path'; +import { Server } from 'hapi'; +import type { UsageCollectionSetup } from '../../../plugins/usage_collection/server'; +import { getKibanaTranslationPaths } from './get_kibana_translation_paths'; +import KbnServer, { KibanaConfig } from '../kbn_server'; +import { registerLocalizationUsageCollector } from './localization'; + +export async function i18nMixin( + kbnServer: KbnServer, + server: Server, + config: Pick +) { + const locale = config.get('i18n.locale') as string; + + const translationPaths = await getKibanaTranslationPaths(config); + + const currentTranslationPaths = ([] as string[]) + .concat(...translationPaths) + .filter((translationPath) => basename(translationPath, '.json') === locale); + i18nLoader.registerTranslationFiles(currentTranslationPaths); + + const translations = await i18nLoader.getTranslationsByLocale(locale); + i18n.init( + Object.freeze({ + locale, + ...translations, + }) + ); + + const getTranslationsFilePaths = () => currentTranslationPaths; + + server.decorate('server', 'getTranslationsFilePaths', getTranslationsFilePaths); + + if (kbnServer.newPlatform.setup.plugins.usageCollection) { + const { usageCollection } = kbnServer.newPlatform.setup.plugins as { + usageCollection: UsageCollectionSetup; + }; + registerLocalizationUsageCollector(usageCollection, { + getLocale: () => config.get('i18n.locale') as string, + getTranslationsFilePaths, + }); + } +} diff --git a/src/legacy/server/i18n/index.ts b/src/legacy/server/i18n/index.ts index 61caefb2fb599..a7ef49f44532c 100644 --- a/src/legacy/server/i18n/index.ts +++ b/src/legacy/server/i18n/index.ts @@ -17,60 +17,4 @@ * under the License. */ -import { i18n, i18nLoader } from '@kbn/i18n'; -import { basename } from 'path'; -import { Server } from 'hapi'; -import { fromRoot } from '../../../core/server/utils'; -import type { UsageCollectionSetup } from '../../../plugins/usage_collection/server'; -import { getTranslationPaths } from './get_translations_path'; -import { I18N_RC } from './constants'; -import KbnServer, { KibanaConfig } from '../kbn_server'; -import { registerLocalizationUsageCollector } from './localization'; - -export async function i18nMixin(kbnServer: KbnServer, server: Server, config: KibanaConfig) { - const locale = config.get('i18n.locale') as string; - - const translationPaths = await Promise.all([ - getTranslationPaths({ - cwd: fromRoot('.'), - glob: `*/${I18N_RC}`, - }), - ...(config.get('plugins.paths') as string[]).map((cwd) => - getTranslationPaths({ cwd, glob: I18N_RC }) - ), - ...(config.get('plugins.scanDirs') as string[]).map((cwd) => - getTranslationPaths({ cwd, glob: `*/${I18N_RC}` }) - ), - getTranslationPaths({ - cwd: fromRoot('../kibana-extra'), - glob: `*/${I18N_RC}`, - }), - ]); - - const currentTranslationPaths = ([] as string[]) - .concat(...translationPaths) - .filter((translationPath) => basename(translationPath, '.json') === locale); - i18nLoader.registerTranslationFiles(currentTranslationPaths); - - const translations = await i18nLoader.getTranslationsByLocale(locale); - i18n.init( - Object.freeze({ - locale, - ...translations, - }) - ); - - const getTranslationsFilePaths = () => currentTranslationPaths; - - server.decorate('server', 'getTranslationsFilePaths', getTranslationsFilePaths); - - if (kbnServer.newPlatform.setup.plugins.usageCollection) { - const { usageCollection } = kbnServer.newPlatform.setup.plugins as { - usageCollection: UsageCollectionSetup; - }; - registerLocalizationUsageCollector(usageCollection, { - getLocale: () => config.get('i18n.locale') as string, - getTranslationsFilePaths, - }); - } -} +export { i18nMixin } from './i18n_mixin'; From 8b1ff4ca5942034e0f8d96afc877e02b5c6bf9c1 Mon Sep 17 00:00:00 2001 From: ymao1 Date: Tue, 27 Oct 2020 13:34:23 -0400 Subject: [PATCH 7/9] [Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (#81778) * Adding hasAuth to server and client * Adding migration and fixing tests * Fixing test * Adding spacing * Adding functional test * Fixing migration Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../builtin_action_types/webhook.test.ts | 13 +- .../server/builtin_action_types/webhook.ts | 5 +- .../server/saved_objects/migrations.test.ts | 57 +++++ .../server/saved_objects/migrations.ts | 12 +- .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - .../components/builtin_action_types/types.ts | 1 + .../webhook/webhook.test.tsx | 35 ++- .../builtin_action_types/webhook/webhook.tsx | 28 ++- .../webhook/webhook_connectors.test.tsx | 6 +- .../webhook/webhook_connectors.tsx | 206 ++++++++++-------- .../actions/builtin_action_types/webhook.ts | 1 + .../spaces_only/tests/actions/migrations.ts | 17 ++ .../functional/es_archives/actions/data.json | 54 +++++ 14 files changed, 334 insertions(+), 103 deletions(-) diff --git a/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts index 23ce527d4ae0d..74feb8ee57d48 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -90,8 +90,9 @@ describe('config validation', () => { }; test('config validation passes when only required fields are provided', () => { - const config: Record = { + const config: Record = { url: 'http://mylisteningserver:9200/endpoint', + hasAuth: true, }; expect(validateConfig(actionType, config)).toEqual({ ...defaultValues, @@ -101,9 +102,10 @@ describe('config validation', () => { test('config validation passes when valid methods are provided', () => { ['post', 'put'].forEach((method) => { - const config: Record = { + const config: Record = { url: 'http://mylisteningserver:9200/endpoint', method, + hasAuth: true, }; expect(validateConfig(actionType, config)).toEqual({ ...defaultValues, @@ -127,8 +129,9 @@ describe('config validation', () => { }); test('config validation passes when a url is specified', () => { - const config: Record = { + const config: Record = { url: 'http://mylisteningserver:9200/endpoint', + hasAuth: true, }; expect(validateConfig(actionType, config)).toEqual({ ...defaultValues, @@ -155,6 +158,7 @@ describe('config validation', () => { headers: { 'Content-Type': 'application/json', }, + hasAuth: true, }; expect(validateConfig(actionType, config)).toEqual({ ...defaultValues, @@ -184,6 +188,7 @@ describe('config validation', () => { headers: { 'Content-Type': 'application/json', }, + hasAuth: true, }; expect(validateConfig(actionType, config)).toEqual({ @@ -263,6 +268,7 @@ describe('execute()', () => { headers: { aheader: 'a value', }, + hasAuth: true, }; await actionType.executor({ actionId: 'some-id', @@ -320,6 +326,7 @@ describe('execute()', () => { headers: { aheader: 'a value', }, + hasAuth: false, }; const secrets: ActionTypeSecretsType = { user: null, password: null }; await actionType.executor({ diff --git a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts index d0ec31721685e..dc9de86d3d951 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts @@ -42,6 +42,7 @@ const configSchemaProps = { defaultValue: WebhookMethods.POST, }), headers: nullableType(HeadersSchema), + hasAuth: schema.boolean({ defaultValue: true }), }; const ConfigSchema = schema.object(configSchemaProps); export type ActionTypeConfigType = TypeOf; @@ -128,12 +129,12 @@ export async function executor( execOptions: WebhookActionTypeExecutorOptions ): Promise> { const actionId = execOptions.actionId; - const { method, url, headers = {} } = execOptions.config; + const { method, url, headers = {}, hasAuth } = execOptions.config; const { body: data } = execOptions.params; const secrets: ActionTypeSecretsType = execOptions.secrets; const basicAuth = - isString(secrets.user) && isString(secrets.password) + hasAuth && isString(secrets.user) && isString(secrets.password) ? { auth: { username: secrets.user, password: secrets.password } } : {}; diff --git a/x-pack/plugins/actions/server/saved_objects/migrations.test.ts b/x-pack/plugins/actions/server/saved_objects/migrations.test.ts index 947d84fcfc638..f1bd1ba2aeb60 100644 --- a/x-pack/plugins/actions/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/actions/server/saved_objects/migrations.test.ts @@ -58,6 +58,63 @@ describe('7.10.0', () => { }); }); +describe('7.11.0', () => { + beforeEach(() => { + jest.resetAllMocks(); + encryptedSavedObjectsSetup.createMigration.mockImplementation( + (shouldMigrateWhenPredicate, migration) => migration + ); + }); + + test('add hasAuth = true for .webhook actions with user and password', () => { + const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0']; + const action = getMockDataForWebhook({}, true); + expect(migration711(action, context)).toMatchObject({ + ...action, + attributes: { + ...action.attributes, + config: { + hasAuth: true, + }, + }, + }); + }); + + test('add hasAuth = false for .webhook actions without user and password', () => { + const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0']; + const action = getMockDataForWebhook({}, false); + expect(migration711(action, context)).toMatchObject({ + ...action, + attributes: { + ...action.attributes, + config: { + hasAuth: false, + }, + }, + }); + }); +}); + +function getMockDataForWebhook( + overwrites: Record = {}, + hasUserAndPassword: boolean +): SavedObjectUnsanitizedDoc { + const secrets = hasUserAndPassword + ? { user: 'test', password: '123' } + : { user: '', password: '' }; + return { + attributes: { + name: 'abc', + actionTypeId: '.webhook', + config: {}, + secrets, + ...overwrites, + }, + id: uuid.v4(), + type: 'action', + }; +} + function getMockDataForEmail( overwrites: Record = {} ): SavedObjectUnsanitizedDoc { diff --git a/x-pack/plugins/actions/server/saved_objects/migrations.ts b/x-pack/plugins/actions/server/saved_objects/migrations.ts index 35d30accecedb..1e2290b14ec1b 100644 --- a/x-pack/plugins/actions/server/saved_objects/migrations.ts +++ b/x-pack/plugins/actions/server/saved_objects/migrations.ts @@ -25,8 +25,18 @@ export function getMigrations( pipeMigrations(renameCasesConfigurationObject, addHasAuthConfigurationObject) ); + const migrationWebhookConnectorHasAuth = encryptedSavedObjects.createMigration< + RawAction, + RawAction + >( + (doc): doc is SavedObjectUnsanitizedDoc => + doc.attributes.actionTypeId === '.webhook', + pipeMigrations(addHasAuthConfigurationObject) + ); + return { '7.10.0': executeMigrationWithErrorHandling(migrationActions, '7.10.0'), + '7.11.0': executeMigrationWithErrorHandling(migrationWebhookConnectorHasAuth, '7.11.0'), }; } @@ -70,7 +80,7 @@ function renameCasesConfigurationObject( const addHasAuthConfigurationObject = ( doc: SavedObjectUnsanitizedDoc ): SavedObjectUnsanitizedDoc => { - if (doc.attributes.actionTypeId !== '.email') { + if (doc.attributes.actionTypeId !== '.email' && doc.attributes.actionTypeId !== '.webhook') { return doc; } const hasAuth = !!doc.attributes.secrets.user || !!doc.attributes.secrets.password; diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 892ed216fe12e..78816c7aeb773 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -20244,7 +20244,6 @@ "xpack.triggersActionsUI.sections.actionsConnectorsList.unableToLoadActionTypesMessage": "アクションタイプを読み込めません", "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHeaderKeyText": "キーが必要です。", "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHeaderValueText": "値が必要です。", - "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText": "ユーザー名が必要です。", "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredMethodText": "メソッドが必要です", "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText": "パスワードが必要です。", "xpack.triggersActionsUI.sections.addAlert.error.greaterThenThreshold0Text": "しきい値 1 はしきい値 0 よりも大きい値にしてください。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 1441491e03261..dcc53cf75afe9 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -20264,7 +20264,6 @@ "xpack.triggersActionsUI.sections.actionsConnectorsList.unableToLoadActionTypesMessage": "无法加载操作类型", "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHeaderKeyText": "“键”必填。", "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHeaderValueText": "“值”必填。", - "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText": "“用户名”必填。", "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredMethodText": "“方法”必填", "xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText": "“密码”必填。", "xpack.triggersActionsUI.sections.addAlert.error.greaterThenThreshold0Text": "阈值 1 应 > 阈值 0。", diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts index 958d77a11c883..e22cd268f9bc5 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts @@ -110,6 +110,7 @@ export interface WebhookConfig { method: string; url: string; headers: Record; + hasAuth: boolean; } export interface WebhookSecrets { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.test.tsx index 337c1f0f18a93..e4d9d3f009c7e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.test.tsx @@ -28,7 +28,7 @@ describe('actionTypeRegistry.get() works', () => { }); describe('webhook connector validation', () => { - test('connector validation succeeds when connector config is valid', () => { + test('connector validation succeeds when hasAuth is true and connector config is valid', () => { const actionConnector = { secrets: { user: 'user', @@ -42,6 +42,35 @@ describe('webhook connector validation', () => { method: 'PUT', url: 'http://test.com', headers: { 'content-type': 'text' }, + hasAuth: true, + }, + } as WebhookActionConnector; + + expect(actionTypeModel.validateConnector(actionConnector)).toEqual({ + errors: { + url: [], + method: [], + user: [], + password: [], + }, + }); + }); + + test('connector validation succeeds when hasAuth is false and connector config is valid', () => { + const actionConnector = { + secrets: { + user: '', + password: '', + }, + id: 'test', + actionTypeId: '.webhook', + name: 'webhook', + isPreconfigured: false, + config: { + method: 'PUT', + url: 'http://test.com', + headers: { 'content-type': 'text' }, + hasAuth: false, }, } as WebhookActionConnector; @@ -65,6 +94,7 @@ describe('webhook connector validation', () => { name: 'webhook', config: { method: 'PUT', + hasAuth: true, }, } as WebhookActionConnector; @@ -73,7 +103,7 @@ describe('webhook connector validation', () => { url: ['URL is required.'], method: [], user: [], - password: ['Password is required.'], + password: ['Password is required when username is used.'], }, }); }); @@ -90,6 +120,7 @@ describe('webhook connector validation', () => { config: { method: 'PUT', url: 'invalid.url', + hasAuth: true, }, } as WebhookActionConnector; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.tsx index 04077738e6015..db3ba9b78cee6 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.tsx @@ -74,26 +74,46 @@ export function getActionType(): ActionTypeModel< ) ); } - if (!action.secrets.user && action.secrets.password) { + if (action.config.hasAuth && !action.secrets.user && !action.secrets.password) { errors.user.push( i18n.translate( - 'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText', + 'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredAuthUserNameText', { defaultMessage: 'Username is required.', } ) ); } - if (!action.secrets.password && action.secrets.user) { + if (action.config.hasAuth && !action.secrets.user && !action.secrets.password) { errors.password.push( i18n.translate( - 'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText', + 'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredAuthPasswordText', { defaultMessage: 'Password is required.', } ) ); } + if (action.secrets.user && !action.secrets.password) { + errors.password.push( + i18n.translate( + 'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText', + { + defaultMessage: 'Password is required when username is used.', + } + ) + ); + } + if (!action.secrets.user && action.secrets.password) { + errors.user.push( + i18n.translate( + 'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredUserText', + { + defaultMessage: 'Username is required when password is used.', + } + ) + ); + } return validationResult; }, validateParams: (actionParams: WebhookActionParams): ValidationResult => { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.test.tsx index 45e4c566f7a27..4c5e78670f0c4 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.test.tsx @@ -24,6 +24,7 @@ describe('WebhookActionConnectorFields renders', () => { method: 'PUT', url: 'http:\\test', headers: { 'content-type': 'text' }, + hasAuth: true, }, } as WebhookActionConnector; const wrapper = mountWithIntl( @@ -50,7 +51,9 @@ describe('WebhookActionConnectorFields renders', () => { secrets: {}, actionTypeId: '.webhook', isPreconfigured: false, - config: {}, + config: { + hasAuth: true, + }, } as WebhookActionConnector; const wrapper = mountWithIntl( { method: 'PUT', url: 'http:\\test', headers: { 'content-type': 'text' }, + hasAuth: true, }, } as WebhookActionConnector; const wrapper = mountWithIntl( diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.tsx index e4f5ef023a529..15d4c6c30450e 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.tsx @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import React, { Fragment, useState } from 'react'; +import React, { Fragment, useEffect, useState } from 'react'; import { FormattedMessage } from '@kbn/i18n/react'; import { @@ -34,12 +34,19 @@ const WebhookActionConnectorFields: React.FunctionComponent> = ({ action, editActionConfig, editActionSecrets, errors, readOnly }) => { const { user, password } = action.secrets; - const { method, url, headers } = action.config; + const { method, url, headers, hasAuth } = action.config; const [httpHeaderKey, setHttpHeaderKey] = useState(''); const [httpHeaderValue, setHttpHeaderValue] = useState(''); const [hasHeaders, setHasHeaders] = useState(false); + useEffect(() => { + if (!action.id) { + editActionConfig('hasAuth', true); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + if (!method) { editActionConfig('method', 'post'); // set method to POST by default } @@ -268,9 +275,9 @@ const WebhookActionConnectorFields: React.FunctionComponent
- +

-
-
- - - - {getEncryptedFieldNotifyLabel(!action.id)} - - - - - - 0 && user !== undefined} + + - 0 && user !== undefined} - name="user" - readOnly={readOnly} - value={user || ''} - data-test-subj="webhookUserInput" - onChange={(e) => { - editActionSecrets('user', e.target.value); - }} - onBlur={() => { - if (!user) { - editActionSecrets('user', ''); - } - }} - /> - - - - 0 && password !== undefined} - label={i18n.translate( - 'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.passwordTextFieldLabel', - { - defaultMessage: 'Password', + disabled={readOnly} + checked={hasAuth} + onChange={(e) => { + editActionConfig('hasAuth', e.target.checked); + if (!e.target.checked) { + editActionSecrets('user', null); + editActionSecrets('password', null); } - )} - > - 0 && password !== undefined} - value={password || ''} - data-test-subj="webhookPasswordInput" - onChange={(e) => { - editActionSecrets('password', e.target.value); - }} - onBlur={() => { - if (!password) { - editActionSecrets('password', ''); - } - }} - /> - + }} + /> - + {hasAuth ? ( + <> + {getEncryptedFieldNotifyLabel(!action.id)} + + + 0 && user !== undefined} + label={i18n.translate( + 'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.userTextFieldLabel', + { + defaultMessage: 'Username', + } + )} + > + 0 && user !== undefined} + name="user" + readOnly={readOnly} + value={user || ''} + data-test-subj="webhookUserInput" + onChange={(e) => { + editActionSecrets('user', e.target.value); + }} + onBlur={() => { + if (!user) { + editActionSecrets('user', ''); + } + }} + /> + + + + 0 && password !== undefined} + label={i18n.translate( + 'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.passwordTextFieldLabel', + { + defaultMessage: 'Password', + } + )} + > + 0 && password !== undefined} + value={password || ''} + data-test-subj="webhookPasswordInput" + onChange={(e) => { + editActionSecrets('password', e.target.value); + }} + onBlur={() => { + if (!password) { + editActionSecrets('password', ''); + } + }} + /> + + + + + ) : null} - - + + + + + + + ); } return ( - + + + + + ); } diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts index ef14dd9ec2eff..64d9711730c7b 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts @@ -20,6 +20,7 @@ import { const defaultValues: Record = { headers: null, method: 'post', + hasAuth: true, }; function parsePort(url: Record): Record { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts index 5992bb54c81fd..d46d60905da1c 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts @@ -55,5 +55,22 @@ export default function createGetTests({ getService }: FtrProviderContext) { projectKey: 'CK', }); }); + + it('7.11.0 migrates webhook connector configurations to have `hasAuth` property', async () => { + const responseWithAuth = await supertest.get( + `${getUrlPrefix(``)}/api/actions/action/949f909b-20a0-46e3-aadb-6a4d117bb592` + ); + + expect(responseWithAuth.status).to.eql(200); + expect(responseWithAuth.body.config).key('hasAuth'); + expect(responseWithAuth.body.config.hasAuth).to.eql(true); + + const responseNoAuth = await supertest.get( + `${getUrlPrefix(``)}/api/actions/action/7434121e-045a-47d6-a0a6-0b6da752397a` + ); + expect(responseNoAuth.status).to.eql(200); + expect(responseNoAuth.body.config).key('hasAuth'); + expect(responseNoAuth.body.config.hasAuth).to.eql(false); + }); }); } diff --git a/x-pack/test/functional/es_archives/actions/data.json b/x-pack/test/functional/es_archives/actions/data.json index aeeca87deb9ff..18d67da1752bc 100644 --- a/x-pack/test/functional/es_archives/actions/data.json +++ b/x-pack/test/functional/es_archives/actions/data.json @@ -56,3 +56,57 @@ "type": "_doc" } } + +{ + "type": "doc", + "value": { + "id": "action:949f909b-20a0-46e3-aadb-6a4d117bb592", + "index": ".kibana_1", + "source": { + "action": { + "actionTypeId": ".webhook", + "config": { + "headers": null, + "method": "post", + "url": "http://localhost" + }, + "name": "A webhook with auth", + "secrets": "LUqlrITACjqPmcWGlbl+H4RsGGOlw8LM0Urq8r7y6jNT7Igv3J7FjKJ2NXfNTaghVBO7e9x3wZOtiycwyoAdviTyYm1pspni24vH+OT70xaSuXcDoxfGwiLEcaG04INDnUJX4dtmRerxqR9ChktC70LNtOU3sqjYI2tWt2vOqGeq" + }, + "migrationVersion": { + "action": "7.10.0" + }, + "references": [ + ], + "type": "action", + "updated_at": "2020-10-26T21:29:47.380Z" + } + } +} + +{ + "type": "doc", + "value": { + "id": "action:7434121e-045a-47d6-a0a6-0b6da752397a", + "index": ".kibana_1", + "source": { + "action": { + "actionTypeId": ".webhook", + "config": { + "headers": null, + "method": "post", + "url": "http://localhost" + }, + "name": "A webhook with no auth", + "secrets": "tOwFq20hbUrcp3FX7stKB5aJaQQdLNQwomSNym8BgnFaBBafPOASv5T0tGdGsTr/CA7VK+N/wYBHQPzt0apF8Z/UYl63ZXqck5tSoFDnQW77zv1VVQ5wEwN1qkAQQcfrXTXU2wYVAYZNSuHkbeRjcasfG0ty1K+J7A==" + }, + "migrationVersion": { + "action": "7.10.0" + }, + "references": [ + ], + "type": "action", + "updated_at": "2020-10-26T21:30:35.146Z" + } + } +} From 59af44bf174d4c00e6f3713e899c254359efbc9b Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Tue, 27 Oct 2020 12:11:22 -0600 Subject: [PATCH 8/9] [Security Solution] critical pref bug with browser fields reducer ## Summary How to test this performance issue? Ensure that you add extra mappings of mapping-test-1k-block-1, 2, 3,4, etc.. that I have included within your advanced settings like below to a Kibana space: ```ts apm-*-transaction*, auditbeat-*, endgame-*, filebeat-*, logs-*, packetbeat-*, winlogbeat-*, mapping-test-1k-block-1, mapping-test-1k-block-2, mapping-test-1k-block-3, mapping-test-1k-block-4 ``` Screen Shot 2020-10-26 at 11 35 12 AM Modify these lines to add manual perf lines like so for the before numbers: file: x-pack/plugins/security_solution/public/common/containers/source/index.tsx ```ts export const getBrowserFields = memoizeOne( (_title: string, fields: IndexField[]): BrowserFields => { console.time('getBrowserFields'); // Start time const value = fields && fields.length > 0 ? fields.reduce( (accumulator: BrowserFields, field: IndexField) => set([field.category, 'fields', field.name], field, accumulator), {} ) : {}; console.timeEnd('getBrowserFields'); // End time return value; }, // Update the value only if _title has changed (newArgs, lastArgs) => newArgs[0] === lastArgs[0] ); ``` Then load any of the SIEM webpages or create a detection engine rule and change around your input indexes. Perf of this before the fix is going to show up in your dev tools console output like this: ```ts getBrowserFields: 7875.5009765625 ms ``` Now, checkout this branch and change the code to be like so for the perf test of the mutatious version for after: file: x-pack/plugins/security_solution/public/common/containers/source/index.tsx ```ts /** * HOT Code path where the fields can be 16087 in length or larger. This is * VERY mutatious on purpose to improve the performance of the transform. */ export const getBrowserFields = memoizeOne( (_title: string, fields: IndexField[]): BrowserFields => { console.time('getBrowserFields'); // Start time // Adds two dangerous casts to allow for mutations within this function type DangerCastForMutation = Record; type DangerCastForBrowserFieldsMutation = Record< string, Omit & { fields: Record } >; // We mutate this instead of using lodash/set to keep this as fast as possible const value = fields.reduce((accumulator, field) => { if (accumulator[field.category] == null) { (accumulator as DangerCastForMutation)[field.category] = {}; } if (accumulator[field.category].fields == null) { accumulator[field.category].fields = {}; } if (accumulator[field.category].fields[field.name] == null) { (accumulator[field.category].fields as DangerCastForMutation)[field.name] = {}; } accumulator[field.category].fields[field.name] = (field as unknown) as BrowserField; return accumulator; }, {}); console.timeEnd('getBrowserFields'); // End time return value; }, // Update the value only if _title has changed (newArgs, lastArgs) => newArgs[0] === lastArgs[0] ); ``` Re-load any and all pages and you should see this now: ```ts getBrowserFields: 11.258056640625 ms ``` ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../common/containers/source/index.test.tsx | 30 +++++++++++++++++ .../public/common/containers/source/index.tsx | 33 ++++++++++++++----- 2 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/common/containers/source/index.test.tsx diff --git a/x-pack/plugins/security_solution/public/common/containers/source/index.test.tsx b/x-pack/plugins/security_solution/public/common/containers/source/index.test.tsx new file mode 100644 index 0000000000000..e81b52f281519 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/containers/source/index.test.tsx @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { IndexField } from '../../../../common/search_strategy/index_fields'; +import { getBrowserFields } from '.'; +import { mockBrowserFields, mocksSource } from './mock'; + +describe('source/index.tsx', () => { + describe('getBrowserFields', () => { + test('it returns an empty object given an empty array', () => { + const fields = getBrowserFields('title 1', []); + expect(fields).toEqual({}); + }); + + test('it returns the same input with the same title', () => { + getBrowserFields('title 1', []); + // Since it is memoized it will return the same output which is empty object given 'title 1' a second time + const fields = getBrowserFields('title 1', mocksSource.indexFields as IndexField[]); + expect(fields).toEqual({}); + }); + + test('it transforms input into output as expected', () => { + const fields = getBrowserFields('title 2', mocksSource.indexFields as IndexField[]); + expect(fields).toEqual(mockBrowserFields); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/common/containers/source/index.tsx b/x-pack/plugins/security_solution/public/common/containers/source/index.tsx index 2cc1c75015e07..47e550b2ced0f 100644 --- a/x-pack/plugins/security_solution/public/common/containers/source/index.tsx +++ b/x-pack/plugins/security_solution/public/common/containers/source/index.tsx @@ -4,7 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -import { set } from '@elastic/safer-lodash-set/fp'; import { keyBy, pick, isEmpty, isEqual, isUndefined } from 'lodash/fp'; import memoizeOne from 'memoize-one'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; @@ -55,15 +54,31 @@ export const getIndexFields = memoizeOne( (newArgs, lastArgs) => newArgs[0] === lastArgs[0] && newArgs[1].length === lastArgs[1].length ); +/** + * HOT Code path where the fields can be 16087 in length or larger. This is + * VERY mutatious on purpose to improve the performance of the transform. + */ export const getBrowserFields = memoizeOne( - (_title: string, fields: IndexField[]): BrowserFields => - fields && fields.length > 0 - ? fields.reduce( - (accumulator: BrowserFields, field: IndexField) => - set([field.category, 'fields', field.name], field, accumulator), - {} - ) - : {}, + (_title: string, fields: IndexField[]): BrowserFields => { + // Adds two dangerous casts to allow for mutations within this function + type DangerCastForMutation = Record; + type DangerCastForBrowserFieldsMutation = Record< + string, + Omit & { fields: Record } + >; + + // We mutate this instead of using lodash/set to keep this as fast as possible + return fields.reduce((accumulator, field) => { + if (accumulator[field.category] == null) { + (accumulator as DangerCastForMutation)[field.category] = {}; + } + if (accumulator[field.category].fields == null) { + accumulator[field.category].fields = {}; + } + accumulator[field.category].fields[field.name] = (field as unknown) as BrowserField; + return accumulator; + }, {}); + }, // Update the value only if _title has changed (newArgs, lastArgs) => newArgs[0] === lastArgs[0] ); From 1066602ab7c4d52139c6156601f182da653dd80c Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Tue, 27 Oct 2020 12:28:33 -0600 Subject: [PATCH 9/9] [Security Solutions][Detection Engine] Changes wording for threat matches and rules (#81334) ## Summary Changes the wording for threat matches and rules cc @marrasherrier @MikePaquette @paulewing Before: Screen Shot 2020-10-21 at 8 52 44 AM After: Screen Shot 2020-10-26 at 10 10 17 PM ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md) --- .../public/common/components/threat_match/translations.ts | 2 +- .../components/rules/description_step/helpers.test.tsx | 2 +- .../components/rules/description_step/translations.tsx | 2 +- .../components/rules/select_rule_type/translations.ts | 5 +++-- .../detections/components/rules/step_define_rule/schema.tsx | 6 +++--- .../scripts/rules/queries/query_with_threat_mapping.json | 2 +- 6 files changed, 10 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/threat_match/translations.ts b/x-pack/plugins/security_solution/public/common/components/threat_match/translations.ts index ca9f6a13856cf..57e7416731486 100644 --- a/x-pack/plugins/security_solution/public/common/components/threat_match/translations.ts +++ b/x-pack/plugins/security_solution/public/common/components/threat_match/translations.ts @@ -13,7 +13,7 @@ export const FIELD = i18n.translate('xpack.securitySolution.threatMatch.fieldDes export const THREAT_FIELD = i18n.translate( 'xpack.securitySolution.threatMatch.threatFieldDescription', { - defaultMessage: 'Threat index field', + defaultMessage: 'Indicator index field', } ); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/helpers.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/helpers.test.tsx index ebdfdcc262b34..ee1edecbdc54a 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/helpers.test.tsx @@ -437,7 +437,7 @@ describe('helpers', () => { it('returns a humanized description for a threat_match type', () => { const [result]: ListItems[] = buildRuleTypeDescription('Test label', 'threat_match'); - expect(result.description).toEqual('Threat Match'); + expect(result.description).toEqual('Indicator Match'); }); }); }); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/translations.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/translations.tsx index d9186c2da7225..04647871f212e 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/translations.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/translations.tsx @@ -65,7 +65,7 @@ export const THRESHOLD_TYPE_DESCRIPTION = i18n.translate( export const THREAT_MATCH_TYPE_DESCRIPTION = i18n.translate( 'xpack.securitySolution.detectionEngine.createRule.threatMatchRuleTypeDescription', { - defaultMessage: 'Threat Match', + defaultMessage: 'Indicator Match', } ); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/translations.ts b/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/translations.ts index 7043aa2d2f956..b9c229fe78f10 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/translations.ts +++ b/x-pack/plugins/security_solution/public/detections/components/rules/select_rule_type/translations.ts @@ -66,13 +66,14 @@ export const THRESHOLD_TYPE_DESCRIPTION = i18n.translate( export const THREAT_MATCH_TYPE_TITLE = i18n.translate( 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.ruleTypeField.threatMatchTitle', { - defaultMessage: 'Threat Match', + defaultMessage: 'Indicator Match', } ); export const THREAT_MATCH_TYPE_DESCRIPTION = i18n.translate( 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.ruleTypeField.threatMatchDescription', { - defaultMessage: 'Upload value lists to write rules around a list of known bad attributes', + defaultMessage: + 'Use indicators from intelligence sources to detect matching events and alerts.', } ); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/schema.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/schema.tsx index ebffb1abf4787..9763125776be2 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/schema.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/schema.tsx @@ -235,7 +235,7 @@ export const schema: FormSchema = { label: i18n.translate( 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldThreatIndexPatternsLabel', { - defaultMessage: 'Threat index patterns', + defaultMessage: 'Indicator Index Patterns', } ), helpText: {THREAT_MATCH_INDEX_HELPER_TEXT}, @@ -265,7 +265,7 @@ export const schema: FormSchema = { label: i18n.translate( 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldThreatMappingLabel', { - defaultMessage: 'Threat Mapping', + defaultMessage: 'Indicator Mapping', } ), validations: [ @@ -301,7 +301,7 @@ export const schema: FormSchema = { label: i18n.translate( 'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.fieldThreatQueryBarLabel', { - defaultMessage: 'Threat index query', + defaultMessage: 'Indicator Index Query', } ), validations: [ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/queries/query_with_threat_mapping.json b/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/queries/query_with_threat_mapping.json index 1e2f217751e96..ed9356f46501c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/queries/query_with_threat_mapping.json +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/queries/query_with_threat_mapping.json @@ -1,5 +1,5 @@ { - "name": "Query with a threat mapping", + "name": "Query with a indicator mapping", "description": "Query with a threat mapping", "rule_id": "threat-mapping", "risk_score": 1,