improve

src/legacy/core_plugins/kibana/public/dashboard/np_ready/application.ts

@@ -19,7 +19,7 @@
 
 import { EuiConfirmModal, EuiIcon } from '@elastic/eui';
 import angular, { IModule } from 'angular';
-import { History } from 'history';
+import { createHashHistory, History } from 'history';
 import { i18nDirective, i18nFilter, I18nProvider } from '@kbn/i18n/angular';
 import {
   AppMountContext,
@@ -28,7 +28,11 @@ import {
   LegacyCoreStart,
   SavedObjectsClientContract,
 } from 'kibana/public';
-import { IKbnUrlStateStorage, Storage } from '../../../../../../plugins/kibana_utils/public';
+import {
+  createKbnUrlStateStorage,
+  IKbnUrlStateStorage,
+  Storage,
+} from '../../../../../../plugins/kibana_utils/public';
 import {
   configureAppAngularModule,
   confirmModalFactory,
@@ -50,7 +54,7 @@ import { IEmbeddableStart } from '../../../../../../plugins/embeddable/public';
 import { NavigationPublicPluginStart as NavigationStart } from '../../../../../../plugins/navigation/public';
 import {
   DataPublicPluginStart as NpDataStart,
-  syncFilters,
+  syncQuery,
 } from '../../../../../../plugins/data/public';
 import { SharePluginStart } from '../../../../../../plugins/share/public';
 
@@ -69,46 +73,57 @@ export interface RenderDeps {
   embeddables: IEmbeddableStart;
   localStorage: Storage;
   share: SharePluginStart;
-  kbnUrlStateStorage: IKbnUrlStateStorage;
-  history: History;
-
-  // hack: keeping the value nested
-  // as configureAppAngularModule() is called once and initialised with reference to 'hasInheritedGlobalState' ,
-  // so to be able to update underlying value, so angular controller could pick it up
-  // this param is used to determine if time filter saved with dashboard should be applied or not
-  hasInheritedGlobalState?: {
-    value: boolean;
-  };
+
+  // hack:
+  // renderApp() called each time the app is mounted,
+  // but initialisation of angular module happens only once.
+  // On each app mount, new history and kbnUrlStateStorage instances are created
+  // and we have to make sure, that those new instances will be used in DashboardAppController
+  // and not the ones which where created when angular module was initialised.
+  // This is achieved by having reference to initial object, which angular module was initialised with,
+  // and then by mutating that object on subsequent mounts we can pass new instances down to controller.
+  // If not for this hack, we could end up that global state syncing uses different history instance then app state syncing
+  // Which could cause excessive browser history entries.
+  angularGlobalStateHacks?: AngularGlobalStateHacks;
 }
 
+interface AngularGlobalStateHacks {
+  hasInheritedGlobalState?: boolean;
+  kbnUrlStateStorage?: IKbnUrlStateStorage;
+  history?: History;
+}
 let angularModuleInstance: IModule | null = null;
-const hasInheritedGlobalStateRef: { value: boolean } = { value: false };
+const angularGlobalStateHacks: AngularGlobalStateHacks = {};
 
 export const renderApp = (element: HTMLElement, appBasePath: string, deps: RenderDeps) => {
-  const { stop: stopSyncingGlobalFilters, hasInheritedFiltersFromUrl } = syncFilters(
-    deps.kbnUrlStateStorage,
-    deps.npDataStart.query.filterManager,
-    deps.npDataStart.query.timefilter.timefilter
+  const history = createHashHistory();
+  const kbnUrlStateStorage = createKbnUrlStateStorage({
+    history,
+    useHash: deps.uiSettings.get('state:storeInSessionStorage'),
+  });
+  const { stop: stopSyncingGlobalState, hasInheritedQueryFromUrl } = syncQuery(
+    deps.npDataStart.query,
+    kbnUrlStateStorage
   );
 
-  // hack: always keeping the latest 'hasInheritedGlobalState' value in the same object - hasInheritedGlobalStateRef
-  // this is needed so angular Controller picks up the latest value, as it has reference to the hasInheritedGlobalStateRef
-  hasInheritedGlobalStateRef.value = hasInheritedFiltersFromUrl;
-  deps.hasInheritedGlobalState = hasInheritedGlobalStateRef;
+  angularGlobalStateHacks.hasInheritedGlobalState = hasInheritedQueryFromUrl;
+  angularGlobalStateHacks.history = history;
+  angularGlobalStateHacks.kbnUrlStateStorage = kbnUrlStateStorage;
 
   if (!angularModuleInstance) {
     angularModuleInstance = createLocalAngularModule(deps.core, deps.navigation);
     // global routing stuff
     configureAppAngularModule(angularModuleInstance, deps.core as LegacyCoreStart, true);
     // custom routing stuff
+    deps.angularGlobalStateHacks = angularGlobalStateHacks;
     initDashboardApp(angularModuleInstance, deps);
   }
 
   const $injector = mountDashboardApp(appBasePath, element);
 
   return () => {
     $injector.get('$rootScope').$destroy();
-    stopSyncingGlobalFilters();
+    stopSyncingGlobalState();
   };
 };
 

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx

@@ -34,10 +34,13 @@ import {
   subscribeWithScope,
 } from '../legacy_imports';
 import {
+  COMPARE_ALL_OPTIONS,
+  compareFilters,
   IndexPattern,
   IndexPatternsContract,
   Query,
   SavedQuery,
+  syncAppFilters,
 } from '../../../../../../plugins/data/public';
 
 import {
@@ -68,13 +71,9 @@ import { getDashboardTitle } from './dashboard_strings';
 import { DashboardAppScope } from './dashboard_app';
 import { convertSavedDashboardPanelToPanelState } from './lib/embeddable_saved_object_converters';
 import { RenderDeps } from './application';
-import {
-  SavedObjectFinderProps,
-  SavedObjectFinderUi,
-} from '../../../../../../plugins/kibana_react/public';
+import { SavedObjectFinderProps, SavedObjectFinderUi } from '../../../../../../plugins/kibana_react/public';
 import { removeQueryParam, unhashUrl } from '../../../../../../plugins/kibana_utils/public';
-import { COMPARE_ALL_OPTIONS, compareFilters } from '../../../../../../plugins/data/public';
-import { startSyncingAppFilters } from './lib/sync_app_filters';
+import { map } from 'rxjs/operators';
 
 export interface DashboardAppControllerDependencies extends RenderDeps {
   $scope: DashboardAppScope;
@@ -112,10 +111,12 @@ export class DashboardAppController {
       },
     },
     core: { notifications, overlays, chrome, injectedMetadata, uiSettings, savedObjects, http },
-    history,
-    kbnUrlStateStorage,
-    hasInheritedGlobalState,
+    angularGlobalStateHacks,
   }: DashboardAppControllerDependencies) {
+    const history = angularGlobalStateHacks!.history!;
+    const kbnUrlStateStorage = angularGlobalStateHacks!.kbnUrlStateStorage!;
+    const hasInheritedGlobalState = angularGlobalStateHacks!.hasInheritedGlobalState!;
+
     const queryFilter = filterManager;
 
     let lastReloadRequestTime = 0;
@@ -133,11 +134,15 @@ export class DashboardAppController {
       history,
     });
 
-    const stopSyncingAppFilters = startSyncingAppFilters(filterManager, dashboardStateManager);
+    const stopSyncingAppFilters = syncAppFilters(filterManager, {
+      set: filters => dashboardStateManager.setFilters(filters),
+      get: () => dashboardStateManager.appState.filters,
+      state$: dashboardStateManager.appState$.pipe(map(state => state.filters)),
+    });
 
     // The hash check is so we only update the time filter on dashboard open, not during
     // normal cross app navigation.
-    if (dashboardStateManager.getIsTimeSavedWithDashboard() && !hasInheritedGlobalState?.value) {
+    if (dashboardStateManager.getIsTimeSavedWithDashboard() && !hasInheritedGlobalState) {
       dashboardStateManager.syncTimefilterWithDashboard(timefilter);
     }
     $scope.showSaveQuery = dashboardCapabilities.saveQuery as boolean;

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts

@@ -19,7 +19,7 @@
 
 import { i18n } from '@kbn/i18n';
 import _ from 'lodash';
-import { Subscription } from 'rxjs';
+import { Observable, Subscription } from 'rxjs';
 import { Moment } from 'moment';
 import { History } from 'history';
 
@@ -75,6 +75,10 @@ export class DashboardStateManager {
     return this.stateContainer.get();
   }
 
+  public get appState$(): Observable<DashboardAppState> {
+    return this.stateContainer.state$;
+  }
+
   private readonly stateContainer: ReduxLikeStateContainer<
     DashboardAppState,
     DashboardAppStateTransitions

src/legacy/core_plugins/kibana/public/dashboard/np_ready/legacy_app.js

@@ -206,8 +206,9 @@ export function initDashboardApp(app, deps) {
                 // See https://github.com/elastic/kibana/issues/10951 for more context.
                 if (error instanceof SavedObjectNotFound && id === 'create') {
                   // Note preserve querystring part is necessary so the state is preserved through the redirect.
-                  deps.history.replace({
-                    ...deps.history.location, // preserve query,
+                  const history = deps.angularGlobalStateHacks.history;
+                  history.replace({
+                    ...history.location, // preserve query,
                     pathname: DashboardConstants.CREATE_NEW_DASHBOARD_URL,
                   });
 

src/legacy/core_plugins/kibana/public/dashboard/plugin.ts

@@ -26,12 +26,11 @@ import {
   SavedObjectsClientContract,
 } from 'kibana/public';
 import { i18n } from '@kbn/i18n';
-import { createHashHistory } from 'history';
 import { RenderDeps } from './np_ready/application';
 import { DataStart } from '../../../data/public';
 import { DataPublicPluginStart as NpDataStart } from '../../../../../plugins/data/public';
 import { IEmbeddableStart } from '../../../../../plugins/embeddable/public';
-import { createKbnUrlStateStorage, Storage } from '../../../../../plugins/kibana_utils/public';
+import { Storage } from '../../../../../plugins/kibana_utils/public';
 import { NavigationPublicPluginStart as NavigationStart } from '../../../../../plugins/navigation/public';
 import { DashboardConstants } from './np_ready/dashboard_constants';
 import {
@@ -97,12 +96,6 @@ export class DashboardPlugin implements Plugin {
           overlays: contextCore.overlays,
         });
 
-        const history = createHashHistory();
-        const kbnUrlStateStorage = createKbnUrlStateStorage({
-          history,
-          useHash: contextCore.uiSettings.get('state:storeInSessionStorage'),
-        });
-
         const deps: RenderDeps = {
           core: contextCore as LegacyCoreStart,
           ...angularDependencies,
@@ -118,8 +111,6 @@ export class DashboardPlugin implements Plugin {
           embeddables,
           dashboardCapabilities: contextCore.application.capabilities.dashboard,
           localStorage: new Storage(localStorage),
-          history,
-          kbnUrlStateStorage,
         };
         const { renderApp } = await import('./np_ready/application');
         return renderApp(params.element, params.appBasePath, deps);

src/plugins/data/public/query/state_sync/index.ts

@@ -17,4 +17,5 @@
  * under the License.
  */
 
-export { syncFilters } from './sync_filters';
+export { syncQuery } from './sync_query';
+export { syncAppFilters } from './sync_app_filters';

src/plugins/data/public/query/state_sync/sync_app_filters.ts

@@ -0,0 +1,65 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import _ from 'lodash';
+import { filter, map } from 'rxjs/operators';
+import { COMPARE_ALL_OPTIONS, compareFilters } from '../filter_manager/lib/compare_filters';
+import { esFilters } from '../../../common';
+import { FilterManager } from '../filter_manager';
+import { BaseStateContainer } from '../../../../../plugins/kibana_utils/public';
+
+/**
+ * Helper utility to sync application's state filters, with filter manager
+ * @param filterManager
+ * @param appState
+ */
+export function syncAppFilters(
+  filterManager: FilterManager,
+  appState: BaseStateContainer<esFilters.Filter[]>
+) {
+  // make sure initial app filters are picked by filterManager
+  filterManager.setAppFilters(_.cloneDeep(appState.get()));
+
+  const subs = [
+    filterManager
+      .getUpdates$()
+      .pipe(
+        map(() => filterManager.getAppFilters()),
+        filter(
+          // continue only if app state filters updated
+          appFilters => !compareFilters(appFilters, appState.get(), COMPARE_ALL_OPTIONS)
+        )
+      )
+      .subscribe(appFilters => {
+        appState.set(appFilters);
+      }),
+
+    // if appFilters in dashboardStateManager changed (e.g browser history update),
+    // sync it to filterManager
+    appState.state$.subscribe(() => {
+      if (!compareFilters(appState.get(), filterManager.getAppFilters(), COMPARE_ALL_OPTIONS)) {
+        filterManager.setAppFilters(_.cloneDeep(appState.get()));
+      }
+    }),
+  ];
+
+  return () => {
+    subs.forEach(s => s.unsubscribe());
+  };
+}