This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-ui.git
commit 4f159b8079b8454bd35b381003b58cb0b742e1fd Author: Alex Heneveld <[email protected]> AuthorDate: Tue Apr 27 12:15:22 2021 +0100 cleanup how parameters and config are stored ensure addition is preserved, and that parameters defined are displayed and interpreted as config --- .../providers/blueprint-service.provider.js | 210 +++++++++++---------- .../app/components/quick-fix/quick-fix.js | 12 +- .../spec-editor/spec-editor.directive.js | 12 +- .../app/components/util/model/entity.model.js | 73 ++++++- .../app/views/main/yaml/yaml.state.js | 7 + 5 files changed, 189 insertions(+), 125 deletions(-) diff --git a/ui-modules/blueprint-composer/app/components/providers/blueprint-service.provider.js b/ui-modules/blueprint-composer/app/components/providers/blueprint-service.provider.js index c98cf69..0132a85 100644 --- a/ui-modules/blueprint-composer/app/components/providers/blueprint-service.provider.js +++ b/ui-modules/blueprint-composer/app/components/providers/blueprint-service.provider.js @@ -90,7 +90,7 @@ function BlueprintService($log, $q, $sce, paletteApi, iconGenerator, dslService) } setBlueprintFromJson(newBlueprint); $log.debug(TAG + 'Blueprint set from YAML', blueprint); - // TODO refresh the blueprint now? see comments in yaml.state.js and on refreshBlueprint + // do we need to refresh the blueprint now? see comments in yaml.state.js and on refreshBlueprint; think not. } function getBlueprint() { @@ -285,7 +285,6 @@ function BlueprintService($log, $q, $sce, paletteApi, iconGenerator, dslService) function refreshTypeMetadata(entity, family) { let deferred = $q.defer(); - if (entity.hasType()) { entity.family = family.id; @@ -350,86 +349,89 @@ function BlueprintService($log, $q, $sce, paletteApi, iconGenerator, dslService) } function refreshConfigConstraints(entity) { + function checkConstraints(config) { + for (let constraintO of config.constraints) { + let message = null; + let key = null, args = null; + if (constraintO instanceof String || typeof constraintO=='string') { + key = constraintO; + } else if (Object.keys(constraintO).length==1) { + key = Object.keys(constraintO)[0]; + args = constraintO[key]; + } else { + $log.warn("Unknown constraint object", typeof constraintO, constraintO, config); + key = constraintO; + } + let val = (k) => entity.config.get(k || config.name); + let isSet = (k) => entity.config.has(k || config.name) && angular.isDefined(val(k)); + let isAnySet = (k) => { + if (!k || !Array.isArray(k)) return false; + return k.some(isSet); + } + let hasDefault = () => angular.isDefined(config.defaultValue); + switch (key) { + case 'Predicates.notNull()': + case 'Predicates.notNull': + if (!isSet() && !hasDefault()) { + message = `<samp>${config.name}</samp> is required`; + } + break; + case 'required': + if (!isSet() && !hasDefault() && val()!='') { + message = `<samp>${config.name}</samp> is required`; + } + break; + case 'regex': + if (isSet() && !(new RegExp(args).test(val))) { + message = `<samp>${config.name}</samp> does not match the required format: <samp>${args}</samp>`; + } + break; + case 'forbiddenIf': + if (isSet() && isSet(args)) { + message = `<samp>${config.name}</samp> and <samp>${args}</samp> cannot both be set`; + } + break; + case 'forbiddenUnless': + if (isSet() && !isSet(args)) { + message = `<samp>${config.name}</samp> can only be set when <samp>${args}</samp> is set`; + } + break; + case 'requiredIf': + if (!isSet() && isSet(args)) { + message = `<samp>${config.name}</samp> must be set if <samp>${args}</samp> is set`; + } + break; + case 'requiredUnless': + if (!isSet() && !isSet(args)) { + message = `<samp>${config.name}</samp> or <samp>${args}</samp> is required`; + } + break; + case 'requiredUnlessAnyOf': + if (!isSet() && !isAnySet(args)) { + message = `<samp>${config.name}</samp> or one of <samp>${args}</samp> is required`; + } + break; + case 'forbiddenUnlessAnyOf': + if (isSet() && !isAnySet(args)) { + message = `<samp>${config.name}</samp> cannot be set if any of <samp>${args}</samp> are set`; + } + break; + default: + $log.warn("Unknown constraint predicate", constraintO, config); + } + if (message !== null) { + entity.addIssue(Issue.builder().group('config').ref(config.name).message($sce.trustAsHtml(message)).build()); + } + } + } return $q((resolve) => { if (entity.miscData.has('config')) { entity.miscData.get('config') .filter(config => config.constraints && config.constraints.length > 0) - .forEach(config => { - for (let constraintO of config.constraints) { - let message = null; - let key = null, args = null; - if (constraintO instanceof String || typeof constraintO=='string') { - key = constraintO; - } else if (Object.keys(constraintO).length==1) { - key = Object.keys(constraintO)[0]; - args = constraintO[key]; - } else { - $log.warn("Unknown constraint object", typeof constraintO, constraintO, config); - key = constraintO; - } - let val = (k) => entity.config.get(k || config.name); - let isSet = (k) => entity.config.has(k || config.name) && angular.isDefined(val(k)); - let isAnySet = (k) => { - if (!k || !Array.isArray(k)) return false; - return k.some(isSet); - } - let hasDefault = () => angular.isDefined(config.defaultValue); - switch (key) { - case 'Predicates.notNull()': - case 'Predicates.notNull': - if (!isSet() && !hasDefault()) { - message = `<samp>${config.name}</samp> is required`; - } - break; - case 'required': - if (!isSet() && !hasDefault() && val()!='') { - message = `<samp>${config.name}</samp> is required`; - } - break; - case 'regex': - if (isSet() && !(new RegExp(args).test(val))) { - message = `<samp>${config.name}</samp> does not match the required format: <samp>${args}</samp>`; - } - break; - case 'forbiddenIf': - if (isSet() && isSet(args)) { - message = `<samp>${config.name}</samp> and <samp>${args}</samp> cannot both be set`; - } - break; - case 'forbiddenUnless': - if (isSet() && !isSet(args)) { - message = `<samp>${config.name}</samp> can only be set when <samp>${args}</samp> is set`; - } - break; - case 'requiredIf': - if (!isSet() && isSet(args)) { - message = `<samp>${config.name}</samp> must be set if <samp>${args}</samp> is set`; - } - break; - case 'requiredUnless': - if (!isSet() && !isSet(args)) { - message = `<samp>${config.name}</samp> or <samp>${args}</samp> is required`; - } - break; - case 'requiredUnlessAnyOf': - if (!isSet() && !isAnySet(args)) { - message = `<samp>${config.name}</samp> or one of <samp>${args}</samp> is required`; - } - break; - case 'forbiddenUnlessAnyOf': - if (isSet() && !isAnySet(args)) { - message = `<samp>${config.name}</samp> cannot be set if any of <samp>${args}</samp> are set`; - } - break; - default: - $log.warn("Unknown constraint predicate", constraintO, config); - } - if (message !== null) { - entity.addIssue(Issue.builder().group('config').ref(config.name).message($sce.trustAsHtml(message)).build()); - } - } - }); + .forEach(checkConstraints); } + // could do same as above to check parameters, but that doesn't make the parameters appear as config to set, + // so instead we merge parameters with config resolve(); }); } @@ -582,49 +584,51 @@ function BlueprintService($log, $q, $sce, paletteApi, iconGenerator, dslService) return $q.all(promises); } - function addConfigKeyDefinition(config, key) { - config.push({ - "name": key, - "label": key, - "description": "", - "priority": 1, - "pinned": true, - "type": "java.lang.String", - "constraints": [], - }); + function addConfigKeyDefinition(entity, key) { + // TODO return type, and below + entity.addConfigKeyDefinition(key, false); } - function addParameterDefinition(params, key) { - params.push({ - "name": key, - "type": "string", - }); + function addParameterDefinition(entity, key) { + entity.addParameterDefinition(key); } function addUnlistedConfigKeysDefinitions(entity) { // copy config key definitions set on this entity into the miscData aggregated view - let allConfig = entity.miscData.get('config') || []; + let allConfig = entity.miscDataOrDefault('configMap', {}); entity.config.forEach((value, key) => { - if (!allConfig.some((e) => e.name === key)) { - addConfigKeyDefinition(allConfig, key); + if (!allConfig[key]) { + entity.addConfigKeyDefinition(key); } }); - entity.miscData.set('config', allConfig); + entity.miscData.set('config', Object.values(allConfig)); + } function addUnlistedParameterDefinitions(entity) { // copy parameter definitions set on this entity into the miscData aggregated view; - // TODO see discussions in PR 112 about whether this is necessary and/or there is a better way - let allParams = entity.miscData.get('parameters') || []; + // see discussions in PR 112 about whether this is necessary and/or there is a better way; but note, this is much updated since + let allParams = entity.miscDataOrDefault('parametersMap', {}); entity.parameters.forEach((param) => { - if (!allParams.some((e) => e.name === param.name)) { - allParams.push(param); + if (!allParams[param.name]) { + allParams[param.name] = param; } }); - entity.miscData.set('parameters', allParams); + entity.miscData.set('parameters', Object.values(allParams)); } function populateEntityFromApiSuccess(entity, data) { + function mapped(list, field) { + let result = {}; + if (list) { + list.forEach(l => { + if (l && l[field]) { + result[l[field]] = l; + } + }); + } + return result; + } entity.clearIssues({group: 'type'}); entity.type = data.symbolicName; entity.icon = data.iconUrl || iconGenerator(data.symbolicName); @@ -637,8 +641,12 @@ function BlueprintService($log, $q, $sce, paletteApi, iconGenerator, dslService) entity.miscData.set('displayName', data.displayName); entity.miscData.set('symbolicName', data.symbolicName); entity.miscData.set('description', data.description); + entity.miscData.set('config', data.config || []); + entity.miscData.set('configMap', mapped(data.config, 'name')); entity.miscData.set('parameters', data.parameters || []); + entity.miscData.set('parametersMap', mapped(data.parameters, 'name')); + entity.miscData.set('sensors', data.sensors || []); entity.miscData.set('traits', data.supertypes || []); entity.miscData.set('tags', data.tags || []); diff --git a/ui-modules/blueprint-composer/app/components/quick-fix/quick-fix.js b/ui-modules/blueprint-composer/app/components/quick-fix/quick-fix.js index d703f11..ffcaa60 100644 --- a/ui-modules/blueprint-composer/app/components/quick-fix/quick-fix.js +++ b/ui-modules/blueprint-composer/app/components/quick-fix/quick-fix.js @@ -33,7 +33,6 @@ export function computeQuickFixes(allIssues) { allIssues.errors.byMessage = {}; Object.values(allIssues.errors.byEntity).forEach(list => { list.forEach(issue => { - // TODO key should be a tuple of group, ref, message let key = issue.group+":"+issue.ref+":"+issue.message; let v = allIssues.errors.byMessage[key]; if (!v) { @@ -126,8 +125,9 @@ function proposeSetFrom() { let createable = qfdef['source-key-createable']; // TODO if root param is required, show error - // TODO make default id containts type name + // TODO make default id contain type name // TODO show default id if no id present + // TODO if id is changed, update all refs // TODO allow graphically selectable @@ -162,7 +162,7 @@ function proposeSetFrom() { let pkey = 'set_from_' + sourceNode.id + '_' + ckey; if (!proposals[pkey]) { - if (sourceNode.create_key) { + if (create) { proposals[pkey] = { text: "Set from new parameter '" + ckey + "' on " + sourceNode.name, tooltip: "This will fix the error by setting the value here equal to the value of a new parameter '" + ckey + "' created on " + sourceNode.name @@ -180,13 +180,13 @@ function proposeSetFrom() { Object.assign(proposals[pkey], { issues: [], apply: (issue, entity) => { - if (sourceNode.create_key) { + if (create) { // check again so we only create once let hasParam = sourceNode.entity.getParameterNamed(ckey); if (!hasParam) { - sourceNode.entity.addParameter(Object.assign( + sourceNode.entity.addParameterDefinition(Object.assign( {name: ckey,}, - qfdef['source-key-parameter-definition'] || {} + qfdef['source-key-parameter-definition'], )); } } diff --git a/ui-modules/blueprint-composer/app/components/spec-editor/spec-editor.directive.js b/ui-modules/blueprint-composer/app/components/spec-editor/spec-editor.directive.js index 892167a..b1e556f 100644 --- a/ui-modules/blueprint-composer/app/components/spec-editor/spec-editor.directive.js +++ b/ui-modules/blueprint-composer/app/components/spec-editor/spec-editor.directive.js @@ -1052,8 +1052,7 @@ export function specEditorDirective($rootScope, $templateCache, $injector, $sani specEditor.addConfigKey = addConfigKey; function addConfigKey(name) { if (name) { - let allConfig = scope.model.miscData.get('config'); - blueprintService.addConfigKeyDefinition(allConfig, name); + blueprintService.addConfigKeyDefinition(scope.model, name); scope.model.addConfig(name, ''); loadLocalConfigFromModel(); scope.state.config.add.value = ''; @@ -1204,14 +1203,7 @@ export function specEditorDirective($rootScope, $templateCache, $injector, $sani } function addParameter(name) { - let allParams = scope.model.miscData.get('parameters'); - if (!allParams) { - allParams = []; - scope.model.miscData.set('parameters', allParams); - } - blueprintService.addParameterDefinition(allParams, name); - let param = allParams.find(p => p.name === name); - scope.model.addParameter(param); + scope.model.addParameterDefinition(name); loadLocalParametersFromModel(); scope.state.parameters.search = ''; scope.state.parameters.add.value = ''; diff --git a/ui-modules/blueprint-composer/app/components/util/model/entity.model.js b/ui-modules/blueprint-composer/app/components/util/model/entity.model.js index b844450..1eb2343 100644 --- a/ui-modules/blueprint-composer/app/components/util/model/entity.model.js +++ b/ui-modules/blueprint-composer/app/components/util/model/entity.model.js @@ -18,6 +18,7 @@ */ import {Issue} from './issue.model'; import {Dsl, DslParser} from './dsl.model'; +import {blueprintServiceProvider} from "../../providers/blueprint-service.provider"; const MEMBERSPEC_REGEX = /^(\w+\.)*[mM]ember[sS]pec$/; const FIRST_MEMBERSPEC_REGEX = /^(\w+\.)*first[mM]ember[sS]pec$/; @@ -550,6 +551,17 @@ export class Entity { return MISC_DATA.get(this); } + miscDataOrDefault(key, defaultValue) { + let miscData = this.miscData; + let result = miscData.get(key); + if (result) { + return result; + } else { + miscData.set(key, defaultValue); + return defaultValue; + } + } + equals(value) { if (value && value instanceof Entity) { try { @@ -593,10 +605,12 @@ Entity.prototype.setPoliciesFromJson = setPoliciesFromJson; Entity.prototype.getData = getData; Entity.prototype.addConfig = addConfig; -Entity.prototype.addParameter = addParameter; +Entity.prototype.addConfigKeyDefinition = addConfigKeyDefinition; +Entity.prototype.addParameterDefinition = addParameterDefinition; Entity.prototype.addMetadata = addMetadata; Entity.prototype.removeConfig = removeConfig; Entity.prototype.removeParameter = removeParameter; +Entity.prototype.updateParameter = updateParameter; Entity.prototype.removeMetadata = removeMetadata; Entity.prototype.isCluster = isCluster; Entity.prototype.isMemberSpec = isMemberSpec; @@ -639,8 +653,47 @@ function addConfig(key, value) { } } -function addParameter(param) { - PARAMETERS.get(this).push(param); +function addConfigKeyDefinition(param, overwrite) { + if (typeof param === 'string') { + param = { + "name": param, + "label": param, + "description": "", + "priority": 1, + "pinned": true, + "type": "java.lang.String", + "constraints": [], + }; + overwrite = false; + } + let key = (param || {}).name; + if (!key) throw new Error("'name' field must be included when adding parameter; was", param); + + let allConfig = this.miscDataOrDefault('configMap', {}); + allConfig[key] = Object.assign(allConfig[key] || {}, param, overwrite ? null : allConfig[key]); + this.miscData.set('config', Object.values(allConfig)); + + this.touch(); + return this; +} + +function addParameterDefinition(param, overwrite) { + if (typeof param === 'string') { + param = {name: key, type: 'string'}; + overwrite = false; + } + let key = (param || {}).name; + if (!key) throw new Error("'name' field must be included when adding parameter"); + + let allParams = this.miscDataOrDefault('parametersMap', {}); + allParams[key] = Object.assign(allParams[key] || {}, param, overwrite ? null : allParams[key]); + this.miscData.set('parameters', Object.values(allParams)); + + let eps = PARAMETERS.get(this); + this.updateParameter(key, allParams[key], true); + + this.addConfigKeyDefinition(allParams[key], overwrite); + this.touch(); return this; } @@ -685,17 +738,22 @@ function removeParameter(name) { /** - * Update an entry in brooklyn.parameters + * Update an entry in brooklyn.parameters if it exists * @param {string} name * @param {object} definition * @returns {Entity} */ -function updateParameter(name, definition) { - if (this.hasParameters()) { +function updateParameter(name, definition, createIfMissing) { + if (createIfMissing || this.hasParameters()) { let paramIndex = PARAMETERS.get(this).findIndex(e => e.name === name); if (paramIndex != -1) { PARAMETERS.get(this)[paramIndex] = definition; this.touch(); + } else { + if (createIfMissing) { + PARAMETERS.get(this).push(definition); + this.touch(); + } } } return this; @@ -1065,7 +1123,7 @@ function setParametersFromJson(incomingModel) { let self = this; incomingModel.map((param)=> { - self.addParameter(param); + self.addParameterDefinition(param); }); this.touch(); } @@ -1133,7 +1191,6 @@ function getParametersAsArray() { } function getParameterNamed(name) { - // TODO confirm works return PARAMETERS.get(this).find(p => p.name === name); } diff --git a/ui-modules/blueprint-composer/app/views/main/yaml/yaml.state.js b/ui-modules/blueprint-composer/app/views/main/yaml/yaml.state.js index e4e3606..a3d6616 100644 --- a/ui-modules/blueprint-composer/app/views/main/yaml/yaml.state.js +++ b/ui-modules/blueprint-composer/app/views/main/yaml/yaml.state.js @@ -95,6 +95,9 @@ function yamlCampStateController($scope, blueprintService, brSnackbar) { // blueprintService.refreshBlueprintMetadata(); } catch (err) { + + // YAML exceptions detected separately so ignore those + if (!(err instanceof YAMLException)) { issues.push({ from: CodeMirror.Pos(0, 0), @@ -103,6 +106,10 @@ function yamlCampStateController($scope, blueprintService, brSnackbar) { }); } } + if (issues.length) { + // these should be displayed, but sometimes aren't. + console.log("Issues detected in blueprint", issues); + } return issues; });
