From 912c5ccb6a1d368bd6d5e1715fff3a7a208ae9b3 Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Tue, 20 Jun 2023 14:43:49 +0200 Subject: [PATCH] Elasticsearch: fix type-related bug within targetContainsTemplate (#69798) * Elasticsearch: fix type-related bug within targetContainsTemplate * Refactor to fix found regression --- .../elasticsearch/datasource.test.ts | 53 ++++++++++++++++++- .../datasource/elasticsearch/datasource.ts | 28 +++------- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/public/app/plugins/datasource/elasticsearch/datasource.test.ts b/public/app/plugins/datasource/elasticsearch/datasource.test.ts index b559e3b5b13..35950dca68e 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.test.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.test.ts @@ -94,7 +94,6 @@ function getTestContext({ data = { responses: [] }, from = 'now-5m', jsonData, - database = '[test-]YYYY.MM.DD', fetchMockImplementation, templateSrvMock, }: TestContext = {}) { @@ -127,6 +126,7 @@ function getTestContext({ return text; } }, + containsTemplate: jest.fn().mockImplementation((text?: string) => text?.includes('$') ?? false), getAdhocFilters: () => [], } as unknown as TemplateSrv); @@ -1295,6 +1295,57 @@ const logsResponse = { }, }; +describe('targetContainsTemplate', () => { + let ds: ElasticDatasource; + let target: ElasticsearchQuery; + beforeEach(() => { + const context = getTestContext(); + ds = context.ds; + target = { + refId: 'test', + bucketAggs: [{ type: 'date_histogram', field: '@timestamp', id: '1' }], + metrics: [{ type: 'count', id: '1' }], + query: 'escape\\:test', + }; + }); + it('returns false when there is no variable in the query', () => { + expect(ds.targetContainsTemplate(target)).toBe(false); + }); + it('returns true when there are variables in the query alias', () => { + target.alias = '$variable'; + expect(ds.targetContainsTemplate(target)).toBe(true); + }); + it('returns true when there are variables in the query', () => { + target.query = '$variable:something'; + expect(ds.targetContainsTemplate(target)).toBe(true); + }); + it('returns true when there are variables in the bucket aggregation', () => { + target.bucketAggs = [{ type: 'date_histogram', field: '$field', id: '1' }]; + expect(ds.targetContainsTemplate(target)).toBe(true); + target.bucketAggs = [{ type: 'date_histogram', field: '@timestamp', id: '1', settings: { interval: '$interval' } }]; + expect(ds.targetContainsTemplate(target)).toBe(true); + }); + it('returns true when there are variables in the metric aggregation', () => { + target.metrics = [{ type: 'moving_avg', id: '1', settings: { window: '$window' } }]; + expect(ds.targetContainsTemplate(target)).toBe(true); + target.metrics = [{ type: 'moving_avg', id: '1', field: '$field' }]; + expect(ds.targetContainsTemplate(target)).toBe(true); + target.metrics = [{ type: 'extended_stats', id: '1', meta: { something: '$something' } }]; + expect(ds.targetContainsTemplate(target)).toBe(true); + }); + it('returns true when there are variables in an array inside an object in metrics', () => { + target.metrics = [ + { + field: 'counter', + id: '1', + settings: { percents: ['20', '40', '$qqq'] }, + type: 'percentiles', + }, + ]; + expect(ds.targetContainsTemplate(target)).toBe(true); + }); +}); + describe('ElasticDatasource using backend', () => { beforeEach(() => { console.error = jest.fn(); diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts index 933efdbcb9b..91186945344 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.ts @@ -828,37 +828,23 @@ export class ElasticDatasource return false; } - private isPrimitive(obj: unknown) { - if (obj === null || obj === undefined) { - return true; - } - if (['string', 'number', 'boolean'].some((type) => type === typeof true)) { - return true; - } - - return false; - } - private objectContainsTemplate(obj: any) { - if (!obj) { + if (typeof obj === 'string') { + return this.templateSrv.containsTemplate(obj); + } + if (!obj || typeof obj !== 'object') { return false; } for (const key of Object.keys(obj)) { - if (this.isPrimitive(obj[key])) { - if (this.templateSrv.containsTemplate(obj[key])) { - return true; - } - } else if (Array.isArray(obj[key])) { + if (Array.isArray(obj[key])) { for (const item of obj[key]) { if (this.objectContainsTemplate(item)) { return true; } } - } else { - if (this.objectContainsTemplate(obj[key])) { - return true; - } + } else if (this.objectContainsTemplate(obj[key])) { + return true; } }