[Security Solution] Global query string functionality improvements (#147218)

Addresses: https://github.com/elastic/kibana/issues/140263

This PR was inspired by https://github.com/elastic/kibana/pull/146649 and while working on persistent rules table state https://github.com/elastic/kibana/pull/145111.

## Summary

It includes improvements for url search parameters manipulation functionality. In particular `useGetInitialUrlParamValue` and `useReplaceUrlParams` hooks.

The main idea is to isolate the encoding layer ([rison](https://github.com/Nanonid/rison)) as an implementation detail so `useGetInitialUrlParamValue` and `useReplaceUrlParams` consumers can just provide types they wish and the hooks serialize/desirealize the data into appropriate format under the hood.

On top of that after `@kbn/rison` was added in https://github.com/elastic/kibana/pull/146649 it's possible to use this package directly to encode and decode parameters whenever necessary.

The improvements include
- store unserialized url parameters state in the redux store
- encode and decode data by using functions from `@kbn/rison` directly
- let `useReplaceUrlParams` accept an updater object

### Checklist

- [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

packages/kbn-rison/kbn_rison.test.ts

@@ -21,7 +21,7 @@ describe('encoding', () => {
   });
   it('throws if it received undefined', () => {
     expect(() => Rison.encode(undefined)).toThrowErrorMatchingInlineSnapshot(
-      `"unable to encode value into rison, expected a primative value array or object"`
+      `"unable to encode value into rison, expected a primitive value array or object"`
     );
   });
   it('encodes a complex object', () => {

packages/kbn-rison/kbn_rison.ts

@@ -29,7 +29,7 @@ export function encode(obj: any) {
   const rison = encodeUnknown(obj);
   if (rison === undefined) {
     throw new Error(
-      'unable to encode value into rison, expected a primative value array or object'
+      'unable to encode value into rison, expected a primitive value array or object'
     );
   }
   return rison;
@@ -42,6 +42,17 @@ export function decode(rison: string): RisonValue {
   return Rison.decode(rison);
 }
 
+/**
+ * safely parse a rison string into a javascript structure, never throws
+ */
+export function safeDecode(rison: string): RisonValue {
+  try {
+    return decode(rison);
+  } catch {
+    return null;
+  }
+}
+
 /**
  * rison-encode a javascript array without surrounding parens
  */

x-pack/plugins/security_solution/common/utils/format_page_filter_search_param.ts

@@ -5,16 +5,14 @@
  * 2.0.
  */
 
-import rison from '@kbn/rison';
 import type { FilterItemObj } from '../../public/common/components/filter_group/types';
 
 export const formatPageFilterSearchParam = (filters: FilterItemObj[]) => {
-  const modifiedFilters = filters.map((filter) => ({
+  return filters.map((filter) => ({
     title: filter.title ?? filter.fieldName,
     selectedOptions: filter.selectedOptions ?? [],
     fieldName: filter.fieldName,
     existsSelected: filter.existsSelected ?? false,
     exclude: filter.exclude ?? false,
   }));
-  return rison.encode(modifiedFilters);
 };

x-pack/plugins/security_solution/cypress/e2e/detection_alerts/detection_page_filters.cy.ts

@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { encode } from '@kbn/rison';
 import { getNewRule } from '../../objects/rule';
 import {
   CONTROL_FRAMES,
@@ -83,7 +84,7 @@ describe('Detections : Page Filters', () => {
     cy.url().then((url) => {
       const currURL = new URL(url);
 
-      currURL.searchParams.set('pageFilters', formatPageFilterSearchParam(NEW_FILTERS));
+      currURL.searchParams.set('pageFilters', encode(formatPageFilterSearchParam(NEW_FILTERS)));
       cy.visit(currURL.toString());
       waitForAlerts();
       assertFilterControlsWithFilterObject(NEW_FILTERS);
@@ -104,7 +105,7 @@ describe('Detections : Page Filters', () => {
     cy.url().then((url) => {
       const currURL = new URL(url);
 
-      currURL.searchParams.set('pageFilters', pageFilterUrlString);
+      currURL.searchParams.set('pageFilters', encode(pageFilterUrlString));
       cy.visit(currURL.toString());
 
       waitForAlerts();

x-pack/plugins/security_solution/cypress/tsconfig.json

@@ -26,6 +26,7 @@
     {
       "path": "../tsconfig.json",
       "force": true
-    }
+    },
+    "@kbn/rison"
   ]
 }

x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx

@@ -210,7 +210,7 @@ const useExternalAlertsInitialUrlState = () => {
 
   const getInitialUrlParamValue = useGetInitialUrlParamValue<boolean>(EXTERNAL_ALERTS_URL_PARAM);
 
-  const { decodedParam: showExternalAlertsInitialUrlState } = useMemo(
+  const showExternalAlertsInitialUrlState = useMemo(
     () => getInitialUrlParamValue(),
     [getInitialUrlParamValue]
   );
@@ -218,12 +218,9 @@ const useExternalAlertsInitialUrlState = () => {
   useEffect(() => {
     // Only called on component unmount
     return () => {
-      replaceUrlParams([
+      replaceUrlParams({
-        {
+        [EXTERNAL_ALERTS_URL_PARAM]: null,
-          key: EXTERNAL_ALERTS_URL_PARAM,
+      });
-          value: null,
-        },
-      ]);
     };
   }, [replaceUrlParams]);
 
@@ -236,11 +233,8 @@ const useExternalAlertsInitialUrlState = () => {
 const useSyncExternalAlertsUrlState = (showExternalAlerts: boolean) => {
   const replaceUrlParams = useReplaceUrlParams();
   useEffect(() => {
-    replaceUrlParams([
+    replaceUrlParams({
-      {
+      [EXTERNAL_ALERTS_URL_PARAM]: showExternalAlerts ? true : null,
-        key: EXTERNAL_ALERTS_URL_PARAM,
+    });
-        value: showExternalAlerts ? 'true' : null,
-      },
-    ]);
   }, [showExternalAlerts, replaceUrlParams]);
 };

x-pack/plugins/security_solution/public/common/components/markdown_editor/plugins/timeline/parser.ts

@@ -7,8 +7,8 @@
 
 import type { Plugin } from 'unified';
 import type { RemarkTokenizer } from '@elastic/eui';
+import { safeDecode } from '@kbn/rison';
 import { parse } from 'query-string';
-import { decodeRisonUrlState } from '../../../../utils/global_query_string/helpers';
 
 import { ID, PREFIX } from './constants';
 import * as i18n from './translations';
@@ -73,9 +73,14 @@ export const TimelineParser: Plugin = function () {
     try {
       const timelineSearch = timelineUrl.split('?');
       const parseTimelineUrlSearch = parse(timelineSearch[1]) as { timeline: string };
-      const { id: timelineId = '', graphEventId = '' } = decodeRisonUrlState(
+      const decodedTimeline = safeDecode(parseTimelineUrlSearch.timeline ?? '') as {
-        parseTimelineUrlSearch.timeline ?? ''
+        id?: string;
-      ) ?? { id: null, graphEventId: '' };
+        graphEventId?: string;
+      } | null;
+      const { id: timelineId = '', graphEventId = '' } = decodedTimeline ?? {
+        id: null,
+        graphEventId: '',
+      };
 
       if (!timelineId) {
         this.file.info(i18n.NO_TIMELINE_ID_FOUND, {

x-pack/plugins/security_solution/public/common/hooks/timeline/use_query_timeline_by_id_on_url_change.test.ts

@@ -7,7 +7,6 @@
 
 import { queryTimelineById } from '../../../timelines/components/open_timeline/helpers';
 import { useQueryTimelineByIdOnUrlChange } from './use_query_timeline_by_id_on_url_change';
-import * as urlHelpers from '../../utils/global_query_string/helpers';
 import { renderHook } from '@testing-library/react-hooks';
 import { timelineDefaults } from '../../../timelines/store/timeline/defaults';
 
@@ -91,15 +90,11 @@ describe('queryTimelineByIdOnUrlChange', () => {
 
   describe('when decode rison fails', () => {
     it('should not call queryTimelineById', () => {
-      jest.spyOn(urlHelpers, 'decodeRisonUrlState').mockImplementationOnce(() => {
-        throw new Error('Unable to decode');
-      });
-
       mockUseLocation.mockReturnValue({ search: oldTimelineRisonSearchString });
 
       const { rerender } = renderHook(() => useQueryTimelineByIdOnUrlChange());
-      mockUseLocation.mockReturnValue({ search: newTimelineRisonSearchString });
+      mockUseLocation.mockReturnValue({ search: '?foo=bar' });
-      jest.clearAllMocks();
+
       rerender();
 
       expect(queryTimelineById).not.toBeCalled();

x-pack/plugins/security_solution/public/common/hooks/timeline/use_query_timeline_by_id_on_url_change.ts

@@ -10,6 +10,7 @@ import { useEffect, useMemo } from 'react';
 import { useLocation } from 'react-router-dom';
 import usePrevious from 'react-use/lib/usePrevious';
 import { useDispatch } from 'react-redux';
+import { safeDecode } from '@kbn/rison';
 import type { TimelineUrl } from '../../../timelines/store/timeline/model';
 import { timelineActions, timelineSelectors } from '../../../timelines/store/timeline';
 import { TimelineId, TimelineTabs } from '../../../../common/types';
@@ -20,7 +21,6 @@ import {
   queryTimelineById,
 } from '../../../timelines/components/open_timeline/helpers';
 import {
-  decodeRisonUrlState,
   getParamFromQueryString,
   getQueryStringFromLocation,
 } from '../../utils/global_query_string/helpers';
@@ -51,29 +51,14 @@ export const useQueryTimelineByIdOnUrlChange = () => {
       urlKey: URL_PARAM_KEY.timeline,
       search: oldSearch ?? '',
     });
-
     const newUrlStateString = getQueryStringKeyValue({ urlKey: URL_PARAM_KEY.timeline, search });
 
-    if (oldUrlStateString != null && newUrlStateString != null) {
+    return oldUrlStateString != null && newUrlStateString != null
-      let newTimeline = null;
+      ? [
-      let oldTimeline = null;
+          safeDecode(oldUrlStateString) as TimelineUrl | null,
-
+          safeDecode(newUrlStateString) as TimelineUrl | null,
-      try {
+        ]
-        newTimeline = decodeRisonUrlState<TimelineUrl>(newUrlStateString);
+      : [null, null];
-      } catch (error) {
-        // do nothing as timeline is defaulted to null
-      }
-
-      try {
-        oldTimeline = decodeRisonUrlState<TimelineUrl>(oldUrlStateString);
-      } catch (error) {
-        // do nothing as timeline is defaulted to null
-      }
-
-      return [oldTimeline, newTimeline];
-    }
-
-    return [null, null];
   }, [oldSearch, search]);
 
   const oldId = previousTimeline?.id;

x-pack/plugins/security_solution/public/common/hooks/use_resolve_conflict.test.tsx

@@ -9,7 +9,6 @@ import { renderHook } from '@testing-library/react-hooks';
 import { useDeepEqualSelector } from './use_selector';
 import { useKibana } from '../lib/kibana';
 import { useResolveConflict } from './use_resolve_conflict';
-import * as urlHelpers from '../utils/global_query_string/helpers';
 
 jest.mock('react-router-dom', () => {
   const original = jest.requireActual('react-router-dom');
@@ -127,9 +126,6 @@ describe('useResolveConflict', () => {
 
     describe('rison is unable to be decoded', () => {
       it('should use timeline values from redux to create the otherObjectPath', async () => {
-        jest.spyOn(urlHelpers, 'decodeRisonUrlState').mockImplementation(() => {
-          throw new Error('Unable to decode');
-        });
         (useLocation as jest.Mock).mockReturnValue({
           pathname: 'my/cool/path',
           search: '?foo=bar',

x-pack/plugins/security_solution/public/common/hooks/use_resolve_conflict.tsx

@@ -8,12 +8,12 @@
 import React, { useCallback, useMemo } from 'react';
 import { useLocation } from 'react-router-dom';
 import { EuiSpacer } from '@elastic/eui';
+import { safeDecode, encode } from '@kbn/rison';
 import { useDeepEqualSelector } from './use_selector';
 import { TimelineId } from '../../../common/types/timeline';
 import { timelineSelectors } from '../../timelines/store/timeline';
 import type { TimelineUrl } from '../../timelines/store/timeline/model';
 import { timelineDefaults } from '../../timelines/store/timeline/defaults';
-import { decodeRisonUrlState, encodeRisonUrlState } from '../utils/global_query_string/helpers';
 import { useKibana } from '../lib/kibana';
 import { URL_PARAM_KEY } from './use_url_state';
 
@@ -53,12 +53,9 @@ export const useResolveConflict = () => {
       activeTab,
       graphEventId,
     };
-    let timelineSearch: TimelineUrl = currentTimelineState;
+    const timelineSearch =
-    try {
+      (safeDecode(timelineRison ?? '') as TimelineUrl | null) ?? currentTimelineState;
-      timelineSearch = decodeRisonUrlState(timelineRison) ?? currentTimelineState;
+
-    } catch (error) {
-      // do nothing as it's already defaulted on line 77
-    }
     // We have resolved to one object, but another object has a legacy URL alias associated with this ID/page. We should display a
     // callout with a warning for the user, and provide a way for them to navigate to the other object.
     const currentObjectId = timelineSearch?.id;
@@ -68,7 +65,7 @@ export const useResolveConflict = () => {
       ...timelineSearch,
       id: newSavedObjectId,
     };
-    const newTimelineRison = encodeRisonUrlState(newTimelineSearch);
+    const newTimelineRison = encode(newTimelineSearch);
     searchQuery.set(URL_PARAM_KEY.timeline, newTimelineRison);
 
     const newPath = `${pathname}?${searchQuery.toString()}${window.location.hash}`;

x-pack/plugins/security_solution/public/common/hooks/use_resolve_redirect.test.ts

@@ -10,7 +10,6 @@ import { renderHook } from '@testing-library/react-hooks';
 import { useDeepEqualSelector } from './use_selector';
 import { useKibana } from '../lib/kibana';
 import { useResolveRedirect } from './use_resolve_redirect';
-import * as urlHelpers from '../utils/global_query_string/helpers';
 
 jest.mock('react-router-dom', () => {
   const original = jest.requireActual('react-router-dom');
@@ -101,9 +100,6 @@ describe('useResolveRedirect', () => {
 
     describe('rison is unable to be decoded', () => {
       it('should use timeline values from redux to create the redirect path', async () => {
-        jest.spyOn(urlHelpers, 'decodeRisonUrlState').mockImplementation(() => {
-          throw new Error('Unable to decode');
-        });
         (useLocation as jest.Mock).mockReturnValue({
           pathname: 'my/cool/path',
           search: '?foo=bar',

x-pack/plugins/security_solution/public/common/hooks/use_resolve_redirect.ts

@@ -7,11 +7,11 @@
 
 import { useCallback, useEffect, useMemo, useState } from 'react';
 import { useLocation } from 'react-router-dom';
+import { safeDecode, encode } from '@kbn/rison';
 import { useDeepEqualSelector } from './use_selector';
 import { TimelineId } from '../../../common/types/timeline';
 import { timelineSelectors } from '../../timelines/store/timeline';
 import { timelineDefaults } from '../../timelines/store/timeline/defaults';
-import { decodeRisonUrlState, encodeRisonUrlState } from '../utils/global_query_string/helpers';
 import { useKibana } from '../lib/kibana';
 import type { TimelineUrl } from '../../timelines/store/timeline/model';
 import { URL_PARAM_KEY } from './use_url_state';
@@ -42,12 +42,8 @@ export const useResolveRedirect = () => {
       activeTab,
       graphEventId,
     };
-    let timelineSearch: TimelineUrl = currentTimelineState;
+    const timelineSearch =
-    try {
+      (safeDecode(timelineRison ?? '') as TimelineUrl | null) ?? currentTimelineState;
-      timelineSearch = decodeRisonUrlState(timelineRison) ?? currentTimelineState;
-    } catch (error) {
-      // do nothing as it's already defaulted on line 77
-    }
 
     if (
       hasRedirected ||
@@ -64,7 +60,7 @@ export const useResolveRedirect = () => {
       ...timelineSearch,
       id: newObjectId,
     };
-    const newTimelineRison = encodeRisonUrlState(newTimelineSearch);
+    const newTimelineRison = encode(newTimelineSearch);
     searchQuery.set(URL_PARAM_KEY.timeline, newTimelineRison);
     const newPath = `${pathname}?${searchQuery.toString()}`;
     spaces.ui.redirectLegacyUrl({

x-pack/plugins/security_solution/public/common/store/global_url_param/actions.ts

@@ -5,16 +5,17 @@
  * 2.0.
  */
 
+import type { RisonValue } from '@kbn/rison';
 import actionCreatorFactory from 'typescript-fsa';
 
 const actionCreator = actionCreatorFactory('x-pack/security_solution/local/global_url_param');
 
-export const registerUrlParam = actionCreator<{ key: string; initialValue: string | null }>(
+export const registerUrlParam = actionCreator<{ key: string; initialValue: RisonValue | null }>(
   'REGISTER_URL_PARAM'
 );
 
 export const deregisterUrlParam = actionCreator<{ key: string }>('DEREGISTER_URL_PARAM');
 
-export const updateUrlParam = actionCreator<{ key: string; value: string | null }>(
+export const updateUrlParam = actionCreator<{ key: string; value: RisonValue | null }>(
   'UPDATE_URL_PARAM'
 );

x-pack/plugins/security_solution/public/common/store/global_url_param/reducer.ts

@@ -5,10 +5,12 @@
  * 2.0.
  */
 
+import type { RisonValue } from '@kbn/rison';
+import deepEqual from 'fast-deep-equal';
 import { reducerWithInitialState } from 'typescript-fsa-reducers';
 import { registerUrlParam, updateUrlParam, deregisterUrlParam } from './actions';
 
-export type GlobalUrlParam = Record<string, string | null>;
+export type GlobalUrlParam = Record<string, RisonValue | null>;
 
 export const initialGlobalUrlParam: GlobalUrlParam = {};
 
@@ -34,8 +36,7 @@ export const globalUrlParamReducer = reducerWithInitialState(initialGlobalUrlPar
     return nextState;
   })
   .case(updateUrlParam, (state, { key, value }) => {
-    // Only update the URL after the query param is registered and if the current value is different than the previous value
+    if (state[key] === undefined || deepEqual(state[key], value)) {
-    if (state[key] === undefined || state[key] === value) {
       return state;
     }
 

x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts

@@ -5,7 +5,8 @@
  * 2.0.
  */
 
-import { decode, encode } from '@kbn/rison';
+import type { RisonValue } from '@kbn/rison';
+import { safeDecode, encode } from '@kbn/rison';
 import type { ParsedQuery } from 'query-string';
 import { parse, stringify } from 'query-string';
 import { url } from '@kbn/kibana-utils-plugin/public';
@@ -19,20 +20,6 @@ export const isDetectionsPages = (pageName: string) =>
   pageName === SecurityPageName.rulesCreate ||
   pageName === SecurityPageName.exceptions;
 
-export const decodeRisonUrlState = <T>(value: string | undefined): T | null => {
-  try {
-    return value ? (decode(value) as unknown as T) : null;
-  } catch (error) {
-    if (error instanceof Error && error.message.startsWith('rison decoder error')) {
-      return null;
-    }
-    throw error;
-  }
-};
-
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const encodeRisonUrlState = (state: any) => encode(state);
-
 export const getQueryStringFromLocation = (search: string) => search.substring(1);
 
 export const getParamFromQueryString = (
@@ -51,18 +38,19 @@ export const getParamFromQueryString = (
  * It doesn't update when the URL changes.
  *
  */
-export const useGetInitialUrlParamValue = <State>(urlParamKey: string) => {
+export const useGetInitialUrlParamValue = <State extends RisonValue>(
+  urlParamKey: string
+): (() => State | null) => {
   // window.location.search provides the most updated representation of the url search.
   // It also guarantees that we don't overwrite URL param managed outside react-router.
-  const getInitialUrlParamValue = useCallback(() => {
+  const getInitialUrlParamValue = useCallback((): State | null => {
-    const param = getParamFromQueryString(
+    const rawParamValue = getParamFromQueryString(
       getQueryStringFromLocation(window.location.search),
       urlParamKey
     );
+    const paramValue = safeDecode(rawParamValue ?? '') as State | null;
 
-    const decodedParam = decodeRisonUrlState<State>(param ?? undefined);
+    return paramValue;
-
-    return { param, decodedParam };
   }, [urlParamKey]);
 
   return getInitialUrlParamValue;
@@ -71,22 +59,30 @@ export const useGetInitialUrlParamValue = <State>(urlParamKey: string) => {
 export const encodeQueryString = (urlParams: ParsedQuery<string>): string =>
   stringify(url.encodeQuery(urlParams), { sort: false, encode: false });
 
-export const useReplaceUrlParams = () => {
+export const useReplaceUrlParams = (): ((params: Record<string, RisonValue | null>) => void) => {
   const history = useHistory();
 
   const replaceUrlParams = useCallback(
-    (params: Array<{ key: string; value: string | null }>) => {
+    (params: Record<string, RisonValue | null>): void => {
       // window.location.search provides the most updated representation of the url search.
       // It prevents unnecessary re-renders which useLocation would create because 'replaceUrlParams' does update the location.
       // window.location.search also guarantees that we don't overwrite URL param managed outside react-router.
       const search = window.location.search;
       const urlParams = parse(search, { sort: false });
 
-      params.forEach(({ key, value }) => {
+      Object.keys(params).forEach((key) => {
+        const value = params[key];
+
         if (value == null || value === '') {
           delete urlParams[key];
-        } else {
+          return;
-          urlParams[key] = value;
+        }
+
+        try {
+          urlParams[key] = encode(value);
+        } catch {
+          // eslint-disable-next-line no-console
+          console.error('Unable to encode url param value');
         }
       });
 

x-pack/plugins/security_solution/public/common/utils/global_query_string/index.test.tsx

@@ -130,7 +130,7 @@ describe('global query string', () => {
       expect(mockDispatch).toBeCalledWith(
         globalUrlParamActions.registerUrlParam({
           key: urlParamKey,
-          initialValue: initialValue.toString(),
+          initialValue,
         })
       );
     });
@@ -140,7 +140,6 @@ describe('global query string', () => {
     it('dispatch updateUrlParam action', () => {
       const urlParamKey = 'testKey';
       const value = { test: 123 };
-      const encodedVaue = '(test:123)';
 
       const globalUrlParam = {
         [urlParamKey]: 'oldValue',
@@ -156,7 +155,7 @@ describe('global query string', () => {
       expect(mockDispatch).toBeCalledWith(
         globalUrlParamActions.updateUrlParam({
           key: urlParamKey,
-          value: encodedVaue,
+          value,
         })
       );
     });
@@ -186,10 +185,12 @@ describe('global query string', () => {
         {
           ...mockGlobalState,
           globalUrlParam: {
-            testNumber: '123',
+            testNumber: 123,
-            testObject: '(test:321)',
+            testObject: { testKey: 321 },
+            testEmptyObject: {},
+            testEmptyArray: [],
             testNull: null,
-            testEmpty: '',
+            testEmptyString: '',
           },
         },
         SUB_PLUGINS_REDUCER,
@@ -202,7 +203,7 @@ describe('global query string', () => {
 
       const { result } = renderHook(() => useGlobalQueryString(), { wrapper });
 
-      expect(result.current).toEqual('testNumber=123&testObject=(test:321)');
+      expect(result.current).toEqual(`testNumber=123&testObject=(testKey:321)`);
     });
   });
 
@@ -218,7 +219,7 @@ describe('global query string', () => {
       renderHook(() => useSyncGlobalQueryString(), { wrapper: makeWrapper(globalUrlParam) });
 
       expect(mockHistory.replace).toHaveBeenCalledWith({
-        search: `firstKey=111&${urlParamKey}=${value}&lastKey=999`,
+        search: `firstKey=111&${urlParamKey}='${value}'&lastKey=999`,
       });
     });
 
@@ -236,7 +237,7 @@ describe('global query string', () => {
       renderHook(() => useSyncGlobalQueryString(), { wrapper: makeWrapper(globalUrlParam) });
 
       expect(mockHistory.replace).toHaveBeenCalledWith({
-        search: `${urlParamKey1}=${value1}&${urlParamKey2}=${value2}`,
+        search: `${urlParamKey1}='${value1}'&${urlParamKey2}='${value2}'`,
       });
     });
 

x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts

@@ -9,12 +9,8 @@ import { useCallback, useEffect, useMemo } from 'react';
 import { difference, isEmpty, pickBy } from 'lodash/fp';
 import { useDispatch } from 'react-redux';
 import usePrevious from 'react-use/lib/usePrevious';
-import {
+import { encode } from '@kbn/rison';
-  encodeQueryString,
+import { encodeQueryString, useGetInitialUrlParamValue, useReplaceUrlParams } from './helpers';
-  encodeRisonUrlState,
-  useGetInitialUrlParamValue,
-  useReplaceUrlParams,
-} from './helpers';
 import { useShallowEqualSelector } from '../../hooks/use_selector';
 import { globalUrlParamActions, globalUrlParamSelectors } from '../../store/global_url_param';
 import { useRouteSpy } from '../route/use_route_spy';
@@ -29,7 +25,7 @@ import { getLinkInfo } from '../../links';
  * @param urlParamKey Must not change.
  * @param onInitialize Called once when initializing. It must not change.
  */
-export const useInitializeUrlParam = <State>(
+export const useInitializeUrlParam = <State extends {}>(
   urlParamKey: string,
   /**
    * @param state Decoded URL param value.
@@ -38,20 +34,20 @@ export const useInitializeUrlParam = <State>(
 ) => {
   const dispatch = useDispatch();
 
-  const getInitialUrlParamValue = useGetInitialUrlParamValue<State>(urlParamKey);
+  const getInitialUrlParamValue = useGetInitialUrlParamValue(urlParamKey);
 
   useEffect(() => {
-    const { param: initialValue, decodedParam: decodedInitialValue } = getInitialUrlParamValue();
+    const value = getInitialUrlParamValue();
 
     dispatch(
       globalUrlParamActions.registerUrlParam({
         key: urlParamKey,
-        initialValue: initialValue ?? null,
+        initialValue: value,
       })
     );
 
     // execute consumer initialization
-    onInitialize(decodedInitialValue);
+    onInitialize(value as State);
 
     return () => {
       dispatch(globalUrlParamActions.deregisterUrlParam({ key: urlParamKey }));
@@ -65,13 +61,12 @@ export const useInitializeUrlParam = <State>(
  *
  * Make sure to call `useInitializeUrlParam` before calling this function.
  */
-export const useUpdateUrlParam = <State>(urlParamKey: string) => {
+export const useUpdateUrlParam = <State extends {}>(urlParamKey: string) => {
   const dispatch = useDispatch();
 
   const updateUrlParam = useCallback(
     (value: State | null) => {
-      const encodedValue = value !== null ? encodeRisonUrlState(value) : null;
+      dispatch(globalUrlParamActions.updateUrlParam({ key: urlParamKey, value }));
-      dispatch(globalUrlParamActions.updateUrlParam({ key: urlParamKey, value: encodedValue }));
     },
     [dispatch, urlParamKey]
   );
@@ -81,11 +76,29 @@ export const useUpdateUrlParam = <State>(urlParamKey: string) => {
 
 export const useGlobalQueryString = (): string => {
   const globalUrlParam = useShallowEqualSelector(globalUrlParamSelectors.selectGlobalUrlParam);
+  const globalQueryString = useMemo(() => {
+    const encodedGlobalUrlParam: Record<string, string> = {};
 
-  const globalQueryString = useMemo(
+    if (!globalUrlParam) {
-    () => encodeQueryString(pickBy((value) => !isEmpty(value), globalUrlParam)),
+      return '';
-    [globalUrlParam]
+    }
-  );
+
+    Object.keys(globalUrlParam).forEach((paramName) => {
+      const value = globalUrlParam[paramName];
+
+      if (!value || (typeof value === 'object' && isEmpty(value))) {
+        return;
+      }
+
+      try {
+        encodedGlobalUrlParam[paramName] = encode(value);
+      } catch {
+        // Just ignore parameters which unable to encode
+      }
+    });
+
+    return encodeQueryString(pickBy((value) => !isEmpty(value), encodedGlobalUrlParam));
+  }, [globalUrlParam]);
 
   return globalQueryString;
 };
@@ -100,29 +113,29 @@ export const useSyncGlobalQueryString = () => {
   const previousGlobalUrlParams = usePrevious(globalUrlParam);
   const replaceUrlParams = useReplaceUrlParams();
 
-  // Url params that got deleted from GlobalUrlParams
-  const unregisteredKeys = useMemo(
-    () => difference(Object.keys(previousGlobalUrlParams ?? {}), Object.keys(globalUrlParam)),
-    [previousGlobalUrlParams, globalUrlParam]
-  );
-
   useEffect(() => {
     const linkInfo = getLinkInfo(pageName) ?? { skipUrlState: true };
-    const params = Object.entries(globalUrlParam).map(([key, value]) => ({
+    const paramsToUpdate = { ...globalUrlParam };
-      key,
+
-      value: linkInfo.skipUrlState ? null : value,
+    if (linkInfo.skipUrlState) {
-    }));
+      Object.keys(paramsToUpdate).forEach((key) => {
+        paramsToUpdate[key] = null;
+      });
+    }
+
+    // Url params that got deleted from GlobalUrlParams
+    const unregisteredKeys = difference(
+      Object.keys(previousGlobalUrlParams ?? {}),
+      Object.keys(globalUrlParam)
+    );
 
     // Delete unregistered Url params
     unregisteredKeys.forEach((key) => {
-      params.push({
+      paramsToUpdate[key] = null;
-        key,
-        value: null,
-      });
     });
 
-    if (params.length > 0) {
+    if (Object.keys(paramsToUpdate).length > 0) {
-      replaceUrlParams(params);
+      replaceUrlParams(paramsToUpdate);
     }
-  }, [globalUrlParam, pageName, unregisteredKeys, replaceUrlParams]);
+  }, [previousGlobalUrlParams, globalUrlParam, pageName, replaceUrlParams]);
 };

x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table/__mocks__/mock_rules_table_persistent_state.ts

@@ -19,9 +19,7 @@ export function mockRulesTablePersistedState({
   urlState: RulesTableUrlSavedState | null;
   storageState: RulesTableStorageSavedState | null;
 }): void {
-  (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(
+  (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(jest.fn().mockReturnValue(urlState));
-    jest.fn().mockReturnValue({ decodedParam: urlState })
-  );
   (useKibana as jest.Mock).mockReturnValue({
     services: { sessionStorage: { get: jest.fn().mockReturnValue(storageState) } },
   });

x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table/use_initialize_rules_table_saved_state.ts

@@ -62,7 +62,7 @@ export function useInitializeRulesTableSavedState(): void {
   } = useKibana();
 
   useEffect(() => {
-    const { decodedParam: urlState } = getUrlParam();
+    const urlState = getUrlParam();
     const storageState = readStorageState(sessionStorage);
 
     if (!urlState && !storageState) {

x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table/use_sync_rules_table_saved_state.test.tsx

@@ -55,12 +55,9 @@ describe('useSyncRulesTableSavedState', () => {
 
     renderHook(() => useSyncRulesTableSavedState());
 
-    expect(replaceUrlParams).toHaveBeenCalledWith([
+    expect(replaceUrlParams).toHaveBeenCalledWith({
-      {
+      [URL_PARAM_KEY.rulesTable]: expectedUrlState,
-        key: URL_PARAM_KEY.rulesTable,
+    });
-        value: expectedUrlState,
-      },
-    ]);
   };
 
   const expectStateToSyncWithStorage = (
@@ -96,7 +93,7 @@ describe('useSyncRulesTableSavedState', () => {
 
     renderHook(() => useSyncRulesTableSavedState());
 
-    expect(replaceUrlParams).toHaveBeenCalledWith([{ key: URL_PARAM_KEY.rulesTable, value: null }]);
+    expect(replaceUrlParams).toHaveBeenCalledWith({ [URL_PARAM_KEY.rulesTable]: null });
     expect(setStorage).not.toHaveBeenCalled();
     expect(removeStorage).toHaveBeenCalledWith(RULES_TABLE_STATE_STORAGE_KEY);
   });
@@ -136,9 +133,7 @@ describe('useSyncRulesTableSavedState', () => {
 
     renderHook(() => useSyncRulesTableSavedState());
 
-    expect(replaceUrlParams).toHaveBeenCalledWith([
+    expect(replaceUrlParams).toHaveBeenCalledWith({ [URL_PARAM_KEY.rulesTable]: expectedUrlState });
-      { key: URL_PARAM_KEY.rulesTable, value: expectedUrlState },
-    ]);
     expect(setStorage).toHaveBeenCalledWith(RULES_TABLE_STATE_STORAGE_KEY, expectedStorageState);
   });
 

x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table/use_sync_rules_table_saved_state.ts

@@ -6,10 +6,7 @@
  */
 
 import { useEffect } from 'react';
-import {
+import { useReplaceUrlParams } from '../../../../../common/utils/global_query_string/helpers';
-  encodeRisonUrlState,
-  useReplaceUrlParams,
-} from '../../../../../common/utils/global_query_string/helpers';
 import { useKibana } from '../../../../../common/lib/kibana';
 import { URL_PARAM_KEY } from '../../../../../common/hooks/use_url_state';
 import { RULES_TABLE_STATE_STORAGE_KEY } from '../constants';
@@ -76,7 +73,7 @@ export function useSyncRulesTableSavedState(): void {
     const hasStorageStateToSave = Object.keys(storageStateToSave).length > 0;
 
     if (!hasUrlStateToSave) {
-      replaceUrlParams([{ key: URL_PARAM_KEY.rulesTable, value: null }]);
+      replaceUrlParams({ [URL_PARAM_KEY.rulesTable]: null });
     }
 
     if (!hasStorageStateToSave) {
@@ -87,9 +84,7 @@ export function useSyncRulesTableSavedState(): void {
       return;
     }
 
-    replaceUrlParams([
+    replaceUrlParams({ [URL_PARAM_KEY.rulesTable]: urlStateToSave });
-      { key: URL_PARAM_KEY.rulesTable, value: encodeRisonUrlState(urlStateToSave) },
-    ]);
     sessionStorage.set(RULES_TABLE_STATE_STORAGE_KEY, storageStateToSave);
   }, [replaceUrlParams, sessionStorage, state]);
 }

x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rule_from_timeline.test.ts

@@ -100,9 +100,7 @@ describe('useRuleFromTimeline', () => {
     jest.clearAllMocks();
     appToastsMock = useAppToastsMock.create();
     (useAppToasts as jest.Mock).mockReturnValue(appToastsMock);
-    (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(() => ({
+    (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(() => timelineId);
-      decodedParam: timelineId,
-    }));
     (resolveTimeline as jest.Mock).mockResolvedValue(selectedTimeline);
   });
 
@@ -139,9 +137,7 @@ describe('useRuleFromTimeline', () => {
         });
     });
     it('if no timeline id in URL, loading: false and query not set', async () => {
-      (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(() => ({
+      (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(() => undefined);
-        decodedParam: undefined,
-      }));
       const { result } = renderHook(() => useRuleFromTimeline(setRuleQuery));
 
       expect(result.current.loading).toEqual(false);
@@ -227,9 +223,7 @@ describe('useRuleFromTimeline', () => {
     });
 
     it('Sets rule from timeline query via callback', async () => {
-      (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(() => ({
+      (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(() => undefined);
-        decodedParam: undefined,
-      }));
       const { result } = renderHook(() => useRuleFromTimeline(setRuleQuery));
       expect(result.current.loading).toEqual(false);
       await act(async () => {
@@ -280,9 +274,7 @@ describe('useRuleFromTimeline', () => {
     });
 
     it('Handles error when query is malformed', async () => {
-      (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(() => ({
+      (useGetInitialUrlParamValue as jest.Mock).mockReturnValue(() => undefined);
-        decodedParam: undefined,
-      }));
       const { result } = renderHook(() => useRuleFromTimeline(setRuleQuery));
       expect(result.current.loading).toEqual(false);
       const tl = {

x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_rule_from_timeline.tsx

@@ -181,9 +181,7 @@ export const useRuleFromTimeline = (setRuleQuery: SetRuleQuery): RuleFromTimelin
 
   // start handle set rule from timeline id
   const getInitialUrlParamValue = useGetInitialUrlParamValue<string>(RULE_FROM_TIMELINE_URL_PARAM);
-  const { decodedParam: timelineIdFromUrl } = useMemo(getInitialUrlParamValue, [
+  const timelineIdFromUrl = useMemo(getInitialUrlParamValue, [getInitialUrlParamValue]);
-    getInitialUrlParamValue,
-  ]);
 
   const getTimelineById = useCallback(
     (timelineId: string) => {

x-pack/plugins/security_solution/public/timelines/components/open_timeline/index.tsx

@@ -7,9 +7,9 @@
 
 import React, { useEffect, useState, useCallback, useMemo } from 'react';
 import { useDispatch } from 'react-redux';
+import { encode } from '@kbn/rison';
 
 import { RULE_FROM_TIMELINE_URL_PARAM } from '../../../detections/containers/detection_engine/rules/use_rule_from_timeline';
-import { encodeRisonUrlState } from '../../../common/utils/global_query_string/helpers';
 import { useNavigation } from '../../../common/lib/kibana';
 import { SecurityPageName } from '../../../../common/constants';
 import { useShallowEqualSelector } from '../../../common/hooks/use_selector';
@@ -283,7 +283,7 @@ export const StatefulOpenTimelineComponent = React.memo<OpenTimelineOwnProps>(
       (savedObjectId) =>
         navigateTo({
           deepLinkId: SecurityPageName.rulesCreate,
-          path: `?${RULE_FROM_TIMELINE_URL_PARAM}=${encodeRisonUrlState(savedObjectId)}`,
+          path: `?${RULE_FROM_TIMELINE_URL_PARAM}=${encode(savedObjectId)}`,
         }),
       [navigateTo]
     );