Merge branch 'duplicate_key_errors_take2' into develop

# Conflicts:
#	src/js/util.js

src/js/History.js

@@ -1,5 +1,7 @@
 'use strict';
 
+var util = require('./util');
+
 /**
  * @constructor History
  * Store action history, enables undo and redo
@@ -121,6 +123,10 @@ function History (editor) {
         var nodes = params.paths.map(findNode);
         nodes.forEach(function (node) {
           var clone = node.clone();
+          if (parentNode.type === 'object') {
+            var existingFieldNames = parentNode.getFieldNames();
+            clone.field = util.findUniqueName(node.field, existingFieldNames);
+          }
           parentNode.insertAfter(clone, afterNode);
           afterNode = clone;
         });

src/js/Node.js

@@ -268,7 +268,7 @@ Node.prototype.setError = function (error, child) {
  * Render the error
  */
 Node.prototype.updateError = function() {
-  var error = this.error;
+  var error = this.fieldError || this.valueError || this.error;
   var tdError = this.dom.tdError;
   if (error && this.dom && this.dom.tr) {
     util.addClassName(this.dom.tr, 'jsoneditor-validation-error');
@@ -1312,15 +1312,6 @@ Node.select = function(editableDiv) {
   }, 0);
 };
 
-/**
- * Update the values from the DOM field and value of this node
- */
-Node.prototype.blur = function() {
-  // retrieve the actual field and value from the DOM.
-  this._getDomValue(false);
-  this._getDomField(false);
-};
-
 /**
  * Check if given node is a child. The method will check recursively to find
  * this node.
@@ -1541,11 +1532,11 @@ Node.prototype.deepEqual = function (json) {
 
 /**
  * Retrieve value from DOM
- * @param {boolean} [silent]  If true (default), no errors will be thrown in
- *                            case of invalid data
  * @private
  */
-Node.prototype._getDomValue = function(silent) {
+Node.prototype._getDomValue = function() {
+  this._clearValueError();
+
   if (this.dom.value && this.type != 'array' && this.type != 'object') {
     this.valueInnerText = util.getInnerText(this.dom.value);
   }
@@ -1567,15 +1558,50 @@ Node.prototype._getDomValue = function(silent) {
       }
     }
     catch (err) {
-      this.value = undefined;
+      // keep the previous value
-      // TODO: sent an action with the new, invalid value?
+      this._setValueError(translate('cannotParseValueError'));
-      if (silent !== true) {
-        throw err;
-      }
     }
   }
 };
 
+/**
+ * Show a local error in case of invalid value
+ * @param {string} message
+ * @private
+ */
+Node.prototype._setValueError = function (message) {
+  this.valueError = {
+    message: message
+  };
+  this.updateError();
+}
+
+Node.prototype._clearValueError = function () {
+  if (this.valueError) {
+    this.valueError = null;
+    this.updateError();
+  }
+}
+
+/**
+ * Show a local error in case of invalid or duplicate field
+ * @param {string} message
+ * @private
+ */
+Node.prototype._setFieldError = function (message) {
+  this.fieldError = {
+    message: message
+  };
+  this.updateError();
+}
+
+Node.prototype._clearFieldError = function () {
+  if (this.fieldError) {
+    this.fieldError = null;
+    this.updateError();
+  }
+}
+
 /**
  * Handle a changed value
  * @private
@@ -1882,31 +1908,50 @@ Node.prototype._updateDomField = function () {
 
 /**
  * Retrieve field from DOM
- * @param {boolean} [silent]  If true (default), no errors will be thrown in
+ * @param {boolean} [forceUnique]  If true, the field name will be changed
- *                            case of invalid data
+ *                                 into a unique name in case it is a duplicate.
  * @private
  */
-Node.prototype._getDomField = function(silent) {
+Node.prototype._getDomField = function(forceUnique) {
+  this._clearFieldError();
+
   if (this.dom.field && this.fieldEditable) {
     this.fieldInnerText = util.getInnerText(this.dom.field);
   }
 
-  if (this.fieldInnerText != undefined) {
+  if (this.fieldInnerText !== undefined) {
     try {
       var field = this._unescapeHTML(this.fieldInnerText);
 
+      var existingFieldNames = this.parent.getFieldNames(this);
+      var isDuplicate = existingFieldNames.indexOf(field) !== -1;
+
+      if (!isDuplicate) {
         if (field !== this.field) {
           this.field = field;
           this._debouncedOnChangeField();
         }
       }
-    catch (err) {
+      else {
-      this.field = undefined;
+        if (forceUnique) {
-      // TODO: sent an action here, with the new, invalid value?
+          // fix duplicate field: change it into a unique name
-      if (silent !== true) {
+          field = util.findUniqueName(field, existingFieldNames);
-        throw err;
+          if (field !== this.field) {
+            this.field = field;
+
+            // TODO: don't debounce but resolve right away, and cancel current debounce
+            this._debouncedOnChangeField();
           }
         }
+        else {
+          this._setFieldError(translate('duplicateFieldError'));
+        }
+      }
+    }
+    catch (err) {
+      // keep the previous field value
+      this._setFieldError(translate('cannotParseFieldError'));
+    }
   }
 };
 
@@ -1941,54 +1986,6 @@ Node.prototype._updateDomDefault = function () {
   }
 };
 
-/**
- * Validate this node and all it's childs
- * @return {Array.<{node: Node, error: {message: string}}>} Returns a list with duplicates
- */
-Node.prototype.validate = function () {
-  var errors = [];
-
-  // find duplicate keys
-  if (this.type === 'object') {
-    var keys = {};
-    var duplicateKeys = [];
-    for (var i = 0; i < this.childs.length; i++) {
-      var child = this.childs[i];
-      if (keys.hasOwnProperty(child.field)) {
-        duplicateKeys.push(child.field);
-      }
-      keys[child.field] = true;
-    }
-
-    if (duplicateKeys.length > 0) {
-      errors = this.childs
-          .filter(function (node) {
-            return duplicateKeys.indexOf(node.field) !== -1;
-          })
-          .map(function (node) {
-            return {
-              node: node,
-              error: {
-                message: translate('duplicateKey') + ' "' + node.field + '"'
-              }
-            }
-          });
-    }
-  }
-
-  // recurse over the childs
-  if (this.childs) {
-    for (var i = 0; i < this.childs.length; i++) {
-      var e = this.childs[i].validate();
-      if (e.length > 0) {
-        errors = errors.concat(e);
-      }
-    }
-  }
-
-  return errors;
-};
-
 /**
  * Clear the dom of the node
  */
@@ -2461,6 +2458,7 @@ Node.prototype.setSelected = function (selected, isFirst) {
 Node.prototype.updateValue = function (value) {
   this.value = value;
   this.previousValue = value;
+  this.valueError = undefined;
   this.updateDom();
 };
 
@@ -2471,6 +2469,7 @@ Node.prototype.updateValue = function (value) {
 Node.prototype.updateField = function (field) {
   this.field = field;
   this.previousField = field;
+  this.fieldError = undefined;
   this.updateDom();
 };
 
@@ -2913,7 +2912,8 @@ Node.prototype.onEvent = function (event) {
     switch (type) {
       case 'blur':
       case 'change':
-        this._getDomValue(true);
+        this._getDomValue();
+        this._clearValueError();
         this._updateDomValue();
         if (this.value) {
           domValue.innerHTML = this._escapeHTML(this.value);
@@ -2922,7 +2922,7 @@ Node.prototype.onEvent = function (event) {
 
       case 'input':
         //this._debouncedGetDomValue(true); // TODO
-        this._getDomValue(true);
+        this._getDomValue();
         this._updateDomValue();
         break;
 
@@ -2944,14 +2944,14 @@ Node.prototype.onEvent = function (event) {
 
       case 'keyup':
         //this._debouncedGetDomValue(true); // TODO
-        this._getDomValue(true);
+        this._getDomValue();
         this._updateDomValue();
         break;
 
       case 'cut':
       case 'paste':
         setTimeout(function () {
-          node._getDomValue(true);
+          node._getDomValue();
           node._updateDomValue();
         }, 1);
         break;
@@ -2963,7 +2963,6 @@ Node.prototype.onEvent = function (event) {
   if (target == domField) {
     switch (type) {
       case 'blur':
-      case 'change':
         this._getDomField(true);
         this._updateDomField();
         if (this.field) {
@@ -2972,7 +2971,7 @@ Node.prototype.onEvent = function (event) {
         break;
 
       case 'input':
-        this._getDomField(true);
+        this._getDomField();
         this._updateSchema();
         this._updateDomField();
         this._updateDomValue();
@@ -2984,14 +2983,14 @@ Node.prototype.onEvent = function (event) {
         break;
 
       case 'keyup':
-        this._getDomField(true);
+        this._getDomField();
         this._updateDomField();
         break;
 
       case 'cut':
       case 'paste':
         setTimeout(function () {
-          node._getDomField(true);
+          node._getDomField();
           node._updateDomField();
         }, 1);
         break;
@@ -3467,6 +3466,25 @@ Node.prototype._showColorPicker = function () {
   }
 };
 
+/**
+ * Get all field names of an object
+ * @param {Node} [excludeNode] Optional node to be excluded from the returned field names
+ * @return {string[]}
+ */
+Node.prototype.getFieldNames = function (excludeNode) {
+  if (this.type === 'object') {
+    return this.childs
+        .filter(function (child) {
+          return child !== excludeNode;
+        })
+        .map(function (child) {
+          return child.field;
+        });
+  }
+
+  return [];
+}
+
 /**
  * Remove nodes
  * @param {Node[] | Node} nodes
@@ -3531,6 +3549,10 @@ Node.onDuplicate = function(nodes) {
     var afterNode = lastNode;
     var clones = nodes.map(function (node) {
       var clone = node.clone();
+      if (node.parent.type === 'object') {
+        var existingFieldNames = node.parent.getFieldNames();
+        clone.field = util.findUniqueName(node.field, existingFieldNames);
+      }
       parent.insertAfter(clone, afterNode);
       afterNode = clone;
       return clone;

src/js/i18n.js

@@ -22,6 +22,9 @@ var _defs = {
     duplicateText: 'Duplicate',
     duplicateTitle: 'Duplicate selected fields (Ctrl+D)',
     duplicateField: 'Duplicate this field (Ctrl+D)',
+    duplicateFieldError: 'Duplicate field name',
+    cannotParseFieldError: 'Cannot parse field into JSON',
+    cannotParseValueError: 'Cannot parse value into JSON',
     empty: 'empty',
     expandAll: 'Expand all fields',
     expandTitle: 'Click to expand/collapse this field (Ctrl+E). \n' +
@@ -106,6 +109,9 @@ var _defs = {
     duplicateText: '复制',
     duplicateTitle: '复制选中字段(Ctrl+D)',
     duplicateField: '复制该字段(Ctrl+D)',
+    duplicateFieldError: '重复的字段名称',
+    cannotParseFieldError: '无法将字段解析为JSON',
+    cannotParseValueError: '无法将值解析为JSON',
     empty: '清空',
     expandAll: '展开所有字段',
     expandTitle: '点击 展开/收缩 该字段(Ctrl+E). \n' +
@@ -190,6 +196,9 @@ var _defs = {
     duplicateText: 'Duplicar',
     duplicateTitle: 'Duplicar campos selecionados (Ctrl+D)',
     duplicateField: 'Duplicar este campo (Ctrl+D)',
+    duplicateFieldError: 'Nome do campo duplicado',
+    cannotParseFieldError: 'Não é possível analisar o campo no JSON',
+    cannotParseValueError: 'Não é possível analisar o valor em JSON',
     empty: 'vazio',
     expandAll: 'Expandir todos campos',
     expandTitle: 'Clique para expandir/encolher este campo (Ctrl+E). \n' +
@@ -286,6 +295,9 @@ var _defs = {
     duplicateText: 'Aşağıya kopyala',
     duplicateTitle: 'Seçili alanlardan bir daha oluştur (Ctrl+D)',
     duplicateField: 'Bu alandan bir daha oluştur (Ctrl+D)',
+    duplicateFieldError: 'Duplicate field name',
+    cannotParseFieldError: 'Alan JSON\'a ayrıştırılamıyor',
+    cannotParseValueError: 'JSON\'a değer ayrıştırılamıyor',
     empty: 'boş',
     expandAll: 'Tüm alanları aç',
     expandTitle: 'Bu alanı açmak/kapatmak için tıkla (Ctrl+E). \n' +

src/js/treemode.js

@@ -283,13 +283,7 @@ treemode.update = function (json) {
  * @return {Object | undefined} json
  */
 treemode.get = function () {
-  // remove focus from currently edited node
+  // TODO: resolve pending debounced input changes if any, but do not resolve invalid inputs
-  if (this.focusTarget) {
-    var node = Node.getNodeFromTarget(this.focusTarget);
-    if (node) {
-      node.blur();
-    }
-  }
 
   if (this.node) {
     return this.node.getValue();
@@ -577,9 +571,6 @@ treemode.validate = function () {
 
   var json = root.getValue();
 
-  // check for duplicate keys
-  var duplicateErrors = root.validate();
-
   // execute JSON schema validation
   var schemaErrors = [];
   if (this.validateSchema) {
@@ -611,7 +602,7 @@ treemode.validate = function () {
         .then(function (customValidationErrors) {
           // only apply when there was no other validation started whilst resolving async results
           if (seq === me.validationSequence) {
-            var errorNodes = [].concat(duplicateErrors, schemaErrors, customValidationErrors || []);
+            var errorNodes = [].concat(schemaErrors, customValidationErrors || []);
             me._renderValidationErrors(errorNodes);
           }
         })

src/js/util.js

@@ -1167,3 +1167,23 @@ exports.get = function (object, path) {
 
   return value;
 }
+
+/**
+ * Find a unique name. Suffix the name with ' (copy)', '(copy 2)', etc
+ * until a unique name is found
+ * @param {string} name
+ * @param {Array} existingPropNames    Array with existing prop names
+ */
+exports.findUniqueName = function(name, existingPropNames) {
+  var strippedName = name.replace(/ \(copy( \d+)?\)$/, '')
+  var validName = strippedName
+  var i = 1
+
+  while (existingPropNames.indexOf(validName) !== -1) {
+    var copy = 'copy' + (i > 1 ? (' ' + i) : '')
+    validName = strippedName + ' (' + copy + ')'
+    i++
+  }
+
+  return validName
+}

test/util.test.js

@@ -247,5 +247,51 @@ describe('util', function () {
       assert.strictEqual(util.makeFieldTooltip({examples: ['foo']}, 'pt-BR'), 'Exemplos\n"foo"');
     });
   });
+
+  it('should find a unique name', function () {
+    assert.strictEqual(util.findUniqueName('other', [
+      'a',
+      'b',
+      'c'
+    ]), 'other')
+
+    assert.strictEqual(util.findUniqueName('b', [
+      'a',
+      'b',
+      'c'
+    ]), 'b (copy)')
+
+    assert.strictEqual(util.findUniqueName('b', [
+      'a',
+      'b',
+      'c',
+      'b (copy)'
+    ]), 'b (copy 2)')
+
+    assert.strictEqual(util.findUniqueName('b', [
+      'a',
+      'b',
+      'c',
+      'b (copy)',
+      'b (copy 2)'
+    ]), 'b (copy 3)')
+
+    assert.strictEqual(util.findUniqueName('b (copy)', [
+      'a',
+      'b',
+      'b (copy)',
+      'b (copy 2)',
+      'c'
+    ]), 'b (copy 3)')
+
+    assert.strictEqual(util.findUniqueName('b (copy 2)', [
+      'a',
+      'b',
+      'b (copy)',
+      'b (copy 2)',
+      'c'
+    ]), 'b (copy 3)')
+  })
+
   // TODO: thoroughly test all util methods
 });