From 88fdc6dd75f1119ffa8e54dbfd721b6ed722b779 Mon Sep 17 00:00:00 2001 From: "andre.schmid" Date: Wed, 23 Mar 2022 15:39:45 +0000 Subject: Fix Service/VF set value to list/map properties In the Service Property Assignment page, setting a value to a property of type list or map was having the type replaced by the schema type and the value incorrectly set. Add test cases to cover the problem. Include small refactors. Issue-ID: SDC-3926 Change-Id: I1257dbb02e18b103118672ec52d663707d53229c Signed-off-by: andre.schmid --- .../be/components/impl/InputsBusinessLogic.java | 2 +- .../be/components/impl/ServiceBusinessLogic.java | 2 +- .../PropertyValueConstraintValidationUtil.java | 135 ++++++++++----------- .../sdc/be/servlets/ComponentPropertyServlet.java | 10 +- 4 files changed, 73 insertions(+), 76 deletions(-) (limited to 'catalog-be/src/main/java') diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/InputsBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/InputsBusinessLogic.java index 28faf73c21..33bb8650ae 100644 --- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/InputsBusinessLogic.java +++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/InputsBusinessLogic.java @@ -336,7 +336,7 @@ public class InputsBusinessLogic extends BaseBusinessLogic { } private Either validateInputValueConstraint(List inputs, final String model) { - PropertyValueConstraintValidationUtil propertyValueConstraintValidationUtil = PropertyValueConstraintValidationUtil.getInstance(); + PropertyValueConstraintValidationUtil propertyValueConstraintValidationUtil = new PropertyValueConstraintValidationUtil(); List inputDefinitions = new ArrayList<>(); for (InputDefinition inputDefinition : inputs) { InputDefinition inputDef = new InputDefinition(); diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ServiceBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ServiceBusinessLogic.java index 24ba0ead46..aa011e91f4 100644 --- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ServiceBusinessLogic.java +++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/ServiceBusinessLogic.java @@ -548,7 +548,7 @@ public class ServiceBusinessLogic extends ComponentBusinessLogic { if (Objects.nonNull(operationInputDefinition.getParentPropertyType())) { inputDefinition.setProperties(Collections.singletonList(propertyDefinition)); } - return PropertyValueConstraintValidationUtil.getInstance().validatePropertyConstraints(Collections.singletonList(inputDefinition), + return new PropertyValueConstraintValidationUtil().validatePropertyConstraints(Collections.singletonList(inputDefinition), applicationDataTypeCache, model); } 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 e7c6ef386d..25ca7d650e 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 @@ -15,6 +15,7 @@ */ package org.openecomp.sdc.be.datamodel.utils; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import fj.data.Either; @@ -53,27 +54,23 @@ public class PropertyValueConstraintValidationUtil { private static final Logger logger = LoggerFactory.getLogger(PropertyValueConstraintValidationUtil.class); private static final String IGNORE_PROPERTY_VALUE_START_WITH = "{\"get_input\":"; private Map dataTypeDefinitionCache; - private ObjectMapper objectMapper = new ObjectMapper(); - private List errorMessages = new ArrayList<>(); + private final ObjectMapper objectMapper = new ObjectMapper(); + private final List errorMessages = new ArrayList<>(); private StringBuilder completePropertyName; private String completeInputName; - public static PropertyValueConstraintValidationUtil getInstance() { - return new PropertyValueConstraintValidationUtil(); - } - public Either validatePropertyConstraints(final Collection propertyDefinitionList, final ApplicationDataTypeCache applicationDataTypeCache, final String model) { - ResponseFormatManager responseFormatManager = getResponseFormatManager(); + dataTypeDefinitionCache = applicationDataTypeCache.getAll(model).left().value(); - CollectionUtils.emptyIfNull(propertyDefinitionList).stream().filter(this::isValuePresent) + CollectionUtils.emptyIfNull(propertyDefinitionList).stream() + .filter(this::isValuePresent) .forEach(this::evaluatePropertyTypeForConstraintValidation); if (CollectionUtils.isNotEmpty(errorMessages)) { - logger.error("Properties with Invalid Data:", errorMessages); - ResponseFormat inputResponse = responseFormatManager - .getResponseFormat(ActionStatus.INVALID_PROPERTY_VALUES, String.join(",", errorMessages)); - return Either.right(inputResponse); + final String errorMsgAsString = String.join(",", errorMessages); + logger.debug("Properties with Invalid Data: {}", errorMsgAsString); + return Either.right(getResponseFormatManager().getResponseFormat(ActionStatus.INVALID_PROPERTY_VALUES, errorMsgAsString)); } return Either.left(Boolean.TRUE); } @@ -86,30 +83,30 @@ public class PropertyValueConstraintValidationUtil { } private void evaluatePropertyTypeForConstraintValidation(PropertyDefinition propertyDefinition) { - if (Objects.nonNull(propertyDefinition.getType()) && dataTypeDefinitionCache.containsKey(propertyDefinition.getType())) { - completeInputName = ""; - completePropertyName = new StringBuilder(); - if (propertyDefinition instanceof InputDefinition) { - completeInputName = propertyDefinition.getName(); - propertyDefinition = getPropertyDefinitionObjectFromInputs(propertyDefinition); - } - if (Objects.nonNull(propertyDefinition)) { - if (ToscaType.isPrimitiveType(propertyDefinition.getType())) { - propertyDefinition.setConstraints(org.openecomp.sdc.be.dao.utils.CollectionUtils.merge(propertyDefinition.safeGetConstraints(), - dataTypeDefinitionCache.get(propertyDefinition.getType()).safeGetConstraints())); - evaluateConstraintsOnProperty(propertyDefinition); - } else if (ToscaType.isCollectionType(propertyDefinition.getType())) { - propertyDefinition.setConstraints(org.openecomp.sdc.be.dao.utils.CollectionUtils.merge(propertyDefinition.safeGetConstraints(), - dataTypeDefinitionCache.get(propertyDefinition.getType()).safeGetConstraints())); - evaluateConstraintsOnProperty(propertyDefinition); - evaluateCollectionTypeProperties(propertyDefinition); - } else { - setCompletePropertyName(propertyDefinition); - evaluateComplexTypeProperties(propertyDefinition); - } - } - } else { + if (propertyDefinition == null || propertyDefinition.getType() == null || !dataTypeDefinitionCache.containsKey(propertyDefinition.getType())) { errorMessages.add("\nUnsupported datatype found for property " + getCompletePropertyName(propertyDefinition)); + return; + } + completeInputName = ""; + completePropertyName = new StringBuilder(); + if (propertyDefinition instanceof InputDefinition) { + completeInputName = propertyDefinition.getName(); + propertyDefinition = getPropertyDefinitionObjectFromInputs(propertyDefinition); + } + if (propertyDefinition != null) { + if (ToscaType.isPrimitiveType(propertyDefinition.getType())) { + propertyDefinition.setConstraints(org.openecomp.sdc.be.dao.utils.CollectionUtils.merge(propertyDefinition.safeGetConstraints(), + dataTypeDefinitionCache.get(propertyDefinition.getType()).safeGetConstraints())); + evaluateConstraintsOnProperty(propertyDefinition); + } else if (ToscaType.isCollectionType(propertyDefinition.getType())) { + propertyDefinition.setConstraints(org.openecomp.sdc.be.dao.utils.CollectionUtils.merge(propertyDefinition.safeGetConstraints(), + dataTypeDefinitionCache.get(propertyDefinition.getType()).safeGetConstraints())); + evaluateConstraintsOnProperty(propertyDefinition); + evaluateCollectionTypeProperties(propertyDefinition); + } else { + setCompletePropertyName(propertyDefinition); + evaluateComplexTypeProperties(propertyDefinition); + } } } @@ -157,7 +154,7 @@ public class PropertyValueConstraintValidationUtil { List propertyDefinitions = dataTypeDefinitionCache.get(propertyDefinition.getType()).getProperties(); try { Map valueMap = MapUtils - .emptyIfNull(ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference>() { + .emptyIfNull(ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() { })); if (CollectionUtils.isEmpty(propertyDefinitions)) { checkAndEvaluatePrimitiveProperty(propertyDefinition, dataTypeDefinitionCache.get(propertyDefinition.getType())); @@ -174,13 +171,13 @@ public class PropertyValueConstraintValidationUtil { try { if (valueMap.containsKey(prop.getName())) { if (ToscaType.isPrimitiveType(prop.getType())) { - evaluateConstraintsOnProperty(createPropertyDefinition(prop, String.valueOf(valueMap.get(prop.getName())))); + evaluateConstraintsOnProperty(copyPropertyWithNewValue(prop, String.valueOf(valueMap.get(prop.getName())))); } else if (ToscaType.isCollectionType(prop.getType())) { - evaluateCollectionTypeProperties(createPropertyDefinition(prop, objectMapper.writeValueAsString(valueMap.get(prop.getName())))); + evaluateCollectionTypeProperties(copyPropertyWithNewValue(prop, objectMapper.writeValueAsString(valueMap.get(prop.getName())))); } else { completePropertyName.append(UNDERSCORE); completePropertyName.append(prop.getName()); - evaluateComplexTypeProperties(createPropertyDefinition(prop, objectMapper.writeValueAsString(valueMap.get(prop.getName())))); + evaluateComplexTypeProperties(copyPropertyWithNewValue(prop, objectMapper.writeValueAsString(valueMap.get(prop.getName())))); } } } catch (IOException e) { @@ -201,8 +198,7 @@ public class PropertyValueConstraintValidationUtil { private void evaluateListType(PropertyDefinition propertyDefinition) { try { String schemaType = propertyDefinition.getSchemaType(); - List list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference>() { - }); + List list = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {}); evaluateCollectionType(propertyDefinition, list, schemaType); } catch (ConstraintValueDoNotMatchPropertyTypeException e) { logger.debug(e.getMessage(), e); @@ -213,8 +209,7 @@ public class PropertyValueConstraintValidationUtil { private void evaluateMapType(PropertyDefinition propertyDefinition) { try { String schemaType = propertyDefinition.getSchemaType(); - Map map = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference>() { - }); + Map map = ConstraintUtil.parseToCollection(propertyDefinition.getValue(), new TypeReference<>() {}); evaluateCollectionType(propertyDefinition, map.values(), schemaType); } catch (ConstraintValueDoNotMatchPropertyTypeException e) { logger.debug(e.getMessage(), e); @@ -222,56 +217,60 @@ public class PropertyValueConstraintValidationUtil { } } - private void evaluateCollectionPrimitiveSchemaType(PropertyDefinition propertyDefinition, Object value, String schemaType) { - if (Objects.nonNull(propertyDefinition.getSchema()) && propertyDefinition.getSchema().getProperty() instanceof PropertyDefinition) { + private void evaluateCollectionPrimitiveSchemaType(final PropertyDefinition propertyDefinition, + final String schemaType) throws JsonProcessingException { + if (propertyDefinition.getSchema() != null && propertyDefinition.getSchema().getProperty() instanceof PropertyDefinition) { propertyDefinition.setConstraints(((PropertyDefinition) propertyDefinition.getSchema().getProperty()).getConstraints()); - propertyDefinition.setValue(String.valueOf(value)); + propertyDefinition.setValue(objectMapper.readValue(propertyDefinition.getValue(), String.class)); propertyDefinition.setType(schemaType); evaluateConstraintsOnProperty(propertyDefinition); } } - private void evaluateCollectionType(PropertyDefinition propertyDefinition, Collection valueList, String schemaType) { - for (Object value : valueList) { + private void evaluateCollectionType(final PropertyDefinition propertyDefinition, final Collection valueList, final String schemaType) { + for (final Object value : valueList) { try { + final PropertyDefinition propertyDefinition1 = copyPropertyWithNewValue(propertyDefinition, objectMapper.writeValueAsString(value)); if (ToscaType.isPrimitiveType(schemaType)) { - evaluateCollectionPrimitiveSchemaType(propertyDefinition, value, schemaType); + evaluateCollectionPrimitiveSchemaType(propertyDefinition1, schemaType); } else if (ToscaType.isCollectionType(schemaType)) { - propertyDefinition.setValue(objectMapper.writeValueAsString(value)); - propertyDefinition.setType(schemaType); - evaluateCollectionTypeProperties(propertyDefinition); + propertyDefinition1.setType(schemaType); + propertyDefinition1.setSchemaType(propertyDefinition.getSchemaProperty().getSchemaType()); + evaluateCollectionTypeProperties(propertyDefinition1); } else { - propertyDefinition.setValue(objectMapper.writeValueAsString(value)); - propertyDefinition.setType(schemaType); + propertyDefinition1.setType(schemaType); completePropertyName.append(UNDERSCORE); - completePropertyName.append(propertyDefinition.getName()); - evaluateComplexTypeProperties(propertyDefinition); + completePropertyName.append(propertyDefinition1.getName()); + evaluateComplexTypeProperties(propertyDefinition1); } - } catch (IOException e) { + } catch (final Exception e) { logger.debug(e.getMessage(), e); errorMessages.add(String.format(VALUE_PROVIDED_IN_INVALID_FORMAT_FOR_PROPERTY, getCompletePropertyName(propertyDefinition))); } } } - private String getCompletePropertyName(PropertyDefinition propertyDefinition) { - return StringUtils.isNotBlank(completeInputName) ? completeInputName - : StringUtils.isBlank(completePropertyName) ? propertyDefinition.getName() - : completePropertyName + UNDERSCORE + propertyDefinition.getName(); + private String getCompletePropertyName(final PropertyDefinition propertyDefinition) { + if (StringUtils.isNotBlank(completeInputName)) { + return completeInputName; + } + + final String propertyName = propertyDefinition == null ? "" : propertyDefinition.getName(); + if (StringUtils.isNotBlank(completePropertyName)) { + return completePropertyName + UNDERSCORE + propertyName; + } + + return propertyName; } - private PropertyDefinition createPropertyDefinition(PropertyDefinition prop, String value) { - PropertyDefinition propertyDefinition = new PropertyDefinition(); - propertyDefinition.setName(prop.getName()); + private PropertyDefinition copyPropertyWithNewValue(final PropertyDefinition propertyToCopy, final String value) { + final var propertyDefinition = new PropertyDefinition(propertyToCopy); propertyDefinition.setValue(value); - propertyDefinition.setType(prop.getType()); - propertyDefinition.setConstraints(prop.getConstraints()); - propertyDefinition.setSchema(prop.getSchema()); return propertyDefinition; } private boolean isValidValueConstraintPresent(List propertyConstraints) { - return propertyConstraints.stream().anyMatch(propertyConstraint -> propertyConstraint instanceof ValidValuesConstraint); + return propertyConstraints != null && propertyConstraints.stream().anyMatch(ValidValuesConstraint.class::isInstance); } private PropertyDefinition getPropertyDefinitionObjectFromInputs(PropertyDefinition property) { 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 fec5dd8b7b..b411a97a87 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 @@ -24,9 +24,7 @@ import io.swagger.v3.oas.annotations.media.Content; import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.servers.Server; -import io.swagger.v3.oas.annotations.servers.Servers; import io.swagger.v3.oas.annotations.tags.Tag; -import io.swagger.v3.oas.annotations.tags.Tags; import java.util.List; import java.util.Map; import javax.inject.Inject; @@ -66,8 +64,8 @@ import org.slf4j.LoggerFactory; @Loggable(prepend = true, value = Loggable.DEBUG, trim = false) @Path("/v1/catalog") -@Tags({@Tag(name = "SDCE-2 APIs")}) -@Servers({@Server(url = "/sdc2/rest")}) +@Tag(name = "SDCE-2 APIs") +@Server(url = "/sdc2/rest") @Singleton public class ComponentPropertyServlet extends BeGenericServlet { @@ -316,7 +314,7 @@ public class ComponentPropertyServlet extends BeGenericServlet { return buildErrorResponse(responseFormat); } //Validate value and Constraint of property and Fetch all data types from cache - Either constraintValidatorResponse = PropertyValueConstraintValidationUtil.getInstance() + Either constraintValidatorResponse = new PropertyValueConstraintValidationUtil() .validatePropertyConstraints(properties.values(), applicationDataTypeCache, propertyBusinessLogic.getComponentModelByComponentId(componentId)); if (constraintValidatorResponse.isRight()) { @@ -328,7 +326,7 @@ public class ComponentPropertyServlet extends BeGenericServlet { Either, ResponseFormat> status = propertyBusinessLogic .updateComponentProperty(componentId, propertyDefinition.getUniqueId(), propertyDefinition, userId); if (status.isRight()) { - log.info("Failed to update Property. Reason - ", status.right().value()); + log.info("Failed to update Property. Reason - {}", status.right().value()); return buildErrorResponse(status.right().value()); } EntryData property = status.left().value(); -- cgit 1.2.3-korg