Fix property constraints validation

Fix property constraints validation behaviour when a property
is not required shouldnt be validated vs constraints if no
value is provided. Also add constraints validation for length
measures in list, map and string types.

Issue-ID: SDC-4222
Signed-off-by: franciscovila <javier.paradela.vila@est.tech>
Change-Id: I48ebb46b3de9ddac3d9dd91400ea0fad983aa94d

diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ComponentInterfaceOperationBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ComponentInterfaceOperationBusinessLogic.java
index d194944..8a2ab89 100644 (file)
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ComponentInterfaceOperationBusinessLogic.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ComponentInterfaceOperationBusinessLogic.java
@@ -406,6 +406,7 @@ public class ComponentInterfaceOperationBusinessLogic extends BaseBusinessLogic
             for (OperationInputDefinition operationInputDefinition : inputDefinitionList) {
                 PropertyDefinition propertyDefinition = new PropertyDefinition();
                 propertyDefinition.setValue(operationInputDefinition.getValue());
+                propertyDefinition.setUniqueId(operationInputDefinition.getUniqueId());
                 propertyDefinition.setType(operationInputDefinition.getType());
                 propertyDefinition.setName(operationInputDefinition.getName());
                 propertyDefinition.setDefaultValue(operationInputDefinition.getDefaultValue());

diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/datamodel/utils/PropertyValueConstraintValidationUtil.java b/catalog-be/src/main/java/org/openecomp/sdc/be/datamodel/utils/PropertyValueConstraintValidationUtil.java
index 37b86db..1c80c49 100644 (file)
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/datamodel/utils/PropertyValueConstraintValidationUtil.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/datamodel/utils/PropertyValueConstraintValidationUtil.java
@@ -22,6 +22,7 @@ import fj.data.Either;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -42,7 +43,9 @@ import org.openecomp.sdc.be.model.PropertyDefinition;
 import org.openecomp.sdc.be.model.cache.ApplicationDataTypeCache;
 import org.openecomp.sdc.be.model.tosca.ToscaType;
 import org.openecomp.sdc.be.model.tosca.constraints.ConstraintUtil;
-import org.openecomp.sdc.be.model.tosca.constraints.ValidValuesConstraint;
+import org.openecomp.sdc.be.model.tosca.constraints.LengthConstraint;
+import org.openecomp.sdc.be.model.tosca.constraints.MaxLengthConstraint;
+import org.openecomp.sdc.be.model.tosca.constraints.MinLengthConstraint;
 import org.openecomp.sdc.be.model.tosca.constraints.exception.ConstraintValueDoNotMatchPropertyTypeException;
 import org.openecomp.sdc.be.model.tosca.constraints.exception.ConstraintViolationException;
 import org.openecomp.sdc.exception.ResponseFormat;
@@ -180,25 +183,64 @@ public class PropertyValueConstraintValidationUtil {
 
     private void evaluateRegularComplexType(PropertyDefinition propertyDefinition, PropertyDefinition prop, Map<String, Object> valueMap) {
         try {
-            if (valueMap.containsKey(prop.getName())) {
+            PropertyDefinition newPropertyWithValue;
+            if (valueMap.containsKey(prop.getName()) ) {
                 if (ToscaType.isPrimitiveType(prop.getType())) {
-                    evaluateConstraintsOnProperty(copyPropertyWithNewValue(prop, String.valueOf(valueMap.get(prop.getName()))));
+                    newPropertyWithValue = copyPropertyWithNewValue(prop, String.valueOf(valueMap.get(prop.getName())));
+                    if (isPropertyToEvaluate(newPropertyWithValue)) {
+                        evaluateConstraintsOnProperty(newPropertyWithValue);
+                    }
                 } else if (ToscaType.isCollectionType(prop.getType())) {
-                    evaluateCollectionTypeProperties(copyPropertyWithNewValue(prop, objectMapper.writeValueAsString(valueMap.get(prop.getName()))));
+                    newPropertyWithValue =
+                        copyPropertyWithNewValue(prop,
+                            objectMapper.writeValueAsString(valueMap.get(prop.getName())));
+                    if (isPropertyToEvaluate(newPropertyWithValue)) {
+                        evaluateCollectionTypeProperties(newPropertyWithValue);
+                    }
                 } else {
-                    completePropertyName.append(UNDERSCORE);
-                    completePropertyName.append(prop.getName());
-                    evaluateComplexTypeProperties(copyPropertyWithNewValue(prop, objectMapper.writeValueAsString(valueMap.get(prop.getName()))));
+                    newPropertyWithValue =
+                        copyPropertyWithNewValue(prop,
+                            objectMapper.writeValueAsString(valueMap.get(prop.getName())));
+                    if (isPropertyToEvaluate(newPropertyWithValue)) {
+                        evaluateComplexTypeProperties(newPropertyWithValue);
+                    }
                 }
             }
-        } catch (IOException e) {
+        } catch (IOException | ConstraintValueDoNotMatchPropertyTypeException e) {
             logger.error(e.getMessage(), e);
             errorMessages.add(String.format(VALUE_PROVIDED_IN_INVALID_FORMAT_FOR_PROPERTY, getCompletePropertyName(propertyDefinition)));
         }
     }
 
+    private boolean isPropertyToEvaluate(PropertyDefinition propertyDefinition) throws ConstraintValueDoNotMatchPropertyTypeException {
+        if (Boolean.FALSE.equals(propertyDefinition.isRequired())) {
+            if (!ToscaType.isCollectionType(propertyDefinition.getType())) {
+                return StringUtils.isNotEmpty(propertyDefinition.getValue()) &&
+                    !"null".equals(propertyDefinition.getValue());
+            } else if (ToscaType.LIST == ToscaType.isValidType(propertyDefinition.getType())) {
+                Collection<Object> list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {});
+                return CollectionUtils.isNotEmpty(list);
+            } else {
+                Map<String, Object> valueMap = MapUtils
+                    .emptyIfNull(ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {
+                    }));
+                return MapUtils.isNotEmpty(valueMap);
+            }
+        } else {
+            return true;
+        }
+    }
+
     private void evaluateCollectionTypeProperties(PropertyDefinition propertyDefinition) {
         ToscaType toscaPropertyType = ToscaType.isValidType(propertyDefinition.getType());
+        try {
+            if (isPropertyToEvaluate(propertyDefinition)) {
+                evaluateCollectionConstraints(propertyDefinition, toscaPropertyType);
+            }
+        } catch (ConstraintValueDoNotMatchPropertyTypeException e) {
+            logger.error(e.getMessage(), e);
+            errorMessages.add(String.format(VALUE_PROVIDED_IN_INVALID_FORMAT_FOR_PROPERTY, getCompletePropertyName(propertyDefinition)));
+        }
         if (ToscaType.LIST == toscaPropertyType) {
             evaluateListType(propertyDefinition);
         } else if (ToscaType.MAP == toscaPropertyType) {
@@ -206,12 +248,52 @@ public class PropertyValueConstraintValidationUtil {
         }
     }
 
+    private void evaluateCollectionConstraints(PropertyDefinition propertyDefinition, ToscaType toscaPropertyType) {
+        List<PropertyConstraint> constraintsList = propertyDefinition.getConstraints();
+
+        if (CollectionUtils.isEmpty(constraintsList)) {
+            return;
+        }
+        ToscaType toscaPropertyType1;
+        if (null == toscaPropertyType) {
+            toscaPropertyType1 = ToscaType.isValidType(propertyDefinition.getType());
+        } else {
+            toscaPropertyType1 = toscaPropertyType;
+        }
+        constraintsList.stream()
+            .filter(this::isACollectionConstraint)
+            .forEach(propertyConstraint -> {
+                try {
+                    if (ToscaType.LIST == toscaPropertyType1) {
+                        Collection<Object> list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {});
+                        propertyConstraint.validate(list);
+                    } else if (ToscaType.MAP == toscaPropertyType1) {
+                        final Map<String, Object> map = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {});
+                        propertyConstraint.validate(map);
+                    }
+                } catch (ConstraintValueDoNotMatchPropertyTypeException | ConstraintViolationException exception) {
+                    errorMessages.add("\n" + propertyConstraint.getErrorMessage(toscaPropertyType1, exception,
+                        getCompletePropertyName(propertyDefinition)));
+                }
+            });
+    }
+
+    private boolean isACollectionConstraint(PropertyConstraint constraint) {
+        if (constraint instanceof MaxLengthConstraint){
+            return true;
+        }
+        if (constraint instanceof MinLengthConstraint) {
+            return true;
+        }
+        return constraint instanceof LengthConstraint;
+    }
+
     private void evaluateListType(PropertyDefinition propertyDefinition) {
         try {
             if (propertyDefinition.getSchemaType() == null) {
                 propertyDefinition.setSchema(createStringSchema());
             }
-            List<Object> list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {});
+            Collection<Object> list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {});
             evaluateCollectionType(propertyDefinition, list);
         } catch (ConstraintValueDoNotMatchPropertyTypeException e) {
             logger.debug(e.getMessage(), e);
@@ -293,10 +375,6 @@ public class PropertyValueConstraintValidationUtil {
         return propertyDefinition;
     }
 
-    private boolean isValidValueConstraintPresent(List<PropertyConstraint> propertyConstraints) {
-        return propertyConstraints != null && propertyConstraints.stream().anyMatch(ValidValuesConstraint.class::isInstance);
-    }
-
     private PropertyDefinition getPropertyDefinitionObjectFromInputs(PropertyDefinition property) {
         InputDefinition inputDefinition = (InputDefinition) property;
         PropertyDefinition propertyDefinition = null;

diff --git a/catalog-be/src/test/java/org/openecomp/sdc/be/datamodel/utils/PropertyValueConstraintValidationUtilTest.java b/catalog-be/src/test/java/org/openecomp/sdc/be/datamodel/utils/PropertyValueConstraintValidationUtilTest.java
index a9350ed..6e9c6dd 100644 (file)
--- a/catalog-be/src/test/java/org/openecomp/sdc/be/datamodel/utils/PropertyValueConstraintValidationUtilTest.java
+++ b/catalog-be/src/test/java/org/openecomp/sdc/be/datamodel/utils/PropertyValueConstraintValidationUtilTest.java
@@ -39,6 +39,8 @@ import java.util.List;
 import java.util.Map;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -180,7 +182,7 @@ class PropertyValueConstraintValidationUtilTest {
 
                PropertyDefinition propertyDefinition = new PropertyDefinition();
                propertyDefinition.setType("org.openecomp.datatypes.heat.network.neutron.Subnet");
-               propertyDefinition.setValue("{\"value_specs\":{\"key\":\"slaac\"}}");
+        propertyDefinition.setValue("{\"value_specs\":{\"key\":\"slaac\", \"key2\":\"slaac\"}}");
 
                Either<Boolean, ResponseFormat> responseEither =
                                propertyValueConstraintValidationUtil.validatePropertyConstraints(
@@ -291,6 +293,81 @@ class PropertyValueConstraintValidationUtilTest {
                assertEquals(schemaType, propertyDefinition.getSchemaType());
        }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"[{\"ipv6_address_mode\": \"dhcpv6-stateful\"}, {\"ipv6_address_mode\": \"dhcpv6-stateless\"}, " +
+        "{\"dns_nameservers\": [\"server1\"]}]",
+        "[{\"ipv6_address_mode\": \"dhcpv6-stateful\"}, {\"ipv6_address_mode\": \"dhcpv6-stateless\"}, " +
+        "{\"dns_nameservers\": [\"server1\", \"server2\", \"server3\", \"server4\", \"server5\"]}]",
+        "[{\"ipv6_address_mode\": \"dhcpv6-stateful\"}, {\"ipv6_address_mode\": \"dhcpv6-stateless\"}, " +
+            "{\"subnetpool\": \"h\"}]",
+        "[{\"ipv6_address_mode\": \"dhcpv6-stateful\"}, {\"ipv6_address_mode\": \"dhcpv6-stateless\"}, " +
+            "{\"subnetpool\": \"123456\"}]",
+        "{\"value_specs\":{\"key\":\"slaac\"}}"})
+    void listOfComplexWithConstrainedFails(String value) {
+        when(applicationDataTypeCache.getAll(null)).thenReturn(Either.left(dataTypeDefinitionMap));
+        final var propertyDefinition = new PropertyDefinition();
+        final String type = "list";
+        propertyDefinition.setType(type);
+        final SchemaDefinition schemaDefinition = new SchemaDefinition();
+        final PropertyDataDefinition schemaProperty = new PropertyDataDefinition();
+        final String schemaType = "org.openecomp.datatypes.heat.network.neutron.Subnet";
+        schemaProperty.setType(schemaType);
+        schemaDefinition.setProperty(schemaProperty);
+        propertyDefinition.setSchema(schemaDefinition);
+        propertyDefinition.setValue(value);
+        final String name = "listOfComplex";
+        propertyDefinition.setName(name);
+
+        Either<Boolean, ResponseFormat> responseEither =
+            propertyValueConstraintValidationUtil
+                .validatePropertyConstraints(Collections.singletonList(propertyDefinition), applicationDataTypeCache, null);
+
+        assertTrue(responseEither.isRight());
+        //original object values should not be changed
+        assertEquals(name, propertyDefinition.getName());
+        assertEquals(type, propertyDefinition.getType());
+        assertEquals(value, propertyDefinition.getValue());
+        assertEquals(schemaType, propertyDefinition.getSchemaType());
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {"[{\"ipv6_address_mode\": \"dhcpv6-stateful\"}, {\"ipv6_address_mode\": \"dhcpv6-stateless\"}, " +
+        "{\"dns_nameservers\": [\"server1\", \"server2\", \"server3\", \"server4\"]}]",
+        "[{\"ipv6_address_mode\": \"dhcpv6-stateful\"}, {\"ipv6_address_mode\": \"dhcpv6-stateless\"}, " +
+            "{\"dns_nameservers\": [\"server1\", \"server1\"]}]",
+        "[{\"ipv6_address_mode\": \"dhcpv6-stateful\"}, {\"ipv6_address_mode\": \"dhcpv6-stateless\"}, " +
+            "{\"subnetpool\": \"123\"}]",
+        "[{\"ipv6_address_mode\": \"dhcpv6-stateful\"}, {\"ipv6_address_mode\": \"dhcpv6-stateless\"}, " +
+            "{\"subnetpool\": \"1234\"}]",
+        "[{\"value_specs\":{\"key1\":\"slaac\",\"key2\":\"slaac\"}}]"})
+    void listOfComplexWithConstrainedSuccess(String value) {
+        when(applicationDataTypeCache.getAll(null)).thenReturn(Either.left(dataTypeDefinitionMap));
+
+        final var propertyDefinition = new PropertyDefinition();
+        final String type = "list";
+        propertyDefinition.setType(type);
+        final SchemaDefinition schemaDefinition = new SchemaDefinition();
+        final PropertyDataDefinition schemaProperty = new PropertyDataDefinition();
+        final String schemaType = "org.openecomp.datatypes.heat.network.neutron.Subnet";
+        schemaProperty.setType(schemaType);
+        schemaDefinition.setProperty(schemaProperty);
+        propertyDefinition.setSchema(schemaDefinition);
+        propertyDefinition.setValue(value);
+        final String name = "listOfComplex";
+        propertyDefinition.setName(name);
+
+        Either<Boolean, ResponseFormat> responseEither =
+            propertyValueConstraintValidationUtil
+                .validatePropertyConstraints(Collections.singletonList(propertyDefinition), applicationDataTypeCache, null);
+
+        assertTrue(responseEither.isLeft());
+        //original object values should not be changed
+        assertEquals(name, propertyDefinition.getName());
+        assertEquals(type, propertyDefinition.getType());
+        assertEquals(value, propertyDefinition.getValue());
+        assertEquals(schemaType, propertyDefinition.getSchemaType());
+    }
+
        @Test
        void listOfComplexSuccessTest1() {
                when(applicationDataTypeCache.getAll(null)).thenReturn(Either.left(dataTypeDefinitionMap));

diff --git a/catalog-be/src/test/resources/types/datatypes/constraintTest.json b/catalog-be/src/test/resources/types/datatypes/constraintTest.json
index 83b4253..a9d1469 100644 (file)
--- a/catalog-be/src/test/resources/types/datatypes/constraintTest.json
+++ b/catalog-be/src/test/resources/types/datatypes/constraintTest.json
@@ -128,6 +128,11 @@
         "uniqueId": "org.openecomp.datatypes.heat.network.neutron.Subnet.datatype.value_specs",
         "type": "map",
         "required": false,
+        "constraints": [
+          {
+            "minLength": 2
+          }
+        ],
         "definition": false,
         "defaultValue": "{}",
         "description": "Extra parameters to include in the request",
@@ -186,6 +191,14 @@
         "uniqueId": "org.openecomp.datatypes.heat.network.neutron.Subnet.datatype.subnetpool",
         "type": "string",
         "required": false,
+        "constraints": [
+          {
+            "minLength": "2"
+          },
+          {
+            "maxLength": "4"
+          }
+        ],
         "definition": false,
         "description": "The name or ID of the subnet pool",
         "password": false,
@@ -198,6 +211,14 @@
         "uniqueId": "org.openecomp.datatypes.heat.network.neutron.Subnet.datatype.dns_nameservers",
         "type": "list",
         "required": false,
+        "constraints": [
+          {
+            "minLength": "2"
+          },
+          {
+            "maxLength": "4"
+          }
+        ],
         "definition": false,
         "defaultValue": "[]",
         "description": "A specified set of DNS name servers to be used",

diff --git a/catalog-model/src/main/java/org/openecomp/sdc/be/model/tosca/constraints/LengthConstraint.java b/catalog-model/src/main/java/org/openecomp/sdc/be/model/tosca/constraints/LengthConstraint.java
index 191993a..d2c8b8c 100644 (file)
--- a/catalog-model/src/main/java/org/openecomp/sdc/be/model/tosca/constraints/LengthConstraint.java
+++ b/catalog-model/src/main/java/org/openecomp/sdc/be/model/tosca/constraints/LengthConstraint.java
@@ -77,6 +77,6 @@ public class LengthConstraint extends AbstractPropertyConstraint {
 
     @Override
     public String getErrorMessage(ToscaType toscaType, ConstraintFunctionalException e, String propertyName) {
-        return getErrorMessage(toscaType, e, propertyName, "%s length must be %s", String.valueOf(length));
+        return getErrorMessage(toscaType, e, propertyName, "%s length must be [%s]", String.valueOf(length));
     }
 }