Fix impossible to drag and drop same service twice into the composition

When a service proxy instance including underscore in its name is added into
the composition, instance ID suffix is not properly calculated and that results
in duplicate key error due to ID conflict.
This change normalizes the name to find existing proxy instances to get suffix
number calculated properly.

Change-Id: I6f2ceb4b9cf020e1523264a99e1347b6caa98965
Issue-ID: SDC-2043
Signed-off-by: Satoshi Fujii <fujii-satoshi@jp.fujitsu.com>

2 files changed, 51 insertions, 1 deletions

diff --git a/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacade.java b/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacade.java
index d5e0675203..0d39bb264f 100644
--- a/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacade.java
+++ b/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacade.java
@@ -778,7 +778,7 @@ public class ToscaOperationFacade {
         if (StringUtils.isEmpty(componentInstance.getIcon())) {
             componentInstance.setIcon(origComponent.getIcon());
         }
-        String nameToFindForCounter = componentInstance.getOriginType() == OriginTypeEnum.ServiceProxy ? componentInstance.getSourceModelName() + PROXY_SUFFIX : origComponent.getName();
+        String nameToFindForCounter = componentInstance.getOriginType() == OriginTypeEnum.ServiceProxy ? ValidationUtils.normaliseComponentName(componentInstance.getSourceModelName()) + PROXY_SUFFIX : origComponent.getName();
         String nextComponentInstanceCounter = getNextComponentInstanceCounter(containerComponent, nameToFindForCounter);
         Either<ImmutablePair<TopologyTemplate, String>, StorageOperationStatus> addResult = nodeTemplateOperation.addComponentInstanceToTopologyTemplate(ModelConverter.convertToToscaElement(containerComponent),
                 ModelConverter.convertToToscaElement(origComponent), nextComponentInstanceCounter, componentInstance, allowDeleted, user);
diff --git a/catalog-model/src/test/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacadeTest.java b/catalog-model/src/test/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacadeTest.java
index 1bbbd1c087..424ea2bd17 100644
--- a/catalog-model/src/test/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacadeTest.java
+++ b/catalog-model/src/test/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacadeTest.java
@@ -30,6 +30,7 @@
 package org.openecomp.sdc.be.model.jsonjanusgraph.operations;
 
 import fj.data.Either;
+import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -48,13 +49,17 @@ import org.openecomp.sdc.be.dao.jsongraph.types.VertexTypeEnum;
 import org.openecomp.sdc.be.datatypes.enums.ComponentTypeEnum;
 import org.openecomp.sdc.be.datatypes.enums.GraphPropertyEnum;
 import org.openecomp.sdc.be.datatypes.enums.JsonPresentationFields;
+import org.openecomp.sdc.be.datatypes.enums.OriginTypeEnum;
 import org.openecomp.sdc.be.datatypes.enums.ResourceTypeEnum;
 import org.openecomp.sdc.be.model.Component;
+import org.openecomp.sdc.be.model.ComponentInstance;
 import org.openecomp.sdc.be.model.ComponentInstanceProperty;
 import org.openecomp.sdc.be.model.Resource;
 import org.openecomp.sdc.be.model.LifecycleStateEnum;
 import org.openecomp.sdc.be.model.ComponentParametersView;
 import org.openecomp.sdc.be.model.PolicyDefinition;
+import org.openecomp.sdc.be.model.Service;
+import org.openecomp.sdc.be.model.User;
 import org.openecomp.sdc.be.model.jsonjanusgraph.datamodel.NodeType;
 import org.openecomp.sdc.be.model.jsonjanusgraph.datamodel.TopologyTemplate;
 import org.openecomp.sdc.be.model.jsonjanusgraph.datamodel.ToscaElement;
@@ -97,6 +102,9 @@ public class ToscaOperationFacadeTest {
     private static final String PROPERTY1_TYPE = "string";
     private static final String PROPERTY2_NAME = "prop2";
     private static final String PROPERTY2_TYPE = "integer";
+    private static final String ICON_NAME = "icon";
+    private static final String SERVICE_MODEL_NAME = "Test_Service";
+    private static final String SERVICE_PROXY_INSTANCE0_NAME = "testservice_proxy0";
 
     @InjectMocks
     private ToscaOperationFacade testInstance;
@@ -630,6 +638,48 @@ public class ToscaOperationFacadeTest {
         assertTrue(resultProps.stream().anyMatch(e -> e.getName().equals(PROPERTY2_NAME)));
     }
 
+    @Test
+    public void testAddComponentInstanceToTopologyTemplate() {
+        Component containerComponent = new Service();
+        Component originalComponent = new Service();
+        ComponentInstance componentInstance = new ComponentInstance();
+        ComponentInstance existingComponentInstance = new ComponentInstance();
+        User user = new User();
+
+        containerComponent.setComponentType(ComponentTypeEnum.SERVICE);
+
+        originalComponent.setComponentType(ComponentTypeEnum.SERVICE);
+        originalComponent.setIcon(ICON_NAME);
+
+        componentInstance.setOriginType(OriginTypeEnum.ServiceProxy);
+        componentInstance.setSourceModelName(SERVICE_MODEL_NAME);
+
+        List<ComponentInstance> existingInstances = new ArrayList<>();
+        existingComponentInstance.setNormalizedName(SERVICE_PROXY_INSTANCE0_NAME);
+        existingInstances.add(existingComponentInstance);
+        containerComponent.setComponentInstances(existingInstances);
+
+        when(nodeTemplateOperationMock
+            .addComponentInstanceToTopologyTemplate(any(), any(), eq("1"), eq(componentInstance), eq(false), eq(user)))
+            .thenReturn(Either.left(new ImmutablePair<>(new TopologyTemplate(), COMPONENT_ID)));
+        TopologyTemplate topologyTemplate = new TopologyTemplate();
+        // preset COMPONENT_TYPE field for internal ModelConverter call
+        topologyTemplate.setMetadataValue(JsonPresentationFields.COMPONENT_TYPE, ComponentTypeEnum.SERVICE.name());
+        when(topologyTemplateOperationMock.getToscaElement(containerComponent.getUniqueId()))
+            .thenReturn(Either.left(topologyTemplate));
+
+        Either<ImmutablePair<Component, String>, StorageOperationStatus> result =
+            testInstance.addComponentInstanceToTopologyTemplate(
+                containerComponent, originalComponent, componentInstance, false, user);
+
+        assertTrue(result.isLeft());
+        assertEquals(componentInstance.getIcon(), ICON_NAME);
+        assertEquals(result.left().value().getRight(), COMPONENT_ID);
+        // the instance counter must be 1 because the service proxy instance with suffix 0 already exists.
+        verify(nodeTemplateOperationMock, times(1))
+            .addComponentInstanceToTopologyTemplate(any(), any(), eq("1"), eq(componentInstance), eq(false), eq(user));
+    }
+
     private Either<PolicyDefinition, StorageOperationStatus> associatePolicyToComponentWithStatus(StorageOperationStatus status) {
         PolicyDefinition policy = new PolicyDefinition();
         String componentId = "componentId";