[8.7] [embeddable] centralize should fetch embeddable observable logic (#151799) (#152838)

# Backport This will backport the following commits from `main` to `8.7`: - [[embeddable] centralize should fetch embeddable observable logic (#151799)](https://github.com/elastic/kibana/pull/151799)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
2025-04-24 09:48:58 -04:00 · 2023-03-07 18:55:12 +01:00 · 2023-03-07 18:55:12 +01:00 · 09ce818e81
commit 09ce818e81
parent 26c8ad7b45
9 changed files with 381 additions and 57 deletions
--- a/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_functions.ts
+++ b/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/diff_state/dashboard_diffing_functions.ts
@ -15,6 +15,7 @@ import {
  isFilterPinned,
  onlyDisabledFiltersChanged,
 } from '@kbn/es-query';
+import { shouldRefreshFilterCompareOptions } from '@kbn/embeddable-plugin/public';

 import { DashboardContainer } from '../../dashboard_container';
 import { DashboardContainerByValueInput } from '../../../../../common';
@ -117,12 +118,6 @@ export const unsavedChangesDiffingFunctions: DashboardDiffFunctions = {
  viewMode: () => false, // When compared view mode is always considered unequal so that it gets backed up.
 };

-const shouldRefreshFilterCompareOptions = {
-  ...COMPARE_ALL_OPTIONS,
-  // do not compare $state to avoid refreshing when filter is pinned/unpinned (which does not impact results)
-  state: false,
-};
-
 export const shouldRefreshDiffingFunctions: DashboardDiffFunctions = {
  ...unsavedChangesDiffingFunctions,
  filters: ({ currentValue, lastValue }) =>
--- a/src/plugins/embeddable/public/index.ts
+++ b/src/plugins/embeddable/public/index.ts
@ -87,6 +87,8 @@ export {
  EmbeddableRenderer,
  useEmbeddableFactory,
  isFilterableEmbeddable,
+  shouldFetch$,
+  shouldRefreshFilterCompareOptions,
 } from './lib';

 export { AttributeService, ATTRIBUTE_SERVICE_KEY } from './lib/attribute_service';
--- a/src/plugins/embeddable/public/lib/filterable_embeddable/index.ts
+++ b/src/plugins/embeddable/public/lib/filterable_embeddable/index.ts
@ -8,3 +8,4 @@

 export type { FilterableEmbeddable } from './types';
 export { isFilterableEmbeddable } from './types';
+export { shouldFetch$, shouldRefreshFilterCompareOptions } from './should_fetch';
--- a/src/plugins/embeddable/public/lib/filterable_embeddable/should_fetch.tsx
+++ b/src/plugins/embeddable/public/lib/filterable_embeddable/should_fetch.tsx
@ -0,0 +1,44 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import fastIsEqual from 'fast-deep-equal';
+import { Observable } from 'rxjs';
+import { map, distinctUntilChanged, skip, startWith } from 'rxjs/operators';
+import { COMPARE_ALL_OPTIONS, onlyDisabledFiltersChanged } from '@kbn/es-query';
+import type { FilterableEmbeddableInput } from './types';
+
+export const shouldRefreshFilterCompareOptions = {
+  ...COMPARE_ALL_OPTIONS,
+  // do not compare $state to avoid refreshing when filter is pinned/unpinned (which does not impact results)
+  state: false,
+};
+
+export function shouldFetch$<
+  TFilterableEmbeddableInput extends FilterableEmbeddableInput = FilterableEmbeddableInput
+>(
+  updated$: Observable<unknown>,
+  getInput: () => TFilterableEmbeddableInput
+): Observable<TFilterableEmbeddableInput> {
+  return updated$.pipe(map(() => getInput())).pipe(
+    // wrapping distinctUntilChanged with startWith and skip to prime distinctUntilChanged with an initial input value.
+    startWith(getInput()),
+    distinctUntilChanged((a: TFilterableEmbeddableInput, b: TFilterableEmbeddableInput) => {
+      if (
+        !fastIsEqual(
+          [a.searchSessionId, a.query, a.timeRange, a.timeslice],
+          [b.searchSessionId, b.query, b.timeRange, b.timeslice]
+        )
+      ) {
+        return false;
+      }
+
+      return onlyDisabledFiltersChanged(a.filters, b.filters, shouldRefreshFilterCompareOptions);
+    }),
+    skip(1)
+  );
+}
--- a/src/plugins/embeddable/public/lib/filterable_embeddable/types.ts
+++ b/src/plugins/embeddable/public/lib/filterable_embeddable/types.ts
@ -6,7 +6,15 @@
 * Side Public License, v 1.
 */

-import { type AggregateQuery, type Filter, type Query } from '@kbn/es-query';
+import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query';
+import { EmbeddableInput } from '../embeddables';
+
+export type FilterableEmbeddableInput = EmbeddableInput & {
+  filters?: Filter[];
+  query?: Query;
+  timeRange?: TimeRange;
+  timeslice?: [number, number];
+};

 /**
 * All embeddables that implement this interface should support being filtered
--- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx
+++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx
@ -55,6 +55,7 @@ import {
  cellValueTrigger,
  CELL_VALUE_TRIGGER,
  type CellValueContext,
+  shouldFetch$,
 } from '@kbn/embeddable-plugin/public';
 import type { Action, UiActionsStart } from '@kbn/ui-actions-plugin/public';
 import type { DataViewsContract, DataView } from '@kbn/data-views-plugin/public';
@ -432,20 +433,11 @@ export class Embeddable

    // Update search context and reload on changes related to search
    this.inputReloadSubscriptions.push(
-      this.getUpdated$()
-        .pipe(map(() => this.getInput()))
-        .pipe(
-          distinctUntilChanged((a, b) =>
-            fastIsEqual(
-              [a.filters, a.query, a.timeRange, a.searchSessionId],
-              [b.filters, b.query, b.timeRange, b.searchSessionId]
-            )
-          ),
-          skip(1)
-        )
-        .subscribe(async (input) => {
+      shouldFetch$<LensEmbeddableInput>(this.getUpdated$(), () => this.getInput()).subscribe(
+        (input) => {
          this.onContainerStateChanged(input);
-        })
+        }
+      )
    );
  }

--- a/x-pack/plugins/maps/public/embeddable/map_embeddable.test.tsx
+++ b/x-pack/plugins/maps/public/embeddable/map_embeddable.test.tsx
@ -0,0 +1,290 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import React from 'react';
+import { v4 as uuidv4 } from 'uuid';
+import { getControlledBy, MapEmbeddable } from './map_embeddable';
+import { buildExistsFilter, disableFilter, pinFilter, toggleFilterNegated } from '@kbn/es-query';
+import type { DataViewFieldBase, DataViewBase } from '@kbn/es-query';
+import { MapEmbeddableConfig, MapEmbeddableInput } from './types';
+import { MapSavedObjectAttributes } from '../../common/map_saved_object_type';
+
+jest.mock('../kibana_services', () => {
+  return {
+    getHttp() {
+      return {
+        basePath: {
+          prepend: (url: string) => url,
+        },
+      };
+    },
+    getMapsCapabilities() {
+      return { save: true };
+    },
+    getSearchService() {
+      return {
+        session: {
+          getSearchOptions() {
+            return undefined;
+          },
+        },
+      };
+    },
+    getShowMapsInspectorAdapter() {
+      return false;
+    },
+    getTimeFilter() {
+      return {
+        getTime() {
+          return { from: 'now-7d', to: 'now' };
+        },
+      };
+    },
+  };
+});
+
+jest.mock('../connected_components/map_container', () => {
+  return {
+    MapContainer: () => {
+      return <div>mockLayerTOC</div>;
+    },
+  };
+});
+
+jest.mock('../routes/map_page', () => {
+  class MockSavedMap {
+    // eslint-disable-next-line @typescript-eslint/no-var-requires
+    private _store = require('../reducers/store').createMapStore();
+    private _attributes: MapSavedObjectAttributes = {
+      title: 'myMap',
+    };
+
+    whenReady = async function () {};
+
+    getStore() {
+      return this._store;
+    }
+    getAttributes() {
+      return this._attributes;
+    }
+    getAutoFitToBounds() {
+      return true;
+    }
+    getSharingSavedObjectProps() {
+      return null;
+    }
+  }
+  return { SavedMap: MockSavedMap };
+});
+
+function untilInitialized(mapEmbeddable: MapEmbeddable): Promise<void> {
+  return new Promise((resolve) => {
+    // @ts-expect-error setInitializationFinished is protected but we are overriding it to know when embeddable is initialized
+    mapEmbeddable.setInitializationFinished = () => {
+      resolve();
+    };
+  });
+}
+
+function onNextTick(): Promise<void> {
+  // wait one tick to give observables time to fire
+  return new Promise((resolve) => setTimeout(resolve, 0));
+}
+
+describe('shouldFetch$', () => {
+  test('should not fetch when search context does not change', async () => {
+    const mapEmbeddable = new MapEmbeddable(
+      {} as unknown as MapEmbeddableConfig,
+      {
+        id: 'map1',
+      } as unknown as MapEmbeddableInput
+    );
+    await untilInitialized(mapEmbeddable);
+
+    const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery');
+
+    mapEmbeddable.updateInput({
+      title: 'updated map title',
+    });
+
+    await onNextTick();
+
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
+  describe('on filters change', () => {
+    test('should fetch on filter change', async () => {
+      const existsFilter = buildExistsFilter(
+        {
+          name: 'myFieldName',
+        } as DataViewFieldBase,
+        {
+          id: 'myDataViewId',
+        } as DataViewBase
+      );
+      const mapEmbeddable = new MapEmbeddable(
+        {} as unknown as MapEmbeddableConfig,
+        {
+          id: 'map1',
+          filters: [existsFilter],
+        } as unknown as MapEmbeddableInput
+      );
+      await untilInitialized(mapEmbeddable);
+
+      const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery');
+
+      mapEmbeddable.updateInput({
+        filters: [toggleFilterNegated(existsFilter)],
+      });
+
+      await onNextTick();
+
+      expect(fetchSpy).toHaveBeenCalled();
+    });
+
+    test('should not fetch on disabled filter change', async () => {
+      const disabledFilter = disableFilter(
+        buildExistsFilter(
+          {
+            name: 'myFieldName',
+          } as DataViewFieldBase,
+          {
+            id: 'myDataViewId',
+          } as DataViewBase
+        )
+      );
+      const mapEmbeddable = new MapEmbeddable(
+        {} as unknown as MapEmbeddableConfig,
+        {
+          id: 'map1',
+          filters: [disabledFilter],
+        } as unknown as MapEmbeddableInput
+      );
+      await untilInitialized(mapEmbeddable);
+
+      const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery');
+
+      mapEmbeddable.updateInput({
+        filters: [toggleFilterNegated(disabledFilter)],
+      });
+
+      await onNextTick();
+
+      expect(fetchSpy).not.toHaveBeenCalled();
+    });
+
+    test('should not fetch when unpinned filter is pinned', async () => {
+      const unpinnedFilter = buildExistsFilter(
+        {
+          name: 'myFieldName',
+        } as DataViewFieldBase,
+        {
+          id: 'myDataViewId',
+        } as DataViewBase
+      );
+      const mapEmbeddable = new MapEmbeddable(
+        {} as unknown as MapEmbeddableConfig,
+        {
+          id: 'map1',
+          filters: [unpinnedFilter],
+        } as unknown as MapEmbeddableInput
+      );
+      await untilInitialized(mapEmbeddable);
+
+      const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery');
+
+      mapEmbeddable.updateInput({
+        filters: [pinFilter(unpinnedFilter)],
+      });
+
+      await onNextTick();
+
+      expect(fetchSpy).not.toHaveBeenCalled();
+    });
+
+    test('should not fetch on filter controlled by map embeddable change', async () => {
+      const embeddableId = 'map1';
+      const filter = buildExistsFilter(
+        {
+          name: 'myFieldName',
+        } as DataViewFieldBase,
+        {
+          id: 'myDataViewId',
+        } as DataViewBase
+      );
+      const controlledByFilter = {
+        ...filter,
+        meta: {
+          ...filter.meta,
+          controlledBy: getControlledBy(embeddableId),
+        },
+      };
+      const mapEmbeddable = new MapEmbeddable(
+        {} as unknown as MapEmbeddableConfig,
+        {
+          id: embeddableId,
+          filters: [controlledByFilter],
+        } as unknown as MapEmbeddableInput
+      );
+      await untilInitialized(mapEmbeddable);
+
+      const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery');
+
+      mapEmbeddable.updateInput({
+        filters: [toggleFilterNegated(controlledByFilter)],
+      });
+
+      await onNextTick();
+
+      expect(fetchSpy).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('on searchSessionId change', () => {
+    test('should fetch when filterByMapExtent is false', async () => {
+      const mapEmbeddable = new MapEmbeddable(
+        {} as unknown as MapEmbeddableConfig,
+        {
+          id: 'map1',
+          filterByMapExtent: false,
+        } as unknown as MapEmbeddableInput
+      );
+      await untilInitialized(mapEmbeddable);
+
+      const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery');
+
+      mapEmbeddable.updateInput({
+        searchSessionId: uuidv4(),
+      });
+
+      await onNextTick();
+
+      expect(fetchSpy).toHaveBeenCalled();
+    });
+
+    test('should not fetch when filterByMapExtent is true', async () => {
+      const mapEmbeddable = new MapEmbeddable(
+        {} as unknown as MapEmbeddableConfig,
+        {
+          id: 'map1',
+          filterByMapExtent: true,
+        } as unknown as MapEmbeddableInput
+      );
+      await untilInitialized(mapEmbeddable);
+
+      const fetchSpy = jest.spyOn(mapEmbeddable, '_dispatchSetQuery');
+
+      mapEmbeddable.updateInput({
+        searchSessionId: uuidv4(),
+      });
+
+      await onNextTick();
+
+      expect(fetchSpy).not.toHaveBeenCalled();
+    });
+  });
+});
--- a/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx
+++ b/x-pack/plugins/maps/public/embeddable/map_embeddable.tsx
@ -14,7 +14,7 @@ import { render, unmountComponentAtNode } from 'react-dom';
 import { Subscription } from 'rxjs';
 import { Unsubscribe } from 'redux';
 import { EuiEmptyPrompt } from '@elastic/eui';
-import { type Filter, compareFilters, type TimeRange, type Query } from '@kbn/es-query';
+import { type Filter } from '@kbn/es-query';
 import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
 import {
  Embeddable,
@ -24,6 +24,7 @@ import {
  VALUE_CLICK_TRIGGER,
  omitGenericEmbeddableInput,
  FilterableEmbeddable,
+  shouldFetch$,
 } from '@kbn/embeddable-plugin/public';
 import { ActionExecutionContext } from '@kbn/ui-actions-plugin/public';
 import { APPLY_FILTER_TRIGGER } from '@kbn/data-plugin/public';
@ -104,6 +105,10 @@ function getIsRestore(searchSessionId?: string) {
  return searchSessionOptions ? searchSessionOptions.isRestore : false;
 }

+export function getControlledBy(id: string) {
+  return `mapEmbeddablePanel${id}`;
+}
+
 export class MapEmbeddable
  extends Embeddable<MapEmbeddableInput, MapEmbeddableOutput>
  implements ReferenceOrValueEmbeddable<MapByValueInput, MapByReferenceInput>, FilterableEmbeddable
@ -114,15 +119,10 @@ export class MapEmbeddable
  private _isActive: boolean;
  private _savedMap: SavedMap;
  private _renderTooltipContent?: RenderToolTipContent;
-  private _subscription: Subscription;
+  private _subscriptions: Subscription[] = [];
  private _prevIsRestore: boolean = false;
  private _prevMapExtent?: MapExtent;
-  private _prevTimeRange?: TimeRange;
-  private _prevTimeslice?: [number, number];
-  private _prevQuery?: Query;
-  private _prevFilters: Filter[] = [];
  private _prevSyncColors?: boolean;
-  private _prevSearchSessionId?: string;
  private _domNode?: HTMLElement;
  private _unsubscribeFromStore?: Unsubscribe;
  private _isInitialized = false;
@ -145,8 +145,8 @@ export class MapEmbeddable
    this._isActive = true;
    this._savedMap = new SavedMap({ mapEmbeddableInput: initialInput });
    this._initializeSaveMap();
-    this._subscription = this.getUpdated$().subscribe(() => this.onUpdate());
-    this._controlledBy = `mapEmbeddablePanel${this.id}`;
+    this._subscriptions.push(this.getUpdated$().subscribe(() => this.onUpdate()));
+    this._controlledBy = getControlledBy(this.id);
  }

  public reportsEmbeddableLoad() {
@ -193,9 +193,20 @@ export class MapEmbeddable
    // Passing callback into redux store instead of regular pattern of getting redux state changes for performance reasons
    store.dispatch(setOnMapMove(this._propogateMapMovement));

-    this._dispatchSetQuery({
-      forceRefresh: false,
-    });
+    this._dispatchSetQuery({ forceRefresh: false });
+    this._subscriptions.push(
+      shouldFetch$<MapEmbeddableInput>(this.getUpdated$(), () => {
+        return {
+          ...this.getInput(),
+          filters: this._getInputFilters(),
+          searchSessionId: this._getSearchSessionId(),
+        };
+      }).subscribe(() => {
+        this._dispatchSetQuery({
+          forceRefresh: false,
+        });
+      })
+    );

    const mapStateJSON = this._savedMap.getAttributes().mapStateJSON;
    if (mapStateJSON) {
@ -309,18 +320,6 @@ export class MapEmbeddable
  }

  onUpdate() {
-    if (
-      !_.isEqual(this.input.timeRange, this._prevTimeRange) ||
-      !_.isEqual(this.input.timeslice, this._prevTimeslice) ||
-      !_.isEqual(this.input.query, this._prevQuery) ||
-      !compareFilters(this._getFilters(), this._prevFilters) ||
-      this._getSearchSessionId() !== this._prevSearchSessionId
-    ) {
-      this._dispatchSetQuery({
-        forceRefresh: false,
-      });
-    }
-
    if (this.input.syncColors !== this._prevSyncColors) {
      this._dispatchSetChartsPaletteServiceGetColor(this.input.syncColors);
    }
@ -382,7 +381,7 @@ export class MapEmbeddable
    }
  };

-  _getFilters() {
+  _getInputFilters() {
    return this.input.filters
      ? this.input.filters.filter(
          (filter) => !filter.meta.disabled && filter.meta.controlledBy !== this._controlledBy
@ -401,15 +400,9 @@ export class MapEmbeddable
  }

  _dispatchSetQuery({ forceRefresh }: { forceRefresh: boolean }) {
-    const filters = this._getFilters();
-    this._prevTimeRange = this.input.timeRange;
-    this._prevTimeslice = this.input.timeslice;
-    this._prevQuery = this.input.query;
-    this._prevFilters = filters;
-    this._prevSearchSessionId = this._getSearchSessionId();
    this._savedMap.getStore().dispatch<any>(
      setQuery({
-        filters,
+        filters: this._getInputFilters(),
        query: this.input.query,
        timeFilters: this.input.timeRange,
        timeslice: this.input.timeslice
@ -675,9 +668,9 @@ export class MapEmbeddable
      unmountComponentAtNode(this._domNode);
    }

-    if (this._subscription) {
-      this._subscription.unsubscribe();
-    }
+    this._subscriptions.forEach((subscription) => {
+      subscription.unsubscribe();
+    });
  }

  reload() {
--- a/x-pack/plugins/maps/public/embeddable/types.ts
+++ b/x-pack/plugins/maps/public/embeddable/types.ts
@ -5,7 +5,6 @@
 * 2.0.
 */

-import type { Filter } from '@kbn/es-query';
 import type { DataView } from '@kbn/data-plugin/common';
 import {
  Embeddable,
@ -13,7 +12,7 @@ import {
  EmbeddableOutput,
  SavedObjectEmbeddableInput,
 } from '@kbn/embeddable-plugin/public';
-import type { Query, TimeRange } from '@kbn/es-query';
+import type { Filter, Query, TimeRange } from '@kbn/es-query';
 import { MapCenterAndZoom, MapExtent, MapSettings } from '../../common/descriptor_types';
 import { MapSavedObjectAttributes } from '../../common/map_saved_object_type';