Add 5s minRefreshInterval to timefilter with validation (#188881)

Adds enforcement for the global time filter to limit the min refresh interval.
elastic · Aug 23, 2024 · 94b657e · 94b657e
1 parent af4f482
commit 94b657e
Show file tree

Hide file tree

Showing 27 changed files with 236 additions and 45 deletions.
diff --git a/src/plugins/data/config.ts b/src/plugins/data/config.ts
@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import { schema, TypeOf } from '@kbn/config-schema';
+import { offeringBasedSchema, schema, TypeOf } from '@kbn/config-schema';
 
 export const searchSessionsConfigSchema = schema.object({
   /**
@@ -84,7 +84,23 @@ export const searchConfigSchema = schema.object({
   sessions: searchSessionsConfigSchema,
 });
 
+export const queryConfigSchema = schema.object({
+  /**
+   * Config for timefilter
+   */
+  timefilter: schema.object({
+    /**
+     * Lower limit of refresh interval (in milliseconds)
+     */
+    minRefreshInterval: offeringBasedSchema<number>({
+      serverless: schema.number({ min: 1000, defaultValue: 5000 }),
+      traditional: schema.number({ min: 1000, defaultValue: 1000 }),
+    }),
+  }),
+});
+
 export const configSchema = schema.object({
+  query: queryConfigSchema,
   search: searchConfigSchema,
   /**
    * Turns on/off limit validations for the registered uiSettings.
@@ -96,4 +112,6 @@ export type ConfigSchema = TypeOf<typeof configSchema>;
 
 export type SearchConfigSchema = TypeOf<typeof searchConfigSchema>;
 
+export type QueryConfigSchema = TypeOf<typeof queryConfigSchema>;
+
 export type SearchSessionsConfigSchema = TypeOf<typeof searchSessionsConfigSchema>;
diff --git a/src/plugins/data/public/plugin.ts b/src/plugins/data/public/plugin.ts
@@ -59,7 +59,9 @@ export class DataPublicPlugin
 
   constructor(initializerContext: PluginInitializerContext<ConfigSchema>) {
     this.searchService = new SearchService(initializerContext);
-    this.queryService = new QueryService();
+    this.queryService = new QueryService(
+      initializerContext.config.get().query.timefilter.minRefreshInterval
+    );
 
     this.storage = new Storage(window.localStorage);
     this.nowProvider = new NowProvider();

diff --git a/src/plugins/data/public/query/query_service.test.ts b/src/plugins/data/public/query/query_service.test.ts
@@ -18,6 +18,8 @@ import { StubBrowserStorage } from '@kbn/test-jest-helpers';
 import { TimefilterContract } from './timefilter';
 import { createNowProviderMock } from '../now_provider/mocks';
 
+const minRefreshIntervalDefault = 1000;
+
 const setupMock = coreMock.createSetup();
 const startMock = coreMock.createStart();
 
@@ -48,6 +50,7 @@ describe('query_service', () => {
       uiSettings: setupMock.uiSettings,
       storage: new Storage(new StubBrowserStorage()),
       nowProvider: createNowProviderMock(),
+      minRefreshInterval: minRefreshIntervalDefault,
     });
     queryServiceStart = queryService.start({
       uiSettings: setupMock.uiSettings,
@@ -82,7 +85,7 @@ describe('query_service', () => {
     const filters = [getFilter(FilterStateStore.GLOBAL_STATE, true, true, 'key1', 'value1')];
     const query = { language: 'kql', query: 'query' };
     const time = { from: new Date().toISOString(), to: new Date().toISOString() };
-    const refreshInterval = { pause: false, value: 10 };
+    const refreshInterval = { pause: false, value: 2000 };
 
     filterManager.setFilters(filters);
     queryStringManager.setQuery(query);

diff --git a/src/plugins/data/public/query/query_service.ts b/src/plugins/data/public/query/query_service.ts
@@ -39,6 +39,7 @@ interface QueryServiceSetupDependencies {
   storage: IStorageWrapper;
   uiSettings: IUiSettingsClient;
   nowProvider: NowProviderInternalContract;
+  minRefreshInterval?: number;
 }
 
 interface QueryServiceStartDependencies {
@@ -81,13 +82,21 @@ export class QueryService implements PersistableStateService<QueryState> {
 
   state$!: QueryState$;
 
-  public setup({ storage, uiSettings, nowProvider }: QueryServiceSetupDependencies): QuerySetup {
+  constructor(private minRefreshInterval: number = 5000) {}
+
+  public setup({
+    storage,
+    uiSettings,
+    nowProvider,
+    minRefreshInterval = this.minRefreshInterval,
+  }: QueryServiceSetupDependencies): QuerySetup {
     this.filterManager = new FilterManager(uiSettings);
 
     const timefilterService = new TimefilterService(nowProvider);
     this.timefilter = timefilterService.setup({
       uiSettings,
       storage,
+      minRefreshInterval,
     });
 
     this.queryStringManager = new QueryStringManager(storage, uiSettings);

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
@@ -66,7 +66,7 @@ describe('connect_to_global_state', () => {
   let aF2: Filter;
 
   beforeEach(() => {
-    const queryService = new QueryService();
+    const queryService = new QueryService(1000);
     queryService.setup({
       uiSettings: setupMock.uiSettings,
       storage: new Storage(new StubBrowserStorage()),
@@ -120,11 +120,21 @@ describe('connect_to_global_state', () => {
   });
 
   test('when refresh interval changes, state container contains updated refresh interval', () => {
+    const stop = connectToQueryGlobalState(queryServiceStart, globalState);
+    timeFilter.setRefreshInterval({ pause: true, value: 5000 });
+    expect(globalState.get().refreshInterval).toEqual({
+      pause: true,
+      value: 5000,
+    });
+    stop();
+  });
+
+  test('when refresh interval is set below min, state container contains min refresh interval', () => {
     const stop = connectToQueryGlobalState(queryServiceStart, globalState);
     timeFilter.setRefreshInterval({ pause: true, value: 100 });
     expect(globalState.get().refreshInterval).toEqual({
       pause: true,
-      value: 100,
+      value: 1000,
     });
     stop();
   });
@@ -135,14 +145,14 @@ describe('connect_to_global_state', () => {
     globalState.set({
       ...globalState.get(),
       filters: [gF1, gF2],
-      refreshInterval: { pause: true, value: 100 },
+      refreshInterval: { pause: true, value: 5000 },
       time: { from: 'now-30m', to: 'now' },
     });
 
     expect(globalStateChangeTriggered).toBeCalledTimes(1);
 
     expect(filterManager.getGlobalFilters()).toHaveLength(2);
-    expect(timeFilter.getRefreshInterval()).toEqual({ pause: true, value: 100 });
+    expect(timeFilter.getRefreshInterval()).toEqual({ pause: true, value: 5000 });
     expect(timeFilter.getTime()).toEqual({ from: 'now-30m', to: 'now' });
     stop();
   });

diff --git a/src/plugins/data/public/query/state_sync/sync_state_with_url.test.ts b/src/plugins/data/public/query/state_sync/sync_state_with_url.test.ts
@@ -25,6 +25,8 @@ import { syncQueryStateWithUrl } from './sync_state_with_url';
 import { GlobalQueryStateFromUrl } from './types';
 import { createNowProviderMock } from '../../now_provider/mocks';
 
+const minRefreshIntervalDefault = 1000;
+
 const setupMock = coreMock.createSetup();
 const startMock = coreMock.createStart();
 
@@ -65,6 +67,7 @@ describe('sync_query_state_with_url', () => {
       uiSettings: setupMock.uiSettings,
       storage: new Storage(new StubBrowserStorage()),
       nowProvider: createNowProviderMock(),
+      minRefreshInterval: minRefreshIntervalDefault,
     });
     queryServiceStart = queryService.start({
       uiSettings: startMock.uiSettings,
@@ -93,7 +96,7 @@ describe('sync_query_state_with_url', () => {
     filterManager.setFilters([gF, aF]);
     kbnUrlStateStorage.kbnUrlControls.flush(); // sync force location change
     expect(history.location.hash).toMatchInlineSnapshot(
-      `"#?_g=(filters:!(('$state':(store:globalState),meta:(alias:!n,disabled:!t,index:'logstash-*',key:query,negate:!t,type:custom,value:'%7B%22match%22:%7B%22key1%22:%22value1%22%7D%7D'),query:(match:(key1:value1)))),refreshInterval:(pause:!t,value:0),time:(from:now-15m,to:now))"`
+      `"#?_g=(filters:!(('$state':(store:globalState),meta:(alias:!n,disabled:!t,index:'logstash-*',key:query,negate:!t,type:custom,value:'%7B%22match%22:%7B%22key1%22:%22value1%22%7D%7D'),query:(match:(key1:value1)))),refreshInterval:(pause:!t,value:1000),time:(from:now-15m,to:now))"`
     );
     stop();
   });
@@ -116,11 +119,21 @@ describe('sync_query_state_with_url', () => {
   });
 
   test('when refresh interval changes, refresh interval is synced to urlStorage', () => {
+    const { stop } = syncQueryStateWithUrl(queryServiceStart, kbnUrlStateStorage);
+    timefilter.setRefreshInterval({ pause: true, value: 5000 });
+    expect(kbnUrlStateStorage.get<GlobalQueryStateFromUrl>('_g')?.refreshInterval).toEqual({
+      pause: true,
+      value: 5000,
+    });
+    stop();
+  });
+
+  test('when refresh interval is set below min, refresh interval is not synced to urlStorage', () => {
     const { stop } = syncQueryStateWithUrl(queryServiceStart, kbnUrlStateStorage);
     timefilter.setRefreshInterval({ pause: true, value: 100 });
     expect(kbnUrlStateStorage.get<GlobalQueryStateFromUrl>('_g')?.refreshInterval).toEqual({
       pause: true,
-      value: 100,
+      value: minRefreshIntervalDefault,
     });
     stop();
   });

diff --git a/src/plugins/data/public/query/timefilter/timefilter.test.ts b/src/plugins/data/public/query/timefilter/timefilter.test.ts
@@ -17,15 +17,23 @@ import { RefreshInterval } from '../../../common';
 import { createNowProviderMock } from '../../now_provider/mocks';
 
 import { timefilterServiceMock } from './timefilter_service.mock';
+import { TimefilterConfig } from './types';
+
 const timefilterSetupMock = timefilterServiceMock.createSetupContract();
+const minRefreshIntervalDefault = 1000;
 
-const timefilterConfig = {
+const defaultTimefilterConfig: TimefilterConfig = {
   timeDefaults: { from: 'now-15m', to: 'now' },
-  refreshIntervalDefaults: { pause: false, value: 0 },
+  refreshIntervalDefaults: { pause: false, value: minRefreshIntervalDefault },
+  minRefreshIntervalDefault,
 };
 
 const nowProviderMock = createNowProviderMock();
-const timefilter = new Timefilter(timefilterConfig, timefilterSetupMock.history, nowProviderMock);
+let timefilter = new Timefilter(
+  defaultTimefilterConfig,
+  timefilterSetupMock.history,
+  nowProviderMock
+);
 
 function stubNowTime(nowTime: any) {
   nowProviderMock.get.mockImplementation(() => (nowTime ? new Date(nowTime) : new Date()));
@@ -118,21 +126,28 @@ describe('setRefreshInterval', () => {
   let fetchSub: Subscription;
   let refreshSub: Subscription;
   let autoRefreshSub: Subscription;
+  let prevTimefilter = timefilter;
 
-  beforeEach(() => {
+  function setup() {
     update = sinon.spy();
     fetch = sinon.spy();
     autoRefreshFetch = sinon.spy((done) => done());
     timefilter.setRefreshInterval({
       pause: false,
-      value: 0,
+      value: minRefreshIntervalDefault,
     });
     refreshSub = timefilter.getRefreshIntervalUpdate$().subscribe(update);
     fetchSub = timefilter.getFetch$().subscribe(fetch);
     autoRefreshSub = timefilter.getAutoRefreshFetch$().subscribe(autoRefreshFetch);
+  }
+
+  beforeEach(() => {
+    prevTimefilter = timefilter;
+    setup();
   });
 
   afterEach(() => {
+    timefilter = prevTimefilter;
     refreshSub.unsubscribe();
     fetchSub.unsubscribe();
     autoRefreshSub.unsubscribe();
@@ -142,16 +157,50 @@ describe('setRefreshInterval', () => {
     expect(timefilter.isRefreshIntervalTouched()).toBe(false);
   });
 
+  test('should limit initial default value to minRefreshIntervalDefault', () => {
+    timefilter = new Timefilter(
+      {
+        ...defaultTimefilterConfig,
+        refreshIntervalDefaults: { pause: false, value: minRefreshIntervalDefault - 100 },
+      },
+      timefilterSetupMock.history,
+      nowProviderMock
+    );
+    setup();
+
+    expect(timefilter.isRefreshIntervalTouched()).toBe(false);
+    expect(timefilter.getRefreshInterval()).toEqual({
+      pause: false,
+      value: minRefreshIntervalDefault,
+    });
+  });
+
+  test('should pause if initial value is set to 0 regardless of minRefreshInterval', () => {
+    timefilter = new Timefilter(
+      {
+        ...defaultTimefilterConfig,
+        refreshIntervalDefaults: { pause: false, value: 0 },
+      },
+      timefilterSetupMock.history,
+      nowProviderMock
+    );
+
+    expect(timefilter.getRefreshInterval()).toEqual({
+      pause: true,
+      value: minRefreshIntervalDefault,
+    });
+  });
+
   test('should register changes to the initial interval', () => {
-    timefilter.setRefreshInterval(timefilterConfig.refreshIntervalDefaults);
+    timefilter.setRefreshInterval(defaultTimefilterConfig.refreshIntervalDefaults);
     expect(timefilter.isRefreshIntervalTouched()).toBe(false);
-    timefilter.setRefreshInterval({ pause: false, value: 1000 });
+    timefilter.setRefreshInterval({ pause: false, value: 5000 });
     expect(timefilter.isRefreshIntervalTouched()).toBe(true);
   });
 
   test('should update refresh interval', () => {
-    timefilter.setRefreshInterval({ pause: true, value: 10 });
-    expect(timefilter.getRefreshInterval()).toEqual({ pause: true, value: 10 });
+    timefilter.setRefreshInterval({ pause: true, value: 5000 });
+    expect(timefilter.getRefreshInterval()).toEqual({ pause: true, value: 5000 });
   });
 
   test('should not add unexpected object keys to refreshInterval state', () => {
@@ -165,23 +214,40 @@ describe('setRefreshInterval', () => {
   });
 
   test('should allow partial updates to refresh interval', () => {
-    timefilter.setRefreshInterval({ value: 10 });
-    expect(timefilter.getRefreshInterval()).toEqual({ pause: true, value: 10 });
+    const { pause } = timefilter.getRefreshInterval();
+    timefilter.setRefreshInterval({ value: 5000 });
+    expect(timefilter.getRefreshInterval()).toEqual({ pause, value: 5000 });
   });
 
   test('should not allow negative intervals', () => {
     timefilter.setRefreshInterval({ value: -10 });
-    expect(timefilter.getRefreshInterval()).toEqual({ pause: true, value: 0 });
+    expect(timefilter.getRefreshInterval()).toEqual({ pause: false, value: 1000 });
   });
 
   test('should set pause to true when interval is changed to zero from non-zero', () => {
+    timefilter = new Timefilter(
+      {
+        ...defaultTimefilterConfig,
+        minRefreshIntervalDefault: 0,
+      },
+      timefilterSetupMock.history,
+      nowProviderMock
+    );
+    setup();
+
     timefilter.setRefreshInterval({ value: 1000, pause: false });
     timefilter.setRefreshInterval({ value: 0, pause: false });
     expect(timefilter.getRefreshInterval()).toEqual({ pause: true, value: 0 });
   });
 
+  test('should pause when interval is set to zero regardless of minRefreshInterval', () => {
+    timefilter.setRefreshInterval({ value: 5000, pause: false });
+    timefilter.setRefreshInterval({ value: 0 });
+    expect(timefilter.getRefreshInterval()).toEqual({ pause: true, value: 1000 });
+  });
+
   test('not emit anything if nothing has changed', () => {
-    timefilter.setRefreshInterval({ pause: false, value: 0 });
+    timefilter.setRefreshInterval(timefilter.getRefreshInterval());
     expect(update.called).toBe(false);
     expect(fetch.called).toBe(false);
   });
@@ -192,7 +258,25 @@ describe('setRefreshInterval', () => {
     expect(fetch.called).toBe(false);
   });
 
+  test('should not emit update, nor fetch, when setting interval below min', () => {
+    const prevInterval = timefilter.getRefreshInterval();
+    timefilter.setRefreshInterval({ value: minRefreshIntervalDefault - 100 });
+    expect(update.called).toBe(false);
+    expect(fetch.called).toBe(false);
+    expect(timefilter.getRefreshInterval()).toEqual(prevInterval);
+  });
+
   test('emit update, not fetch, when switching to value: 0', () => {
+    timefilter = new Timefilter(
+      {
+        ...defaultTimefilterConfig,
+        minRefreshIntervalDefault: 0,
+      },
+      timefilterSetupMock.history,
+      nowProviderMock
+    );
+    setup();
+
     timefilter.setRefreshInterval({ pause: false, value: 5000 });
     expect(update.calledOnce).toBe(true);
     expect(fetch.calledOnce).toBe(true);