From 4188c46f83f5b95a9aca5fe1f23c7ffc2a360dcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Sat, 17 Sep 2016 11:28:45 +0200 Subject: [PATCH] feat(templating): more work on new variable handling code, #6048 --- .../features/templating/custom_variable.ts | 9 +- .../templating/datasource_variable.ts | 9 +- .../features/templating/interval_variable.ts | 18 ++- .../app/features/templating/query_variable.ts | 38 ++---- .../templating/specs/variable_specs.ts | 6 + .../specs/variable_srv_init_specs.ts | 113 ++++++++++++++++++ .../templating/specs/variable_srv_specs.ts | 46 ------- public/app/features/templating/variable.ts | 14 ++- .../app/features/templating/variable_srv.ts | 34 ++++-- 9 files changed, 192 insertions(+), 95 deletions(-) create mode 100644 public/app/features/templating/specs/variable_srv_init_specs.ts diff --git a/public/app/features/templating/custom_variable.ts b/public/app/features/templating/custom_variable.ts index 99dd278f899..6b119d0947c 100644 --- a/public/app/features/templating/custom_variable.ts +++ b/public/app/features/templating/custom_variable.ts @@ -11,11 +11,12 @@ export class CustomVariable implements Variable { includeAll: boolean; /** @ngInject */ - constructor(private model, private timeSrv, private templateSrv) { + constructor(private model, private timeSrv, private templateSrv, private variableSrv) { _.extend(this, model); } setValue(option) { + this.variableSrv.setOptionAsCurrent(this, option); } updateOptions() { @@ -33,9 +34,13 @@ export class CustomVariable implements Variable { this.options.unshift({text: 'All', value: "$__all"}); } - dependsOn(variableName) { + dependsOn(variable) { return false; } + + setValueFromUrl(urlValue) { + return this.variableSrv.setOptionFromUrl(this, urlValue); + } } variableConstructorMap['custom'] = CustomVariable; diff --git a/public/app/features/templating/datasource_variable.ts b/public/app/features/templating/datasource_variable.ts index bd13c43f6e4..ff89152dbc0 100644 --- a/public/app/features/templating/datasource_variable.ts +++ b/public/app/features/templating/datasource_variable.ts @@ -11,11 +11,12 @@ export class DatasourceVariable implements Variable { options: any; /** @ngInject */ - constructor(private model, private datasourceSrv) { + constructor(private model, private datasourceSrv, private variableSrv) { _.extend(this, model); } setValue(option) { + this.variableSrv.setOptionAsCurrent(this, option); } updateOptions() { @@ -48,9 +49,13 @@ export class DatasourceVariable implements Variable { this.options = options; } - dependsOn(variableName) { + dependsOn(variable) { return false; } + + setValueFromUrl(urlValue) { + return this.variableSrv.setOptionFromUrl(this, urlValue); + } } variableConstructorMap['datasource'] = DatasourceVariable; diff --git a/public/app/features/templating/interval_variable.ts b/public/app/features/templating/interval_variable.ts index 681b088ed0b..409a0824aea 100644 --- a/public/app/features/templating/interval_variable.ts +++ b/public/app/features/templating/interval_variable.ts @@ -13,17 +13,20 @@ export class IntervalVariable implements Variable { query: string; /** @ngInject */ - constructor(private model, private timeSrv, private templateSrv) { + constructor(private model, private timeSrv, private templateSrv, private variableSrv) { _.extend(this, model); } setValue(option) { - if (this.auto) { - this.updateAutoValue(); - } + this.updateAutoValue(); + this.variableSrv.setOptionAsCurrent(this, option); } updateAutoValue() { + if (!this.auto) { + return; + } + // add auto option if missing if (this.options.length && this.options[0].text !== 'auto') { this.options.unshift({ text: 'auto', value: '$__auto_interval' }); @@ -44,9 +47,14 @@ export class IntervalVariable implements Variable { } } - dependsOn(variableName) { + dependsOn(variable) { return false; } + + setValueFromUrl(urlValue) { + this.updateAutoValue(); + return this.variableSrv.setOptionFromUrl(this, urlValue); + } } variableConstructorMap['interval'] = IntervalVariable; diff --git a/public/app/features/templating/query_variable.ts b/public/app/features/templating/query_variable.ts index 784b401a52a..6ba95b05e00 100644 --- a/public/app/features/templating/query_variable.ts +++ b/public/app/features/templating/query_variable.ts @@ -24,31 +24,11 @@ export class QueryVariable implements Variable { } setValue(option){ - this.current = _.cloneDeep(option); - - if (_.isArray(this.current.text)) { - this.current.text = this.current.text.join(' + '); - } - - this.variableSrv.selectOptionsForCurrentValue(this); - return this.variableSrv.variableUpdated(this); + this.variableSrv.setOptionAsCurrent(this, option); } setValueFromUrl(urlValue) { - var promise = this.$q.when(); - - if (this.refresh) { - promise = this.updateOptions(); - } - - return promise.then(() => { - var option = _.find(this.options, op => { - return op.text === urlValue || op.value === urlValue; - }); - - option = option || { text: urlValue, value: urlValue }; - return this.setValue(option); - }); + return this.variableSrv.setOptionFromUrl(this, urlValue); } updateOptions() { @@ -126,11 +106,11 @@ export class QueryVariable implements Variable { } else if (sortType === 2) { options = _.sortBy(options, function(opt) { var matches = opt.text.match(/.*?(\d+).*/); -if (!matches) { - return 0; -} else { - return parseInt(matches[1], 10); -} + if (!matches) { + return 0; + } else { + return parseInt(matches[1], 10); + } }); } @@ -141,8 +121,8 @@ if (!matches) { return options; } - dependsOn(variableName) { - return containsVariable(this.query, variableName) || containsVariable(this.datasource, variableName); + dependsOn(variable) { + return containsVariable(this.query, this.datasource, variable.name); } } diff --git a/public/app/features/templating/specs/variable_specs.ts b/public/app/features/templating/specs/variable_specs.ts index a87cd744aaa..a11f6272425 100644 --- a/public/app/features/templating/specs/variable_specs.ts +++ b/public/app/features/templating/specs/variable_specs.ts @@ -30,6 +30,12 @@ describe('containsVariable', function() { var contains = containsVariable('$env', 'env'); expect(contains).to.be(true); }); + + it('should be able to pass in multiple test strings', function() { + var contains = containsVariable('asd','asd2.$env', 'env'); + expect(contains).to.be(true); + }); + }); }); diff --git a/public/app/features/templating/specs/variable_srv_init_specs.ts b/public/app/features/templating/specs/variable_srv_init_specs.ts new file mode 100644 index 00000000000..59c82ed26c5 --- /dev/null +++ b/public/app/features/templating/specs/variable_srv_init_specs.ts @@ -0,0 +1,113 @@ +import {describe, beforeEach, it, sinon, expect, angularMocks} from 'test/lib/common'; + +import _ from 'lodash'; +import helpers from 'test/specs/helpers'; +import '../all'; + +describe('VariableSrv init', function() { + var ctx = new helpers.ControllerTestContext(); + + beforeEach(angularMocks.module('grafana.core')); + beforeEach(angularMocks.module('grafana.controllers')); + beforeEach(angularMocks.module('grafana.services')); + + beforeEach(ctx.providePhase(['datasourceSrv', 'timeSrv', 'templateSrv', '$location'])); + beforeEach(angularMocks.inject(($rootScope, $q, $location, $injector) => { + ctx.$q = $q; + ctx.$rootScope = $rootScope; + ctx.$location = $location; + ctx.variableSrv = $injector.get('variableSrv'); + ctx.variableSrv.init({templating: {list: []}}); + ctx.$rootScope.$digest(); + })); + + function describeInitScenario(desc, fn) { + describe(desc, function() { + var scenario: any = { + urlParams: {}, + setup: setupFn => { + scenario.setupFn = setupFn; + } + }; + + beforeEach(function() { + scenario.setupFn(); + ctx.datasource = {}; + ctx.datasource.metricFindQuery = sinon.stub().returns(ctx.$q.when(scenario.queryResult)); + + ctx.datasourceSrv.get = sinon.stub().returns(ctx.$q.when(ctx.datasource)); + ctx.datasourceSrv.getMetricSources = sinon.stub().returns(scenario.metricSources); + + ctx.$location.search = sinon.stub().returns(scenario.urlParams); + + ctx.dashboard = {templating: {list: scenario.variables}}; + ctx.variableSrv.init(ctx.dashboard); + ctx.$rootScope.$digest(); + + scenario.variables = ctx.variableSrv.variables; + }); + + fn(scenario); + }); + } + + ['query', 'interval', 'custom', 'datasource'].forEach(type => { + describeInitScenario('when setting ' + type + ' variable via url', scenario => { + scenario.setup(() => { + scenario.variables = [{ + name: 'apps', + type: type, + current: {text: "test", value: "test"}, + options: [{text: "test", value: "test"}] + }]; + scenario.urlParams["var-apps"] = "new"; + }); + + it('should update current value', () => { + expect(scenario.variables[0].current.value).to.be("new"); + expect(scenario.variables[0].current.text).to.be("new"); + }); + }); + + }); + + describe('given dependent variables', () => { + var variableList = [ + { + name: 'app', + type: 'query', + query: '', + current: {text: "app1", value: "app1"}, + options: [{text: "app1", value: "app1"}] + }, + { + name: 'server', + type: 'query', + refresh: 1, + query: '$app.*', + current: {text: "server1", value: "server1"}, + options: [{text: "server1", value: "server1"}] + }, + ]; + + describeInitScenario('when setting parent var from url', scenario => { + scenario.setup(() => { + scenario.variables = _.cloneDeep(variableList); + scenario.urlParams["var-app"] = "google"; + scenario.queryResult = [{text: 'google-server1'}, {text: 'google-server2'}]; + }); + + it('should update child variable', () => { + expect(scenario.variables[1].options.length).to.be(2); + expect(scenario.variables[1].current.text).to.be("google-server1"); + }); + + it('should only update it once', () => { + expect(ctx.datasource.metricFindQuery.callCount).to.be(1); + }); + + }); + }); + +}); + diff --git a/public/app/features/templating/specs/variable_srv_specs.ts b/public/app/features/templating/specs/variable_srv_specs.ts index f6a7224da42..7dc007c663c 100644 --- a/public/app/features/templating/specs/variable_srv_specs.ts +++ b/public/app/features/templating/specs/variable_srv_specs.ts @@ -21,52 +21,6 @@ describe('VariableSrv', function() { ctx.$rootScope.$digest(); })); - function describeInitSceneario(desc, fn) { - describe(desc, function() { - var scenario: any = { - urlParams: {}, - setup: setupFn => { - scenario.setupFn = setupFn; - } - }; - - beforeEach(function() { - scenario.setupFn(); - var ds: any = {}; - ds.metricFindQuery = sinon.stub().returns(ctx.$q.when(scenario.queryResult)); - ctx.datasourceSrv.get = sinon.stub().returns(ctx.$q.when(ds)); - ctx.datasourceSrv.getMetricSources = sinon.stub().returns(scenario.metricSources); - - ctx.$location.search = sinon.stub().returns(scenario.urlParams); - - ctx.dashboard = {templating: {list: scenario.variables}}; - ctx.variableSrv.init(ctx.dashboard); - ctx.$rootScope.$digest(); - - scenario.variables = ctx.variableSrv.variables; - }); - - fn(scenario); - }); - } - - describeInitSceneario('when setting simple variable via url', scenario => { - scenario.setup(() => { - scenario.variables = [{ - name: 'apps', - type: 'query', - current: {text: "test", value: "test"}, - options: [{text: "test", value: "test"}] - }]; - scenario.urlParams["var-apps"] = "new"; - }); - - it('should update current value', () => { - expect(scenario.variables[0].current.value).to.be("new"); - expect(scenario.variables[0].current.text).to.be("new"); - }); - }); - function describeUpdateVariable(desc, fn) { describe(desc, function() { var scenario: any = {}; diff --git a/public/app/features/templating/variable.ts b/public/app/features/templating/variable.ts index cbee8b0b9c1..da9c280aa33 100644 --- a/public/app/features/templating/variable.ts +++ b/public/app/features/templating/variable.ts @@ -1,9 +1,14 @@ +/// +import _ from 'lodash'; import kbn from 'app/core/utils/kbn'; -export function containsVariable(str, variableName) { - if (!str) { - return false; +export function containsVariable(...args: any[]) { + var variableName = args[args.length-1]; + var str = args[0] || ''; + + for (var i = 1; i < args.length-1; i++) { + str += args[i] || ''; } variableName = kbn.regexEscape(variableName); @@ -15,7 +20,8 @@ export function containsVariable(str, variableName) { export interface Variable { setValue(option); updateOptions(); - dependsOn(variableName); + dependsOn(variable); + setValueFromUrl(urlValue); } diff --git a/public/app/features/templating/variable_srv.ts b/public/app/features/templating/variable_srv.ts index f03cd8a576d..a1409e4cc2e 100644 --- a/public/app/features/templating/variable_srv.ts +++ b/public/app/features/templating/variable_srv.ts @@ -40,12 +40,6 @@ export class VariableSrv { this.variableLock[variable.name] = this.$q.defer(); } - var promises = []; - - for (let variable of this.variables) { - promises.push(this.processVariable(variable, queryParams)); - } - return this.$q.all(this.variables.map(variable => { return this.processVariable(variable, queryParams); })); @@ -66,7 +60,6 @@ export class VariableSrv { if (urlValue !== void 0) { return variable.setValueFromUrl(urlValue).then(lock.resolve); } - if (variable.refresh === 1 || variable.refresh === 2) { return variable.updateOptions().then(() => { // if (_.isEmpty(variable.current) && variable.options.length) { @@ -174,6 +167,33 @@ export class VariableSrv { } } + setOptionFromUrl(variable, urlValue) { + var promise = this.$q.when(); + + if (variable.refresh) { + promise = variable.updateOptions(); + } + + return promise.then(() => { + var option = _.find(variable.options, op => { + return op.text === urlValue || op.value === urlValue; + }); + + option = option || {text: urlValue, value: urlValue}; + return variable.setValue(option); + }); + } + + setOptionAsCurrent(variable, option) { + variable.current = _.cloneDeep(option); + + if (_.isArray(variable.current.text)) { + variable.current.text = variable.current.text.join(' + '); + } + + this.selectOptionsForCurrentValue(variable); + return this.variableUpdated(variable); + } } coreModule.service('variableSrv', VariableSrv);