Support 'strict' flag for restrictions

Closes-Bug: #1476627

Change-Id: Iba64887e9d693264a6d0a859becd51cd11929a16
This commit is contained in:
Vitaly Kramskikh 2015-07-30 15:11:33 +03:00
parent 62105b2e22
commit aafd74b430
5 changed files with 64 additions and 47 deletions

View File

@ -44,7 +44,8 @@ structure includes the following attributes::
* *label* is a setting title that is displayed on UI
* *weight* defines the order in which this setting is displayed in its group.
This attribute is desirable
* *type* defines the type of UI control to use for the setting. The following types are supported:
* *type* defines the type of UI control to use for the setting.
The following types are supported:
* *text* - single line input
* *password* - password input
@ -71,8 +72,8 @@ Restrictions
------------
Restrictions define when settings and setting groups should be available.
Each restriction is defined as a *condition* with optional *action* and
*message*::
Each restriction is defined as a *condition* with optional *action*, *message*
and *strict*::
restrictions:
- condition: "settings:common.libvirt_type.value != 'kvm'"
@ -90,6 +91,17 @@ Each restriction is defined as a *condition* with optional *action* and
* *message* is a message that is shown if *condition* is satisfied. This field
is optional.
* *strict* is a boolean flag which specifies how to handle non-existent keys
in expressions. If it is set to true (default value), exception is thrown in
case of non-existent key. Otherwise values of such keys have null value.
Setting this flag to false is useful for conditions which rely on settings
provided by plugins::
restrictions:
- condition: "settings:other_plugin == null or settings:other_plugin.metadata.enabled != true"
strict: false
message: "Other plugin must be installed and enabled"
There are also short forms of restrictions::
restrictions:

View File

@ -32,6 +32,8 @@ class TestExpressionParser(BaseTestCase):
}
hypervisor = models['settings']['common']['libvirt_type']['value']
# if you change/add test cases, please also modify
# static/tests/unit/expression.js
test_cases = (
# test scalars
('true', True),
@ -77,21 +79,26 @@ class TestExpressionParser(BaseTestCase):
'!= "{0}")'.format(hypervisor), True),
# test nonexistent keys
('cluster:nonexistentkey', TypeError),
('cluster:nonexistentkey == null', True, False),
# test evaluation flow
('cluster:mode != "ha_compact" and cluster:nonexistentkey == 1',
('cluster:mode != "ha_compact" and cluster:nonexistentkey == null',
False),
('cluster:mode == "ha_compact" and cluster:nonexistentkey == 1',
('cluster:mode == "ha_compact" and cluster:nonexistentkey == null',
TypeError),
('cluster:mode == "ha_compact" and cluster:nonexistentkey == null',
True, False),
)
def evaluate_expression(expression, models):
return Expression(expression, models).evaluate()
def evaluate_expression(expression, models, strict):
return Expression(expression, models, strict).evaluate()
for test_case in test_cases:
expression, result = test_case
expression = test_case[0]
result = test_case[1]
strict = test_case[2] if len(test_case) > 2 else True
if inspect.isclass(result) and issubclass(result, Exception):
self.assertRaises(result, evaluate_expression,
expression, models)
expression, models, strict)
else:
self.assertEqual(evaluate_expression(expression, models),
result)
self.assertEqual(evaluate_expression(expression, models,
strict), result)

View File

@ -107,7 +107,7 @@ define([
var restrictions = (this.expandedRestrictions || {})[path];
if (action) restrictions = _.where(restrictions, {action: action});
var satisfiedRestrictions = _.filter(restrictions, function(restriction) {
return new Expression(restriction.condition, models).evaluate();
return new Expression(restriction.condition, models, restriction).evaluate();
});
return {result: !!satisfiedRestrictions.length, message: _.compact(_.pluck(satisfiedRestrictions, 'message')).join(' ')};
},

View File

@ -9,6 +9,7 @@ define([
'use strict';
registerSuite({
name: 'Expression',
'Expression parser test': function() {
var hypervisor = 'kvm';
var testModels = {
@ -16,6 +17,9 @@ define([
settings: new models.Settings({common: {libvirt_type: {value: hypervisor}}}),
release: new models.Release({roles: ['controller', 'compute']})
};
// if you change/add test cases, please also modify
// nailgun/test/unit/test_expression_parser.py
var testCases = [
// test scalars
['true', true],
@ -58,25 +62,26 @@ define([
['cluster:mode == "ha_compact" and not (settings:common.libvirt_type.value != "' + hypervisor + '")', true],
// test nonexistent keys
['cluster:nonexistentkey', Error],
['cluster:nonexistentkey == null', true, false],
// test evaluation flow
['cluster:mode != "ha_compact" and cluster:nonexistentkey == 1', false],
['cluster:mode == "ha_compact" and cluster:nonexistentkey == 1', Error]
['cluster:mode != "ha_compact" and cluster:nonexistentkey == null', false],
['cluster:mode == "ha_compact" and cluster:nonexistentkey == null', Error],
['cluster:mode == "ha_compact" and cluster:nonexistentkey == null', true, false]
];
function evaluate(expression) {
var result = Expression(expression, testModels).evaluate();
function evaluate(expression, options) {
var result = Expression(expression, testModels, options).evaluate();
return result instanceof expressionObjects.ModelPath ? result.get() : result;
}
_.each(testCases, function(testCase) {
var expression = testCase[0];
var result = testCase[1];
_.each(testCases, _.spread(function(expression, result, strict) {
var options = {strict: strict};
if (result === Error) {
assert.throws(evaluate.bind(null, expression), Error, '', expression + ' throws an error');
assert.throws(_.partial(evaluate, expression, options), Error, '', expression + ' throws an error');
} else {
assert.strictEqual(evaluate(expression), result, expression + ' evaluates correctly');
assert.strictEqual(evaluate(expression, options), result, expression + ' evaluates correctly');
}
});
}));
}
});
});

View File

@ -329,11 +329,11 @@ function($, _, i18n, React, utils, models, Expression, componentMixins, controls
getValuesToCheck: function(setting, valueAttribute) {
return setting.values ? _.without(_.pluck(setting.values, 'data'), setting[valueAttribute]) : [!setting[valueAttribute]];
},
checkValues: function(values, path, currentValue, condition) {
checkValues: function(values, path, currentValue, restriction) {
var extraModels = {settings: this.props.settingsForChecks};
var result = _.all(values, function(value) {
this.props.settingsForChecks.set(path, value);
return new Expression(condition, this.props.configModels).evaluate(extraModels);
return new Expression(restriction.condition, this.props.configModels, restriction).evaluate(extraModels);
}, this);
this.props.settingsForChecks.set(path, currentValue);
return result;
@ -350,9 +350,8 @@ function($, _, i18n, React, utils, models, Expression, componentMixins, controls
return _.compact(this.props.allocatedRoles.map(function(roleName) {
var role = roles.findWhere({name: roleName});
if (_.any(role.expandedRestrictions.restrictions, function(restriction) {
var condition = restriction.condition;
if (_.contains(condition, 'settings:' + path) && !(new Expression(condition, this.props.configModels).evaluate())) {
return this.checkValues(valuesToCheck, pathToCheck, setting[valueAttribute], condition);
if (_.contains(restriction.condition, 'settings:' + path) && !(new Expression(restriction.condition, this.props.configModels, restriction).evaluate())) {
return this.checkValues(valuesToCheck, pathToCheck, setting[valueAttribute], restriction);
}
return false;
}, this)) return role.get('label');
@ -362,13 +361,16 @@ function($, _, i18n, React, utils, models, Expression, componentMixins, controls
var path = this.props.makePath(groupName, settingName),
currentSetting = this.props.settings.get(path);
if (!this.areCalсulationsPossible(currentSetting)) return [];
var getDependentRestrictions = _.bind(function(pathToCheck) {
return _.pluck(_.filter(this.props.settings.expandedRestrictions[pathToCheck], function(restriction) {
var dependentRestrictions = {};
var addDependentRestrictions = _.bind(function(pathToCheck, label) {
var result = _.filter(this.props.settings.expandedRestrictions[pathToCheck], function(restriction) {
return restriction.action == 'disable' && _.contains(restriction.condition, 'settings:' + path);
}), 'condition');
});
if (result.length) {
dependentRestrictions[label] = result.concat(dependentRestrictions[label] || []);
}
}, this);
// collect dependent settings
var dependentSettings = {};
// collect dependencies
_.each(this.props.settings.attributes, function(group, groupName) {
// don't take into account hidden dependent settings
if (this.props.checkRestrictions('hide', this.props.makePath(groupName, 'metadata')).result) return;
@ -376,31 +378,22 @@ function($, _, i18n, React, utils, models, Expression, componentMixins, controls
// we support dependecies on checkboxes, toggleable setting groups, dropdowns and radio groups
var pathToCheck = this.props.makePath(groupName, settingName);
if (!this.areCalсulationsPossible(setting) || pathToCheck == path || this.props.checkRestrictions('hide', pathToCheck).result) return;
var dependentRestrictions;
if (setting[this.props.getValueAttribute(settingName)] == true) {
dependentRestrictions = getDependentRestrictions(pathToCheck);
if (dependentRestrictions.length) {
dependentSettings[setting.label] = _.union(dependentSettings[setting.label], dependentRestrictions);
}
addDependentRestrictions(pathToCheck, setting.label);
} else {
var activeOption = _.find(setting.values, {data: setting.value});
if (activeOption) {
dependentRestrictions = getDependentRestrictions(this.props.makePath(pathToCheck, activeOption.data));
if (dependentRestrictions.length) {
dependentSettings[setting.label] = _.union(dependentSettings[setting.label], dependentRestrictions);
}
}
if (activeOption) addDependentRestrictions(this.props.makePath(pathToCheck, activeOption.data), setting.label);
}
}, this);
}, this);
// evaluate dependencies
if (!_.isEmpty(dependentSettings)) {
if (!_.isEmpty(dependentRestrictions)) {
var valueAttribute = this.props.getValueAttribute(settingName),
pathToCheck = this.props.makePath(path, valueAttribute),
valuesToCheck = this.getValuesToCheck(currentSetting, valueAttribute),
checkValues = _.bind(this.checkValues, this, valuesToCheck, pathToCheck, currentSetting[valueAttribute]);
return _.compact(_.map(dependentSettings, function(conditions, label) {
if (_.any(conditions, checkValues)) return label;
checkValues = _.partial(this.checkValues, valuesToCheck, pathToCheck, currentSetting[valueAttribute]);
return _.compact(_.map(dependentRestrictions, function(restrictions, label) {
if (_.any(restrictions, checkValues)) return label;
}));
}
return [];