Fixed declared properties are not updated properly

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

2 files changed, 72 insertions, 30 deletions

diff --git a/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsontitan/operations/ToscaOperationFacade.java b/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsontitan/operations/ToscaOperationFacade.java
index 4482f7f943..dd9c3ca29e 100644
--- a/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsontitan/operations/ToscaOperationFacade.java
+++ b/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsontitan/operations/ToscaOperationFacade.java
@@ -20,7 +20,6 @@
 
 package org.openecomp.sdc.be.model.jsontitan.operations;
 
-import com.datastax.driver.core.DataType;
 import fj.data.Either;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
@@ -47,7 +46,6 @@ import org.openecomp.sdc.be.model.operations.api.StorageOperationStatus;
 import org.openecomp.sdc.be.model.operations.impl.DaoStatusConverter;
 import org.openecomp.sdc.be.model.operations.impl.UniqueIdBuilder;
 import org.openecomp.sdc.be.resources.data.ComponentMetadataData;
-import org.openecomp.sdc.be.resources.data.DataTypeData;
 import org.openecomp.sdc.common.jsongraph.util.CommonUtility;
 import org.openecomp.sdc.common.jsongraph.util.CommonUtility.LogLevelEnum;
 import org.openecomp.sdc.common.log.wrappers.Logger;
@@ -1236,30 +1234,31 @@ public class ToscaOperationFacade {
 
     public Either<Map<String, List<ComponentInstanceProperty>>, StorageOperationStatus> addComponentInstancePropertiesToComponent(Component containerComponent, Map<String, List<ComponentInstanceProperty>> instProperties) {
         requireNonNull(instProperties);
-        StorageOperationStatus status = null;
         for (Entry<String, List<ComponentInstanceProperty>> entry : instProperties.entrySet()) {
             List<ComponentInstanceProperty> props = entry.getValue();
             String componentInstanceId = entry.getKey();
-
+            List<ComponentInstanceProperty> originalComponentInstProps =
+                containerComponent.getComponentInstancesProperties().get(componentInstanceId);
             Map<String, List<CapabilityDefinition>> containerComponentCapabilities = containerComponent.getCapabilities();
 
-            if (!isEmpty(props)) {
-                for (ComponentInstanceProperty property : props) {
-                    String propertyParentUniqueId = property.getParentUniqueId();
-                    Optional<CapabilityDefinition>
-                            capPropDefinition = getPropertyCapability(propertyParentUniqueId, containerComponent);
-                    if(capPropDefinition.isPresent() && MapUtils.isNotEmpty(containerComponentCapabilities)) {
-                        status = populateAndUpdateInstanceCapProperty(containerComponent, componentInstanceId,
-                                containerComponentCapabilities, property, capPropDefinition.get());
-                    }
-                    if(status ==  null) {
-                        List<ComponentInstanceProperty> instanceProperties = containerComponent
-                                .getComponentInstancesProperties().get(componentInstanceId);
-                        status = updateInstanceProperty(containerComponent, componentInstanceId, instanceProperties, property);
-                    }
-                    if(status != StorageOperationStatus.OK) {
-                        return Either.right(status);
-                    }
+            if(isEmpty(props)) {
+                continue;
+            }
+            for (ComponentInstanceProperty property : props) {
+                StorageOperationStatus status = null;
+                String propertyParentUniqueId = property.getParentUniqueId();
+                Optional<CapabilityDefinition>
+                        capPropDefinition = getPropertyCapability(propertyParentUniqueId, containerComponent);
+                if(capPropDefinition.isPresent() && MapUtils.isNotEmpty(containerComponentCapabilities)) {
+                    status = populateAndUpdateInstanceCapProperty(containerComponent, componentInstanceId,
+                            containerComponentCapabilities, property, capPropDefinition.get());
+                }
+                if(status == null) {
+                    status = updateOrAddComponentInstanceProperty(containerComponent, componentInstanceId,
+                        originalComponentInstProps, property);
+                }
+                if(status != StorageOperationStatus.OK) {
+                    return Either.right(status);
                 }
             }
         }
@@ -1299,23 +1298,24 @@ public class ToscaOperationFacade {
                 .findAny();
     }
 
-    private StorageOperationStatus updateInstanceProperty(Component containerComponent, String componentInstanceId,
-                                                          List<ComponentInstanceProperty> instanceProperties,
-                                                          ComponentInstanceProperty property) {
+    private StorageOperationStatus updateOrAddComponentInstanceProperty(Component containerComponent,
+        String componentInstanceId, List<ComponentInstanceProperty> originalComponentInstProps,
+        ComponentInstanceProperty property)
+    {
         StorageOperationStatus status;
-        Optional<ComponentInstanceProperty> instanceProperty = instanceProperties.stream()
-                .filter(p -> p.getUniqueId().equals(property.getUniqueId()))
-                .findAny();
+        // check if the property already exists or not
+        Optional<ComponentInstanceProperty> instanceProperty = originalComponentInstProps.stream()
+                .filter(p -> p.getUniqueId().equals(property.getUniqueId())).findAny();
         if (instanceProperty.isPresent()) {
             status = updateComponentInstanceProperty(containerComponent, componentInstanceId, property);
         } else {
             status = addComponentInstanceProperty(containerComponent, componentInstanceId, property);
         }
         if (status != StorageOperationStatus.OK) {
-            log.debug("Failed to update instance property {} for instance {} error {} ", property, componentInstanceId, status);
-            return status;
+            log.debug("Failed to update instance property {} for instance {} error {} ",
+                property, componentInstanceId, status);
         }
-        return StorageOperationStatus.OK;
+        return status;
     }
 
     public StorageOperationStatus updateInstanceCapabilityProperty(Component containerComponent, String componentInstanceId,
diff --git a/catalog-model/src/test/java/org/openecomp/sdc/be/model/jsontitan/operations/ToscaOperationFacadeTest.java b/catalog-model/src/test/java/org/openecomp/sdc/be/model/jsontitan/operations/ToscaOperationFacadeTest.java
index 0eb0b3c6d2..b43b632ad1 100644
--- a/catalog-model/src/test/java/org/openecomp/sdc/be/model/jsontitan/operations/ToscaOperationFacadeTest.java
+++ b/catalog-model/src/test/java/org/openecomp/sdc/be/model/jsontitan/operations/ToscaOperationFacadeTest.java
@@ -51,6 +51,7 @@ import org.openecomp.sdc.be.datatypes.enums.GraphPropertyEnum;
 import org.openecomp.sdc.be.datatypes.enums.JsonPresentationFields;
 import org.openecomp.sdc.be.datatypes.enums.ResourceTypeEnum;
 import org.openecomp.sdc.be.model.Component;
+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;
@@ -82,6 +83,8 @@ import java.util.Arrays;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyMap;
@@ -91,6 +94,12 @@ import static org.mockito.ArgumentMatchers.eq;
 
 @RunWith(MockitoJUnitRunner.class)
 public class ToscaOperationFacadeTest {
+    private static final String COMPONENT_ID = "componentId";
+    private static final String PROPERTY1_NAME = "prop1";
+    private static final String PROPERTY1_TYPE = "string";
+    private static final String PROPERTY2_NAME = "prop2";
+    private static final String PROPERTY2_TYPE = "integer";
+
     @InjectMocks
     private ToscaOperationFacade testInstance;
 
@@ -103,6 +112,9 @@ public class ToscaOperationFacadeTest {
     @Mock
     private NodeTypeOperation nodeTypeOperation;
 
+    @Mock
+    private NodeTemplateOperation nodeTemplateOperationMock;
+
     @Before
     public void setUp() throws Exception {
         testInstance = new ToscaOperationFacade();
@@ -584,6 +596,36 @@ public class ToscaOperationFacadeTest {
         assertEquals(datatype, result);
     }
 
+    @Test
+    public void testAddComponentInstancePropertiesToComponent() {
+        // set up component object
+        Component component = new Resource();
+        component.setUniqueId(COMPONENT_ID);
+        List<ComponentInstanceProperty> instanceProps = new ArrayList<>();
+        ComponentInstanceProperty instanceProp = new ComponentInstanceProperty();
+        instanceProp.setName(PROPERTY1_NAME);
+        instanceProp.setType(PROPERTY1_TYPE);
+        instanceProps.add(instanceProp);
+        instanceProp = new ComponentInstanceProperty();
+        instanceProp.setName(PROPERTY2_NAME);
+        instanceProp.setType(PROPERTY2_TYPE);
+        instanceProps.add(instanceProp);
+        Map<String, List<ComponentInstanceProperty>> instancePropsMap =
+            Collections.singletonMap(COMPONENT_ID, instanceProps);
+        component.setComponentInstancesProperties(Collections.singletonMap(COMPONENT_ID, new ArrayList<>()));
+
+        when(nodeTemplateOperationMock.addComponentInstanceProperty(any(), any(), any()))
+            .thenReturn(StorageOperationStatus.OK);
+
+        Either<Map<String, List<ComponentInstanceProperty>>, StorageOperationStatus> result =
+            testInstance.addComponentInstancePropertiesToComponent(component, instancePropsMap);
+        assertTrue(result.isLeft());
+        verify(nodeTemplateOperationMock, times(2)).addComponentInstanceProperty(any(), any(), any());
+        List<ComponentInstanceProperty> resultProps = result.left().value().get(COMPONENT_ID);
+        assertTrue(resultProps.stream().anyMatch(e -> e.getName().equals(PROPERTY1_NAME)));
+        assertTrue(resultProps.stream().anyMatch(e -> e.getName().equals(PROPERTY2_NAME)));
+    }
+
     private Either<PolicyDefinition, StorageOperationStatus> associatePolicyToComponentWithStatus(StorageOperationStatus status) {
         PolicyDefinition policy = new PolicyDefinition();
         String componentId = "componentId";