From 7925b9f7e2caaec4744a350608bab9b3999a8696 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Fri, 16 Dec 2022 18:18:41 +0000 Subject: [PATCH 01/12] connect without container Signed-off-by: abbyhu2000 --- src/plugins/data/public/index.ts | 1 + .../state_sync/connect_to_query_state.ts | 175 +++++++++++++++++- .../data/public/query/state_sync/index.ts | 2 +- .../public/application/components/top_nav.tsx | 6 + 4 files changed, 182 insertions(+), 2 deletions(-) diff --git a/src/plugins/data/public/index.ts b/src/plugins/data/public/index.ts index 8b495a2a19b..32c48ee206c 100644 --- a/src/plugins/data/public/index.ts +++ b/src/plugins/data/public/index.ts @@ -444,6 +444,7 @@ export { Filter, Query, RefreshInterval, TimeRange } from '../common'; export { createSavedQueryService, + useQueryStateWithNoContainer, connectToQueryState, syncQueryStateWithUrl, QueryState, diff --git a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts index 2f492fd28f7..101259bbaca 100644 --- a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts +++ b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts @@ -31,12 +31,185 @@ import { Subscription } from 'rxjs'; import { filter, map } from 'rxjs/operators'; import _ from 'lodash'; -import { BaseStateContainer } from '../../../../opensearch_dashboards_utils/public'; +import { + BaseStateContainer, + IOsdUrlStateStorage, +} from '../../../../opensearch_dashboards_utils/public'; import { QuerySetup, QueryStart } from '../query_service'; import { QueryState, QueryStateChange } from './types'; import { FilterStateStore, COMPARE_ALL_OPTIONS, compareFilters } from '../../../common'; import { validateTimeRange } from '../timefilter'; +export const useQueryStateWithNoContainer = ( + { + timefilter: { timefilter }, + filterManager, + queryString, + state$, + }: Pick, + OsdUrlStateStorage: IOsdUrlStateStorage, + syncConfig: { + filters?: FilterStateStore | boolean; + query?: boolean; + } +) => { + const syncKeys: Array = []; + if (syncConfig.query) { + syncKeys.push('query'); + } + if (syncConfig.filters) { + switch (syncConfig.filters) { + case true: + syncKeys.push('filters'); + break; + case FilterStateStore.APP_STATE: + syncKeys.push('appFilters'); + break; + case FilterStateStore.GLOBAL_STATE: + syncKeys.push('globalFilters'); + break; + } + } + + const initialStateFromURL: QueryState = OsdUrlStateStorage.get('_q') ?? { + query: queryString.getDefaultQuery(), + filters: filterManager.getGlobalFilters(), + }; + let initialDirty = false; + + if (syncConfig.query && !_.isEqual(initialStateFromURL.query, queryString.getQuery())) { + initialStateFromURL.query = queryString.getQuery(); + initialDirty = true; + } + + if (syncConfig.filters) { + if (syncConfig.filters === true) { + if ( + !initialStateFromURL.filters || + !compareFilters( + initialStateFromURL.filters, + filterManager.getFilters(), + COMPARE_ALL_OPTIONS + ) + ) { + initialStateFromURL.filters = filterManager.getFilters(); + initialDirty = true; + } + } else if (syncConfig.filters === FilterStateStore.GLOBAL_STATE) { + if ( + !initialStateFromURL.filters || + !compareFilters(initialStateFromURL.filters, filterManager.getGlobalFilters(), { + ...COMPARE_ALL_OPTIONS, + state: false, + }) + ) { + initialStateFromURL.filters = filterManager.getGlobalFilters(); + initialDirty = true; + } + } else if (syncConfig.filters === FilterStateStore.APP_STATE) { + if ( + !initialStateFromURL.filters || + !compareFilters(initialStateFromURL.filters, filterManager.getAppFilters(), { + ...COMPARE_ALL_OPTIONS, + state: false, + }) + ) { + initialStateFromURL.filters = filterManager.getAppFilters(); + initialDirty = true; + } + } + } + + if (initialDirty) { + OsdUrlStateStorage.set('_q', initialStateFromURL, { + replace: true, + }); + } + + // to ignore own state updates + const updateInProgress = false; + + const subs: Subscription[] = [ + state$ + .pipe( + filter(({ changes, state }) => { + if (updateInProgress) return false; + return syncKeys.some((syncKey) => changes[syncKey]); + }), + map(({ changes }) => { + const newState: QueryState = {}; + if (syncConfig.query && changes.query) { + newState.query = queryString.getQuery(); + } + if (syncConfig.filters) { + if (syncConfig.filters === true && changes.filters) { + newState.filters = filterManager.getFilters(); + } else if ( + syncConfig.filters === FilterStateStore.GLOBAL_STATE && + changes.globalFilters + ) { + newState.filters = filterManager.getGlobalFilters(); + } else if (syncConfig.filters === FilterStateStore.APP_STATE && changes.appFilters) { + newState.filters = filterManager.getAppFilters(); + } + } + return newState; + }) + ) + .subscribe((newState) => { + OsdUrlStateStorage.set('_q', newState, { + replace: true, + }); + }), + /* stateContainer.state$.subscribe((state) => { + updateInProgress = true; + + // cloneDeep is required because services are mutating passed objects + // and state in state container is frozen + + if (syncConfig.query) { + const curQuery = state.query || queryString.getQuery(); + if (!_.isEqual(curQuery, queryString.getQuery())) { + queryString.setQuery(_.cloneDeep(curQuery)); + } + } + + if (syncConfig.filters) { + const filters = state.filters || []; + if (syncConfig.filters === true) { + if (!compareFilters(filters, filterManager.getFilters(), COMPARE_ALL_OPTIONS)) { + filterManager.setFilters(_.cloneDeep(filters)); + } + } else if (syncConfig.filters === FilterStateStore.APP_STATE) { + if ( + !compareFilters(filters, filterManager.getAppFilters(), { + ...COMPARE_ALL_OPTIONS, + state: false, + }) + ) { + filterManager.setAppFilters(_.cloneDeep(filters)); + } + } else if (syncConfig.filters === FilterStateStore.GLOBAL_STATE) { + if ( + !compareFilters(filters, filterManager.getGlobalFilters(), { + ...COMPARE_ALL_OPTIONS, + state: false, + }) + ) { + filterManager.setGlobalFilters(_.cloneDeep(filters)); + } + } + } + + updateInProgress = false; + }),*/ + ]; + + return () => { + subs.forEach((s) => s.unsubscribe()); + }; +}; + /** * Helper to setup two-way syncing of global data and a state container * @param QueryService: either setup or start diff --git a/src/plugins/data/public/query/state_sync/index.ts b/src/plugins/data/public/query/state_sync/index.ts index 7109d7d8702..6a98d8bc4bb 100644 --- a/src/plugins/data/public/query/state_sync/index.ts +++ b/src/plugins/data/public/query/state_sync/index.ts @@ -28,6 +28,6 @@ * under the License. */ -export { connectToQueryState } from './connect_to_query_state'; +export { connectToQueryState, useQueryStateWithNoContainer } from './connect_to_query_state'; export { syncQueryStateWithUrl } from './sync_state_with_url'; export { QueryState, QueryStateChange } from './types'; diff --git a/src/plugins/vis_builder/public/application/components/top_nav.tsx b/src/plugins/vis_builder/public/application/components/top_nav.tsx index fe9b735b5ac..621fcfe513b 100644 --- a/src/plugins/vis_builder/public/application/components/top_nav.tsx +++ b/src/plugins/vis_builder/public/application/components/top_nav.tsx @@ -18,6 +18,8 @@ import { setEditorState } from '../utils/state_management/metadata_slice'; import { useCanSave } from '../utils/use/use_can_save'; import { saveStateToSavedObject } from '../../saved_visualizations/transforms'; import { TopNavMenuData } from '../../../../navigation/public'; +import { opensearchFilters } from '../../../../data/public'; +import { useQueryStateWithNoContainer } from '../../../../data/public'; export const TopNav = () => { // id will only be set for the edit route @@ -34,6 +36,10 @@ export const TopNav = () => { const saveDisabledReason = useCanSave(); const savedVisBuilderVis = useSavedVisBuilderVis(visualizationIdFromUrl); + useQueryStateWithNoContainer(services.data.query, services.osdUrlStateStorage, { + filters: opensearchFilters.FilterStateStore.APP_STATE, + query: true, + }); const { selected: indexPattern } = useIndexPatterns(); const [config, setConfig] = useState(); const originatingApp = useTypedSelector((state) => { From bd6e6ba28ac52d19c7e85943f0e8a73dc7daf029 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Fri, 16 Dec 2022 18:58:49 +0000 Subject: [PATCH 02/12] Query and filter persistence working Signed-off-by: abbyhu2000 --- .../state_sync/connect_to_query_state.ts | 93 ++++++------------- 1 file changed, 27 insertions(+), 66 deletions(-) diff --git a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts index 101259bbaca..7c1caf7ccde 100644 --- a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts +++ b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts @@ -49,8 +49,8 @@ export const useQueryStateWithNoContainer = ( }: Pick, OsdUrlStateStorage: IOsdUrlStateStorage, syncConfig: { - filters?: FilterStateStore | boolean; - query?: boolean; + filters: FilterStateStore | boolean; + query: boolean; } ) => { const syncKeys: Array = []; @@ -75,11 +75,18 @@ export const useQueryStateWithNoContainer = ( query: queryString.getDefaultQuery(), filters: filterManager.getGlobalFilters(), }; - let initialDirty = false; + + // set up initial '_q' flag in the URL to sync query and filter changes + if (!OsdUrlStateStorage.get('_q')) { + OsdUrlStateStorage.set('_q', initialStateFromURL, { + replace: true, + }); + } if (syncConfig.query && !_.isEqual(initialStateFromURL.query, queryString.getQuery())) { - initialStateFromURL.query = queryString.getQuery(); - initialDirty = true; + if (initialStateFromURL.query) { + queryString.setQuery(_.cloneDeep(initialStateFromURL.query)); + } } if (syncConfig.filters) { @@ -92,8 +99,9 @@ export const useQueryStateWithNoContainer = ( COMPARE_ALL_OPTIONS ) ) { - initialStateFromURL.filters = filterManager.getFilters(); - initialDirty = true; + if (initialStateFromURL.filters) { + filterManager.setFilters(_.cloneDeep(initialStateFromURL.filters)); + } } } else if (syncConfig.filters === FilterStateStore.GLOBAL_STATE) { if ( @@ -103,8 +111,9 @@ export const useQueryStateWithNoContainer = ( state: false, }) ) { - initialStateFromURL.filters = filterManager.getGlobalFilters(); - initialDirty = true; + if (initialStateFromURL.filters) { + filterManager.setGlobalFilters(_.cloneDeep(initialStateFromURL.filters)); + } } } else if (syncConfig.filters === FilterStateStore.APP_STATE) { if ( @@ -114,30 +123,24 @@ export const useQueryStateWithNoContainer = ( state: false, }) ) { - initialStateFromURL.filters = filterManager.getAppFilters(); - initialDirty = true; + if (initialStateFromURL.filters) { + filterManager.setAppFilters(_.cloneDeep(initialStateFromURL.filters)); + } } } } - if (initialDirty) { - OsdUrlStateStorage.set('_q', initialStateFromURL, { - replace: true, - }); - } - - // to ignore own state updates - const updateInProgress = false; - const subs: Subscription[] = [ state$ .pipe( - filter(({ changes, state }) => { - if (updateInProgress) return false; + filter(({ changes }) => { return syncKeys.some((syncKey) => changes[syncKey]); }), - map(({ changes }) => { - const newState: QueryState = {}; + map(({ changes, state }) => { + const newState: QueryState = { + query: state.query, + filters: state.filters, + }; if (syncConfig.query && changes.query) { newState.query = queryString.getQuery(); } @@ -161,48 +164,6 @@ export const useQueryStateWithNoContainer = ( replace: true, }); }), - /* stateContainer.state$.subscribe((state) => { - updateInProgress = true; - - // cloneDeep is required because services are mutating passed objects - // and state in state container is frozen - - if (syncConfig.query) { - const curQuery = state.query || queryString.getQuery(); - if (!_.isEqual(curQuery, queryString.getQuery())) { - queryString.setQuery(_.cloneDeep(curQuery)); - } - } - - if (syncConfig.filters) { - const filters = state.filters || []; - if (syncConfig.filters === true) { - if (!compareFilters(filters, filterManager.getFilters(), COMPARE_ALL_OPTIONS)) { - filterManager.setFilters(_.cloneDeep(filters)); - } - } else if (syncConfig.filters === FilterStateStore.APP_STATE) { - if ( - !compareFilters(filters, filterManager.getAppFilters(), { - ...COMPARE_ALL_OPTIONS, - state: false, - }) - ) { - filterManager.setAppFilters(_.cloneDeep(filters)); - } - } else if (syncConfig.filters === FilterStateStore.GLOBAL_STATE) { - if ( - !compareFilters(filters, filterManager.getGlobalFilters(), { - ...COMPARE_ALL_OPTIONS, - state: false, - }) - ) { - filterManager.setGlobalFilters(_.cloneDeep(filters)); - } - } - } - - updateInProgress = false; - }),*/ ]; return () => { From def62ca0b0db00a7c86aacfff5049fc1b0a0cbf8 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Fri, 16 Dec 2022 19:04:30 +0000 Subject: [PATCH 03/12] Rebase and changelog Signed-off-by: abbyhu2000 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 478cad6cadd..4b6ef6a7eb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Table Visualization] Refactor table visualization using React and DataGrid component ([#2863](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2863)) - [Vis Builder] Add redux store persistence ([#3088](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3088)) - [Multi DataSource] Improve test connection ([#3110](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3110)) +- [Vis Builder] Add app filter and query persistence without using state container ([#3100](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3100)) ### 🐛 Bug Fixes From 797856ed937b5ff78a298922b7d514d7a3b1a972 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Fri, 16 Dec 2022 19:14:30 +0000 Subject: [PATCH 04/12] Simplify filter management Signed-off-by: abbyhu2000 --- .../state_sync/connect_to_query_state.ts | 133 ++++++------------ 1 file changed, 44 insertions(+), 89 deletions(-) diff --git a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts index 7c1caf7ccde..934acd4c860 100644 --- a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts +++ b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts @@ -42,14 +42,13 @@ import { validateTimeRange } from '../timefilter'; export const useQueryStateWithNoContainer = ( { - timefilter: { timefilter }, filterManager, queryString, state$, }: Pick, OsdUrlStateStorage: IOsdUrlStateStorage, syncConfig: { - filters: FilterStateStore | boolean; + filters: FilterStateStore; query: boolean; } ) => { @@ -57,23 +56,13 @@ export const useQueryStateWithNoContainer = ( if (syncConfig.query) { syncKeys.push('query'); } - if (syncConfig.filters) { - switch (syncConfig.filters) { - case true: - syncKeys.push('filters'); - break; - case FilterStateStore.APP_STATE: - syncKeys.push('appFilters'); - break; - case FilterStateStore.GLOBAL_STATE: - syncKeys.push('globalFilters'); - break; - } + if (syncConfig.filters === FilterStateStore.APP_STATE) { + syncKeys.push('appFilters'); } const initialStateFromURL: QueryState = OsdUrlStateStorage.get('_q') ?? { query: queryString.getDefaultQuery(), - filters: filterManager.getGlobalFilters(), + filters: filterManager.getAppFilters(), }; // set up initial '_q' flag in the URL to sync query and filter changes @@ -89,86 +78,52 @@ export const useQueryStateWithNoContainer = ( } } - if (syncConfig.filters) { - if (syncConfig.filters === true) { - if ( - !initialStateFromURL.filters || - !compareFilters( - initialStateFromURL.filters, - filterManager.getFilters(), - COMPARE_ALL_OPTIONS - ) - ) { - if (initialStateFromURL.filters) { - filterManager.setFilters(_.cloneDeep(initialStateFromURL.filters)); - } - } - } else if (syncConfig.filters === FilterStateStore.GLOBAL_STATE) { - if ( - !initialStateFromURL.filters || - !compareFilters(initialStateFromURL.filters, filterManager.getGlobalFilters(), { - ...COMPARE_ALL_OPTIONS, - state: false, - }) - ) { - if (initialStateFromURL.filters) { - filterManager.setGlobalFilters(_.cloneDeep(initialStateFromURL.filters)); - } - } - } else if (syncConfig.filters === FilterStateStore.APP_STATE) { - if ( - !initialStateFromURL.filters || - !compareFilters(initialStateFromURL.filters, filterManager.getAppFilters(), { - ...COMPARE_ALL_OPTIONS, - state: false, - }) - ) { - if (initialStateFromURL.filters) { - filterManager.setAppFilters(_.cloneDeep(initialStateFromURL.filters)); - } + if (syncConfig.filters === FilterStateStore.APP_STATE) { + if ( + !initialStateFromURL.filters || + !compareFilters(initialStateFromURL.filters, filterManager.getAppFilters(), { + ...COMPARE_ALL_OPTIONS, + state: false, + }) + ) { + if (initialStateFromURL.filters) { + filterManager.setAppFilters(_.cloneDeep(initialStateFromURL.filters)); } } - } - const subs: Subscription[] = [ - state$ - .pipe( - filter(({ changes }) => { - return syncKeys.some((syncKey) => changes[syncKey]); - }), - map(({ changes, state }) => { - const newState: QueryState = { - query: state.query, - filters: state.filters, - }; - if (syncConfig.query && changes.query) { - newState.query = queryString.getQuery(); - } - if (syncConfig.filters) { - if (syncConfig.filters === true && changes.filters) { - newState.filters = filterManager.getFilters(); - } else if ( - syncConfig.filters === FilterStateStore.GLOBAL_STATE && - changes.globalFilters - ) { - newState.filters = filterManager.getGlobalFilters(); - } else if (syncConfig.filters === FilterStateStore.APP_STATE && changes.appFilters) { + const subs: Subscription[] = [ + state$ + .pipe( + filter(({ changes }) => { + return syncKeys.some((syncKey) => changes[syncKey]); + }), + map(({ changes, state }) => { + const newState: QueryState = { + query: state.query, + filters: state.filters, + }; + if (syncConfig.query && changes.query) { + newState.query = queryString.getQuery(); + } + + if (syncConfig.filters === FilterStateStore.APP_STATE && changes.appFilters) { newState.filters = filterManager.getAppFilters(); } - } - return newState; - }) - ) - .subscribe((newState) => { - OsdUrlStateStorage.set('_q', newState, { - replace: true, - }); - }), - ]; - return () => { - subs.forEach((s) => s.unsubscribe()); - }; + return newState; + }) + ) + .subscribe((newState) => { + OsdUrlStateStorage.set('_q', newState, { + replace: true, + }); + }), + ]; + + return () => { + subs.forEach((s) => s.unsubscribe()); + }; + } }; /** From 16667adff5b1e264bd8e31902ffae3af7accf221 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Wed, 28 Dec 2022 16:49:31 +0000 Subject: [PATCH 05/12] change function name Signed-off-by: abbyhu2000 --- src/plugins/data/public/index.ts | 2 +- .../data/public/query/state_sync/connect_to_query_state.ts | 2 +- src/plugins/data/public/query/state_sync/index.ts | 2 +- .../vis_builder/public/application/components/top_nav.tsx | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plugins/data/public/index.ts b/src/plugins/data/public/index.ts index 32c48ee206c..3f96955e22b 100644 --- a/src/plugins/data/public/index.ts +++ b/src/plugins/data/public/index.ts @@ -444,7 +444,7 @@ export { Filter, Query, RefreshInterval, TimeRange } from '../common'; export { createSavedQueryService, - useQueryStateWithNoContainer, + connectStorageToQueryState, connectToQueryState, syncQueryStateWithUrl, QueryState, diff --git a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts index 934acd4c860..9548bbb3dba 100644 --- a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts +++ b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts @@ -40,7 +40,7 @@ import { QueryState, QueryStateChange } from './types'; import { FilterStateStore, COMPARE_ALL_OPTIONS, compareFilters } from '../../../common'; import { validateTimeRange } from '../timefilter'; -export const useQueryStateWithNoContainer = ( +export const connectStorageToQueryState = ( { filterManager, queryString, diff --git a/src/plugins/data/public/query/state_sync/index.ts b/src/plugins/data/public/query/state_sync/index.ts index 6a98d8bc4bb..5c5619461e0 100644 --- a/src/plugins/data/public/query/state_sync/index.ts +++ b/src/plugins/data/public/query/state_sync/index.ts @@ -28,6 +28,6 @@ * under the License. */ -export { connectToQueryState, useQueryStateWithNoContainer } from './connect_to_query_state'; +export { connectToQueryState, connectStorageToQueryState } from './connect_to_query_state'; export { syncQueryStateWithUrl } from './sync_state_with_url'; export { QueryState, QueryStateChange } from './types'; diff --git a/src/plugins/vis_builder/public/application/components/top_nav.tsx b/src/plugins/vis_builder/public/application/components/top_nav.tsx index 621fcfe513b..b21afc5dcf7 100644 --- a/src/plugins/vis_builder/public/application/components/top_nav.tsx +++ b/src/plugins/vis_builder/public/application/components/top_nav.tsx @@ -19,7 +19,7 @@ import { useCanSave } from '../utils/use/use_can_save'; import { saveStateToSavedObject } from '../../saved_visualizations/transforms'; import { TopNavMenuData } from '../../../../navigation/public'; import { opensearchFilters } from '../../../../data/public'; -import { useQueryStateWithNoContainer } from '../../../../data/public'; +import { connectStorageToQueryState } from '../../../../data/public'; export const TopNav = () => { // id will only be set for the edit route @@ -36,7 +36,7 @@ export const TopNav = () => { const saveDisabledReason = useCanSave(); const savedVisBuilderVis = useSavedVisBuilderVis(visualizationIdFromUrl); - useQueryStateWithNoContainer(services.data.query, services.osdUrlStateStorage, { + connectStorageToQueryState(services.data.query, services.osdUrlStateStorage, { filters: opensearchFilters.FilterStateStore.APP_STATE, query: true, }); From 053c0df23557d489cd3d548e2edab0f3d4f41596 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Wed, 28 Dec 2022 18:28:31 +0000 Subject: [PATCH 06/12] add unit test for function connect storage to query Signed-off-by: abbyhu2000 --- .../state_sync/connect_to_query_state.test.ts | 123 +++++++++++++++++- 1 file changed, 120 insertions(+), 3 deletions(-) diff --git a/src/plugins/data/public/query/state_sync/connect_to_query_state.test.ts b/src/plugins/data/public/query/state_sync/connect_to_query_state.test.ts index 5b4e0f22806..6b540c3da5c 100644 --- a/src/plugins/data/public/query/state_sync/connect_to_query_state.test.ts +++ b/src/plugins/data/public/query/state_sync/connect_to_query_state.test.ts @@ -31,19 +31,32 @@ import { Subscription } from 'rxjs'; import { FilterManager } from '../filter_manager'; import { getFilter } from '../filter_manager/test_helpers/get_stub_filter'; -import { Filter, FilterStateStore, UI_SETTINGS } from '../../../common'; +import { Filter, FilterStateStore, Query, UI_SETTINGS } from '../../../common'; import { coreMock } from '../../../../../core/public/mocks'; import { BaseStateContainer, createStateContainer, + IOsdUrlStateStorage, + createOsdUrlStateStorage, Storage, } from '../../../../opensearch_dashboards_utils/public'; import { QueryService, QueryStart } from '../query_service'; import { StubBrowserStorage } from '../../../../../test_utils/public/stub_browser_storage'; -import { connectToQueryState } from './connect_to_query_state'; +import { connectStorageToQueryState, connectToQueryState } from './connect_to_query_state'; import { TimefilterContract } from '../timefilter'; import { QueryState } from './types'; - +import { createBrowserHistory, History } from 'history'; +import { QueryStringContract } from '../query_string'; + +const connectStorageToQueryStateFn = ( + query: QueryStart, + OsdUrlStateStorage: IOsdUrlStateStorage +) => { + connectStorageToQueryState(query, OsdUrlStateStorage, { + filters: FilterStateStore.APP_STATE, + query: true, + }); +}; const connectToQueryGlobalState = (query: QueryStart, state: BaseStateContainer) => connectToQueryState(query, state, { refreshInterval: true, @@ -74,6 +87,110 @@ setupMock.uiSettings.get.mockImplementation((key: string) => { } }); +describe('connect_storage_to_query_state', () => { + let queryServiceStart: QueryStart; + let queryString: QueryStringContract; + let queryChangeSub: Subscription; + let queryChangeTriggered = jest.fn(); + let filterManager: FilterManager; + let filterManagerChangeSub: Subscription; + let filterManagerChangeTriggered = jest.fn(); + let osdUrlStateStorage: IOsdUrlStateStorage; + let history: History; + let gF1: Filter; + let gF2: Filter; + let aF1: Filter; + let aF2: Filter; + let q1: Query; + + beforeEach(() => { + const queryService = new QueryService(); + queryService.setup({ + uiSettings: setupMock.uiSettings, + storage: new Storage(new StubBrowserStorage()), + }); + queryServiceStart = queryService.start({ + uiSettings: setupMock.uiSettings, + storage: new Storage(new StubBrowserStorage()), + savedObjectsClient: startMock.savedObjects.client, + }); + + queryString = queryServiceStart.queryString; + queryChangeTriggered = jest.fn(); + queryChangeSub = queryString.getUpdates$().subscribe(queryChangeTriggered); + + filterManager = queryServiceStart.filterManager; + filterManagerChangeTriggered = jest.fn(); + filterManagerChangeSub = filterManager.getUpdates$().subscribe(filterManagerChangeTriggered); + + window.location.href = '/'; + history = createBrowserHistory(); + osdUrlStateStorage = createOsdUrlStateStorage({ useHash: false, history }); + + gF1 = getFilter(FilterStateStore.GLOBAL_STATE, true, true, 'key1', 'value1'); + gF2 = getFilter(FilterStateStore.GLOBAL_STATE, false, false, 'key2', 'value2'); + aF1 = getFilter(FilterStateStore.APP_STATE, true, true, 'key3', 'value3'); + aF2 = getFilter(FilterStateStore.APP_STATE, false, false, 'key4', 'value4'); + + q1 = { + query: 'count is less than 100', + language: 'kuery', + }; + }); + + afterEach(() => { + filterManagerChangeSub.unsubscribe(); + queryChangeSub.unsubscribe(); + }); + + test('state is initialized with default state', () => { + expect(osdUrlStateStorage.get('_q')).toBeNull(); + connectStorageToQueryStateFn(queryServiceStart, osdUrlStateStorage); + + expect(osdUrlStateStorage.get('_q')).toEqual({ + query: queryString.getDefaultQuery(), + filters: filterManager.getAppFilters(), + }); + }); + + test('state is initialized with URL states', () => { + const initialStates = { + filters: [aF1, aF2], + query: q1, + }; + osdUrlStateStorage.set('_q', initialStates, { + replace: true, + }); + connectStorageToQueryStateFn(queryServiceStart, osdUrlStateStorage); + expect(filterManager.getFilters().length).toBe(2); + expect(queryString.getQuery()).toStrictEqual(q1); + }); + + test('when global filter changes, filter in storage should not be updated', () => { + connectStorageToQueryStateFn(queryServiceStart, osdUrlStateStorage); + const previousStorage = osdUrlStateStorage.get('_q'); + filterManager.setFilters([gF1, gF1]); + const updatedStorage = osdUrlStateStorage.get('_q'); + expect(previousStorage).toStrictEqual(updatedStorage); + }); + + test('when app filter changes, filter storage should be updated', () => { + connectStorageToQueryStateFn(queryServiceStart, osdUrlStateStorage); + const previousStorage = osdUrlStateStorage.get('_q'); + filterManager.setFilters([aF1, aF1]); + const updatedStorage = osdUrlStateStorage.get('_q'); + expect(previousStorage).not.toStrictEqual(updatedStorage); + }); + + test('when query changes, state updates query', () => { + connectStorageToQueryStateFn(queryServiceStart, osdUrlStateStorage); + const previousStorage = osdUrlStateStorage.get('_q'); + queryString.setQuery(q1); + const updatedStorage = osdUrlStateStorage.get('_q'); + expect(previousStorage).not.toStrictEqual(updatedStorage); + }); +}); + describe('connect_to_global_state', () => { let queryServiceStart: QueryStart; let filterManager: FilterManager; From 6f25bfd7c95afd0ef3a34a1a31c6e75c88f24b90 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Wed, 28 Dec 2022 19:39:01 +0000 Subject: [PATCH 07/12] changelog change Signed-off-by: abbyhu2000 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b6ef6a7eb9..8a65499044b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,7 +49,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [I18n] Register ru, ru-RU locale ([#2817](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2817)) - Add yarn opensearch arg to setup plugin dependencies ([#2544](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2544)) - [Multi DataSource] Test the connection to an external data source when creating or updating ([#2973](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2973)) -- [Doc] Add current plugin persistence implementation readme ([#3081](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3081)) - [Table Visualization] Refactor table visualization using React and DataGrid component ([#2863](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2863)) - [Vis Builder] Add redux store persistence ([#3088](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3088)) - [Multi DataSource] Improve test connection ([#3110](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3110)) @@ -104,6 +103,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add `current-usage.md` and more details to `README.md` of `charts` plugin ([#2695](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2695)) - [Doc] Add readme for global query persistence ([#3001](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3001)) - Updates NOTICE file, adds validation to GitHub CI ([#3051](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3051)) +- [Doc] Add current plugin persistence implementation readme ([#3081](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3081)) ### 🛠 Maintenance From d001a7bc2d34b175fb6af7c372a9664902b192a8 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Wed, 4 Jan 2023 16:17:42 +0000 Subject: [PATCH 08/12] add comments and functional documentation in readme Signed-off-by: abbyhu2000 --- .../data/public/query/state_sync/README.md | 23 +++++++++++++++++++ .../state_sync/connect_to_query_state.ts | 8 +++++++ 2 files changed, 31 insertions(+) diff --git a/src/plugins/data/public/query/state_sync/README.md b/src/plugins/data/public/query/state_sync/README.md index 6b9b1581005..f54688afccf 100644 --- a/src/plugins/data/public/query/state_sync/README.md +++ b/src/plugins/data/public/query/state_sync/README.md @@ -1,3 +1,26 @@ # Query state syncing utilities Set of helpers to connect data services to state containers and state syncing utilities + +# Connect to query state + +This set of functions help sync state storage and state container with query managers. + +1. `connectStorageToQueryState()` + - This function take three input parameters: query state managers, `OsdUrlStateStorage`, and the two configs that it helps syncing, `app filters` and `query` + - If `OsdUrlStateStorage` is empty, then we initialize the `OsdUrlStateStorage` using the default app filter and query by calling `getDefaultQuery()` and `getAppFilters()` + - If the current query state and filter state differentiate from the url state storage, we update the filter and query values using state managers for filter and query from the data plugin. This step ensures that if we refresh the page, filter and query still persists their previous values. + - Then we set up subscriptions for both filter and query, so whenever we change the values for either of them, the new state get synced up with `OsdUrlStateStorage`. + - In the return function, we unsubscribe each one of them. + +2. `connectToQueryState()` + - This function take three input parameters: query state managers, `state container`, and the four configs that it helps syncing, `filter`, `query`, `time` and `refresh intervals` + - For initial syncing, we get the initial values from the state managers in the data plugin, and we store the values in the state container + - Then we set up subscriptions for each one of the config in data plugin, so whenever we change the values for any one of them, the new state get saved and sync with the state container. + - We also set up subscriptions for the states in state container, so whenever the value in state container get changed, the state managers in the data plugin will also be updated. + - In the return function, we unsubscribe each one of them. + +There are a couple differences between the above two functions: +1. `connectStorageToQueryState()` uses `OsdUrlStateStorage` for syncing, while `connectToQueryState()` uses `state container` for syncing, and `state container` is then synced up with `OsdUrlStateStorage`. +2. `connectStorageToQueryState()` can be used for persisting the app states, specifically app filter and query values, while `connectToQueryState()` can be used for persisting both app states and global states, specificlly app filter and query which are part of app states, global filter, time range, time refresh interval which are parts of global states. +3. `connectStorageToQueryState()` sets up a one way syncing from data to `OsdUrlStateStorage`, while `connectToQueryState()` sets up two-way syncing of the data and `state container`. Both of the functions serve to connect data services to achieve state syncing. \ No newline at end of file diff --git a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts index 9548bbb3dba..c8a75facc2b 100644 --- a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts +++ b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts @@ -40,6 +40,14 @@ import { QueryState, QueryStateChange } from './types'; import { FilterStateStore, COMPARE_ALL_OPTIONS, compareFilters } from '../../../common'; import { validateTimeRange } from '../timefilter'; +/** + * Helper function to sync up filter and query services in data plugin + * with a URL state storage so plugins can persist the app filter and query + * values across refresh + * @param QueryService: either setup or start + * @param OsdUrlStateStorage to use for syncing and store data + * @param syncConfig app filter and query + */ export const connectStorageToQueryState = ( { filterManager, From 400a2266eba2215ab55bd1e6888fe894e32be43a Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Wed, 4 Jan 2023 16:42:29 +0000 Subject: [PATCH 09/12] add more documentation in data persistence readme about vis builder persistence Signed-off-by: abbyhu2000 --- docs/plugins/data_persistence.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/plugins/data_persistence.md b/docs/plugins/data_persistence.md index 84e71c9c324..8c4504db216 100644 --- a/docs/plugins/data_persistence.md +++ b/docs/plugins/data_persistence.md @@ -196,6 +196,8 @@ There are two types for data persistence: 4. In `useEditorUpdates()`, we use the saved appState to load the visualize editor +5. Since we implement state management in Vis Builder with redux store, the app state persistence implementation for vis builder is also different than other plugins. While the global state persistence is the same, we implement app state persistence without using any state containers or state syncing utils. For the actual visual data, we directly hook up redux store with `OsdUrlStateStorage` to sync the values by using `saveReduxState()` and `loadReduxState()`. For app filter and query, since they are part of the app states but not part of the redux store, we directly hook up their state managers from data plugin with `OsdUrlStateStorage` and `connectStorageToQueryState()`. + # Refresh When we refresh the page, both app state and global state should persist: From 0cf3695795837708a5e4a2d065ef36a8f67fdffa Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Wed, 4 Jan 2023 16:47:27 +0000 Subject: [PATCH 10/12] error handling Signed-off-by: abbyhu2000 --- .../state_sync/connect_to_query_state.ts | 126 +++++++++--------- 1 file changed, 65 insertions(+), 61 deletions(-) diff --git a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts index c8a75facc2b..8b850b36eab 100644 --- a/src/plugins/data/public/query/state_sync/connect_to_query_state.ts +++ b/src/plugins/data/public/query/state_sync/connect_to_query_state.ts @@ -60,77 +60,81 @@ export const connectStorageToQueryState = ( query: boolean; } ) => { - const syncKeys: Array = []; - if (syncConfig.query) { - syncKeys.push('query'); - } - if (syncConfig.filters === FilterStateStore.APP_STATE) { - syncKeys.push('appFilters'); - } - - const initialStateFromURL: QueryState = OsdUrlStateStorage.get('_q') ?? { - query: queryString.getDefaultQuery(), - filters: filterManager.getAppFilters(), - }; + try { + const syncKeys: Array = []; + if (syncConfig.query) { + syncKeys.push('query'); + } + if (syncConfig.filters === FilterStateStore.APP_STATE) { + syncKeys.push('appFilters'); + } - // set up initial '_q' flag in the URL to sync query and filter changes - if (!OsdUrlStateStorage.get('_q')) { - OsdUrlStateStorage.set('_q', initialStateFromURL, { - replace: true, - }); - } + const initialStateFromURL: QueryState = OsdUrlStateStorage.get('_q') ?? { + query: queryString.getDefaultQuery(), + filters: filterManager.getAppFilters(), + }; - if (syncConfig.query && !_.isEqual(initialStateFromURL.query, queryString.getQuery())) { - if (initialStateFromURL.query) { - queryString.setQuery(_.cloneDeep(initialStateFromURL.query)); + // set up initial '_q' flag in the URL to sync query and filter changes + if (!OsdUrlStateStorage.get('_q')) { + OsdUrlStateStorage.set('_q', initialStateFromURL, { + replace: true, + }); } - } - if (syncConfig.filters === FilterStateStore.APP_STATE) { - if ( - !initialStateFromURL.filters || - !compareFilters(initialStateFromURL.filters, filterManager.getAppFilters(), { - ...COMPARE_ALL_OPTIONS, - state: false, - }) - ) { - if (initialStateFromURL.filters) { - filterManager.setAppFilters(_.cloneDeep(initialStateFromURL.filters)); + if (syncConfig.query && !_.isEqual(initialStateFromURL.query, queryString.getQuery())) { + if (initialStateFromURL.query) { + queryString.setQuery(_.cloneDeep(initialStateFromURL.query)); } } - const subs: Subscription[] = [ - state$ - .pipe( - filter(({ changes }) => { - return syncKeys.some((syncKey) => changes[syncKey]); - }), - map(({ changes, state }) => { - const newState: QueryState = { - query: state.query, - filters: state.filters, - }; - if (syncConfig.query && changes.query) { - newState.query = queryString.getQuery(); - } + if (syncConfig.filters === FilterStateStore.APP_STATE) { + if ( + !initialStateFromURL.filters || + !compareFilters(initialStateFromURL.filters, filterManager.getAppFilters(), { + ...COMPARE_ALL_OPTIONS, + state: false, + }) + ) { + if (initialStateFromURL.filters) { + filterManager.setAppFilters(_.cloneDeep(initialStateFromURL.filters)); + } + } - if (syncConfig.filters === FilterStateStore.APP_STATE && changes.appFilters) { - newState.filters = filterManager.getAppFilters(); - } + const subs: Subscription[] = [ + state$ + .pipe( + filter(({ changes }) => { + return syncKeys.some((syncKey) => changes[syncKey]); + }), + map(({ changes, state }) => { + const newState: QueryState = { + query: state.query, + filters: state.filters, + }; + if (syncConfig.query && changes.query) { + newState.query = queryString.getQuery(); + } - return newState; - }) - ) - .subscribe((newState) => { - OsdUrlStateStorage.set('_q', newState, { - replace: true, - }); - }), - ]; + if (syncConfig.filters === FilterStateStore.APP_STATE && changes.appFilters) { + newState.filters = filterManager.getAppFilters(); + } - return () => { - subs.forEach((s) => s.unsubscribe()); - }; + return newState; + }) + ) + .subscribe((newState) => { + OsdUrlStateStorage.set('_q', newState, { + replace: true, + }); + }), + ]; + + return () => { + subs.forEach((s) => s.unsubscribe()); + }; + } + } catch (err) { + return; } }; From 6552ab5e589ad556280dd36439a7950b6f9cd2e5 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Wed, 4 Jan 2023 17:39:42 +0000 Subject: [PATCH 11/12] add function definition in data plugin api doc Signed-off-by: abbyhu2000 --- src/plugins/data/public/public.api.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 615ca3c2e0d..4651f4a2b70 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -368,6 +368,20 @@ export enum BUCKET_TYPES { // @public export const castOpenSearchToOsdFieldTypeName: (opensearchType: OPENSEARCH_FIELD_TYPES | string) => OSD_FIELD_TYPES; +// @public +export const connectStorageToQueryState = ( + { + filterManager, + queryString, + state$, + }: Pick, + OsdUrlStateStorage: IOsdUrlStateStorage, + syncConfig: { + filters: FilterStateStore; + query: boolean; + } +) => () => void; + // Warning: (ae-forgotten-export) The symbol "QuerySetup" needs to be exported by the entry point index.d.ts // Warning: (ae-forgotten-export) The symbol "BaseStateContainer" needs to be exported by the entry point index.d.ts // Warning: (ae-missing-release-tag) "connectToQueryState" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) From 8595a427408e76d72305d338778dfa1c507e0345 Mon Sep 17 00:00:00 2001 From: abbyhu2000 Date: Fri, 6 Jan 2023 16:23:31 +0000 Subject: [PATCH 12/12] document and comments Signed-off-by: abbyhu2000 --- docs/plugins/data_persistence.md | 2 +- .../vis_builder/public/application/components/top_nav.tsx | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/plugins/data_persistence.md b/docs/plugins/data_persistence.md index 8c4504db216..b4141e8a57a 100644 --- a/docs/plugins/data_persistence.md +++ b/docs/plugins/data_persistence.md @@ -196,7 +196,7 @@ There are two types for data persistence: 4. In `useEditorUpdates()`, we use the saved appState to load the visualize editor -5. Since we implement state management in Vis Builder with redux store, the app state persistence implementation for vis builder is also different than other plugins. While the global state persistence is the same, we implement app state persistence without using any state containers or state syncing utils. For the actual visual data, we directly hook up redux store with `OsdUrlStateStorage` to sync the values by using `saveReduxState()` and `loadReduxState()`. For app filter and query, since they are part of the app states but not part of the redux store, we directly hook up their state managers from data plugin with `OsdUrlStateStorage` and `connectStorageToQueryState()`. +5. We can also choose to do state sync without using state container by directing hooking up the state managers with the URL data storage. For example, we implemented state management in Vis Builder with redux store, while the global state persistence implementation is the same, we implemented app state persistence without using any state containers or state syncing utils. For the actual visual data, we directly hook up redux store with `OsdUrlStateStorage` to sync the values by using `saveReduxState()` and `loadReduxState()`. For app filter and query, since they are part of the app states but not part of the redux store, we directly hook up their state managers from data plugin with `OsdUrlStateStorage` and `connectStorageToQueryState()`. # Refresh When we refresh the page, both app state and global state should persist: diff --git a/src/plugins/vis_builder/public/application/components/top_nav.tsx b/src/plugins/vis_builder/public/application/components/top_nav.tsx index b21afc5dcf7..d64352fb4d6 100644 --- a/src/plugins/vis_builder/public/application/components/top_nav.tsx +++ b/src/plugins/vis_builder/public/application/components/top_nav.tsx @@ -18,8 +18,7 @@ import { setEditorState } from '../utils/state_management/metadata_slice'; import { useCanSave } from '../utils/use/use_can_save'; import { saveStateToSavedObject } from '../../saved_visualizations/transforms'; import { TopNavMenuData } from '../../../../navigation/public'; -import { opensearchFilters } from '../../../../data/public'; -import { connectStorageToQueryState } from '../../../../data/public'; +import { opensearchFilters, connectStorageToQueryState } from '../../../../data/public'; export const TopNav = () => { // id will only be set for the edit route