From bd98f6033c9dddb50507c04ff70095955c346273 Mon Sep 17 00:00:00 2001 From: Chris André Date: Fri, 24 Apr 2020 19:43:18 -0400 Subject: Work around potential NullPointerExceptions in ComponentInstanceOperation - Rewrite of `updateAttributeValueInResourceInstance` Issue-ID: SDC-2923 Signed-off-by: Chris Andre Change-Id: I1becdec8c10515976835e7d8fb0208b9cbad10bb --- .../impl/ComponentInstanceOperation.java | 73 +++++++++++----------- 1 file changed, 37 insertions(+), 36 deletions(-) (limited to 'catalog-model') diff --git a/catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/ComponentInstanceOperation.java b/catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/ComponentInstanceOperation.java index 38ce067ee5..4284afea0f 100644 --- a/catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/ComponentInstanceOperation.java +++ b/catalog-model/src/main/java/org/openecomp/sdc/be/model/operations/impl/ComponentInstanceOperation.java @@ -20,6 +20,7 @@ package org.openecomp.sdc.be.model.operations.impl; +import fj.F; import org.janusgraph.core.JanusGraph; import org.janusgraph.core.JanusGraphVertex; import fj.data.Either; @@ -261,8 +262,6 @@ public class ComponentInstanceOperation extends AbstractOperation { * @return */ private Either updateAttributeOfResourceInstance(ComponentInstanceProperty resourceInstanceAttribute, String resourceInstanceId) { - - Either result = null; Wrapper errorWrapper = new Wrapper<>(); UpdateDataContainer updateDataContainer = new UpdateDataContainer<>(GraphEdgeLabels.ATTRIBUTE_IMPL, (() -> AttributeData.class), (() -> AttributeValueData.class), NodeTypeEnum.Attribute, NodeTypeEnum.AttributeValue); @@ -271,20 +270,13 @@ public class ComponentInstanceOperation extends AbstractOperation { AttributeValueData attributeValueData = updateDataContainer.getValueDataWrapper().getInnerElement(); attributeValueData.setHidden(resourceInstanceAttribute.isHidden()); attributeValueData.setValue(resourceInstanceAttribute.getValue()); - Either updateRes = janusGraphGenericDao - .updateNode(attributeValueData, AttributeValueData.class); - if (updateRes.isRight()) { - JanusGraphOperationStatus status = updateRes.right().value(); - errorWrapper.setInnerElement(status); - } else { - result = Either.left(updateRes.left().value()); - } - } - if (!errorWrapper.isEmpty()) { - result = Either.right(errorWrapper.getInnerElement()); - } - return result; + return janusGraphGenericDao.updateNode( + attributeValueData, AttributeValueData.class + ); + } else { + return Either.right(errorWrapper.getInnerElement()); + } } private Either addAttributeToResourceInstance(ComponentInstanceProperty attributeInstanceProperty, String resourceInstanceId, Integer index) { @@ -587,32 +579,41 @@ public class ComponentInstanceOperation extends AbstractOperation { return new ComponentInstanceProperty(hidden, resourceInstanceAttribute, uid); } - public Either updateAttributeValueInResourceInstance(ComponentInstanceProperty resourceInstanceAttribute, String resourceInstanceId, boolean inTransaction) { - + public Either updateAttributeValueInResourceInstance( + ComponentInstanceProperty resourceInstanceAttribute, + String resourceInstanceId, + boolean inTransaction + ) { + //TODO This null could bubble up. Shouldn't we set a default value (such as Either.left(StorageOperationStatus.GENERAL_ERROR)) ? Either result = null; - try { - Either eitherAttributeValue = updateAttributeOfResourceInstance(resourceInstanceAttribute, resourceInstanceId); - - if (eitherAttributeValue.isRight()) { - log.error("Failed to add attribute value {} to resource instance {} in Graph. status is {}", resourceInstanceAttribute, resourceInstanceId, eitherAttributeValue.right().value().name()); - result = Either.right(DaoStatusConverter.convertJanusGraphStatusToStorageStatus(eitherAttributeValue.right().value())); - return result; - } else { - AttributeValueData attributeValueData = eitherAttributeValue.left().value(); - - ComponentInstanceProperty attributeValueResult = buildResourceInstanceAttribute(attributeValueData, resourceInstanceAttribute); - log.debug("The returned ResourceInstanceAttribute is {}", attributeValueResult); - - result = Either.left(attributeValueResult); - return result; - } - } - - finally { + result = updateAttributeOfResourceInstance(resourceInstanceAttribute, resourceInstanceId) + .bimap( + buildResourceInstanceAttribute(resourceInstanceAttribute), + handleFailedAttributeAdditionError(resourceInstanceAttribute, resourceInstanceId)); + } finally { handleTransactionCommitRollback(inTransaction, result); } + return result; + } + + private F handleFailedAttributeAdditionError( + ComponentInstanceProperty resourceInstanceAttribute, String resourceInstanceId) { + return status -> { + log.error("Failed to add attribute value {} to resource instance {} in Graph. status is {}", + resourceInstanceAttribute, resourceInstanceId, status.name()); + return DaoStatusConverter.convertJanusGraphStatusToStorageStatus(status); + }; + } + private F buildResourceInstanceAttribute( + ComponentInstanceProperty resourceInstanceAttribute) { + return attributeValueData -> { + ComponentInstanceProperty attributeValueResult = + buildResourceInstanceAttribute(attributeValueData, resourceInstanceAttribute); + log.debug("The returned ResourceInstanceAttribute is {}", attributeValueResult); + return attributeValueResult; + }; } public Either addInputValueToResourceInstance(ComponentInstanceInput resourceInstanceInput, String resourceInstanceId, Integer index, boolean inTransaction) { -- cgit 1.2.3-korg