From e0cc7f46ade967a1a55cd53835a2c5a7bcf2a248 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 12 Aug 2020 12:04:57 -0400 Subject: [PATCH 1/8] working on query string tests --- .../test_utilities/simulator/index.tsx | 5 ++ .../public/resolver/view/query_params.test.ts | 54 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 x-pack/plugins/security_solution/public/resolver/view/query_params.test.ts 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 cae6a18576ebd4..24e7aa9a5914f5 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 @@ -191,8 +191,13 @@ export class Simulator { return this.domNodes('[data-test-subj="resolver:map:node-submenu-item"]'); } + public get historyLocationSearch() { + return this.history.location.search; + } + /** * Return the selected node query string values. + * @deprecated */ public queryStringValues(): { selectedNode: string[] } { const urlSearchParams = new URLSearchParams(this.history.location.search); diff --git a/x-pack/plugins/security_solution/public/resolver/view/query_params.test.ts b/x-pack/plugins/security_solution/public/resolver/view/query_params.test.ts new file mode 100644 index 00000000000000..4cf4fd6097b169 --- /dev/null +++ b/x-pack/plugins/security_solution/public/resolver/view/query_params.test.ts @@ -0,0 +1,54 @@ +/* + * 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 { noAncestorsTwoChildren } from '../data_access_layer/mocks/no_ancestors_two_children'; +import { Simulator } from '../test_utilities/simulator'; +// Extend jest with a custom matcher +import '../test_utilities/extend_jest'; +import { urlSearch } from '../test_utilities/url_search'; + +let simulator: Simulator; +let databaseDocumentID: string; +let entityIDs: { origin: string; firstChild: string; secondChild: string }; + +// the resolver component instance ID, used by the react code to distinguish piece of global state from those used by other resolver instances +const resolverComponentInstanceID = 'resolverComponentInstanceID'; + +describe('Resolver, when analyzing a tree that has no ancestors and 2 children', () => { + beforeEach(async () => { + // create a mock data access layer + const { metadata: dataAccessLayerMetadata, dataAccessLayer } = noAncestorsTwoChildren(); + + // save a reference to the entity IDs exposed by the mock data layer + entityIDs = dataAccessLayerMetadata.entityIDs; + + // save a reference to the `_id` supported by the mock data layer + databaseDocumentID = dataAccessLayerMetadata.databaseDocumentID; + + // create a resolver simulator, using the data access layer and an arbitrary component instance ID + simulator = new Simulator({ databaseDocumentID, dataAccessLayer, resolverComponentInstanceID }); + }); + + describe("when the second child node's first button has been clicked", () => { + beforeEach(async () => { + const node = await simulator.resolveWrapper(() => + simulator.processNodeElements({ entityID: entityIDs.secondChild }).find('button') + ); + if (node) { + // Click the first button under the second child element. + node.first().simulate('click'); + } + }); + const expectedSearch = urlSearch(resolverComponentInstanceID, { + selectedEntityID: 'secondChild', + }); + it(`should have a url search of ${expectedSearch}`, () => { + expect(simulator.map(() => simulator.historyLocationSearch)).toYieldEqualTo( + urlSearch(resolverComponentInstanceID, { selectedEntityID: 'secondChild' }) + ); + }); + }); +}); From 475255292aa9b3dacea6793dd3063e63b5ad0765 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 13 Aug 2020 13:35:53 -0400 Subject: [PATCH 2/8] doit --- .../test_utilities/simulator/index.tsx | 43 +++++++++++-------- .../resolver/view/clickthrough.test.tsx | 10 ++--- .../public/resolver/view/panel.test.tsx | 8 ++-- .../public/resolver/view/query_params.test.ts | 41 ++++++++++++++++-- .../view/resolver_without_providers.tsx | 8 +--- .../view/use_resolver_query_params.ts | 43 ++++++++++--------- 6 files changed, 98 insertions(+), 55 deletions(-) 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 24e7aa9a5914f5..f341b5792e2de7 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 @@ -21,10 +21,6 @@ import { ResolverAction } from '../../store/actions'; * Test a Resolver instance using jest, enzyme, and a mock data layer. */ export class Simulator { - /** - * A string that uniquely identifies this Resolver instance among others mounted in the DOM. - */ - private readonly resolverComponentInstanceID: string; /** * The redux store, creating in the constructor using the `dataAccessLayer`. * This code subscribes to state transitions. @@ -63,7 +59,6 @@ export class Simulator { databaseDocumentID?: string; history?: HistoryPackageHistoryInterface; }) { - this.resolverComponentInstanceID = resolverComponentInstanceID; // create the spy middleware (for debugging tests) this.spyMiddleware = spyMiddlewareFactory(); @@ -90,7 +85,7 @@ export class Simulator { // Render Resolver via the `MockResolver` component, using `enzyme`. this.wrapper = mount( ({ // the query string has a key showing that the second child is selected - queryStringSelectedNode: simulator.queryStringValues().selectedNode, + search: simulator.historyLocationSearch, // the second child is rendered in the DOM, and shows up as selected selectedSecondChildNodeCount: simulator.selectedProcessNode(entityIDs.secondChild) .length, @@ -98,7 +99,9 @@ describe('Resolver, when analyzing a tree that has no ancestors and 2 children', })) ).toYieldEqualTo({ // Just the second child should be marked as selected in the query string - queryStringSelectedNode: [entityIDs.secondChild], + search: urlSearch(resolverComponentInstanceID, { + selectedEntityID: entityIDs.secondChild, + }), // The second child is rendered and has `[aria-selected]` selectedSecondChildNodeCount: 1, // The origin child is rendered and doesn't have `[aria-selected]` @@ -166,9 +169,6 @@ describe('Resolver, when analyzing a tree that has two related events for the or await expect( simulator.map(() => simulator.processNodeSubmenuItems().map((node) => node.text())) ).toYieldEqualTo(['2 registry']); - await expect( - simulator.map(() => simulator.processNodeSubmenuItems().length) - ).toYieldEqualTo(1); }); }); describe('and when the related events button is clicked again', () => { 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 4d391a6c9ce59c..e42c179e7e7676 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 @@ -152,9 +152,11 @@ describe(`Resolver: when analyzing a tree with no ancestors and two children, an ]); }); it("should have the first node's ID in the query string", async () => { - await expect(simulator().map(() => simulator().queryStringValues())).toYieldEqualTo({ - selectedNode: [entityIDs.origin], - }); + await expect(simulator().map(() => simulator().historyLocationSearch)).toYieldEqualTo( + urlSearch(resolverComponentInstanceID, { + selectedEntityID: entityIDs.origin, + }) + ); }); describe('and when the node list link has been clicked', () => { beforeEach(async () => { diff --git a/x-pack/plugins/security_solution/public/resolver/view/query_params.test.ts b/x-pack/plugins/security_solution/public/resolver/view/query_params.test.ts index 4cf4fd6097b169..26c25cfab2c215 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/query_params.test.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/query_params.test.ts @@ -15,7 +15,7 @@ let databaseDocumentID: string; let entityIDs: { origin: string; firstChild: string; secondChild: string }; // the resolver component instance ID, used by the react code to distinguish piece of global state from those used by other resolver instances -const resolverComponentInstanceID = 'resolverComponentInstanceID'; +const resolverComponentInstanceID = 'oldID'; describe('Resolver, when analyzing a tree that has no ancestors and 2 children', () => { beforeEach(async () => { @@ -45,10 +45,45 @@ describe('Resolver, when analyzing a tree that has no ancestors and 2 children', const expectedSearch = urlSearch(resolverComponentInstanceID, { selectedEntityID: 'secondChild', }); - it(`should have a url search of ${expectedSearch}`, () => { - expect(simulator.map(() => simulator.historyLocationSearch)).toYieldEqualTo( + it(`should have a url search of ${expectedSearch}`, async () => { + await expect(simulator.map(() => simulator.historyLocationSearch)).toYieldEqualTo( urlSearch(resolverComponentInstanceID, { selectedEntityID: 'secondChild' }) ); }); + describe('when the resolver component gets unmounted', () => { + beforeEach(() => { + simulator.unmount(); + }); + it('should have a history location search of `""`', async () => { + await expect(simulator.map(() => simulator.historyLocationSearch)).toYieldEqualTo(''); + }); + }); + describe('when the resolver component has its component instance ID changed', () => { + const newInstanceID = 'newID'; + beforeEach(() => { + simulator.resolverComponentInstanceID = newInstanceID; + }); + it('should have a history location search of `""`', async () => { + await expect(simulator.map(() => simulator.historyLocationSearch)).toYieldEqualTo(''); + }); + describe("when the user clicks the second child node's button again", () => { + beforeEach(async () => { + const node = await simulator.resolveWrapper(() => + simulator.processNodeElements({ entityID: entityIDs.secondChild }).find('button') + ); + if (node) { + // Click the first button under the second child element. + node.first().simulate('click'); + } + }); + it(`should have a url search of ${urlSearch(newInstanceID, { + selectedEntityID: 'secondChild', + })}`, async () => { + await expect(simulator.map(() => simulator.historyLocationSearch)).toYieldEqualTo( + urlSearch(newInstanceID, { selectedEntityID: 'secondChild' }) + ); + }); + }); + }); }); }); diff --git a/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx b/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx index 5f1e5f18e575dd..4bfe5cd37c2408 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx @@ -6,9 +6,8 @@ /* eslint-disable react/display-name */ -import React, { useContext, useCallback } from 'react'; +import React, { useContext, useCallback, useEffect } from 'react'; import { useSelector } from 'react-redux'; -import { useEffectOnce } from 'react-use'; import { EuiLoadingSpinner } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; import * as selectors from '../store/selectors'; @@ -70,11 +69,6 @@ export const ResolverWithoutProviders = React.memo( const hasError = useSelector(selectors.hasError); const activeDescendantId = useSelector(selectors.ariaActiveDescendant); const { colorMap } = useResolverTheme(); - const { cleanUpQueryParams } = useResolverQueryParams(); - - useEffectOnce(() => { - return () => cleanUpQueryParams(); - }); return ( diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts index ed514a61d4e068..931ce2923c1988 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { useCallback, useMemo } from 'react'; +import { useCallback, useMemo, useEffect } from 'react'; // eslint-disable-next-line import/no-nodejs-modules import querystring from 'querystring'; import { useSelector } from 'react-redux'; @@ -20,24 +20,24 @@ export function useResolverQueryParams() { const history = useHistory(); const urlSearch = useLocation().search; const resolverComponentInstanceID = useSelector(selectors.resolverComponentInstanceID); - const uniqueCrumbIdKey: string = `resolver-${resolverComponentInstanceID}-id`; - const uniqueCrumbEventKey: string = `resolver-${resolverComponentInstanceID}-event`; + const idKey: string = `resolver-${resolverComponentInstanceID}-id`; + const eventKey: string = `resolver-${resolverComponentInstanceID}-event`; const pushToQueryParams = useCallback( (newCrumbs: CrumbInfo) => { // Construct a new set of parameters from the current set (minus empty parameters) // by assigning the new set of parameters provided in `newCrumbs` const crumbsToPass = { ...querystring.parse(urlSearch.slice(1)), - [uniqueCrumbIdKey]: newCrumbs.crumbId, - [uniqueCrumbEventKey]: newCrumbs.crumbEvent, + [idKey]: newCrumbs.crumbId, + [eventKey]: newCrumbs.crumbEvent, }; // If either was passed in as empty, remove it from the record if (newCrumbs.crumbId === '') { - delete crumbsToPass[uniqueCrumbIdKey]; + delete crumbsToPass[idKey]; } if (newCrumbs.crumbEvent === '') { - delete crumbsToPass[uniqueCrumbEventKey]; + delete crumbsToPass[eventKey]; } const relativeURL = { search: querystring.stringify(crumbsToPass) }; @@ -45,12 +45,12 @@ export function useResolverQueryParams() { // trail of these, thus `.replace` instead of `.push` return history.replace(relativeURL); }, - [history, urlSearch, uniqueCrumbIdKey, uniqueCrumbEventKey] + [history, urlSearch, idKey, eventKey] ); const queryParams: CrumbInfo = useMemo(() => { const parsed = querystring.parse(urlSearch.slice(1)); - const crumbEvent = parsed[uniqueCrumbEventKey]; - const crumbId = parsed[uniqueCrumbIdKey]; + const crumbEvent = parsed[eventKey]; + const crumbId = parsed[idKey]; function valueForParam(param: string | string[]): string { if (Array.isArray(param)) { return param[0] || ''; @@ -61,21 +61,24 @@ export function useResolverQueryParams() { crumbEvent: valueForParam(crumbEvent), crumbId: valueForParam(crumbId), }; - }, [urlSearch, uniqueCrumbIdKey, uniqueCrumbEventKey]); + }, [urlSearch, idKey, eventKey]); - const cleanUpQueryParams = () => { - const crumbsToPass = { - ...querystring.parse(urlSearch.slice(1)), + useEffect(() => { + const oldIdKey = idKey; + const oldEventKey = eventKey; + return () => { + const crumbsToPass = { + ...querystring.parse(history.location.search.slice(1)), + }; + delete crumbsToPass[oldIdKey]; + delete crumbsToPass[oldEventKey]; + const relativeURL = { search: querystring.stringify(crumbsToPass) }; + history.replace(relativeURL); }; - delete crumbsToPass[uniqueCrumbIdKey]; - delete crumbsToPass[uniqueCrumbEventKey]; - const relativeURL = { search: querystring.stringify(crumbsToPass) }; - history.replace(relativeURL); - }; + }, [idKey, eventKey, history]); return { pushToQueryParams, queryParams, - cleanUpQueryParams, }; } From a675bb0bbd08f3215baac89fbb9ae9ec621eadec Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 13 Aug 2020 13:48:01 -0400 Subject: [PATCH 3/8] cleanup the url handling code --- .../view/use_resolver_query_params.ts | 67 ++++++++++--------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts index 931ce2923c1988..6c804c465bf077 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts @@ -5,8 +5,6 @@ */ import { useCallback, useMemo, useEffect } from 'react'; -// eslint-disable-next-line import/no-nodejs-modules -import querystring from 'querystring'; import { useSelector } from 'react-redux'; import { useHistory, useLocation } from 'react-router-dom'; import * as selectors from '../store/selectors'; @@ -23,24 +21,21 @@ export function useResolverQueryParams() { const idKey: string = `resolver-${resolverComponentInstanceID}-id`; const eventKey: string = `resolver-${resolverComponentInstanceID}-event`; const pushToQueryParams = useCallback( - (newCrumbs: CrumbInfo) => { - // Construct a new set of parameters from the current set (minus empty parameters) - // by assigning the new set of parameters provided in `newCrumbs` - const crumbsToPass = { - ...querystring.parse(urlSearch.slice(1)), - [idKey]: newCrumbs.crumbId, - [eventKey]: newCrumbs.crumbEvent, - }; + (queryStringState: CrumbInfo) => { + const urlSearchParams = new URLSearchParams(urlSearch); - // If either was passed in as empty, remove it from the record - if (newCrumbs.crumbId === '') { - delete crumbsToPass[idKey]; + urlSearchParams.set(idKey, queryStringState.crumbId); + urlSearchParams.set(eventKey, queryStringState.crumbEvent); + + // If either was passed in as empty, remove it + if (queryStringState.crumbId === '') { + urlSearchParams.delete(idKey); } - if (newCrumbs.crumbEvent === '') { - delete crumbsToPass[eventKey]; + if (queryStringState.crumbEvent === '') { + urlSearchParams.delete(eventKey); } - const relativeURL = { search: querystring.stringify(crumbsToPass) }; + const relativeURL = { search: urlSearchParams.toString() }; // We probably don't want to nuke the user's history with a huge // trail of these, thus `.replace` instead of `.push` return history.replace(relativeURL); @@ -48,31 +43,37 @@ export function useResolverQueryParams() { [history, urlSearch, idKey, eventKey] ); const queryParams: CrumbInfo = useMemo(() => { - const parsed = querystring.parse(urlSearch.slice(1)); - const crumbEvent = parsed[eventKey]; - const crumbId = parsed[idKey]; - function valueForParam(param: string | string[]): string { - if (Array.isArray(param)) { - return param[0] || ''; - } - return param || ''; - } + const urlSearchParams = new URLSearchParams(urlSearch); return { - crumbEvent: valueForParam(crumbEvent), - crumbId: valueForParam(crumbId), + // Use `''` for backwards compatibility with deprecated code. + crumbEvent: urlSearchParams.get(eventKey) ?? '', + crumbId: urlSearchParams.get(idKey) ?? '', }; }, [urlSearch, idKey, eventKey]); useEffect(() => { + /** + * Keep track of the old query string keys so we can remove them. + */ const oldIdKey = idKey; const oldEventKey = eventKey; + /** + * When `idKey` or `eventKey` changes (such as when the `resolverComponentInstanceID` has changed) or when the component unmounts, remove any state from the query string. + */ return () => { - const crumbsToPass = { - ...querystring.parse(history.location.search.slice(1)), - }; - delete crumbsToPass[oldIdKey]; - delete crumbsToPass[oldEventKey]; - const relativeURL = { search: querystring.stringify(crumbsToPass) }; + /** + * This effect must not be invalidated when `search` changes. + * Use the current location.search via the `history` object. + * `history` doesn't change so this is effectively like accessing `search` via a ref. + */ + const urlSearchParams = new URLSearchParams(history.location.search); + + /** + * Remove old keys from the url + */ + urlSearchParams.delete(oldIdKey); + urlSearchParams.delete(oldEventKey); + const relativeURL = { search: urlSearchParams.toString() }; history.replace(relativeURL); }; }, [idKey, eventKey, history]); From 6e1511e22b48733a29f08a2956bf46467d065206 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 13 Aug 2020 14:24:41 -0400 Subject: [PATCH 4/8] remove unused stuff --- .../public/resolver/view/resolver_without_providers.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx b/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx index 4bfe5cd37c2408..caa1365044407e 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx @@ -6,7 +6,7 @@ /* eslint-disable react/display-name */ -import React, { useContext, useCallback, useEffect } from 'react'; +import React, { useContext, useCallback } from 'react'; import { useSelector } from 'react-redux'; import { EuiLoadingSpinner } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; @@ -17,7 +17,6 @@ import { ProcessEventDot } from './process_event_dot'; import { useCamera } from './use_camera'; import { SymbolDefinitions, useResolverTheme } from './assets'; import { useStateSyncingActions } from './use_state_syncing_actions'; -import { useResolverQueryParams } from './use_resolver_query_params'; import { StyledMapContainer, StyledPanel, GraphContainer } from './styles'; import { entityIDSafeVersion } from '../../../common/endpoint/models/event'; import { SideEffectContext } from './side_effect_context'; From 7f1f45392911a15909677015868a04b4e7ce2942 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 13 Aug 2020 14:38:04 -0400 Subject: [PATCH 5/8] refactor --- .../view/resolver_without_providers.tsx | 3 + .../resolver/view/use_query_string_keys.ts | 21 +++++++ .../view/use_resolver_query_params.ts | 6 +- .../view/use_resolver_query_params_cleaner.ts | 55 +++++++++++++++++++ 4 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/resolver/view/use_query_string_keys.ts create mode 100644 x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params_cleaner.ts diff --git a/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx b/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx index caa1365044407e..59564b08628ee7 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx @@ -10,6 +10,7 @@ import React, { useContext, useCallback } from 'react'; import { useSelector } from 'react-redux'; import { EuiLoadingSpinner } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; +import { useResolverQueryParamCleaner } from './use_resolver_query_params_cleaner'; import * as selectors from '../store/selectors'; import { EdgeLine } from './edge_line'; import { GraphControls } from './graph_controls'; @@ -33,6 +34,8 @@ export const ResolverWithoutProviders = React.memo( { className, databaseDocumentID, resolverComponentInstanceID }: ResolverProps, refToForward ) { + // + useResolverQueryParamCleaner(); /** * This is responsible for dispatching actions that include any external data. * `databaseDocumentID` diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_query_string_keys.ts b/x-pack/plugins/security_solution/public/resolver/view/use_query_string_keys.ts new file mode 100644 index 00000000000000..11f1a30db72fcf --- /dev/null +++ b/x-pack/plugins/security_solution/public/resolver/view/use_query_string_keys.ts @@ -0,0 +1,21 @@ +/* + * 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 { useSelector } from 'react-redux'; +import * as selectors from '../store/selectors'; + +/** + * Get the query string keys used by this Resolver instance. + */ +export function useQueryStringKeys(): { idKey: string; eventKey: string } { + const resolverComponentInstanceID = useSelector(selectors.resolverComponentInstanceID); + const idKey: string = `resolver-${resolverComponentInstanceID}-id`; + const eventKey: string = `resolver-${resolverComponentInstanceID}-event`; + return { + idKey, + eventKey, + }; +} diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts index 6c804c465bf077..3c76b8015aa301 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts @@ -5,8 +5,8 @@ */ import { useCallback, useMemo, useEffect } from 'react'; -import { useSelector } from 'react-redux'; import { useHistory, useLocation } from 'react-router-dom'; +import { useQueryStringKeys } from './use_query_string_keys'; import * as selectors from '../store/selectors'; import { CrumbInfo } from './panels/panel_content_utilities'; @@ -17,9 +17,7 @@ export function useResolverQueryParams() { */ const history = useHistory(); const urlSearch = useLocation().search; - const resolverComponentInstanceID = useSelector(selectors.resolverComponentInstanceID); - const idKey: string = `resolver-${resolverComponentInstanceID}-id`; - const eventKey: string = `resolver-${resolverComponentInstanceID}-event`; + const { idKey, eventKey } = useQueryStringKeys(); const pushToQueryParams = useCallback( (queryStringState: CrumbInfo) => { const urlSearchParams = new URLSearchParams(urlSearch); diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params_cleaner.ts b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params_cleaner.ts new file mode 100644 index 00000000000000..f88df7707848a0 --- /dev/null +++ b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params_cleaner.ts @@ -0,0 +1,55 @@ +/* + * 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 { useRef, useEffect } from 'react'; +import { useLocation, useHistory } from 'react-router-dom'; + +import { useQueryStringKeys } from './use_query_string_keys'; +/** + * Cleanup any query string keys that were added by this Resolver instance. + * This works by having a React effect that just has behavior in the 'cleanup' function. + */ +export function useResolverQueryParamCleaner() { + /** + * Keep a reference to the current search value. This is used in the cleanup function. + * This value of useLocation().search isn't used directly since that would change and + * we only want the cleanup to run on unmount or when the resolverComponentInstanceID + * changes. + */ + const searchRef = useRef(); + searchRef.current = useLocation().search; + + const history = useHistory(); + + const { idKey, eventKey } = useQueryStringKeys(); + + useEffect(() => { + /** + * Keep track of the old query string keys so we can remove them. + */ + const oldIdKey = idKey; + const oldEventKey = eventKey; + /** + * When `idKey` or `eventKey` changes (such as when the `resolverComponentInstanceID` has changed) or when the component unmounts, remove any state from the query string. + */ + return () => { + /** + * This effect must not be invalidated when `search` changes. + * Use the current location.search via the `history` object. + * `history` doesn't change so this is effectively like accessing `search` via a ref. + */ + const urlSearchParams = new URLSearchParams(searchRef.current); + + /** + * Remove old keys from the url + */ + urlSearchParams.delete(oldIdKey); + urlSearchParams.delete(oldEventKey); + const relativeURL = { search: urlSearchParams.toString() }; + history.replace(relativeURL); + }; + }, [idKey, eventKey, history]); +} From b7258ed7e0015e569ad51eb91223513abf6adaf4 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 13 Aug 2020 14:44:28 -0400 Subject: [PATCH 6/8] remove old cleanup effect --- .../view/use_resolver_query_params.ts | 30 +------------------ 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts index 3c76b8015aa301..aa0851916a7b4e 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params.ts @@ -4,10 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { useCallback, useMemo, useEffect } from 'react'; +import { useCallback, useMemo } from 'react'; import { useHistory, useLocation } from 'react-router-dom'; import { useQueryStringKeys } from './use_query_string_keys'; -import * as selectors from '../store/selectors'; import { CrumbInfo } from './panels/panel_content_utilities'; export function useResolverQueryParams() { @@ -49,33 +48,6 @@ export function useResolverQueryParams() { }; }, [urlSearch, idKey, eventKey]); - useEffect(() => { - /** - * Keep track of the old query string keys so we can remove them. - */ - const oldIdKey = idKey; - const oldEventKey = eventKey; - /** - * When `idKey` or `eventKey` changes (such as when the `resolverComponentInstanceID` has changed) or when the component unmounts, remove any state from the query string. - */ - return () => { - /** - * This effect must not be invalidated when `search` changes. - * Use the current location.search via the `history` object. - * `history` doesn't change so this is effectively like accessing `search` via a ref. - */ - const urlSearchParams = new URLSearchParams(history.location.search); - - /** - * Remove old keys from the url - */ - urlSearchParams.delete(oldIdKey); - urlSearchParams.delete(oldEventKey); - const relativeURL = { search: urlSearchParams.toString() }; - history.replace(relativeURL); - }; - }, [idKey, eventKey, history]); - return { pushToQueryParams, queryParams, From d1b3c2a83efc28ca6cef92b0ad4cb79bf8e2c62d Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 13 Aug 2020 15:47:34 -0400 Subject: [PATCH 7/8] durp --- .../public/resolver/view/resolver_without_providers.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx b/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx index 59564b08628ee7..32faeec043f2da 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx +++ b/x-pack/plugins/security_solution/public/resolver/view/resolver_without_providers.tsx @@ -34,7 +34,6 @@ export const ResolverWithoutProviders = React.memo( { className, databaseDocumentID, resolverComponentInstanceID }: ResolverProps, refToForward ) { - // useResolverQueryParamCleaner(); /** * This is responsible for dispatching actions that include any external data. From 6a2601877d87907d9e4ce087e399a815c795be1c Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 13 Aug 2020 15:49:27 -0400 Subject: [PATCH 8/8] hacked --- .../public/resolver/view/use_resolver_query_params_cleaner.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params_cleaner.ts b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params_cleaner.ts index f88df7707848a0..a84eb0490aae2f 100644 --- a/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params_cleaner.ts +++ b/x-pack/plugins/security_solution/public/resolver/view/use_resolver_query_params_cleaner.ts @@ -38,8 +38,6 @@ export function useResolverQueryParamCleaner() { return () => { /** * This effect must not be invalidated when `search` changes. - * Use the current location.search via the `history` object. - * `history` doesn't change so this is effectively like accessing `search` via a ref. */ const urlSearchParams = new URLSearchParams(searchRef.current);