Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 5s minRefreshInterval to timefilter with validation #188881

Merged
merged 29 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6bbdd7f
Add 5s min refresh interval to timefilter with validation
nickofthyme Jul 22, 2024
85eb553
Merge branch 'main' into min-refresh-interval
nickofthyme Jul 29, 2024
f37e7f4
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Jul 29, 2024
2be42e1
remove `setMinRefreshInterval` method
nickofthyme Aug 7, 2024
44e5bbd
remove data dep from navigation plugin
nickofthyme Aug 7, 2024
ea3519e
Merge branch 'main' into min-refresh-interval
nickofthyme Aug 7, 2024
b858a86
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 7, 2024
ca79323
add config option for minRefreshInterval default
nickofthyme Aug 8, 2024
2187cc3
change `QueryService` service to allow usage in `security_solution` p…
nickofthyme Aug 8, 2024
00451eb
fix: jest tests and type errors
nickofthyme Aug 8, 2024
986728e
Merge branch 'main' into min-refresh-interval
nickofthyme Aug 8, 2024
64ed4dc
test: update and add timefilter tests
nickofthyme Aug 8, 2024
b3f7e55
chore: fix type errors in tests
nickofthyme Aug 8, 2024
1edd8d0
fix expected config check
nickofthyme Aug 8, 2024
e1c065a
Merge branch 'main' into min-refresh-interval
nickofthyme Aug 8, 2024
99760dd
fix more tests
nickofthyme Aug 8, 2024
ec3bc54
fix config test
nickofthyme Aug 9, 2024
b4622ce
Merge branch 'main' into min-refresh-interval
nickofthyme Aug 9, 2024
d3aebad
pause when interval is 0, cleanup default intervals in test code
nickofthyme Aug 9, 2024
ce76a1f
Merge branch 'main' into min-refresh-interval
nickofthyme Aug 12, 2024
59b39bc
revert SO changes, add and fix tests
nickofthyme Aug 12, 2024
e852cf0
fix: failing to start refresh loop on browser refresh
nickofthyme Aug 14, 2024
6612e77
chore: update discover data loop check
nickofthyme Aug 14, 2024
ffb3dc5
Merge branch 'main' into min-refresh-interval
nickofthyme Aug 14, 2024
5892855
update config optional type
nickofthyme Aug 14, 2024
3cc60d8
Merge branch 'main' into min-refresh-interval
mbondyra Aug 19, 2024
ce93360
Merge branch 'main' into min-refresh-interval
mbondyra Aug 20, 2024
155744c
Merge branch 'main' into min-refresh-interval
nickofthyme Aug 22, 2024
40509ff
Merge branch 'main' into min-refresh-interval
nickofthyme Aug 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/plugins/data/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
/**
Expand Down Expand Up @@ -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.
Expand All @@ -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>;
4 changes: 3 additions & 1 deletion src/plugins/data/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved

this.storage = new Storage(window.localStorage);
this.nowProvider = new NowProvider();
Expand Down
5 changes: 4 additions & 1 deletion src/plugins/data/public/query/query_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 10 additions & 1 deletion src/plugins/data/public/query/query_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ interface QueryServiceSetupDependencies {
storage: IStorageWrapper;
uiSettings: IUiSettingsClient;
nowProvider: NowProviderInternalContract;
minRefreshInterval?: number;
}

interface QueryServiceStartDependencies {
Expand Down Expand Up @@ -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 {
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
this.filterManager = new FilterManager(uiSettings);

const timefilterService = new TimefilterService(nowProvider);
this.timefilter = timefilterService.setup({
uiSettings,
storage,
minRefreshInterval,
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
});

this.queryStringManager = new QueryStringManager(storage, uiSettings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand Down
110 changes: 97 additions & 13 deletions src/plugins/data/public/query/timefilter/timefilter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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();
Expand All @@ -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', () => {
Expand All @@ -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);
});
Expand All @@ -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);
Expand Down
Loading