Improve auto generation of list key values. (RIFT-15923)

Change-Id: Ia5a9444b5420f1fe0afd5d6f50792b124139fada
Signed-off-by: Bob Gallagher <bob.gallagher@riftio.com>
diff --git a/skyquake/plugins/composer/src/src/components/EditDescriptorModelProperties.js b/skyquake/plugins/composer/src/src/components/EditDescriptorModelProperties.js
index f3b2c7a..aa18963 100644
--- a/skyquake/plugins/composer/src/src/components/EditDescriptorModelProperties.js
+++ b/skyquake/plugins/composer/src/src/components/EditDescriptorModelProperties.js
@@ -138,7 +138,10 @@
 				create(model, path, property);
 			} else {
 				const name = path.join('.');
-				const value = Property.createModelInstance(property);
+				// get a unique name for the new list item based on the current list content
+				// some lists, based on the key, may not get a uniqueName generated here
+				const uniqueName = DescriptorModelMetaFactory.generateItemUniqueName(container.model[property.name], property);
+				const value = Property.createModelInstance(property, uniqueName);
 				utils.assignPathValue(this.model, name, value);
 			}
 			CatalogItemsActions.catalogItemDescriptorChanged(this.getRoot());
diff --git a/skyquake/plugins/composer/src/src/libraries/model/DescriptorModelMetaFactory.js b/skyquake/plugins/composer/src/src/libraries/model/DescriptorModelMetaFactory.js
index 164c850..5488e77 100644
--- a/skyquake/plugins/composer/src/src/libraries/model/DescriptorModelMetaFactory.js
+++ b/skyquake/plugins/composer/src/src/libraries/model/DescriptorModelMetaFactory.js
@@ -64,9 +64,17 @@
 
 		return cachedDescriptorModelMetaRequest;
 	},
-	createModelInstanceForType(typeOrPath) {
+	/**
+	 * Create a new instance of the indicated property and, if relevent, use the given
+	 * unique name for the instance's key (see generateItemUniqueName)
+	 * 
+	 * @param {Object | string} typeOrPath a property definition object or a path to a property 
+	 * @param [{string}] uniqueName optional 
+	 * @returns 
+	 */
+	createModelInstanceForType(typeOrPath, uniqueName) {
 		const modelMeta = this.getModelMetaForType(typeOrPath);
-		return DescriptorModelMetaProperty.createModelInstance(modelMeta);
+		return DescriptorModelMetaProperty.createModelInstance(modelMeta, uniqueName);
 	},
 	getModelMetaForType(typeOrPath, filterProperties = () => true) {
 		// resolve paths like 'nsd' or 'vnfd.vdu' or 'nsd.constituent-vnfd'
@@ -98,5 +106,46 @@
 			return result;
 		}
 		console.warn('no model uiState found for type', typeOrPath);
+	},
+	/**
+	 * For a list with a single valued key that is of type string, generate a unique name
+	 * for a new entry to be added to the indicated list. This name will use the provided
+	 * prefix (or the list's name) followed by a number. The number will be based on the
+	 * current length of the array but will insure there is no collision with an existing
+	 * name.
+	 * 
+	 * @param {Array} list the list model data
+	 * @param {prooerty} property the schema definition of the list 
+	 * @param [{any} prefix] the perferred prefix for the name. If not provide property.name
+	 *  will be used. 
+	 * @returns {string}
+	 */
+	generateItemUniqueName (list, property, prefix) {
+		if (   property.type !== 'list' 
+			|| property.key.length !== 1
+			|| property.properties.find(prop => prop.name === property.key[0])['data-type'] !== 'string') {
+			// only support list with a single key of type string
+			return null;
+		}
+		if (!prefix) {
+			prefix = property.name;
+		}
+		let key = property.key[0];
+		let suffix = list ? list.length + 1 : 1
+		let keyValue = prefix + '-' + suffix;
+		function makeUniqueName() {
+			if (list) {
+				for (let i = 0; i < list.length; i = ++i) {
+					if (list[i][key] === keyValue) {
+						keyValue = keyValue + '-' + (i+1);
+						makeUniqueName(); // not worried about recursing too deep (chances ??)
+						break;
+					}
+				}
+			}
+		}
+		makeUniqueName();
+		return keyValue;
 	}
+
 }
diff --git a/skyquake/plugins/composer/src/src/libraries/model/DescriptorModelMetaProperty.js b/skyquake/plugins/composer/src/src/libraries/model/DescriptorModelMetaProperty.js
index 4318c5b..2955e55 100644
--- a/skyquake/plugins/composer/src/src/libraries/model/DescriptorModelMetaProperty.js
+++ b/skyquake/plugins/composer/src/src/libraries/model/DescriptorModelMetaProperty.js
@@ -147,22 +147,36 @@
 		}
 		return /uuid/.test(property['data-type']);
 	},
-	createModelInstance(property) {
+	/**
+	 * Create a new instance of the indicated property and, if relevent, use the given
+	 * unique name for the instance's key (see generateItemUniqueName)
+	 * 
+	 * @param {Object} typeOrPath - property definition
+	 * @param {any} uniqueName 
+	 * @returns 
+	 */
+	createModelInstance(property, uniqueName) {
 		const Property = this;
 		const defaultValue = Property.defaultValue.bind(this);
-		function createModel(uiState, parentMeta) {
+		function createModel(uiState, parentMeta, uniqueName) {
 			const model = {};
 			if (Property.isLeaf(uiState)) {
 				if (uiState.name === 'name') {
-					return changeCase.param(parentMeta.name) + '-' + InstanceCounter.count(parentMeta[':qualified-type']);
+					return uniqueName || (changeCase.param(parentMeta.name) + '-' + InstanceCounter.count(parentMeta[':qualified-type']));
 				}
 				if (_isArray(parentMeta.key) && _includes(parentMeta.key, uiState.name)) {
 					if (/uuid/.test(uiState['data-type'])) {
 						return guid();
 					}
 					if (uiState['data-type'] === 'string') {
-						const prefix = uiState.name.replace('id', '');
-						return  (prefix ? changeCase.param(prefix) + '-' : '') + guid(5);
+						// if there is only one key property and we were given a 
+						// unique name (probably because creating a list entry
+						// property) then use the unique name otherwise make one up.
+						if (parentMeta.key.length > 1 || !uniqueName) {
+							const prefix = uiState.name.replace('id', '');
+							uniqueName = (prefix ? changeCase.param(prefix) + '-' : '') + guid(5);
+						}
+						return uniqueName;
 					}
 					if (/int/.test(uiState['data-type'])) {
 						return InstanceCounter.count(uiState[':qualified-type']);
@@ -173,7 +187,7 @@
 				return [];
 			} else {
 				uiState.properties.forEach(p => {
-					model[p.name] = createModel(p, uiState);
+					model[p.name] = createModel(p, uiState, uniqueName);
 				});
 			}
 			return model;
@@ -188,7 +202,7 @@
 			if (/list/.test(property.type)) {
 				property.type = 'container';
 			}
-			const modelInstance = createModel(property, property);
+			const modelInstance = createModel(property, property, uniqueName);
 			modelInstance.uiState = {type: property.name};
 			const modelFragment = DescriptorTemplateFactory.createModelForType(property[':qualified-type'] || property.name) || {};
 			Object.assign(modelInstance, modelFragment);
diff --git a/skyquake/plugins/composer/src/src/libraries/model/DescriptorTemplates.js b/skyquake/plugins/composer/src/src/libraries/model/DescriptorTemplates.js
index 2808412..1183d4e 100644
--- a/skyquake/plugins/composer/src/src/libraries/model/DescriptorTemplates.js
+++ b/skyquake/plugins/composer/src/src/libraries/model/DescriptorTemplates.js
@@ -23,18 +23,18 @@
 
 'use strict';
 
-import guid from './../guid'
-import InstanceCounter from './../InstanceCounter'
-
-const generateName = (prefix, counter) => prefix + '-' + InstanceCounter.count(counter);
-
+//
+// note: values can be expressions. After the object is created the funtion will be 
+// invoked. if you use the arrow function syntax the this pointer will reference
+// the created object.
+//
 export default {
 	'vnfd': {
 		'description': 'A simple VNF descriptor w/ one VDU',
 		'version': '1.0',
 		'connection-point': [
 			{
-				'name': 'cp1',
+				'name': 'connection-point-1',
 				'type': 'VPORT'
 			}
 		],
@@ -43,8 +43,8 @@
 				'uiState': {
 					'type': 'vdu'
 				},
-				'id': () => guid(5),
-				'name': () => generateName('vdu', 'vnfd.vdu'),
+				'id': 'vdu-1',
+				'name': 'vdu-1',
 				'vm-flavor': {
 					'vcpu-count': 4,
 					'memory-mb': 16384,
@@ -54,7 +54,7 @@
 				'external-interface': [
 					{
 						'name': 'eth0',
-						'vnfd-connection-point-ref': 'cp1',
+						'vnfd-connection-point-ref': 'connection-point-1',
 						'virtual-interface': {
 							'type': 'VIRTIO'
 						}
@@ -64,8 +64,6 @@
 		]
 	},
 	'vnfd.internal-vld': {
-		'id': () => guid(),
-		'name': () => generateName('vld', 'new.vnfd.internal-vld'),
 		'description': 'Virtual link for internal fabric',
 		'type': 'ELAN'
 	}
diff --git a/skyquake/plugins/composer/src/src/libraries/model/descriptors/ForwardingGraph.js b/skyquake/plugins/composer/src/src/libraries/model/descriptors/ForwardingGraph.js
index 35b3077..1173296 100644
--- a/skyquake/plugins/composer/src/src/libraries/model/descriptors/ForwardingGraph.js
+++ b/skyquake/plugins/composer/src/src/libraries/model/descriptors/ForwardingGraph.js
@@ -56,7 +56,9 @@
 	}
 
 	createRsp() {
-		const model = DescriptorModelMetaFactory.createModelInstanceForType('nsd.vnffgd.rsp');
+		const property = DescriptorModelMetaFactory.getModelMetaForType('nsd.vnffgd.rsp');
+		const uniqueName = DescriptorModelMetaFactory.generateItemUniqueName(this.rsp, property);
+		const model = DescriptorModelMetaFactory.createModelInstanceForType('nsd.vnffgd.rsp', uniqueName);
 		return this.rsp = new RecordServicePath(model, this);
 	}
 
diff --git a/skyquake/plugins/composer/src/src/libraries/model/descriptors/NetworkService.js b/skyquake/plugins/composer/src/src/libraries/model/descriptors/NetworkService.js
index 17646aa..b10fceb 100644
--- a/skyquake/plugins/composer/src/src/libraries/model/descriptors/NetworkService.js
+++ b/skyquake/plugins/composer/src/src/libraries/model/descriptors/NetworkService.js
@@ -118,7 +118,9 @@
 	}
 
 	createVld() {
-		const model = DescriptorModelMetaFactory.createModelInstanceForType('nsd.vld');
+		const property = DescriptorModelMetaFactory.getModelMetaForType('nsd.vld');
+		const uniqueName = DescriptorModelMetaFactory.generateItemUniqueName(this.vld, property);
+		const model = DescriptorModelMetaFactory.createModelInstanceForType('nsd.vld', uniqueName);
 		return this.vld = DescriptorModelFactory.newVirtualLink(model, this);
 	}
 
@@ -138,15 +140,13 @@
 	}
 
 	set vnffgd(obj) {
-		const onAddForwardingGraph = (fg) => {
-			const index = this.vnffgd.map(suffixAsInteger('short-name')).reduce(toBiggestValue, this.vnffgd.length);
-			fg.model['short-name'] = 'FG-' + index;
-		};
-		this.updateModelList('vnffgd', obj, ForwardingGraph, onAddForwardingGraph);
+		this.updateModelList('vnffgd', obj, ForwardingGraph);
 	}
 
 	createVnffgd(model) {
-		model = model || DescriptorModelMetaFactory.createModelInstanceForType('nsd.vnffgd');
+		const property = DescriptorModelMetaFactory.getModelMetaForType('nsd.vnffgd');
+		const uniqueName = DescriptorModelMetaFactory.generateItemUniqueName(this.vnffgd, property, 'fg');
+		model = model || DescriptorModelMetaFactory.createModelInstanceForType('nsd.vnffgd', uniqueName);
 		return this.vnffgd = DescriptorModelFactory.newForwardingGraph(model, this);
 	}
 
diff --git a/skyquake/plugins/composer/src/src/libraries/model/descriptors/VirtualNetworkFunction.js b/skyquake/plugins/composer/src/src/libraries/model/descriptors/VirtualNetworkFunction.js
index 73e10d3..5f3ad04 100644
--- a/skyquake/plugins/composer/src/src/libraries/model/descriptors/VirtualNetworkFunction.js
+++ b/skyquake/plugins/composer/src/src/libraries/model/descriptors/VirtualNetworkFunction.js
@@ -52,7 +52,9 @@
 	}
 
 	createVdu() {
-		const model = DescriptorModelMetaFactory.createModelInstanceForType('vnfd.vdu');
+		const property = DescriptorModelMetaFactory.getModelMetaForType('vnfd.vdu');
+		const uniqueName = DescriptorModelMetaFactory.generateItemUniqueName(this.vdu, property);
+		const model = DescriptorModelMetaFactory.createModelInstanceForType('vnfd.vdu', uniqueName);
 		return this.vdu = DescriptorModelFactory.newVirtualDeploymentUnit(model, this);
 	}
 
@@ -71,7 +73,9 @@
 	}
 
 	createVld() {
-		const model = DescriptorModelMetaFactory.createModelInstanceForType('vnfd.internal-vld');
+		const property = DescriptorModelMetaFactory.getModelMetaForType('vnfd.internal-vld');
+		const uniqueName = DescriptorModelMetaFactory.generateItemUniqueName(this['internal-vld'], property);
+		const model = DescriptorModelMetaFactory.createModelInstanceForType('vnfd.internal-vld', uniqueName);
 		return this.vld = DescriptorModelFactory.newInternalVirtualLink(model, this);
 	}