AMBARI-2055. Oozie reconfig forces the user to enter bogus values for two parameters in order to save any changes. (yusaku)

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/trunk@1478193 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -823,6 +823,9 @@ Trunk (unreleased changes):
 
  BUG FIXES
 
+ AMBARI-2055. Oozie reconfig forces the user to enter bogus values for two
+ parameters in order to save any changes. (yusaku)
+
  AMBARI-2054. If "Install from Local Repository" selected in install wizard,
  Add Host wizard not working. (yusaku)
 

ambari-web/app/assets/test/tests.js

@@ -39,4 +39,5 @@ require('test/main/item_test');
 require('test/utils/form_field_test');
 require('test/utils/misc_test');
 require('test/utils/validator_test');
+require('test/utils/config_test');
 require('test/views/common/chart/linear_time_test');

ambari-web/app/data/config_properties.js

@@ -65,6 +65,9 @@
  *     E.g., "General", "Advanced", "NameNode", "DataNode"
  */
 
+var App = require('app');
+require('config');
+
 module.exports =
 {
   "configProperties": [

ambari-web/app/utils/config.js

@@ -72,6 +72,40 @@ App.config = Em.Object.create({
     }
     return category;
   },
+  /**
+   * additional handling for special properties such as
+   * checkbox and digital which values with 'm' at the end
+   * @param config
+   */
+  handleSpecialProperties: function(config){
+    if (config.displayType === 'int' && /\d+m$/.test(config.value)) {
+      config.value = config.value.slice(0, config.value.length - 1);
+      config.defaultValue = config.value;
+    }
+    if (config.displayType === 'checkbox') {
+      config.value = (config.value === 'true') ? config.defaultValue = true : config.defaultValue = false;
+    }
+  },
+  /**
+   * calculate config properties:
+   * category, filename, isRequired, isUserProperty
+   * @param config
+   * @param isAdvanced
+   * @param advancedConfigs
+   */
+  calculateConfigProperties: function(config, isAdvanced, advancedConfigs){
+    if (!isAdvanced || this.get('customFileNames').contains(config.filename)) {
+      var categoryMetaData = this.identifyCategory(config);
+      if (categoryMetaData != null) {
+        config.category = categoryMetaData.get('name');
+        if(!isAdvanced) config.isUserProperty = true;
+      }
+    } else {
+      config.category = 'Advanced';
+      config.filename = isAdvanced && advancedConfigs.findProperty('name', config.name).filename;
+      config.isRequired = false;
+    }
+  },
   /**
    * return:
    *   configs,
@@ -119,30 +153,13 @@ App.config = Em.Object.create({
           serviceConfigObj.isOverridable = configsPropertyDef.isOverridable === undefined ? true : configsPropertyDef.isOverridable;
           serviceConfigObj.serviceName = configsPropertyDef ? configsPropertyDef.serviceName : null;
         }
+        // MAPREDUCE contains core-site properties but doesn't show them
         if(serviceConfigObj.serviceName === 'MAPREDUCE' && serviceConfigObj.filename === 'core-site.xml'){
           serviceConfigObj.isVisible = false;
         }
         if (_tag.siteName === 'global') {
           if (configsPropertyDef) {
-            if (configsPropertyDef.displayType === 'int') {
-              if (/\d+m$/.test(properties[index])) {
-
-                serviceConfigObj.value = properties[index].slice(0, properties[index].length - 1);
-                serviceConfigObj.defaultValue = serviceConfigObj.value;
-              }
-            }
-            if (configsPropertyDef.displayType === 'checkbox') {
-              switch (properties[index]) {
-                case 'true' :
-                  serviceConfigObj.value = true;
-                  serviceConfigObj.defaultValue = true;
-                  break;
-                case 'false' :
-                  serviceConfigObj.value = false;
-                  serviceConfigObj.defaultValue = false;
-                  break;
-              }
-            }
+            this.handleSpecialProperties(serviceConfigObj);
           }
           serviceConfigObj.id = 'puppet var';
           serviceConfigObj.displayName = configsPropertyDef ? configsPropertyDef.displayName : null;
@@ -154,16 +171,7 @@ App.config = Em.Object.create({
           serviceConfigObj.id = 'site property';
           serviceConfigObj.displayType = 'advanced';
           serviceConfigObj.displayName = configsPropertyDef ? configsPropertyDef.displayName : index;
-          if (!isAdvanced || this.get('customFileNames').contains(serviceConfigObj.filename)) {
-            var categoryMetaData = this.identifyCategory(serviceConfigObj);
-            if (categoryMetaData != null) {
-              serviceConfigObj.category = categoryMetaData.get('name');
-              if(!isAdvanced) serviceConfigObj.isUserProperty = true;
-            }
-          } else {
-            serviceConfigObj.category = 'Advanced';
-            serviceConfigObj.filename = isAdvanced && advancedConfigs.findProperty('name', index).filename;
-          }
+          this.calculateConfigProperties(serviceConfigObj, isAdvanced, advancedConfigs);
           configs.push(serviceConfigObj);
         } else {
           mappingConfigs.push(serviceConfigObj);
@@ -219,14 +227,8 @@ App.config = Em.Object.create({
           isOverridable: true,
           overrides: stored.overrides,
           isRequired: !isAdvanced
-        }
-        if (!isAdvanced || this.get('customFileNames').contains(configData.filename)) {
-          var categoryMetaData = this.identifyCategory(configData);
-          if (categoryMetaData != null) {
-            configData.category = categoryMetaData.get('name');
-            if(!isAdvanced) configData.isUserProperty = true;
-          }
-        }
+        };
+        this.calculateConfigProperties(configData, isAdvanced, advancedConfigs);
       } else if (preDefined && !stored) {
         configData = preDefined;
         if (isAdvanced) {

ambari-web/test/utils/config_test.js

@@ -0,0 +1,116 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+var App = require('app');
+require('utils/config');
+
+describe('App.config', function () {
+
+  describe('#identifyCategory', function () {
+    var data = {};
+    it('should return null if config doesn\'t have category', function () {
+      expect(App.config.identifyCategory(data)).to.equal(null);
+    });
+    it('should return "AdvancedCoreSite" if filename "core-site.xml" and serviceName "HDFS"', function () {
+      data = {
+        serviceName: 'HDFS',
+        filename: 'core-site.xml'
+      }
+      expect(App.config.identifyCategory(data).name).to.equal('AdvancedCoreSite');
+    });
+    it('should return "CapacityScheduler" if filename "capacity-scheduler.xml" and serviceName "MAPREDUCE"', function () {
+      data = {
+        serviceName: 'MAPREDUCE',
+        filename: 'capacity-scheduler.xml'
+      }
+      expect(App.config.identifyCategory(data).name).to.equal('CapacityScheduler');
+    });
+  });
+
+  describe('#handleSpecialProperties', function () {
+    var config = {};
+    it('value should be transformed to "1024" from "1024m"', function () {
+      config = {
+        displayType: 'int',
+        value: '1024m',
+        defaultValue: '1024m'
+      };
+      App.config.handleSpecialProperties(config);
+      expect(config.value).to.equal('1024');
+      expect(config.defaultValue).to.equal('1024');
+    });
+    it('value should be transformed to true from "true"', function () {
+      config = {
+        displayType: 'checkbox',
+        value: 'true',
+        defaultValue: 'true'
+      };
+      App.config.handleSpecialProperties(config);
+      expect(config.value).to.equal(true);
+      expect(config.defaultValue).to.equal(true);
+    });
+    it('value should be transformed to false from "false"', function () {
+      config = {
+        displayType: 'checkbox',
+        value: 'false',
+        defaultValue: 'false'
+      };
+      App.config.handleSpecialProperties(config);
+      expect(config.value).to.equal(false);
+      expect(config.defaultValue).to.equal(false);
+    });
+  });
+
+  describe('#calculateConfigProperties', function () {
+    var config = {};
+    var isAdvanced = false;
+    var advancedConfigs = [];
+    it('isUserProperty should be true if config is custom(site.xml) and not advanced', function () {
+      config = {
+        serviceName: 'HDFS',
+        filename: 'core-site.xml'
+      };
+      App.config.calculateConfigProperties(config, isAdvanced, advancedConfigs);
+      expect(config.isUserProperty).to.equal(true);
+    });
+    it('isUserProperty should be false if config from "capacity-scheduler.xml" or "mapred-queue-acls.xml" ', function () {
+      config = {
+        serviceName: 'MAPREDUCE',
+        filename: 'capacity-scheduler.xml',
+        isUserProperty: false
+      };
+      isAdvanced = true;
+      App.config.calculateConfigProperties(config, isAdvanced, advancedConfigs);
+      expect(config.isUserProperty).to.equal(false);
+    });
+    it('isRequired should be false if config is advanced"', function () {
+      config = {
+        name: 'test',
+        serviceName: 'HDFS',
+        filename: 'core-site.xml'
+      };
+      isAdvanced = true;
+      advancedConfigs = [{name:'test', filename: 'test.xml'}];
+      App.config.calculateConfigProperties(config, isAdvanced, advancedConfigs);
+      expect(config.category).to.equal('Advanced');
+      expect(config.isRequired).to.equal(false);
+      expect(config.filename).to.equal('test.xml');
+    });
+  });
+
+});

ambari-web/test/utils/validator_test.js

@@ -253,5 +253,22 @@ describe('validator', function () {
       })
     });
   })
-
+  describe('#isValidConfigKey(value)', function() {
+    var tests = [
+      {m:'"123" - valid',i:'123',e:true},
+      {m:'"abc" - valid',i:'abc',e:true},
+      {m:'"abc123" - valid',i:'abc123',e:true},
+      {m:'".abc." - valid',i:'.abc.',e:true},
+      {m:'"_abc_" - valid',i:'_abc_',e:true},
+      {m:'"-abc-" - valid',i:'-abc-',e:true},
+      {m:'"abc 123" - invalid',i:'abc 123',e:false},
+      {m:'"a"b" - invalid',i:'a"b',e:false},
+      {m:'"a\'b" - invalid',i:'a\'b',e:false}
+    ];
+    tests.forEach(function(test) {
+      it(test.m + ' ', function () {
+        expect(validator.isValidConfigKey(test.i)).to.equal(test.e);
+      })
+    });
+  })
 })