From 86c0a0aa75980ae608f62f074cc8a7566d3c1da5 Mon Sep 17 00:00:00 2001
From: Ricardo Matias <ricardo.matias0@gmail.com>
Date: Wed, 1 Feb 2017 10:42:49 +0100
Subject: [PATCH] feat(util/model): avoid persisting empty property element
 containers

Closes camunda/camunda-modeler#512
---
 lib/util/model/ModelCloneHelper.js     | 42 ++++++++++++++++++++++----
 test/spec/util/ModelCloneHelperSpec.js | 10 +++---
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/lib/util/model/ModelCloneHelper.js b/lib/util/model/ModelCloneHelper.js
index 776fc0ee..2db1e847 100644
--- a/lib/util/model/ModelCloneHelper.js
+++ b/lib/util/model/ModelCloneHelper.js
@@ -39,6 +39,10 @@ module.exports = ModelCloneHelper;
 ModelCloneHelper.prototype.clone = function(oldElement, newElement, properties) {
   this._newElement = newElement;
 
+  // this property allows us to avoid ending up with empty (xml) tags
+  // f.ex: if extensionElements.values is empty, don't set it
+  this._hasNestedProperty = false;
+
   // we want the extensionElements to be cloned last
   // so that they can check certain properties
   properties = sort(properties, function(prop) {
@@ -49,7 +53,7 @@ ModelCloneHelper.prototype.clone = function(oldElement, newElement, properties)
     var oldElementProp = oldElement.get(propName),
         newElementProp = newElement.get(propName),
         propDescriptor = newElement.$model.getPropertyDescriptor(newElement, propName),
-        name;
+        newProperty, name;
 
     // we're not interested in cloning:
     // - same values from simple types
@@ -72,14 +76,25 @@ ModelCloneHelper.prototype.clone = function(oldElement, newElement, properties)
       forEach(oldElementProp, function(extElement) {
         var newProp = this._deepClone(extElement);
 
-        newProp.$parent = newElement;
+        if (this._hasNestedProperty) {
+          newProp.$parent = newElement;
 
-        newElementProp.push(newProp);
+          newElementProp.push(newProp);
+        }
+
+        this._hasNestedProperty = false;
       }, this);
+
     } else {
       name = propName.replace(/bpmn:/, '');
 
-      newElement[name] = this._deepClone(oldElementProp);
+      newProperty = this._deepClone(oldElementProp);
+
+      if (this._hasNestedProperty) {
+        newElement[name] = newProperty;
+      }
+
+      this._hasNestedProperty = false;
     }
   }, this);
 
@@ -110,12 +125,15 @@ ModelCloneHelper.prototype._deepClone = function _deepClone(element) {
     return prop !== '$type';
   });
 
+  if (!properties.length) {
+    this._hasNestedProperty = true;
+  }
+
   forEach(properties, function(propName) {
     // check if the element has this property defined
     if (element[propName] !== undefined && (element[propName].$type || isArray(element[propName]))) {
 
       if (isArray(element[propName])) {
-        newProp[propName] = [];
 
         forEach(element[propName], function(property) {
           var extProp = element.$model.getTypeDescriptor(property.$type),
@@ -145,15 +163,27 @@ ModelCloneHelper.prototype._deepClone = function _deepClone(element) {
 
           newDeepProp.$parent = newProp;
 
+          if (!newProp[propName]) {
+            newProp[propName] = [];
+          }
+
+          this._hasNestedProperty = true;
+
           newProp[propName].push(newDeepProp);
         }, this);
 
       } else if (element[propName].$type) {
         newProp[propName] = this._deepClone(element[propName]);
 
-        newProp[propName].$parent = newProp;
+        if (newProp[propName]) {
+          this._hasNestedProperty = true;
+
+          newProp[propName].$parent = newProp;
+        }
       }
     } else {
+      this._hasNestedProperty = true;
+
       // just assign directly if it's a value
       newProp[propName] = element[propName];
     }
diff --git a/test/spec/util/ModelCloneHelperSpec.js b/test/spec/util/ModelCloneHelperSpec.js
index afd3f069..2d1b30a6 100644
--- a/test/spec/util/ModelCloneHelperSpec.js
+++ b/test/spec/util/ModelCloneHelperSpec.js
@@ -208,7 +208,7 @@ describe('util/ModelCloneHelper', function() {
       var extElem = userTask.extensionElements;
 
       // then
-      expect(extElem.values).to.be.empty;
+      expect(extElem).to.not.exist;
     }));
 
   });
@@ -230,10 +230,12 @@ describe('util/ModelCloneHelper', function() {
       });
 
       var signalEvtDef = moddle.create('bpmn:SignalEventDefinition', {
-        timeDuration: 'foobar'
+        async: true
       });
 
-      var multiInst = moddle.create('bpmn:MultiInstanceLoopCharacteristics');
+      var multiInst = moddle.create('bpmn:MultiInstanceLoopCharacteristics', {
+        elementVariable: 'foobar'
+      });
 
       var timerStartEvent = moddle.create('bpmn:StartEvent', {
         extensionElements: createExtElems(),
@@ -364,7 +366,7 @@ describe('util/ModelCloneHelper', function() {
       var extElems = clonedElement.extensionElements;
 
       // then
-      expect(extElems.values).be.empty;
+      expect(extElems).not.exist;
     }));
 
   });