fix review comments on property assignment

Change-Id: Ia986294cb552f4e26b3ebf1d1b50c9b7941553ab
Issue-ID: SDC-2097
Signed-off-by: Talio <tali.orenbach@amdocs.com>

8 files changed, 64 insertions, 141 deletions

diff --git a/catalog-be/pom.xml b/catalog-be/pom.xml
index 4c0cbbfae0..4ebbf82d39 100644
--- a/catalog-be/pom.xml
+++ b/catalog-be/pom.xml
@@ -596,27 +596,7 @@
         <dependency>
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-collections4</artifactId>
-            <version>4.1</version>
-        </dependency>
-        <dependency>
-            <groupId>org.openecomp.sdc.be</groupId>
-            <artifactId>catalog-model</artifactId>
-            <version>${project.version}</version>
-        </dependency>
-        <dependency>
-            <groupId>org.onap.sdc.common</groupId>
-            <artifactId>onap-tosca-datatype</artifactId>
-            <version>${project.version}</version>
-        </dependency>
-        <dependency>
-            <groupId>org.apache.commons</groupId>
-            <artifactId>commons-collections4</artifactId>
-            <version>4.1</version>
-        </dependency>
-        <dependency>
-            <groupId>org.openecomp.sdc.be</groupId>
-            <artifactId>catalog-model</artifactId>
-            <version>${project.version}</version>
+            <version>${commons.collections.version}</version>
         </dependency>
     </dependencies>
 
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
index f49f531e28..0439dd5ef6 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
@@ -22,27 +22,16 @@ package org.openecomp.sdc.be.components.impl;
 
 import com.google.gson.JsonElement;
 import fj.data.Either;
-import java.util.Map.Entry;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.openecomp.sdc.be.config.BeEcompErrorManager;
 import org.openecomp.sdc.be.dao.api.ActionStatus;
 import org.openecomp.sdc.be.dao.titan.TitanOperationStatus;
-import org.openecomp.sdc.be.datatypes.elements.ListDataDefinition;
-import org.openecomp.sdc.be.datatypes.elements.OperationDataDefinition;
-import org.openecomp.sdc.be.datatypes.elements.OperationInputDefinition;
-import org.openecomp.sdc.be.datatypes.elements.PropertyDataDefinition;
-import org.openecomp.sdc.be.datatypes.elements.SchemaDefinition;
+import org.openecomp.sdc.be.datatypes.elements.*;
 import org.openecomp.sdc.be.datatypes.enums.NodeTypeEnum;
 import org.openecomp.sdc.be.impl.WebAppContextWrapper;
-import org.openecomp.sdc.be.model.Component;
-import org.openecomp.sdc.be.model.ComponentInstanceInterface;
-import org.openecomp.sdc.be.model.ComponentParametersView;
-import org.openecomp.sdc.be.model.DataTypeDefinition;
-import org.openecomp.sdc.be.model.IComplexDefaultValue;
-import org.openecomp.sdc.be.model.InterfaceDefinition;
-import org.openecomp.sdc.be.model.PropertyDefinition;
+import org.openecomp.sdc.be.model.*;
 import org.openecomp.sdc.be.model.operations.api.IElementOperation;
 import org.openecomp.sdc.be.model.operations.api.StorageOperationStatus;
 import org.openecomp.sdc.be.model.operations.utils.ComponentValidationUtils;
@@ -166,7 +155,7 @@ public class PropertyBusinessLogic extends BaseBusinessLogic {
                         String convertedValue = null;
                         if (newPropertyDefinition.getDefaultValue() != null) {
                             convertedValue = converter.convert(
-                                (String) newPropertyDefinition.getDefaultValue(), innerType, allDataTypes.left().value());
+                                newPropertyDefinition.getDefaultValue(), innerType, allDataTypes.left().value());
                             newPropertyDefinition.setDefaultValue(convertedValue);
                         }
                     }
@@ -351,16 +340,14 @@ public class PropertyBusinessLogic extends BaseBusinessLogic {
     }
 
     private boolean isPropertyUsedInCIInterfaces(Map<String, List<ComponentInstanceInterface>> componentInstanceInterfaces, PropertyDefinition propertyDefinitionEntry){
+        Optional<ComponentInstanceInterface> isPropertyExistInOperationInterface = Optional.empty();
         if(MapUtils.isNotEmpty(componentInstanceInterfaces)){
-            for (Entry<String, List<ComponentInstanceInterface>> interfaceEntry : componentInstanceInterfaces.entrySet()) {
-                for (ComponentInstanceInterface instanceInterface : interfaceEntry.getValue()) {
-                    if (isPropertyExistInOperationInterface(propertyDefinitionEntry, instanceInterface)) {
-                        return true;
-                    }
-                }
-            }
+            isPropertyExistInOperationInterface = componentInstanceInterfaces.entrySet().stream()
+                    .flatMap(interfaceEntry -> interfaceEntry.getValue().stream())
+                    .filter(instanceInterface -> isPropertyExistInOperationInterface(propertyDefinitionEntry, instanceInterface))
+                    .findAny();
         }
-        return false;
+        return isPropertyExistInOperationInterface.isPresent();
     }
 
     private boolean isPropertyExistInOperationInterface(PropertyDefinition propertyDefinition,
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/property/DefaultPropertyDeclarator.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/property/DefaultPropertyDeclarator.java
index 437ae2d67d..06e8db0f05 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/property/DefaultPropertyDeclarator.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/property/DefaultPropertyDeclarator.java
@@ -88,7 +88,7 @@ public abstract class DefaultPropertyDeclarator<PROPERTYOWNER extends Properties
 
     private InputDefinition createInput(String componentId, PROPERTYOWNER propertiesOwner, ComponentInstancePropInput propInput, PropertyDataDefinition prop) {
         String generatedInputName = generateInputName(propertiesOwner instanceof
-                Service ? null : propertiesOwner.getNormalizedName(),
+                        Service ? null : propertiesOwner.getNormalizedName(),
             propInput);
         return createInputFromProperty(componentId, propertiesOwner, generatedInputName, propInput, prop);
     }
@@ -108,23 +108,24 @@ public abstract class DefaultPropertyDeclarator<PROPERTYOWNER extends Properties
     }
 
     private String handleInputName(String inputName, String[] parsedPropNames) {
-        String prefix;
+        StringBuilder prefix = new StringBuilder();
         int startingIndex;
 
         if(Objects.isNull(inputName)) {
-            prefix = parsedPropNames[0];
+            prefix.append(parsedPropNames[0]);
             startingIndex = 1;
         } else {
-            prefix = inputName;
+            prefix.append(inputName);
             startingIndex = 0;
         }
 
         while(startingIndex < parsedPropNames.length){
-            prefix += "_"  + parsedPropNames[startingIndex];
+            prefix.append("_");
+            prefix.append(parsedPropNames[startingIndex]);
             startingIndex ++;
         }
 
-        return prefix;
+        return prefix.toString();
     }
 
     private PropertyDataDefinition resolveProperty(List<PROPERTYTYPE> propertiesToCreate, ComponentInstancePropInput propInput) {
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
index 4502012bd9..c321df1449 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
@@ -31,19 +31,7 @@ import org.json.simple.JSONArray;
 import org.json.simple.JSONObject;
 import org.json.simple.parser.JSONParser;
 import org.json.simple.parser.ParseException;
-import org.openecomp.sdc.be.components.impl.ArtifactsBusinessLogic;
-import org.openecomp.sdc.be.components.impl.ComponentBusinessLogic;
-import org.openecomp.sdc.be.components.impl.ComponentInstanceBusinessLogic;
-import org.openecomp.sdc.be.components.impl.ElementBusinessLogic;
-import org.openecomp.sdc.be.components.impl.GroupBusinessLogic;
-import org.openecomp.sdc.be.components.impl.InterfaceOperationBusinessLogic;
-import org.openecomp.sdc.be.components.impl.MonitoringBusinessLogic;
-import org.openecomp.sdc.be.components.impl.PolicyBusinessLogic;
-import org.openecomp.sdc.be.components.impl.PolicyTypeBusinessLogic;
-import org.openecomp.sdc.be.components.impl.ProductBusinessLogic;
-import org.openecomp.sdc.be.components.impl.PropertyBusinessLogic;
-import org.openecomp.sdc.be.components.impl.ResourceBusinessLogic;
-import org.openecomp.sdc.be.components.impl.ServiceBusinessLogic;
+import org.openecomp.sdc.be.components.impl.*;
 import org.openecomp.sdc.be.components.lifecycle.LifecycleBusinessLogic;
 import org.openecomp.sdc.be.components.scheduledtasks.ComponentsCleanBusinessLogic;
 import org.openecomp.sdc.be.components.upgrade.UpgradeBusinessLogic;
@@ -376,7 +364,7 @@ public class BeGenericServlet extends BasicServlet {
             return root;
         } catch (ParseException e) {
             log.info("failed to convert input to json");
-            log.debug("failed to convert to json", e);
+            log.error("failed to convert to json", e);
             return new JSONObject();
         }
 
@@ -398,13 +386,13 @@ public class BeGenericServlet extends BasicServlet {
         } catch (Exception e) {
             // INVALID JSON
             log.info("failed to convert from json");
-            log.debug("failed to convert from json", e);
+            log.error("failed to convert from json", e);
             return Either.right(ActionStatus.INVALID_CONTENT);
         }
         return Either.left(t);
     }
 
-    private <T> Either<String, ActionStatus> convertObjectToJson(PropertyDefinition propertyDefinition) {
+    private Either<String, ActionStatus> convertObjectToJson(PropertyDefinition propertyDefinition) {
         Type constraintType = new TypeToken<PropertyConstraint>() {
         }.getType();
         Gson gson = new GsonBuilder().registerTypeAdapter(constraintType, new PropertyOperation.PropertyConstraintSerialiser()).create();
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
index 0edce61bae..79e63c71b5 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
@@ -18,11 +18,7 @@ package org.openecomp.sdc.be.servlets;
 
 import com.jcabi.aspects.Loggable;
 import fj.data.Either;
-import io.swagger.annotations.Api;
-import io.swagger.annotations.ApiOperation;
-import io.swagger.annotations.ApiParam;
-import io.swagger.annotations.ApiResponse;
-import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.*;
 import org.openecomp.sdc.be.components.impl.PropertyBusinessLogic;
 import org.openecomp.sdc.be.config.BeEcompErrorManager;
 import org.openecomp.sdc.be.dao.api.ActionStatus;
@@ -37,15 +33,7 @@ import org.slf4j.LoggerFactory;
 import javax.inject.Singleton;
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
-import javax.ws.rs.Consumes;
-import javax.ws.rs.DELETE;
-import javax.ws.rs.GET;
-import javax.ws.rs.HeaderParam;
-import javax.ws.rs.POST;
-import javax.ws.rs.PUT;
-import javax.ws.rs.Path;
-import javax.ws.rs.PathParam;
-import javax.ws.rs.Produces;
+import javax.ws.rs.*;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
@@ -59,6 +47,8 @@ import java.util.Map;
 public class ComponentPropertyServlet extends BeGenericServlet {
 
   private static final Logger log = LoggerFactory.getLogger(ComponentPropertyServlet.class);
+  private static final String CREATE_PROPERTY = "Create Property";
+  private static final String DEBUG_MESSAGE = "Start handle request of {} modifier id is {}";
 
   @POST
   @Path("services/{serviceId}/properties")
@@ -236,7 +226,7 @@ public class ComponentPropertyServlet extends BeGenericServlet {
       return buildOkResponse(newPropertyDefinition);
 
     } catch (Exception e) {
-      BeEcompErrorManager.getInstance().logBeRestApiGeneralError("Create Property");
+      BeEcompErrorManager.getInstance().logBeRestApiGeneralError(CREATE_PROPERTY);
       log.debug("create property failed with exception", e);
       ResponseFormat responseFormat =
               getComponentsUtils().getResponseFormat(ActionStatus.GENERAL_ERROR);
@@ -308,7 +298,7 @@ public class ComponentPropertyServlet extends BeGenericServlet {
     ServletContext context = request.getSession().getServletContext();
 
     String url = request.getMethod() + " " + request.getRequestURI();
-    log.debug("Start handle request of {} modifier id is {}", url, userId);
+    log.debug(DEBUG_MESSAGE, url, userId);
 
     try {
       PropertyBusinessLogic propertyBL = getPropertyBL(context);
@@ -322,7 +312,7 @@ public class ComponentPropertyServlet extends BeGenericServlet {
       return buildOkResponse(retrievedPropertyEither.left().value());
 
     } catch (Exception e) {
-      BeEcompErrorManager.getInstance().logBeRestApiGeneralError("Create Property");
+      BeEcompErrorManager.getInstance().logBeRestApiGeneralError(CREATE_PROPERTY);
       log.debug("get property failed with exception", e);
       ResponseFormat responseFormat =
               getComponentsUtils().getResponseFormat(ActionStatus.GENERAL_ERROR);
@@ -333,7 +323,7 @@ public class ComponentPropertyServlet extends BeGenericServlet {
     ServletContext context = request.getSession().getServletContext();
 
     String url = request.getMethod() + " " + request.getRequestURI();
-    log.debug("Start handle request of {} modifier id is {}", url, userId);
+    log.debug(DEBUG_MESSAGE, url, userId);
 
     try {
       PropertyBusinessLogic propertyBL = getPropertyBL(context);
@@ -347,7 +337,7 @@ public class ComponentPropertyServlet extends BeGenericServlet {
       return buildOkResponse(propertiesListEither.left().value());
 
     } catch (Exception e) {
-      BeEcompErrorManager.getInstance().logBeRestApiGeneralError("Create Property");
+      BeEcompErrorManager.getInstance().logBeRestApiGeneralError(CREATE_PROPERTY);
       log.debug("get property failed with exception", e);
       ResponseFormat responseFormat =
               getComponentsUtils().getResponseFormat(ActionStatus.GENERAL_ERROR);
@@ -358,7 +348,7 @@ public class ComponentPropertyServlet extends BeGenericServlet {
     ServletContext context = request.getSession().getServletContext();
 
     String url = request.getMethod() + " " + request.getRequestURI();
-    log.debug("Start handle request of {} modifier id is {}", url, userId);
+    log.debug(DEBUG_MESSAGE, url, userId);
 
     try {
 
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/PropertyConvertor.java b/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/PropertyConvertor.java
index 04c7c69daa..405db5a465 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/PropertyConvertor.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/PropertyConvertor.java
@@ -20,13 +20,11 @@
 
 package org.openecomp.sdc.be.tosca;
 
-import java.io.StringReader;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.function.Supplier;
-
+import com.google.gson.Gson;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonParser;
+import com.google.gson.stream.JsonReader;
+import fj.data.Either;
 import org.apache.commons.lang3.StringUtils;
 import org.openecomp.sdc.be.datatypes.elements.PropertyDataDefinition;
 import org.openecomp.sdc.be.datatypes.elements.SchemaDefinition;
@@ -37,7 +35,6 @@ import org.openecomp.sdc.be.model.Resource;
 import org.openecomp.sdc.be.model.tosca.ToscaPropertyType;
 import org.openecomp.sdc.be.model.tosca.converters.DataTypePropertyConverter;
 import org.openecomp.sdc.be.model.tosca.converters.ToscaMapValueConverter;
-import org.openecomp.sdc.be.model.tosca.converters.ToscaStringConvertor;
 import org.openecomp.sdc.be.model.tosca.converters.ToscaValueBaseConverter;
 import org.openecomp.sdc.be.model.tosca.converters.ToscaValueConverter;
 import org.openecomp.sdc.be.tosca.model.EntrySchema;
@@ -45,12 +42,12 @@ import org.openecomp.sdc.be.tosca.model.ToscaNodeType;
 import org.openecomp.sdc.be.tosca.model.ToscaProperty;
 import org.openecomp.sdc.common.log.wrappers.Logger;
 
-import com.google.gson.Gson;
-import com.google.gson.JsonElement;
-import com.google.gson.JsonParser;
-import com.google.gson.stream.JsonReader;
-
-import fj.data.Either;
+import java.io.StringReader;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.function.Supplier;
 
 public class PropertyConvertor {
     private static PropertyConvertor instance;
@@ -127,12 +124,8 @@ public class PropertyConvertor {
         prop.setType(property.getType());
         prop.setDescription(property.getDescription());
         prop.setRequired(property.isRequired());
-        switch (propertyType) {
-            case CAPABILITY:
-                prop.setStatus(property.getStatus());
-                break;
-            default:
-                break;
+        if(propertyType.equals(PropertyType.CAPABILITY)) {
+            prop.setStatus(property.getStatus());
         }
         return prop;
     }
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/ToscaExportHandler.java b/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/ToscaExportHandler.java
index ecead12478..06cf2afe36 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/ToscaExportHandler.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/tosca/ToscaExportHandler.java
@@ -107,7 +107,7 @@ public class ToscaExportHandler {
 
     private static final Logger log = Logger.getLogger(ToscaExportHandler.class);
 
-    public static final String TOSCA_VERSION = "tosca_simple_yaml_1_1";
+    private static final String TOSCA_VERSION = "tosca_simple_yaml_1_1";
     private static final String SERVICE_NODE_TYPE_PREFIX = "org.openecomp.service.";
     private static final String IMPORTS_FILE_KEY = "file";
     private static final String TOSCA_INTERFACE_NAME = "-interface.yml";
@@ -273,7 +273,7 @@ public class ToscaExportHandler {
             addPoliciesToTopologyTemplate(component, topologyTemplate);
         } catch (SdcResourceNotFoundException e) {
             log.debug("Fail to add policies to topology template:",e);
-            Either.right(ToscaError.GENERAL_ERROR);
+            return Either.right(ToscaError.GENERAL_ERROR);
         }
 
 
@@ -313,26 +313,6 @@ public class ToscaExportHandler {
         return Either.left(toscaNode);
     }
 
-    private Either<ToscaTopolgyTemplate, ToscaError> fillInputs(Component component,
-            ToscaTopolgyTemplate topologyTemplate, Map<String, DataTypeDefinition> dataTypes) {
-        if (log.isDebugEnabled()) {
-            log.debug("fillInputs for component {}", component.getUniqueId());
-        }
-        List<InputDefinition> inputDef = component.getInputs();
-        Map<String, ToscaProperty> inputs = new HashMap<>();
-
-        if (inputDef != null) {
-            inputDef.forEach(i -> {
-                ToscaProperty property = propertyConvertor.convertProperty(dataTypes, i, PropertyConvertor.PropertyType.INPUT);
-                inputs.put(i.getName(), property);
-            });
-            if (!inputs.isEmpty()) {
-                topologyTemplate.setInputs(inputs);
-            }
-        }
-        return Either.left(topologyTemplate);
-    }
-
   private void addGroupsToTopologyTemplate(Component component, ToscaTopolgyTemplate topologyTemplate) {
 
 
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/externalupload/utils/ServiceUtils.java b/catalog-be/src/main/java/org/openecomp/sdc/externalupload/utils/ServiceUtils.java
index ba6f9c233c..20501efab8 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/externalupload/utils/ServiceUtils.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/externalupload/utils/ServiceUtils.java
@@ -17,18 +17,18 @@
 package org.openecomp.sdc.externalupload.utils;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableSet;
 import org.apache.commons.beanutils.BeanUtils;
 
 import java.lang.reflect.Field;
 import java.util.*;
 
 public class ServiceUtils {
-  private static final char[] CHARS = new char[]{
-      '0', '1', '2', '3', '4', '5', '6', '7',
-      '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'
-  };
-  private static final String TYPE = "type";
-  private static final String NODE = "node";
+
+  private static ImmutableSet<Class> collectionClasses = ImmutableSet.of(Map.class, List.class, Set.class);
+  private static ImmutableSet<Class> primitiveTypesClasses = ImmutableSet.of(String.class, Integer.class, Double.class, Float.class);
+
+  private ServiceUtils() {}
 
   public static <T> Optional<T> createObjectUsingSetters(Object objectCandidate,
                                                          Class<T> classToCreate)
@@ -66,15 +66,19 @@ public class ServiceUtils {
   }
 
   private static boolean isComplexClass(Field field) {
-    return !field.getType().equals(Map.class)
-        && !field.getType().equals(String.class)
-        && !field.getType().equals(Integer.class)
-        && !field.getType().equals(Float.class)
-        && !field.getType().equals(Double.class)
-        && !field.getType().equals(Set.class)
-        && !field.getType().equals(Object.class)
-        && !field.getType().equals(List.class);
+    return !isCollectionClass(field)
+        && !isPrimitiveClass(field)
+        && !field.getType().equals(Object.class);
+  }
+
+  private static boolean isCollectionClass(Field field) {
+    return collectionClasses.contains(field.getType());
   }
+
+  private static boolean isPrimitiveClass(Field field) {
+    return primitiveTypesClasses.contains(field.getType());
+  }
+
   public static Map<String, Object> getObjectAsMap(Object obj) {
     return new ObjectMapper().convertValue(obj, Map.class);
   }