Loki query editor: update CompletionDataProvider time range when it changes (#94905)

* Loki query editor: update CompletionDataProvider time range when it changes

* CompletionDataProvider: update test and add regression

* Formatting

* Completion Data Provider: clear cache when the time range changes

* Completion Data Provider: specifically test for undefined values vs empty string
This commit is contained in:
Matias Chomicki 2024-10-21 19:14:27 +02:00 committed by GitHub
parent f3a93a0303
commit 89c215a9ff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 64 additions and 17 deletions

View File

@ -57,11 +57,14 @@ describe('Language completion provider', () => {
it('should again fetch labels on second start with different timerange', async () => {
const languageProvider = new LanguageProvider(datasource);
jest.mocked(datasource.getTimeRangeParams).mockRestore();
const fetchSpy = jest.spyOn(languageProvider, 'fetchLabels').mockResolvedValue([]);
await languageProvider.start();
expect(fetchSpy).toHaveBeenCalledTimes(1);
await languageProvider.start(mockTimeRange);
expect(fetchSpy).toHaveBeenCalledTimes(2);
await languageProvider.start(mockTimeRange);
expect(fetchSpy).toHaveBeenCalledTimes(2);
});
});

View File

@ -57,11 +57,14 @@ export default class LokiLanguageProvider extends LanguageProvider {
*/
start = (timeRange?: TimeRange) => {
const range = timeRange ?? this.getDefaultTimeRange();
const newRangeParams = this.datasource.getTimeRangeParams(range);
const prevRangeParams = this.startedTimeRange ? this.datasource.getTimeRangeParams(this.startedTimeRange) : null;
// refetch labels if either there's not already a start task or the time range has changed
if (
!this.startTask ||
this.startedTimeRange?.from.isSame(range.from) === false ||
this.startedTimeRange?.to.isSame(range.to) === false
!prevRangeParams ||
newRangeParams.start !== prevRangeParams.start ||
newRangeParams.end !== prevRangeParams.end
) {
this.startedTimeRange = range;
this.startTask = this.fetchLabels({ timeRange: range }).then(() => {

View File

@ -119,6 +119,7 @@ const MonacoQueryField = ({
const historyRef = useLatest(history);
const onRunQueryRef = useLatest(onRunQuery);
const onBlurRef = useLatest(onBlur);
const completionDataProviderRef = useRef<CompletionDataProvider | null>(null);
const autocompleteCleanupCallback = useRef<(() => void) | null>(null);
@ -132,6 +133,12 @@ const MonacoQueryField = ({
};
}, []);
useEffect(() => {
if (completionDataProviderRef.current && timeRange) {
completionDataProviderRef.current.setTimeRange(timeRange);
}
}, [timeRange]);
const setPlaceholder = (monaco: Monaco, editor: MonacoEditor) => {
const placeholderDecorators = [
{
@ -213,6 +220,7 @@ const MonacoQueryField = ({
monaco.editor.setModelMarkers(model, 'owner', markers);
});
const dataProvider = new CompletionDataProvider(langProviderRef.current, historyRef, timeRange);
completionDataProviderRef.current = dataProvider;
const completionProvider = getCompletionProvider(monaco, dataProvider);
// completion-providers in monaco are not registered directly to editor-instances,

View File

@ -72,6 +72,15 @@ const mockTimeRange = {
},
};
const otherTimeRange = {
from: dateTime(1234567800000),
to: dateTime(1234567801000),
raw: {
from: dateTime(1234567800000),
to: dateTime(1234567801000),
},
};
describe('CompletionDataProvider', () => {
let completionProvider: CompletionDataProvider, languageProvider: LokiLanguageProvider, datasource: LokiDatasource;
let historyRef: { current: Array<HistoryItem<LokiQuery>> } = { current: [] };
@ -137,21 +146,21 @@ describe('CompletionDataProvider', () => {
});
test('Returns the expected parser and label keys', async () => {
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledTimes(1);
});
test('Returns the expected parser and label keys, cache duplicate query', async () => {
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledTimes(1);
});
test('Returns the expected parser and label keys, unique query is not cached', async () => {
//1
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
//2
expect(await completionProvider.getParserAndLabelKeys('unique')).toEqual(parserAndLabelKeys);
@ -161,23 +170,32 @@ describe('CompletionDataProvider', () => {
expect(await completionProvider.getParserAndLabelKeys('uffdah')).toEqual(parserAndLabelKeys);
// 4
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledTimes(4);
});
test('Clears the cache when the time range changes', async () => {
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
completionProvider.setTimeRange(otherTimeRange);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledTimes(2);
});
test('Returns the expected parser and label keys, cache size is 2', async () => {
//1
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
//2
expect(await completionProvider.getParserAndLabelKeys('unique')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('unique')).toEqual(parserAndLabelKeys);
// 2
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledTimes(2);
// 3
@ -186,13 +204,21 @@ describe('CompletionDataProvider', () => {
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledTimes(3);
// 4
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(await completionProvider.getParserAndLabelKeys('{a="b"}')).toEqual(parserAndLabelKeys);
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledTimes(4);
});
test('Uses time range from CompletionProvider', async () => {
completionProvider.getParserAndLabelKeys('');
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledWith('', { timeRange: mockTimeRange });
completionProvider.getParserAndLabelKeys('{a="b"}');
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledWith('{a="b"}', { timeRange: mockTimeRange });
});
test('Updates the time range from CompletionProvider', async () => {
completionProvider.getParserAndLabelKeys('{a="b"}');
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledWith('{a="b"}', { timeRange: mockTimeRange });
completionProvider.setTimeRange(otherTimeRange);
completionProvider.getParserAndLabelKeys('{a="b"}');
expect(languageProvider.getParserAndLabelKeys).toHaveBeenCalledWith('{a="b"}', { timeRange: otherTimeRange });
});
});

View File

@ -30,6 +30,11 @@ export class CompletionDataProvider {
return `{${allLabelTexts.join(',')}}`;
}
setTimeRange(timeRange: TimeRange) {
this.timeRange = timeRange;
this.queryToLabelKeysCache.clear();
}
getHistory() {
return chain(this.historyRef.current)
.orderBy('ts', 'desc')
@ -82,7 +87,9 @@ export class CompletionDataProvider {
// Make room in the cache for the fresh result by deleting the "first" index
const keys = this.queryToLabelKeysCache.keys();
const firstKey = keys.next().value;
this.queryToLabelKeysCache.delete(firstKey);
if (firstKey !== undefined) {
this.queryToLabelKeysCache.delete(firstKey);
}
}
// Fetch a fresh result from the backend
const labelKeys = await this.languageProvider.getParserAndLabelKeys(logQuery, { timeRange: this.timeRange });