Merge pull request #154 from frogfather/file-uploader-bug

add custom onChange directive to correct file uploader bug
diff --git a/Jenkinsfile b/Jenkinsfile
index 05aca28..50bc00b 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -18,6 +18,12 @@
  */
 
 node(label: 'ubuntu') {
+    properties([
+        pipelineTriggers([
+            issueCommentTrigger('.*test this please.*')
+        ])
+    ])
+
     catchError {
         def environmentDockerImage
 
@@ -36,7 +42,7 @@
             }
 
             stage('Run tests') {
-                environmentDockerImage.inside('-i --name brooklyn-${DOCKER_TAG} --mount type=bind,source="${HOME}/.m2/settings.xml",target=/var/maven/.m2/settings.xml,readonly -v ${WORKSPACE}:/usr/build -w /usr/build') {
+                environmentDockerImage.inside('-i --name brooklyn-${DOCKER_TAG} -v ${WORKSPACE}/.m2:/var/maven/.m2 --mount type=bind,source="${HOME}/.m2/settings.xml",target=/var/maven/.m2/settings.xml,readonly -v ${WORKSPACE}:/usr/build -w /usr/build') {
                     sh 'mvn clean install -Duser.home=/var/maven -Duser.name=jenkins'
                 }
             }
@@ -44,12 +50,10 @@
             // Conditional stage to deploy artifacts, when not building a PR
             if (env.CHANGE_ID == null) {
                 stage('Deploy artifacts') {
-                    environmentDockerImage.inside('-i --name brooklyn-${DOCKER_TAG} --mount type=bind,source="${HOME}/.m2/settings.xml",target=/var/maven/.m2/settings.xml,readonly -v ${WORKSPACE}:/usr/build -w /usr/build') {
+                    environmentDockerImage.inside('-i --name brooklyn-${DOCKER_TAG} -v ${WORKSPACE}/.m2:/var/maven/.m2 --mount type=bind,source="${HOME}/.m2/settings.xml",target=/var/maven/.m2/settings.xml,readonly -v ${WORKSPACE}:/usr/build -w /usr/build') {
                         sh 'mvn deploy -DskipTests -Duser.home=/var/maven -Duser.name=jenkins'
                     }
                 }
-
-                // TODO: Publish docker image to https://hub.docker.com/r/apache/brooklyn/ ?
             }
         }
     }
diff --git a/ui-modules/blueprint-composer/app/views/main/main.controller.js b/ui-modules/blueprint-composer/app/views/main/main.controller.js
index 886b4a1..fc1ac1a 100644
--- a/ui-modules/blueprint-composer/app/views/main/main.controller.js
+++ b/ui-modules/blueprint-composer/app/views/main/main.controller.js
@@ -131,9 +131,28 @@
         
         yaml = edit.type.plan.data;
     }
+    
+    vm.isGraphicalMode = () => {
+        return $state.includes(graphicalState.name);
+    };
+    vm.isYamlMode = () => {
+        return $state.includes(yamlState.name);
+    };
 
     if (yaml) {
-        blueprintService.setFromYaml(yaml);
+        if (vm.isYamlMode()) {
+            // don't set blueprint; yaml mode will take from "initial yaml" 
+            blueprintService.reset();
+            $scope.initialYaml = yaml;
+        } else {
+            try {
+                blueprintService.setFromYaml(yaml);
+            } catch (e) {
+                console.warn("YAML supplied for editing is not valid for a blueprint. It will be ignored unless opened in the YAML editor:", e);
+                blueprintService.reset();
+                $scope.initialYaml = yaml;
+            }
+        }
     } else {
         blueprintService.reset();
     }
@@ -147,10 +166,6 @@
         deployApplication();
     };
 
-    vm.isGraphicalMode = () => {
-        return $state.includes(graphicalState.name);
-    };
-
     vm.getAllActions = () => {
         return actionService.getActions();
     }
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 bfd47f2..f9390ae 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
@@ -39,6 +39,11 @@
         brSnackbar.create(`Cannot load blueprint: ${ex.message}`);
         vm.yaml = '';
     }
+    if ($scope.initialYaml && !vm.yaml) {
+        // either yaml was supplied and yaml mode requested, skipping blueprint setup,
+        // or the yaml was invalid, an error logged, and this was recorded
+        vm.yaml = $scope.initialYaml; 
+    }
 
     if (!CodeMirror.lint.hasOwnProperty('yaml-composer')) {
         CodeMirror.registerGlobalHelper('lint', 'yaml-composer', mode => mode.name === 'yaml', (text, options, cm) => {
diff --git a/ui-modules/utils/quick-launch/quick-launch.js b/ui-modules/utils/quick-launch/quick-launch.js
index e4b1a67..cf85869 100644
--- a/ui-modules/utils/quick-launch/quick-launch.js
+++ b/ui-modules/utils/quick-launch/quick-launch.js
@@ -177,21 +177,87 @@
             return yaml.safeDump(newApp);
         }
 
-        function buildComposerYaml() {
+        function buildComposerYaml(validate) {
             if ($scope.yamlViewDisplayed) {
                 return angular.copy($scope.editorYaml);
             } else {
-                let newApp = {
-                    name: $scope.model.name || $scope.app.displayName,
-                };
-                if ($scope.model.location) {
-                    newApp.location = $scope.model.location;
+                let planText = $scope.app.plan.data || "{}";
+                let result = {};
+                
+                // this is set if we're able to parse the plan's text definition, and then:
+                // - we've had to override a field from the plan's text definition, because a value is set _and_ different; or
+                // - the plan's text definition is indented or JSON rather than YAML (not outdented yaml)
+                // and in either case we use the result _object_ ... 
+                // unless we didn't actually change anything, in which case this is ignored
+                let cannotUsePlanText = false;
+                
+                if (validate) {
+                    result = yaml.safeLoad(planText);
+                    if (typeof result !== 'object') {
+                        throw "The plan is not a YAML map, but of type "+(typeof result);
+                    }
+                    if (!result.services) {
+                        throw "The plan does not have any services.";
+                    }
+                    for (const [k,v] of Object.entries(result) ) {
+                       if (planText.indexOf(k)!=0 && planText.indexOf('\n'+k+':')<0) {
+                          // plan is not outdented yaml, can't use its text mode
+                          cannotUsePlanText = true;
+                          break;
+                       }
+                    }
                 }
-                if ($scope.entityToDeploy[BROOKLYN_CONFIG]) {
-                    newApp[BROOKLYN_CONFIG] = $scope.entityToDeploy[BROOKLYN_CONFIG]
+                
+                let newApp = {};
+                
+                let newName = $scope.model.name || $scope.app.displayName;
+                if (newName && newName != result.name) {
+                    newApp.name = newName;
+                    if (result.name) {
+                        delete result.name;
+                        cannotUsePlanText = true;
+                    }
                 }
-                // TODO if plan data has config in the root (unlikely) this will have errors
-                return yaml.safeDump(newApp) + "\n" + $scope.app.plan.data;
+                
+                let newLocation = $scope.model.location;
+                if (newLocation && newLocation != result.location) {
+                    newApp.location = newLocation;
+                    if (result.location) {
+                        delete result.location;
+                        cannotUsePlanText = true;
+                    }
+                }
+
+                let newConfig = $scope.entityToDeploy[BROOKLYN_CONFIG];
+                if (newConfig) {
+                    if (result[BROOKLYN_CONFIG]) {
+                        let oldConfig = result[BROOKLYN_CONFIG];
+                        let mergedConfig = angular.copy(oldConfig);
+                        for (const [k,v] of Object.entries(newConfig) ) {
+                            if (mergedConfig[k] != v) {
+                                cannotUsePlanText = true;
+                                mergedConfig[k] = v;
+                            }
+                        }
+                        if (cannotUsePlanText) {
+                            newApp[BROOKLYN_CONFIG] = mergedConfig;
+                            delete result[BROOKLYN_CONFIG];
+                        }
+                    } else {
+                        newApp[BROOKLYN_CONFIG] = newConfig;
+                    }
+                }
+                
+                // prefer to use the actual yaml input, but if it's not possible
+                let tryMergeByConcatenate = 
+                    Object.keys(newApp).length ?
+                        (yaml.safeDump(newApp) + "\n" + ((validate && cannotUsePlanText) ? yaml.safeDump(result) : planText))
+                        : planText;
+                if (validate) {
+                    // don't think there's any way we'd wind up with invalid yaml but check to be sure
+                    yaml.safeLoad(tryMergeByConcatenate);
+                }
+                return tryMergeByConcatenate;
             }
         }
 
@@ -205,8 +271,19 @@
         }
 
         function openComposer() {
-            window.location.href = '/brooklyn-ui-blueprint-composer/#!/graphical?'+
-                'yaml='+encodeURIComponent(buildComposerYaml());
+            try {
+              window.location.href = '/brooklyn-ui-blueprint-composer/#!/graphical?'+
+                'yaml='+encodeURIComponent(buildComposerYaml(true));
+            } catch (error) {
+              console.warn("Opening composer in YAML text editor mode because we cannot generate a model for this configuration:", error);
+              window.location.href = '/brooklyn-ui-blueprint-composer/#!/yaml?'+
+                'yaml='+encodeURIComponent(
+                    "# This plan may have items which require attention so is being opened in YAML text editor mode.\n"+
+                    "# The YAML was autogenerated by merging the plan with any values provided in UI, but issues were\n"+
+                    "# detected that mean it might not be correct. Please check the blueprint below carefully.\n"+
+                    "\n"+
+                    buildComposerYaml(false));
+            }
         }
 
         function clearError() {
diff --git a/ui-modules/utils/yaml-editor/addon/lint/lint-yaml-brooklyn.js b/ui-modules/utils/yaml-editor/addon/lint/lint-yaml-brooklyn.js
index 2d5e424..e4cf85c 100644
--- a/ui-modules/utils/yaml-editor/addon/lint/lint-yaml-brooklyn.js
+++ b/ui-modules/utils/yaml-editor/addon/lint/lint-yaml-brooklyn.js
@@ -29,38 +29,31 @@
 import catalogVersionSchema from '../schemas/catalog-version.json';
 import rootSchema from '../schemas/root.json';
 
-CodeMirror.registerGlobalHelper('lint', 'yamlBlueprint', (mode, cm) => (mode.name === 'yaml' && mode.type === 'blueprint'), (text, options, cm) => {
-    let validator = new Validator();
+let blueprintValidator = new Validator();
+let catalogValidator = new Validator();
+let rootValidator = new Validator();
 
+[ blueprintValidator, catalogValidator, rootValidator ].forEach(validator => {
     validator.addSchema(JSON.parse(blueprintSchema), '/Blueprint');
     validator.addSchema(JSON.parse(blueprintEntitySchema), '/Blueprint/Entity');
     validator.addSchema(JSON.parse(blueprintLocationSchema), '/Blueprint/Location');
-
-    return lint(validator, blueprintSchema, text, options, cm);
 });
-CodeMirror.registerGlobalHelper('lint', 'yamlCatalog', (mode, cm) => (mode.name === 'yaml' && mode.type === 'catalog'), (text, options, cm) => {
-    let validator = new Validator();
 
+[ catalogValidator, rootValidator ].forEach(validator => {
     validator.addSchema(JSON.parse(catalogSchema), '/Catalog');
     validator.addSchema(JSON.parse(catalogItemReferenceSchema), '/Catalog/Item/Reference');
     validator.addSchema(JSON.parse(catalogItemInlineSchema), '/Catalog/Item/Inline');
     validator.addSchema(JSON.parse(catalogVersionSchema), '/Catalog/Version');
-
-    return lint(validator, catalogSchema, text, options, cm);
 });
-CodeMirror.registerGlobalHelper('lint', 'yamlBrooklyn', (mode, cm) => (mode.name === 'yaml' && mode.type === 'brooklyn'), (text, options, cm) => {
-    let validator = new Validator();
 
-    validator.addSchema(JSON.parse(blueprintSchema), '/Blueprint');
-    validator.addSchema(JSON.parse(blueprintEntitySchema), '/Blueprint/Entity');
-    validator.addSchema(JSON.parse(blueprintLocationSchema), '/Blueprint/Location');
-    validator.addSchema(JSON.parse(catalogSchema), '/Catalog');
-    validator.addSchema(JSON.parse(catalogItemReferenceSchema), '/Catalog/Item/Reference');
-    validator.addSchema(JSON.parse(catalogItemInlineSchema), '/Catalog/Item/Inline');
-    validator.addSchema(JSON.parse(catalogVersionSchema), '/Catalog/Version');
-
-    return lint(validator, rootSchema, text, options, cm);
-});
+CodeMirror.registerGlobalHelper('lint', 'yamlBlueprint', (mode, cm) => (mode.name === 'yaml' && mode.type === 'blueprint'), 
+    (text, options, cm) => lint(blueprintValidator, blueprintSchema, text, options, cm));
+    
+CodeMirror.registerGlobalHelper('lint', 'yamlCatalog', (mode, cm) => (mode.name === 'yaml' && mode.type === 'catalog'), 
+    (text, options, cm) => lint(catalogValidator, catalogSchema, text, options, cm));
+    
+CodeMirror.registerGlobalHelper('lint', 'yamlBrooklyn', (mode, cm) => (mode.name === 'yaml' && mode.type === 'brooklyn'), 
+    (text, options, cm) => lint(rootValidator, rootSchema, text, options, cm));
 
 function lint(validator, baseSchema, text, options, cm) {
     let issues = [];