diff --git a/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md b/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md new file mode 100644 index 00000000000000..474962e614aa7d --- /dev/null +++ b/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.getupdated_.md @@ -0,0 +1,17 @@ + + +[Home](./index.md) > [kibana-plugin-plugins-embeddable-public](./kibana-plugin-plugins-embeddable-public.md) > [Embeddable](./kibana-plugin-plugins-embeddable-public.embeddable.md) > [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 + +Signature: + +```typescript +getUpdated$(): Readonly>; +``` +Returns: + +`Readonly>` + diff --git a/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.md b/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.md index b1f1bed7541c37..4541afec29fa52 100644 --- a/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.md +++ b/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.md @@ -44,8 +44,9 @@ export declare abstract class Embeddablereload this stream is guarantied to emit later which allows to skip any state handling in case reload 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 reload is calledThe 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) | | | diff --git a/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.reload.md b/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.reload.md index e3b06f414cb5b4..7e2e9f982e505b 100644 --- a/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddable.reload.md +++ b/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$ + Signature: ```typescript diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index 86628abd924296..4d5a3fb9a8cc9f 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/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 = [ + '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() }), }); } }; diff --git a/src/plugins/discover/public/application/embeddable/search_embeddable.ts b/src/plugins/discover/public/application/embeddable/search_embeddable.ts index e592d0b0ec8fd0..b143afd1988e68 100644 --- a/src/plugins/discover/public/application/embeddable/search_embeddable.ts +++ b/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 diff --git a/src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx b/src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx index b020006c0c2bb0..c7c71656bceb22 100644 --- a/src/plugins/embeddable/public/lib/embeddables/embeddable.test.tsx +++ b/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); +}); diff --git a/src/plugins/embeddable/public/lib/embeddables/embeddable.tsx b/src/plugins/embeddable/public/lib/embeddables/embeddable.tsx index 31df5c5085f8ba..2173082d67d3e8 100644 --- a/src/plugins/embeddable/public/lib/embeddables/embeddable.tsx +++ b/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> { + return merge(this.getInput$().pipe(skip(1)), this.getOutput$().pipe(skip(1))).pipe( + debounceTime(0), + mapTo(undefined) + ); + } + public getInput$(): Readonly> { return this.input$.asObservable(); } diff --git a/src/plugins/embeddable/public/public.api.md b/src/plugins/embeddable/public/public.api.md index 1621dfa3f2c3c3..6f45d2781c9a3a 100644 --- a/src/plugins/embeddable/public/public.api.md +++ b/src/plugins/embeddable/public/public.api.md @@ -312,6 +312,7 @@ export abstract class Embeddable>; // (undocumented) readonly id: string; // (undocumented) diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts b/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts index c12a0f0759018d..f7592977858d25 100644 --- a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts +++ b/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; 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(); }; diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx index 3a3258a79c59f8..54517e4ee8c849 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx +++ b/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); }); diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index 56d471be63d3e2..e7d3e1a4bfa5b2 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/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); } } diff --git a/x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/async_search.ts b/x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/async_search.ts index 3a05d5c2853635..1e92019899b5a0 100644 --- a/x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/async_search.ts +++ b/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'); }); }); });