Revert and fix temporary fix of SDC-4352 94/133094/2
authorJvD_Ericsson <jeff.van.dam@est.tech>
Tue, 31 Jan 2023 13:25:38 +0000 (13:25 +0000)
committerMichael Morris <michael.morris@est.tech>
Wed, 1 Feb 2023 12:05:20 +0000 (12:05 +0000)
Revert https://gerrit.onap.org/r/c/sdc/+/133049 and fix
initial issue described by SDC-4352

Fix floats not being imported correctly

Fix not being able to use equal constraint with bool type

Issue-ID: SDC-4352
Signed-off-by: JvD_Ericsson <jeff.van.dam@est.tech>
Change-Id: I90641838aa40bea211f57f47fe65d09bf0573453

catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ComponentInstanceBusinessLogic.java
catalog-be/src/test/java/org/openecomp/sdc/be/impl/ComponentsUtilsTest.java
catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/PropertyOperation.java
catalog-model/src/main/java/org/openecomp/sdc/be/model/tosca/constraints/EqualConstraint.java

index 0c93b1d..d87549f 100644 (file)
@@ -17,6 +17,7 @@
  * limitations under the License.
  * ============LICENSE_END=========================================================
  */
+
 package org.openecomp.sdc.be.components.impl;
 
 import static org.openecomp.sdc.be.components.attribute.GetOutputUtils.isGetOutputValueForOutput;
@@ -167,7 +168,8 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
     private static final String CREATE_OR_UPDATE_PROPERTY_VALUE = "CreateOrUpdatePropertyValue";
     private static final String FAILED_TO_COPY_COMP_INSTANCE_TO_CANVAS = "Failed to copy the component instance to the canvas";
     private static final String COPY_COMPONENT_INSTANCE_OK = "Copy component instance OK";
-    private static final String CANNOT_ATTACH_RESOURCE_INSTANCES_TO_CONTAINER_RESOURCE_OF_TYPE = "Cannot attach resource instances to container resource of type {}";
+    private static final String CANNOT_ATTACH_RESOURCE_INSTANCES_TO_CONTAINER_RESOURCE_OF_TYPE =
+        "Cannot attach resource instances to container resource of type {}";
     private static final String FAILED_TO_UPDATE_COMPONENT_INSTANCE_CAPABILITY = "Failed to update component instance capability on instance {} in "
         + "container {}";
     private static final String SERVICE_PROXY = "serviceProxy";
@@ -185,12 +187,12 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
     private final ComponentInstanceChangeOperationOrchestrator onChangeInstanceOperationOrchestrator;
     private final ForwardingPathOperation forwardingPathOperation;
     private final NodeFilterOperation nodeFilterOperation;
+    private final ToscaFunctionValidator toscaFunctionValidator;
+    private final PropertyBusinessLogic propertyBusinessLogic;
     @Autowired
     private CompositionBusinessLogic compositionBusinessLogic;
     @Autowired
     private ContainerInstanceTypesData containerInstanceTypesData;
-    private final ToscaFunctionValidator toscaFunctionValidator;
-    private final PropertyBusinessLogic propertyBusinessLogic;
 
     @Autowired
     public ComponentInstanceBusinessLogic(IElementOperation elementDao, IGroupOperation groupOperation,
@@ -930,7 +932,8 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
                     }
                     if (CollectionUtils.isNotEmpty(filteredGroups)) {
                         filteredGroups.stream()
-                            .filter(g -> g.getArtifacts() != null && g.getArtifacts().stream().anyMatch(p -> p.equals(artifactDefinition.getGeneratedFromId()))).findFirst()
+                            .filter(g -> g.getArtifacts() != null &&
+                                g.getArtifacts().stream().anyMatch(p -> p.equals(artifactDefinition.getGeneratedFromId()))).findFirst()
                             .ifPresent(g -> fillInstanceArtifactMap(groupInstancesArtifacts, artifactDefinition, g));
                     }
                 }
@@ -1960,7 +1963,7 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
         ComponentInstance foundResourceInstance = resourceInstanceStatus.left().value();
 
         // Validate instance property against it's constrains
-        Either<Boolean, ResponseFormat> constraintValidatorResponse = validatePropertyValueConstraint(properties,componentId);
+        Either<Boolean, ResponseFormat> constraintValidatorResponse = validatePropertyValueConstraint(properties, componentId);
         if (constraintValidatorResponse.isRight()) {
             log.error("Failed validation value and constraint of property: {}", constraintValidatorResponse.right().value());
             return Either.right(constraintValidatorResponse.right().value());
@@ -1976,7 +1979,7 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
             for (ComponentInstanceProperty property : properties) {
                 validateMandatoryFields(property);
                 validatePropertyExistsOnComponent(property, containerComponent, foundResourceInstance);
-                // validatePropertyConstraintsNotChanged(properties, foundResourceInstance);
+                validatePropertyConstraintsNotChanged(properties, foundResourceInstance);
                 String propertyParentUniqueId = property.getParentUniqueId();
                 if (property.isToscaFunction()) {
                     toscaFunctionValidator.validate(property, containerComponent);
@@ -2060,9 +2063,9 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
             jsonArray.put(Integer.parseInt(path.get(0)), valueAsObject);
         } else {
             if (objectForPath instanceof JSONObject) {
-                addE((JSONObject)objectForPath, path.subList(1, path.size()), value);
+                addE((JSONObject) objectForPath, path.subList(1, path.size()), value);
             } else {
-                addE((JSONArray)objectForPath, path.subList(1, path.size()), value);
+                addE((JSONArray) objectForPath, path.subList(1, path.size()), value);
             }
         }
     }
@@ -2071,7 +2074,7 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
 
         Object objectForPath = null;
         if (jsonObject.has(path.get(0))) {
-            objectForPath =  jsonObject.get(path.get(0));
+            objectForPath = jsonObject.get(path.get(0));
         } else {
             if (StringUtils.isNumeric(path.get(0))) {
                 objectForPath = new JSONArray();
@@ -2086,9 +2089,9 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
             jsonObject.put(path.get(0), valueAsObject);
         } else {
             if (objectForPath instanceof JSONObject) {
-                addE((JSONObject)objectForPath, path.subList(1, path.size()), value);
+                addE((JSONObject) objectForPath, path.subList(1, path.size()), value);
             } else {
-                addE((JSONArray)objectForPath, path.subList(1, path.size()), value);
+                addE((JSONArray) objectForPath, path.subList(1, path.size()), value);
             }
         }
     }
@@ -2157,7 +2160,7 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
             propertyDefinition.setUniqueId(componentInstanceAttribute.getUniqueId());
             attributesToValidate.add(propertyDefinition);
         });
-        Either<Boolean, ResponseFormat> constraintValidatorResponse = validatePropertyValueConstraint(attributesToValidate,componentId);
+        Either<Boolean, ResponseFormat> constraintValidatorResponse = validatePropertyValueConstraint(attributesToValidate, componentId);
         if (constraintValidatorResponse.isRight()) {
             log.error("Failed validation value and constraint of attribute: {}", constraintValidatorResponse.right().value());
             return Either.right(constraintValidatorResponse.right().value());
@@ -2219,7 +2222,7 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
     }
 
     private void validatePropertyExistsOnComponent(ComponentInstanceProperty property, Component containerComponent,
-                                                                        ComponentInstance foundResourceInstance) {
+                                                   ComponentInstance foundResourceInstance) {
         List<ComponentInstanceProperty> instanceProperties = containerComponent.getComponentInstancesProperties()
             .get(foundResourceInstance.getUniqueId());
         final boolean hasProperty = instanceProperties.stream().anyMatch(p -> p.getName().equals(property.getName()));
@@ -3142,22 +3145,23 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
     }
 
     private void maintainNodeFilters(
-            ComponentInstance currentResourceInstance,
-            ComponentInstance newComponentInstance,
-            String containerComponentId) {
+        ComponentInstance currentResourceInstance,
+        ComponentInstance newComponentInstance,
+        String containerComponentId) {
         CINodeFilterDataDefinition filterToMaintain = currentResourceInstance.getNodeFilter();
         if (null != filterToMaintain) {
             nodeFilterOperation.addNodeFilterData(
-                    containerComponentId.toLowerCase(),
-                    newComponentInstance.getUniqueId(),
-                    filterToMaintain);
+                containerComponentId.toLowerCase(),
+                newComponentInstance.getUniqueId(),
+                filterToMaintain);
         }
     }
 
     private void checkForExternalReqAndCapabilities(Component component, ComponentInstance resResourceInfo) {
         if (MapUtils.isNotEmpty(component.getRequirements())) {
             component.getRequirements().entrySet().forEach(requirementsMap -> {
-                if (MapUtils.isNotEmpty(resResourceInfo.getRequirements()) && resResourceInfo.getRequirements().containsKey(requirementsMap.getKey())) {
+                if (MapUtils.isNotEmpty(resResourceInfo.getRequirements()) &&
+                    resResourceInfo.getRequirements().containsKey(requirementsMap.getKey())) {
                     List<RequirementDefinition> resourceReqList = resResourceInfo.getRequirements().get(requirementsMap.getKey());
                     for (RequirementDefinition requirements : requirementsMap.getValue()) {
                         String requirementName = requirements.getName();
@@ -3998,7 +4002,7 @@ public class ComponentInstanceBusinessLogic extends BaseBusinessLogic {
     private void validatePropertyConstraintsNotChanged(List<ComponentInstanceProperty> newProperties, ComponentInstance originalResourceInstance) {
         for (ComponentInstanceProperty newProperty : newProperties) {
             Optional<PropertyDefinition> originalProperty = originalResourceInstance.getProperties().stream()
-                    .filter(prop -> prop.getUniqueId().equals(newProperty.getUniqueId())).findAny();
+                .filter(prop -> prop.getUniqueId().equals(newProperty.getUniqueId())).findAny();
             if (originalProperty.isPresent()) {
                 List<PropertyConstraint> originalConstraints = originalProperty.get().getConstraints();
                 List<PropertyConstraint> newConstraints = newProperty.getConstraints();
index 310f479..dae9ec8 100644 (file)
@@ -774,9 +774,9 @@ class ComponentsUtilsTest {
         ComponentInstanceProperty[] properties = response.left().value();
         assertEquals(9, properties.length);
         assertEquals("value", ((EqualConstraint) properties[0].getConstraints().iterator().next()).getEqual());
-        assertEquals("5", ((GreaterOrEqualConstraint) properties[1].getConstraints().iterator().next()).getGreaterOrEqual());
-        assertEquals("7", ((LessThanConstraint) properties[2].getConstraints().iterator().next()).getLessThan());
-        assertEquals("9", ((LessOrEqualConstraint) properties[3].getConstraints().iterator().next()).getLessOrEqual());
+        assertEquals(5, ((GreaterOrEqualConstraint) properties[1].getConstraints().iterator().next()).getGreaterOrEqual());
+        assertEquals(7, ((LessThanConstraint) properties[2].getConstraints().iterator().next()).getLessThan());
+        assertEquals(9, ((LessOrEqualConstraint) properties[3].getConstraints().iterator().next()).getLessOrEqual());
         assertEquals("5", ((InRangeConstraint) properties[4].getConstraints().iterator().next()).getMin().toString());
         assertEquals("10", ((InRangeConstraint) properties[4].getConstraints().iterator().next()).getMax().toString());
         assertEquals(3, ((ValidValuesConstraint) properties[5].getConstraints().iterator().next()).getValidValues().size());
index d5f7ccb..3d2e701 100644 (file)
  * limitations under the License.
  * ============LICENSE_END=========================================================
  */
+
 package org.openecomp.sdc.be.model.operations.impl;
 
-import static org.openecomp.sdc.be.model.tosca.constraints.ConstraintUtil.convertToComparable;
 import static org.openecomp.sdc.common.log.enums.EcompLoggerErrorCode.BUSINESS_PROCESS_ERROR;
 
 import com.fasterxml.jackson.core.ObjectCodec;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.JsonNodeType;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.gson.JsonArray;
@@ -93,7 +94,6 @@ import org.openecomp.sdc.be.model.operations.api.DerivedFromOperation;
 import org.openecomp.sdc.be.model.operations.api.IPropertyOperation;
 import org.openecomp.sdc.be.model.operations.api.StorageOperationStatus;
 import org.openecomp.sdc.be.model.tosca.ToscaPropertyType;
-import org.openecomp.sdc.be.model.tosca.ToscaType;
 import org.openecomp.sdc.be.model.tosca.constraints.EqualConstraint;
 import org.openecomp.sdc.be.model.tosca.constraints.GreaterOrEqualConstraint;
 import org.openecomp.sdc.be.model.tosca.constraints.GreaterThanConstraint;
@@ -125,7 +125,8 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe
     private static final String DATA_TYPE_CANNOT_BE_FOUND_IN_GRAPH_STATUS_IS = "Data type {} cannot be found in graph. status is {}";
     private static final String GOING_TO_EXECUTE_COMMIT_ON_GRAPH = "Going to execute commit on graph.";
     private static final String GOING_TO_EXECUTE_ROLLBACK_ON_GRAPH = "Going to execute rollback on graph.";
-    private static final String FAILED_TO_ASSOCIATE_RESOURCE_TO_PROPERTY_IN_GRAPH_STATUS_IS = "Failed to associate resource {} to property {} in graph. status is {}";
+    private static final String FAILED_TO_ASSOCIATE_RESOURCE_TO_PROPERTY_IN_GRAPH_STATUS_IS =
+        "Failed to associate resource {} to property {} in graph. status is {}";
     private static final String AFTER_ADDING_PROPERTY_TO_GRAPH = "After adding property to graph {}";
     private static final String BEFORE_ADDING_PROPERTY_TO_GRAPH = "Before adding property to graph {}";
     private static final String THE_VALUE_OF_PROPERTY_FROM_TYPE_IS_INVALID = "The value {} of property from type {} is invalid";
@@ -1268,14 +1269,14 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe
                     return Either.right(operationStatus);
                 }
                 propertiesData.put(propertyName, addPropertyToNodeType.left().value());
-            }            
+            }
             DataTypeData dataTypeData = new DataTypeData();
             Either<DataTypeDefinition, StorageOperationStatus> existingNode = getDataTypeByUidWithoutDerived(uniqueId, true);
             if (existingNode.isLeft()) {
                 dataTypeData.getDataTypeDataDefinition().setNormative(existingNode.left().value().isNormative());
             }
             dataTypeData.getDataTypeDataDefinition().setUniqueId(uniqueId);
-            
+
             long modificationTime = System.currentTimeMillis();
             dataTypeData.getDataTypeDataDefinition().setModificationTime(modificationTime);
             Either<DataTypeData, JanusGraphOperationStatus> updateNode = janusGraphGenericDao.updateNode(dataTypeData, DataTypeData.class);
@@ -2319,7 +2320,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe
                 return je.getAsBoolean();
             }
             if (je.isNumber()) {
-                double number = je.getAsNumber().floatValue();
+                float number = je.getAsNumber().floatValue();
                 if ((number % 1) == 0) {
                     return je.getAsNumber().intValue();
                 }
@@ -2354,22 +2355,22 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe
                     } else {
                         switch (constraintType) {
                             case EQUAL:
-                                propertyConstraint = deserializeConstraintWithStringOperand(value, EqualConstraint.class);
+                                propertyConstraint = deserializeConstraint(value, EqualConstraint.class);
                                 break;
                             case IN_RANGE:
                                 propertyConstraint = deserializeInRangeConstraintConstraint(value);
                                 break;
                             case GREATER_THAN:
-                                propertyConstraint = deserializeConstraintWithStringOperand(value, GreaterThanConstraint.class);
+                                propertyConstraint = deserializeConstraint(value, GreaterThanConstraint.class);
                                 break;
                             case LESS_THAN:
-                                propertyConstraint = deserializeConstraintWithStringOperand(value, LessThanConstraint.class);
+                                propertyConstraint = deserializeConstraint(value, LessThanConstraint.class);
                                 break;
                             case GREATER_OR_EQUAL:
-                                propertyConstraint = deserializeConstraintWithStringOperand(value, GreaterOrEqualConstraint.class);
+                                propertyConstraint = deserializeConstraint(value, GreaterOrEqualConstraint.class);
                                 break;
                             case LESS_OR_EQUAL:
-                                propertyConstraint = deserializeConstraintWithStringOperand(value, LessOrEqualConstraint.class);
+                                propertyConstraint = deserializeConstraint(value, LessOrEqualConstraint.class);
                                 break;
                             case VALID_VALUES:
                                 propertyConstraint = deserializeValidValuesConstraint(value);
@@ -2396,11 +2397,25 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe
             return propertyConstraint;
         }
 
-        private PropertyConstraint deserializeConstraintWithStringOperand(JsonNode value, Class<? extends PropertyConstraint> constraintClass) {
-            String asString = value.asText();
-            log.debug("Before adding value to {} object. value = {}", constraintClass, asString);
+        private PropertyConstraint deserializeConstraint(JsonNode value, Class<? extends PropertyConstraint> constraintClass) {
+            JsonNodeType type = value.getNodeType();
+            if (type.equals(JsonNodeType.NUMBER)) {
+                float asFloat = (float) value.asDouble();
+                if ((asFloat % 1) == 0) {
+                    return deserializeConstraintWithObjectOperand(value.asInt(), constraintClass);
+                }
+                return deserializeConstraintWithObjectOperand(asFloat, constraintClass);
+            } else if (type.equals(JsonNodeType.BOOLEAN)) {
+                return deserializeConstraintWithObjectOperand(value.asBoolean(), constraintClass);
+            } else {
+                return deserializeConstraintWithObjectOperand(value.asText(), constraintClass);
+            }
+        }
+
+        private PropertyConstraint deserializeConstraintWithObjectOperand(Object value, Class<? extends PropertyConstraint> constraintClass) {
+            log.debug("Before adding value to {} object. value = {}", constraintClass, value);
             try {
-                return constraintClass.getConstructor(Object.class).newInstance(asString);
+                return constraintClass.getConstructor(Object.class).newInstance(value);
             } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException | NoSuchMethodException
                      | SecurityException exception) {
                 log.error("Error deserializing constraint", exception);
index 7462a72..8ba6da5 100644 (file)
@@ -17,6 +17,7 @@
  * limitations under the License.
  * ============LICENSE_END=========================================================
  */
+
 package org.openecomp.sdc.be.model.tosca.constraints;
 
 import javax.validation.constraints.NotNull;
@@ -51,6 +52,9 @@ public class EqualConstraint extends AbstractComparablePropertyConstraint {
     public void initialize(ToscaType propertyType) throws ConstraintValueDoNotMatchPropertyTypeException {
         if (propertyType.isValidValue(String.valueOf(equal))) {
             typed = propertyType.convert(String.valueOf(equal));
+            if (propertyType.equals(ToscaType.BOOLEAN)) {
+                return;
+            }
             initialize(String.valueOf(equal), propertyType);
         } else {
             throw new ConstraintValueDoNotMatchPropertyTypeException(
@@ -95,6 +99,17 @@ public class EqualConstraint extends AbstractComparablePropertyConstraint {
         return String.valueOf(equal);
     }
 
+    @Override
+    public void validate(Object propertyValue) throws ConstraintViolationException {
+        if (propertyValue == null) {
+            throw new ConstraintViolationException("Value to check is null");
+        }
+        if (!(propertyValue instanceof Boolean)) {
+            super.validate(propertyValue);
+        }
+        doValidate(propertyValue);
+    }
+
     public boolean validateValueType(String propertyType) throws ConstraintValueDoNotMatchPropertyTypeException {
         ToscaType toscaType = ToscaType.getToscaType(propertyType);
         if (toscaType == null) {