From 813ac1c0dfa0f5c90210407c93d98076c86fc9ef Mon Sep 17 00:00:00 2001 From: vasraz Date: Wed, 26 Oct 2022 09:34:53 +0100 Subject: Fix potential NPE in importing property constraints Fix potential NPE introduced by https://gerrit.onap.org/r/c/sdc/+/131472 Signed-off-by: Vasyl Razinkov Change-Id: I10a6f40e5a36c7a1eb33ed6a24ad3a2356672a64 Issue-ID: SDC-4231 --- .../model/operations/impl/PropertyOperation.java | 157 +++++++++++---------- 1 file changed, 81 insertions(+), 76 deletions(-) diff --git a/catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/PropertyOperation.java b/catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/PropertyOperation.java index d797d8a8ee..1068e31db6 100644 --- a/catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/PropertyOperation.java +++ b/catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/PropertyOperation.java @@ -77,6 +77,7 @@ import org.openecomp.sdc.be.dao.neo4j.GraphPropertiesDictionary; import org.openecomp.sdc.be.datatypes.elements.PropertyDataDefinition; import org.openecomp.sdc.be.datatypes.elements.PropertyRule; import org.openecomp.sdc.be.datatypes.elements.SchemaDefinition; +import org.openecomp.sdc.be.datatypes.enums.ConstraintType; import org.openecomp.sdc.be.datatypes.enums.NodeTypeEnum; import org.openecomp.sdc.be.model.Component; import org.openecomp.sdc.be.model.ComponentInstanceProperty; @@ -88,7 +89,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.datatypes.enums.ConstraintType; 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; @@ -843,7 +843,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe } return true; } - + public boolean isPropertyTypeValid(final IComplexDefaultValue property, final Map dataTypes) { if (property == null) { return false; @@ -1154,25 +1154,27 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe } DataTypeData resultCTD = createDataTypeResult.left().value(); List properties = dataTypeDefinition.getProperties(); - Either, JanusGraphOperationStatus> addPropertiesToDataType = addPropertiesToDataType(resultCTD.getUniqueId(), dataTypeDefinition.getModel(), + Either, JanusGraphOperationStatus> addPropertiesToDataType = addPropertiesToDataType(resultCTD.getUniqueId(), + dataTypeDefinition.getModel(), properties); if (addPropertiesToDataType.isRight()) { log.debug("Failed add properties {} to data type {}", properties, dataTypeDefinition.getName()); return Either.right(addPropertiesToDataType.right().value()); } - + final Either modelRelationship = addDataTypeToModel(dataTypeDefinition); if (modelRelationship.isRight()) { return Either.right(modelRelationship.right().value()); - } - + } + String derivedFrom = dataTypeDefinition.getDerivedFromName(); if (derivedFrom != null) { - final Either derivedFromDataType = getDataTypeByNameValidForModel(derivedFrom, dataTypeDefinition.getModel()); + final Either derivedFromDataType = getDataTypeByNameValidForModel(derivedFrom, + dataTypeDefinition.getModel()); if (derivedFromDataType.isRight()) { return Either.right(derivedFromDataType.right().value()); } - + log.debug("Before creating relation between data type {} to its parent {}", dtUniqueId, derivedFrom); UniqueIdData from = new UniqueIdData(NodeTypeEnum.DataType, dtUniqueId); final String deriveFromUid = derivedFromDataType.left().value().getUniqueId(); @@ -1186,17 +1188,17 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe } return Either.left(createDataTypeResult.left().value()); } - + private Either addDataTypeToModel(final DataTypeDefinition dataTypeDefinition) { - final String model = dataTypeDefinition.getModel(); - if (model == null) { - return Either.left(null); - } - final GraphNode from = new UniqueIdData(NodeTypeEnum.Model, UniqueIdBuilder.buildModelUid(model)); - final GraphNode to = new UniqueIdData(NodeTypeEnum.DataType, dataTypeDefinition.getUniqueId()); - log.info("Connecting model {} to type {}", from, to); - return janusGraphGenericDao.createRelation(from , to, GraphEdgeLabels.MODEL_ELEMENT, Collections.emptyMap()); - } + final String model = dataTypeDefinition.getModel(); + if (model == null) { + return Either.left(null); + } + final GraphNode from = new UniqueIdData(NodeTypeEnum.Model, UniqueIdBuilder.buildModelUid(model)); + final GraphNode to = new UniqueIdData(NodeTypeEnum.DataType, dataTypeDefinition.getUniqueId()); + log.info("Connecting model {} to type {}", from, to); + return janusGraphGenericDao.createRelation(from, to, GraphEdgeLabels.MODEL_ELEMENT, Collections.emptyMap()); + } private DataTypeData buildDataTypeData(DataTypeDefinition dataTypeDefinition, String ctUniqueId) { DataTypeData dataTypeData = new DataTypeData(dataTypeDefinition); @@ -1265,7 +1267,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe } return Either.left(propertiesData); } - + public Either getDataTypeByNameValidForModel(final String name, final String modelName) { final Either dataTypesRes = janusGraphGenericDao .getNode(GraphPropertiesDictionary.NAME.getProperty(), name, DataTypeData.class, modelName); @@ -1282,13 +1284,15 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe return Either.right(propertiesStatus); } final Either, JanusGraphOperationStatus> parentNode = janusGraphGenericDao - .getChild(UniqueIdBuilder.getKeyByNodeType(NodeTypeEnum.DataType), dataTypeDefinition.getUniqueId(), GraphEdgeLabels.DERIVED_FROM, NodeTypeEnum.DataType, - DataTypeData.class); + .getChild(UniqueIdBuilder.getKeyByNodeType(NodeTypeEnum.DataType), dataTypeDefinition.getUniqueId(), GraphEdgeLabels.DERIVED_FROM, + NodeTypeEnum.DataType, + DataTypeData.class); log.debug(AFTER_RETRIEVING_DERIVED_FROM_NODE_OF_STATUS_IS, dataTypeDefinition.getUniqueId(), parentNode); if (parentNode.isRight()) { final JanusGraphOperationStatus janusGraphOperationStatus = parentNode.right().value(); if (janusGraphOperationStatus != JanusGraphOperationStatus.NOT_FOUND) { - log.error(BUSINESS_PROCESS_ERROR, "Failed to find the parent data type of data type {}. status is {}", dataTypeDefinition.getUniqueId(), janusGraphOperationStatus); + log.error(BUSINESS_PROCESS_ERROR, "Failed to find the parent data type of data type {}. status is {}", + dataTypeDefinition.getUniqueId(), janusGraphOperationStatus); return Either.right(janusGraphOperationStatus); } } else { @@ -1303,7 +1307,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe dataTypeDefinition.setDerivedFrom(parentDataTypeDefinition); } return Either.left(dataTypeDefinition); - } + } /** * Build Data type object from graph by unique id @@ -1417,7 +1421,8 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe } @Override - public Either getDataTypeByName(final String name, final String validForModel, final boolean inTransaction) { + public Either getDataTypeByName(final String name, final String validForModel, + final boolean inTransaction) { Either result = null; try { Either ctResult = this.getDataTypeByNameValidForModel(name, validForModel); @@ -1448,7 +1453,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe public Either getDataTypeByName(final String name, final String validForModel) { return getDataTypeByName(name, validForModel, true); } - + public Either getDataTypeByUidWithoutDerived(String uid, boolean inTransaction) { Either result = null; try { @@ -1456,7 +1461,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe if (ctResult.isRight()) { JanusGraphOperationStatus status = ctResult.right().value(); if (status != JanusGraphOperationStatus.NOT_FOUND) { - log.error(BUSINESS_PROCESS_ERROR, "Failed to retrieve information on data type {} status is {}", uid, status); + log.error(BUSINESS_PROCESS_ERROR, "Failed to retrieve information on data type {} status is {}", uid, status); } result = Either.right(DaoStatusConverter.convertJanusGraphStatusToStorageStatus(ctResult.right().value())); return result; @@ -1533,7 +1538,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe final Map> dataTypes = new HashMap<>(); Either>, JanusGraphOperationStatus> result = Either.left(dataTypes); final Map allDataTypesFound = new HashMap<>(); - + final Map> dataTypeUidstoModels = dataTypeOperation.getAllDataTypeUidsToModels(); if (dataTypeUidstoModels != null) { @@ -1549,7 +1554,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe } return Either.right(status); } - for (final String model: entry.getValue()) { + for (final String model : entry.getValue()) { if (!dataTypes.containsKey(model)) { dataTypes.put(model, new HashMap()); } @@ -1557,7 +1562,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe dataTypes.get(model).put(dataTypeDefinition.getName(), dataTypeDefinition); } } - + } if (log.isTraceEnabled()) { if (result.isRight()) { @@ -1824,7 +1829,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe String dataTypeName = newDataTypeDefinition.getName(); List propertiesToAdd = new ArrayList<>(); if (isPropertyTypeChanged(dataTypeName, newProperties, oldProperties, propertiesToAdd) - || isDerivedFromNameChanged(dataTypeName, newDerivedFromName, oldDerivedFromName)) { + || isDerivedFromNameChanged(dataTypeName, newDerivedFromName, oldDerivedFromName)) { log.debug("The new data type {} is invalid.", dataTypeName); result = Either.right(StorageOperationStatus.CANNOT_UPDATE_EXISTING_ENTITY); return result; @@ -2111,21 +2116,20 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe @Override public JsonElement serialize(PropertyConstraint src, Type typeOfSrc, JsonSerializationContext context) { - JsonParser parser = new JsonParser(); JsonObject result = new JsonObject(); JsonArray jsonArray = new JsonArray(); if (src instanceof InRangeConstraint) { InRangeConstraint rangeConstraint = (InRangeConstraint) src; - jsonArray.add(parser.parse(rangeConstraint.getRangeMinValue())); - jsonArray.add(parser.parse(rangeConstraint.getRangeMaxValue())); + jsonArray.add(JsonParser.parseString(rangeConstraint.getRangeMinValue())); + jsonArray.add(JsonParser.parseString(rangeConstraint.getRangeMaxValue())); result.add("inRange", jsonArray); } else if (src instanceof GreaterThanConstraint) { GreaterThanConstraint greaterThanConstraint = (GreaterThanConstraint) src; - jsonArray.add(parser.parse(greaterThanConstraint.getGreaterThan())); + jsonArray.add(JsonParser.parseString(greaterThanConstraint.getGreaterThan())); result.add("greaterThan", jsonArray); } else if (src instanceof LessOrEqualConstraint) { LessOrEqualConstraint lessOrEqualConstraint = (LessOrEqualConstraint) src; - jsonArray.add(parser.parse(lessOrEqualConstraint.getLessOrEqual())); + jsonArray.add(JsonParser.parseString(lessOrEqualConstraint.getLessOrEqual())); result.add("lessOrEqual", jsonArray); } else { log.warn("PropertyConstraint {} is not supported. Ignored.", src.getClass().getName()); @@ -2299,55 +2303,56 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe log.warn("ConstraintType was not found for constraint name:{}", field.getKey()); } else { if (value == null) { - log.warn("The value of " + constraintType + " constraint is null"); - } - switch (constraintType) { - case EQUAL: - propertyConstraint = deserializeConstraintWithStringOperand(value, EqualConstraint.class); - break; - case IN_RANGE: - propertyConstraint = deserializeInRangeConstraintConstraint(value); - break; - case GREATER_THAN: - propertyConstraint = deserializeConstraintWithStringOperand(value, GreaterThanConstraint.class); - break; - case LESS_THAN: - propertyConstraint = deserializeConstraintWithStringOperand(value, LessThanConstraint.class); - break; - case GREATER_OR_EQUAL: - propertyConstraint = deserializeConstraintWithStringOperand(value, GreaterOrEqualConstraint.class); - break; - case LESS_OR_EQUAL: - propertyConstraint = deserializeConstraintWithStringOperand(value, LessOrEqualConstraint.class); - break; - case VALID_VALUES: - propertyConstraint = deserializeValidValuesConstraint(value); - break; - case LENGTH: - propertyConstraint = deserializeConstraintWithIntegerOperand(value, LengthConstraint.class); - break; - case MIN_LENGTH: - propertyConstraint = deserializeConstraintWithIntegerOperand(value, MinLengthConstraint.class); - break; - case MAX_LENGTH: - propertyConstraint = deserializeConstraintWithIntegerOperand(value, MaxLengthConstraint.class); - break; - default: - log.warn("Key {} is not supported. Ignored.", field.getKey()); + log.warn("The value of {} constraint is null", constraintType); + } else { + switch (constraintType) { + case EQUAL: + propertyConstraint = deserializeConstraintWithStringOperand(value, EqualConstraint.class); + break; + case IN_RANGE: + propertyConstraint = deserializeInRangeConstraintConstraint(value); + break; + case GREATER_THAN: + propertyConstraint = deserializeConstraintWithStringOperand(value, GreaterThanConstraint.class); + break; + case LESS_THAN: + propertyConstraint = deserializeConstraintWithStringOperand(value, LessThanConstraint.class); + break; + case GREATER_OR_EQUAL: + propertyConstraint = deserializeConstraintWithStringOperand(value, GreaterOrEqualConstraint.class); + break; + case LESS_OR_EQUAL: + propertyConstraint = deserializeConstraintWithStringOperand(value, LessOrEqualConstraint.class); + break; + case VALID_VALUES: + propertyConstraint = deserializeValidValuesConstraint(value); + break; + case LENGTH: + propertyConstraint = deserializeConstraintWithIntegerOperand(value, LengthConstraint.class); + break; + case MIN_LENGTH: + propertyConstraint = deserializeConstraintWithIntegerOperand(value, MinLengthConstraint.class); + break; + case MAX_LENGTH: + propertyConstraint = deserializeConstraintWithIntegerOperand(value, MaxLengthConstraint.class); + break; + default: + log.warn("Key {} is not supported. Ignored.", field.getKey()); + } } } } return propertyConstraint; } - + private PropertyConstraint deserializeConstraintWithStringOperand(JsonNode value, Class constraintClass) { String asString = value.asText(); log.debug("Before adding value to {} object. value = {}", constraintClass, asString); try { return constraintClass.getConstructor(String.class).newInstance(asString); } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException | NoSuchMethodException - | SecurityException exception) { + | SecurityException exception) { log.error("Error deserializing constraint", exception); return null; } @@ -2359,7 +2364,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe try { return constraintClass.getConstructor(Integer.class).newInstance(asInt); } catch (InstantiationException | IllegalAccessException | IllegalArgumentException | InvocationTargetException | NoSuchMethodException - | SecurityException exception) { + | SecurityException exception) { log.error("Error deserializing constraint", exception); return null; } @@ -2387,7 +2392,7 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe } return null; } - + private PropertyConstraint deserializeValidValuesConstraint(JsonNode value) { ArrayNode rangeArray = (ArrayNode) value; if (rangeArray.size() == 0) { @@ -2404,8 +2409,8 @@ public class PropertyOperation extends AbstractOperation implements IPropertyOpe } return null; } - - + + } } -- cgit 1.2.3-korg