[Search] Fix dashboard embeddables don't refetch on searchSessionId change (#84261) (#85562)

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md

@@ -0,0 +1,17 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) &gt; [Embeddable](./kibana-plugin-plugins-embeddable-public.embeddable.md) &gt; [getUpdated$](./kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md)
+
+## Embeddable.getUpdated$() method
+
+Merges input$ and output$ streams and denounces emit till next macro-task Could be useful to batch reactions to input$ and output$ updates that happen separately but synchronously In case corresponding state change triggered `reload` this stream is guarantied to emit later which allows to skip any state handling in case `reload` already handled it
+
+<b>Signature:</b>
+
+```typescript
+getUpdated$(): Readonly<Rx.Observable<void>>;
+```
+<b>Returns:</b>
+
+`Readonly<Rx.Observable<void>>`
+

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.md

@@ -44,8 +44,9 @@ export declare abstract class Embeddable<TEmbeddableInput extends EmbeddableInpu
 |  [getOutput$()](./kibana-plugin-plugins-embeddable-public.embeddable.getoutput_.md) |  |  |
 |  [getRoot()](./kibana-plugin-plugins-embeddable-public.embeddable.getroot.md) |  | Returns the top most parent embeddable, or itself if this embeddable is not within a parent. |
 |  [getTitle()](./kibana-plugin-plugins-embeddable-public.embeddable.gettitle.md) |  |  |
+|  [getUpdated$()](./kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md) |  | Merges input$ and output$ streams and denounces emit till next macro-task Could be useful to batch reactions to input$ and output$ updates that happen separately but synchronously In case corresponding state change triggered <code>reload</code> this stream is guarantied to emit later which allows to skip any state handling in case <code>reload</code> already handled it |
 |  [onFatalError(e)](./kibana-plugin-plugins-embeddable-public.embeddable.onfatalerror.md) |  |  |
-|  [reload()](./kibana-plugin-plugins-embeddable-public.embeddable.reload.md) |  | Reload will be called when there is a request to refresh the data or view, even if the input data did not change. |
+|  [reload()](./kibana-plugin-plugins-embeddable-public.embeddable.reload.md) |  | Reload will be called when there is a request to refresh the data or view, even if the input data did not change.<!-- -->In case if input data did change and reload is requested input$ and output$ would still emit before <code>reload</code> is called<!-- -->The order would be as follows: input$ output$ reload() \-\-\-- updated$ |
 |  [render(el)](./kibana-plugin-plugins-embeddable-public.embeddable.render.md) |  |  |
 |  [supportedTriggers()](./kibana-plugin-plugins-embeddable-public.embeddable.supportedtriggers.md) |  |  |
 |  [updateInput(changes)](./kibana-plugin-plugins-embeddable-public.embeddable.updateinput.md) |  |  |

docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.reload.md

@@ -6,6 +6,10 @@
 
 Reload will be called when there is a request to refresh the data or view, even if the input data did not change.
 
+In case if input data did change and reload is requested input$ and output$ would still emit before `reload` is called
+
+The order would be as follows: input$ output$ reload() \-\-\-- updated$
+
 <b>Signature:</b>
 
 ```typescript

src/plugins/dashboard/public/application/dashboard_app_controller.tsx

@@ -633,9 +633,26 @@ export class DashboardAppController {
           removeQueryParam(history, DashboardConstants.SEARCH_SESSION_ID, true);
         }
 
+        // state keys change in which likely won't need a data fetch
+        const noRefetchKeys: Array<keyof DashboardContainerInput> = [
+          'viewMode',
+          'title',
+          'description',
+          'expandedPanelId',
+          'useMargins',
+          'isEmbeddedExternally',
+          'isFullScreenMode',
+          'isEmptyState',
+        ];
+
+        const shouldRefetch = Object.keys(changes).some(
+          (changeKey) => !noRefetchKeys.includes(changeKey as keyof DashboardContainerInput)
+        );
+
         dashboardContainer.updateInput({
           ...changes,
-          searchSessionId: searchService.session.start(),
+          // do not start a new session if this is irrelevant state change to prevent excessive searches
+          ...(shouldRefetch && { searchSessionId: searchService.session.start() }),
         });
       }
     };

src/plugins/discover/public/application/embeddable/search_embeddable.ts

@@ -19,7 +19,6 @@
 import './search_embeddable.scss';
 import angular from 'angular';
 import _ from 'lodash';
-import * as Rx from 'rxjs';
 import { Subscription } from 'rxjs';
 import { i18n } from '@kbn/i18n';
 import { UiActionsStart, APPLY_FILTER_TRIGGER } from '../../../../ui_actions/public';
@@ -98,6 +97,7 @@ export class SearchEmbeddable
   private prevTimeRange?: TimeRange;
   private prevFilters?: Filter[];
   private prevQuery?: Query;
+  private prevSearchSessionId?: string;
 
   constructor(
     {
@@ -140,7 +140,7 @@ export class SearchEmbeddable
       .timefilter.getAutoRefreshFetch$()
       .subscribe(this.fetch);
 
-    this.subscription = Rx.merge(this.getOutput$(), this.getInput$()).subscribe(() => {
+    this.subscription = this.getUpdated$().subscribe(() => {
       this.panelTitle = this.output.title || '';
 
       if (this.searchScope) {
@@ -262,7 +262,8 @@ export class SearchEmbeddable
   }
 
   public reload() {
-    this.fetch();
+    if (this.searchScope)
+      this.pushContainerStateParamsToScope(this.searchScope, { forceFetch: true });
   }
 
   private fetch = async () => {
@@ -326,12 +327,16 @@ export class SearchEmbeddable
     }
   };
 
-  private pushContainerStateParamsToScope(searchScope: SearchScope) {
+  private pushContainerStateParamsToScope(
+    searchScope: SearchScope,
+    { forceFetch = false }: { forceFetch: boolean } = { forceFetch: false }
+  ) {
     const isFetchRequired =
       !esFilters.onlyDisabledFiltersChanged(this.input.filters, this.prevFilters) ||
       !_.isEqual(this.prevQuery, this.input.query) ||
       !_.isEqual(this.prevTimeRange, this.input.timeRange) ||
-      !_.isEqual(searchScope.sort, this.input.sort || this.savedSearch.sort);
+      !_.isEqual(searchScope.sort, this.input.sort || this.savedSearch.sort) ||
+      this.prevSearchSessionId !== this.input.searchSessionId;
 
     // If there is column or sort data on the panel, that means the original columns or sort settings have
     // been overridden in a dashboard.
@@ -339,13 +344,14 @@ export class SearchEmbeddable
     searchScope.sort = this.input.sort || this.savedSearch.sort;
     searchScope.sharedItemTitle = this.panelTitle;
 
-    if (isFetchRequired) {
+    if (forceFetch || isFetchRequired) {
       this.filtersSearchSource!.setField('filter', this.input.filters);
       this.filtersSearchSource!.setField('query', this.input.query);
+
       this.prevFilters = this.input.filters;
       this.prevQuery = this.input.query;
       this.prevTimeRange = this.input.timeRange;
-
+      this.prevSearchSessionId = this.input.searchSessionId;
       this.fetch();
     } else if (this.searchScope) {
       // trigger a digest cycle to make sure non-fetch relevant changes are propagated

src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx

@@ -19,7 +19,7 @@
 
 /* eslint-disable max-classes-per-file */
 
-import { skip } from 'rxjs/operators';
+import { skip, take } from 'rxjs/operators';
 import { Embeddable } from './embeddable';
 import { EmbeddableOutput, EmbeddableInput } from './i_embeddable';
 import { ViewMode } from '../types';
@@ -112,3 +112,34 @@ test('updating output state retains instance information', async () => {
   expect(outputTest.getOutput().inputUpdatedTimes).toBe(2);
   expect(outputTest.getOutput().testClass).toBeInstanceOf(TestClass);
 });
+
+test('updated$ called after reload and batches input/output changes', async () => {
+  const hello = new ContactCardEmbeddable(
+    { id: '123', firstName: 'Brienne', lastName: 'Tarth' },
+    { execAction: (() => null) as any }
+  );
+
+  const reloadSpy = jest.spyOn(hello, 'reload');
+
+  const input$ = hello.getInput$().pipe(skip(1));
+  let inputEmittedTimes = 0;
+  input$.subscribe(() => {
+    inputEmittedTimes++;
+  });
+
+  const updated$ = hello.getUpdated$();
+  let updatedEmittedTimes = 0;
+  updated$.subscribe(() => {
+    updatedEmittedTimes++;
+  });
+  const updatedPromise = updated$.pipe(take(1)).toPromise();
+
+  hello.updateInput({ nameTitle: 'Sir', lastReloadRequestTime: Date.now() });
+  expect(reloadSpy).toHaveBeenCalledTimes(1);
+  expect(inputEmittedTimes).toBe(1);
+  expect(updatedEmittedTimes).toBe(0);
+
+  await updatedPromise;
+
+  expect(updatedEmittedTimes).toBe(1);
+});

src/plugins/embeddable/public/lib/embeddables/embeddable.tsx

@@ -19,7 +19,8 @@
 
 import { cloneDeep, isEqual } from 'lodash';
 import * as Rx from 'rxjs';
-import { distinctUntilChanged, map } from 'rxjs/operators';
+import { merge } from 'rxjs';
+import { debounceTime, distinctUntilChanged, map, mapTo, skip } from 'rxjs/operators';
 import { RenderCompleteDispatcher } from '../../../../kibana_utils/public';
 import { Adapters } from '../types';
 import { IContainer } from '../containers';
@@ -104,9 +105,31 @@ export abstract class Embeddable<
   /**
    * Reload will be called when there is a request to refresh the data or view, even if the
    * input data did not change.
+   *
+   * In case if input data did change and reload is requested input$ and output$ would still emit before `reload` is called
+   *
+   * The order would be as follows:
+   * input$
+   * output$
+   * reload()
+   * ----
+   * updated$
    */
   public abstract reload(): void;
 
+  /**
+   * Merges input$ and output$ streams and debounces emit till next macro-task.
+   * Could be useful to batch reactions to input$ and output$ updates that happen separately but synchronously.
+   * In case corresponding state change triggered `reload` this stream is guarantied to emit later,
+   * which allows to skip any state handling in case `reload` already handled it.
+   */
+  public getUpdated$(): Readonly<Rx.Observable<void>> {
+    return merge(this.getInput$().pipe(skip(1)), this.getOutput$().pipe(skip(1))).pipe(
+      debounceTime(0),
+      mapTo(undefined)
+    );
+  }
+
   public getInput$(): Readonly<Rx.Observable<TEmbeddableInput>> {
     return this.input$.asObservable();
   }

src/plugins/embeddable/public/public.api.md

@@ -312,6 +312,7 @@ export abstract class Embeddable<TEmbeddableInput extends EmbeddableInput = Embe
     getRoot(): IEmbeddable | IContainer;
     // (undocumented)
     getTitle(): string;
+    getUpdated$(): Readonly<Rx.Observable<void>>;
     // (undocumented)
     readonly id: string;
     // (undocumented)

src/plugins/visualizations/public/embeddable/visualize_embeddable.ts

@@ -19,7 +19,6 @@
 
 import _, { get } from 'lodash';
 import { Subscription } from 'rxjs';
-import * as Rx from 'rxjs';
 import { i18n } from '@kbn/i18n';
 import { VISUALIZE_EMBEDDABLE_TYPE } from './constants';
 import {
@@ -100,6 +99,7 @@ export class VisualizeEmbeddable
   private timeRange?: TimeRange;
   private query?: Query;
   private filters?: Filter[];
+  private searchSessionId?: string;
   private visCustomizations?: Pick<VisualizeInput, 'vis' | 'table'>;
   private subscriptions: Subscription[] = [];
   private expression: string = '';
@@ -155,8 +155,11 @@ export class VisualizeEmbeddable
       .subscribe(this.updateHandler.bind(this));
 
     this.subscriptions.push(
-      Rx.merge(this.getOutput$(), this.getInput$()).subscribe(() => {
-        this.handleChanges();
+      this.getUpdated$().subscribe(() => {
+        const isDirty = this.handleChanges();
+        if (isDirty && this.handler) {
+          this.updateHandler();
+        }
       })
     );
 
@@ -220,7 +223,7 @@ export class VisualizeEmbeddable
     }
   }
 
-  public async handleChanges() {
+  private handleChanges(): boolean {
     this.transferCustomizationsToUiState();
 
     let dirty = false;
@@ -243,13 +246,16 @@ export class VisualizeEmbeddable
       dirty = true;
     }
 
+    if (this.searchSessionId !== this.input.searchSessionId) {
+      this.searchSessionId = this.input.searchSessionId;
+      dirty = true;
+    }
+
     if (this.vis.description && this.domNode) {
       this.domNode.setAttribute('data-description', this.vis.description);
     }
 
-    if (this.handler && dirty) {
-      this.updateHandler();
-    }
+    return dirty;
   }
 
   // this is a hack to make editor still work, will be removed once we clean up editor
@@ -395,6 +401,7 @@ export class VisualizeEmbeddable
   }
 
   private handleVisUpdate = async () => {
+    this.handleChanges();
     this.updateHandler();
   };
 

x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx

@@ -205,6 +205,8 @@ describe('embeddable', () => {
     await embeddable.initializeSavedVis({ id: '123' } as LensEmbeddableInput);
     embeddable.render(mountpoint);
 
+    expect(expressionRenderer).toHaveBeenCalledTimes(1);
+
     embeddable.updateInput({
       timeRange,
       query,
@@ -212,6 +214,10 @@ describe('embeddable', () => {
       searchSessionId: 'searchSessionId',
     });
 
+    expect(expressionRenderer).toHaveBeenCalledTimes(1);
+
+    await new Promise((resolve) => setTimeout(resolve, 0));
+
     expect(expressionRenderer).toHaveBeenCalledTimes(2);
   });
 

x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx

@@ -91,7 +91,7 @@ export class Embeddable
     timeRange?: TimeRange;
     query?: Query;
     filters?: Filter[];
-    lastReloadRequestTime?: number;
+    searchSessionId?: string;
   } = {};
 
   constructor(
@@ -110,7 +110,9 @@ export class Embeddable
 
     this.expressionRenderer = deps.expressionRenderer;
     this.initializeSavedVis(initialInput).then(() => this.onContainerStateChanged(initialInput));
-    this.subscription = this.getInput$().subscribe((input) => this.onContainerStateChanged(input));
+    this.subscription = this.getUpdated$().subscribe(() =>
+      this.onContainerStateChanged(this.input)
+    );
 
     this.autoRefreshFetchSubscription = deps.timefilter
       .getAutoRefreshFetch$()
@@ -162,23 +164,29 @@ export class Embeddable
   }
 
   onContainerStateChanged(containerState: LensEmbeddableInput) {
+    if (this.handleContainerStateChanged(containerState)) this.reload();
+  }
+
+  handleContainerStateChanged(containerState: LensEmbeddableInput): boolean {
+    let isDirty = false;
     const cleanedFilters = containerState.filters
       ? containerState.filters.filter((filter) => !filter.meta.disabled)
       : undefined;
     if (
       !_.isEqual(containerState.timeRange, this.externalSearchContext.timeRange) ||
       !_.isEqual(containerState.query, this.externalSearchContext.query) ||
-      !_.isEqual(cleanedFilters, this.externalSearchContext.filters)
+      !_.isEqual(cleanedFilters, this.externalSearchContext.filters) ||
+      this.externalSearchContext.searchSessionId !== containerState.searchSessionId
     ) {
       this.externalSearchContext = {
         timeRange: containerState.timeRange,
         query: containerState.query,
-        lastReloadRequestTime: this.externalSearchContext.lastReloadRequestTime,
         filters: cleanedFilters,
+        searchSessionId: containerState.searchSessionId,
       };
-
-      this.reload();
+      isDirty = true;
     }
+    return isDirty;
   }
 
   private updateActiveData = (
@@ -205,7 +213,7 @@ export class Embeddable
         expression={this.expression || null}
         searchContext={this.getMergedSearchContext()}
         variables={input.palette ? { theme: { palette: input.palette } } : {}}
-        searchSessionId={this.input.searchSessionId}
+        searchSessionId={this.externalSearchContext.searchSessionId}
         handleEvent={this.handleEvent}
         onData$={this.updateActiveData}
         renderMode={input.renderMode}
@@ -259,16 +267,9 @@ export class Embeddable
   };
 
   async reload() {
-    const currentTime = Date.now();
-    if (this.externalSearchContext.lastReloadRequestTime !== currentTime) {
-      this.externalSearchContext = {
-        ...this.externalSearchContext,
-        lastReloadRequestTime: currentTime,
-      };
-
-      if (this.domNode) {
-        this.render(this.domNode);
-      }
+    this.handleContainerStateChanged(this.input);
+    if (this.domNode) {
+      this.render(this.domNode);
     }
   }
 

x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/async_search.ts

@@ -125,6 +125,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         await testSubjects.missingOrFail('embeddableErrorLabel');
         const data = await PageObjects.visChart.getBarChartData('Sum of bytes');
         expect(data.length).to.be(5);
+
+        // switching dashboard to edit mode (or any other non-fetch required) state change
+        // should leave session state untouched
+        await PageObjects.dashboard.switchToEditMode();
+        await sendToBackground.expectState('restored');
       });
     });
   });