From 715453204e33744a2c784edf33e7720a0f9b7dc4 Mon Sep 17 00:00:00 2001 From: Dan Cech Date: Fri, 14 Apr 2017 17:41:22 -0400 Subject: [PATCH 1/3] make sure graphite queries containing references are properly updated --- .../graphite/partials/query.editor.html | 2 +- .../plugins/datasource/graphite/query_ctrl.ts | 58 ++++++++++++++----- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/public/app/plugins/datasource/graphite/partials/query.editor.html b/public/app/plugins/datasource/graphite/partials/query.editor.html index 60372fb8ab0..5646f005be9 100755 --- a/public/app/plugins/datasource/graphite/partials/query.editor.html +++ b/public/app/plugins/datasource/graphite/partials/query.editor.html @@ -1,7 +1,7 @@
- +
diff --git a/public/app/plugins/datasource/graphite/query_ctrl.ts b/public/app/plugins/datasource/graphite/query_ctrl.ts index f9f34284971..ce2baea0b76 100644 --- a/public/app/plugins/datasource/graphite/query_ctrl.ts +++ b/public/app/plugins/datasource/graphite/query_ctrl.ts @@ -55,7 +55,7 @@ export class GraphiteQueryCtrl extends QueryCtrl { } try { - this.parseTargeRecursive(astNode, null, 0); + this.parseTargetRecursive(astNode, null, 0); } catch (err) { console.log('error parsing target:', err.message); this.error = err.message; @@ -72,7 +72,7 @@ export class GraphiteQueryCtrl extends QueryCtrl { func.params[index] = value; } - parseTargeRecursive(astNode, func, index) { + parseTargetRecursive(astNode, func, index) { if (astNode === null) { return null; } @@ -81,7 +81,7 @@ export class GraphiteQueryCtrl extends QueryCtrl { case 'function': var innerFunc = gfunc.createFuncInstance(astNode.name, { withDefaultParams: false }); _.each(astNode.params, (param, index) => { - this.parseTargeRecursive(param, innerFunc, index); + this.parseTargetRecursive(param, innerFunc, index); }); innerFunc.updateText(); @@ -209,30 +209,56 @@ export class GraphiteQueryCtrl extends QueryCtrl { } targetTextChanged() { - this.parseTarget(); - this.panelCtrl.refresh(); + this.updateModelTarget(); + this.refresh(); } updateModelTarget() { // render query - var metricPath = this.getSegmentPathUpTo(this.segments.length); - this.target.target = _.reduce(this.functions, this.wrapFunction, metricPath); + if (!this.target.textEditor) { + var metricPath = this.getSegmentPathUpTo(this.segments.length); + this.target.target = _.reduce(this.functions, this.wrapFunction, metricPath); + } + // loop through queries and update targetFull as needed + for (const target of this.panelCtrl.panel.targets) { + this.resolveTarget(target); + } + } + + resolveTarget(target) { // render nested query var targetsByRefId = _.keyBy(this.panelCtrl.panel.targets, 'refId'); + + // no references to self + delete targetsByRefId[target.refId]; + var nestedSeriesRefRegex = /\#([A-Z])/g; - var targetWithNestedQueries = this.target.target.replace(nestedSeriesRefRegex, (match, g1) => { - var target = targetsByRefId[g1]; - if (!target) { - return match; + var targetWithNestedQueries = target.target; + + while (targetWithNestedQueries.match(nestedSeriesRefRegex)) { + var updated = targetWithNestedQueries.replace(nestedSeriesRefRegex, (match, g1) => { + var t = targetsByRefId[g1]; + if (!t) { + return match; + } + + // no circular references + delete targetsByRefId[g1]; + + return t.target; + }); + + if (updated === targetWithNestedQueries) { + break; } - return target.targetFull || target.target; - }); + targetWithNestedQueries = updated; + } - delete this.target.targetFull; - if (this.target.target !== targetWithNestedQueries) { - this.target.targetFull = targetWithNestedQueries; + delete target.targetFull; + if (target.target !== targetWithNestedQueries) { + target.targetFull = targetWithNestedQueries; } } From a64e000f1aa5bd3d7a3f71e937439a49b66e90a2 Mon Sep 17 00:00:00 2001 From: Dan Cech Date: Mon, 17 Apr 2017 11:10:55 -0400 Subject: [PATCH 2/3] process this.target separately to fix issues with tests --- public/app/plugins/datasource/graphite/query_ctrl.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/public/app/plugins/datasource/graphite/query_ctrl.ts b/public/app/plugins/datasource/graphite/query_ctrl.ts index ce2baea0b76..7c89ac1a0a9 100644 --- a/public/app/plugins/datasource/graphite/query_ctrl.ts +++ b/public/app/plugins/datasource/graphite/query_ctrl.ts @@ -220,9 +220,13 @@ export class GraphiteQueryCtrl extends QueryCtrl { this.target.target = _.reduce(this.functions, this.wrapFunction, metricPath); } - // loop through queries and update targetFull as needed - for (const target of this.panelCtrl.panel.targets) { - this.resolveTarget(target); + this.resolveTarget(this.target); + + // loop through other queries and update targetFull as needed + for (const target of this.panelCtrl.panel.targets || []) { + if (target.refId !== this.target.refId) { + this.resolveTarget(target); + } } } From 85baa50194226e1c3433531c0eca73331d66e0f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Tue, 18 Apr 2017 16:30:20 +0200 Subject: [PATCH 3/3] recfactor: added unit test for the new scenario, #8143 --- .../plugins/datasource/graphite/query_ctrl.ts | 10 +++++----- .../graphite/specs/query_ctrl_specs.ts | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/public/app/plugins/datasource/graphite/query_ctrl.ts b/public/app/plugins/datasource/graphite/query_ctrl.ts index 7c89ac1a0a9..1cf4406c7b8 100644 --- a/public/app/plugins/datasource/graphite/query_ctrl.ts +++ b/public/app/plugins/datasource/graphite/query_ctrl.ts @@ -28,7 +28,6 @@ export class GraphiteQueryCtrl extends QueryCtrl { } toggleEditorMode() { - this.target.textEditor = !this.target.textEditor; this.parseTarget(); } @@ -220,17 +219,17 @@ export class GraphiteQueryCtrl extends QueryCtrl { this.target.target = _.reduce(this.functions, this.wrapFunction, metricPath); } - this.resolveTarget(this.target); + this.updateRenderedTarget(this.target); // loop through other queries and update targetFull as needed for (const target of this.panelCtrl.panel.targets || []) { if (target.refId !== this.target.refId) { - this.resolveTarget(target); + this.updateRenderedTarget(target); } } } - resolveTarget(target) { + updateRenderedTarget(target) { // render nested query var targetsByRefId = _.keyBy(this.panelCtrl.panel.targets, 'refId'); @@ -240,6 +239,8 @@ export class GraphiteQueryCtrl extends QueryCtrl { var nestedSeriesRefRegex = /\#([A-Z])/g; var targetWithNestedQueries = target.target; + // Keep interpolating until there are no query references + // The reason for the loop is that the referenced query might contain another reference to another query while (targetWithNestedQueries.match(nestedSeriesRefRegex)) { var updated = targetWithNestedQueries.replace(nestedSeriesRefRegex, (match, g1) => { var t = targetsByRefId[g1]; @@ -249,7 +250,6 @@ export class GraphiteQueryCtrl extends QueryCtrl { // no circular references delete targetsByRefId[g1]; - return t.target; }); diff --git a/public/app/plugins/datasource/graphite/specs/query_ctrl_specs.ts b/public/app/plugins/datasource/graphite/specs/query_ctrl_specs.ts index 95691ae0b7b..e88fbc044c1 100644 --- a/public/app/plugins/datasource/graphite/specs/query_ctrl_specs.ts +++ b/public/app/plugins/datasource/graphite/specs/query_ctrl_specs.ts @@ -186,4 +186,24 @@ describe('GraphiteQueryCtrl', function() { expect(ctx.ctrl.target.targetFull).to.be('scaleToSeconds(nested.query.count)'); }); }); + + describe('when updating target used in other query', function() { + beforeEach(function() { + ctx.ctrl.target.target = 'metrics.a.count'; + ctx.ctrl.target.refId = 'A'; + ctx.ctrl.datasource.metricFindQuery = sinon.stub().returns(ctx.$q.when([{expandable: false}])); + ctx.ctrl.parseTarget(); + + ctx.ctrl.panelCtrl.panel.targets = [ + ctx.ctrl.target, {target: 'sumSeries(#A)', refId: 'B'} + ]; + + ctx.ctrl.updateModelTarget(); + }); + + it('targetFull of other query should update', function() { + expect(ctx.ctrl.panel.targets[1].targetFull).to.be('sumSeries(metrics.a.count)'); + }); + }); + });