From 166d45efe6de92ce067bc7b6065a0d7de94a6388 Mon Sep 17 00:00:00 2001 From: franciscovila Date: Tue, 18 Oct 2022 16:02:02 +0100 Subject: 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 Change-Id: I48ebb46b3de9ddac3d9dd91400ea0fad983aa94d --- .../ComponentInterfaceOperationBusinessLogic.java | 1 + .../PropertyValueConstraintValidationUtil.java | 104 ++++++++++++++++++--- .../PropertyValueConstraintValidationUtilTest.java | 79 +++++++++++++++- .../resources/types/datatypes/constraintTest.json | 21 +++++ 4 files changed, 191 insertions(+), 14 deletions(-) (limited to 'catalog-be') 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 d194944786..8a2ab8924e 100644 --- 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 37b86db3cf..1c80c496f2 100644 --- 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 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 list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {}); + return CollectionUtils.isNotEmpty(list); + } else { + Map 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 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 list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {}); + propertyConstraint.validate(list); + } else if (ToscaType.MAP == toscaPropertyType1) { + final Map 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 list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {}); + Collection 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 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 a9350edc18..6e9c6dd784 100644 --- 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 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 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 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 83b4253939..a9d1469068 100644 --- 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", -- cgit 1.2.3-korg