From c9c7f88f77c9cab341485c27e1e67f6294a0b1c2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Chris=20Andr=C3=A9?= Date: Thu, 23 Apr 2020 16:53:42 -0400 Subject: [PATCH] Work around potential NullPointerExceptions in `ToscaOperationFacade` - Add tests in `getToscaElementByOperation` and `updateToscaElement` for null values - rewrite `getLatestCertifiedByToscaResourceName` in a more functional way - Rewrite some other usages of Either in a more functional way Issue-ID: SDC-2922 Signed-off-by: Chris Andre Change-Id: I52b294ec91faf9e1054af572dcca4060e62fe571 --- .../operations/ToscaOperationFacade.java | 105 +++++++++++++-------- 1 file changed, 64 insertions(+), 41 deletions(-) diff --git a/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacade.java b/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacade.java index 5b3aea0868..cde3ef1975 100644 --- a/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacade.java +++ b/catalog-model/src/main/java/org/openecomp/sdc/be/model/jsonjanusgraph/operations/ToscaOperationFacade.java @@ -177,24 +177,30 @@ public class ToscaOperationFacade { return getToscaElementByOperation(componentV, new ComponentParametersView()); } - private Either getToscaElementByOperation(GraphVertex componentV, ComponentParametersView filters) { - VertexTypeEnum label = componentV.getLabel(); - - ToscaElementOperation toscaOperation = getToscaElementOperation(componentV); - log.debug("getToscaElementByOperation: toscaOperation={}", toscaOperation.getClass()); - Either toscaElement; - String componentId = componentV.getUniqueId(); - if (toscaOperation != null) { - log.debug("Need to fetch tosca element for id {}", componentId); - toscaElement = toscaOperation.getToscaElement(componentV, filters); + private Either getToscaElementByOperation(GraphVertex componentV, + ComponentParametersView filters) { + if (componentV == null) { + log.debug("Unexpected null value for `componentV`"); + return Either.right(StorageOperationStatus.GENERAL_ERROR); } else { - log.debug("not supported tosca type {} for id {}", label, componentId); - toscaElement = Either.right(StorageOperationStatus.BAD_REQUEST); - } - if (toscaElement.isRight()) { - return Either.right(toscaElement.right().value()); + VertexTypeEnum label = componentV.getLabel(); + + ToscaElementOperation toscaOperation = getToscaElementOperation(componentV); + if (toscaOperation != null) { + log.debug("getToscaElementByOperation: toscaOperation={}", toscaOperation.getClass()); + } + + Either toscaElement; + String componentId = componentV.getUniqueId(); + if (toscaOperation != null) { + log.debug("Need to fetch tosca element for id {}", componentId); + toscaElement = toscaOperation.getToscaElement(componentV, filters); + } else { + log.debug("not supported tosca type {} for id {}", label, componentId); + toscaElement = Either.right(StorageOperationStatus.BAD_REQUEST); + } + return toscaElement.left().map(ModelConverter::convertFromToscaElement); } - return Either.left(ModelConverter.convertFromToscaElement(toscaElement.left().value())); } // endregion @@ -357,9 +363,9 @@ public class ToscaOperationFacade { return getLatestCertifiedByToscaResourceName(toscaResourceName, VertexTypeEnum.NODE_TYPE, JsonParseFlagEnum.ParseMetadata); } - public Either getLatestCertifiedByToscaResourceName(String toscaResourceName, VertexTypeEnum vertexType, JsonParseFlagEnum parseFlag) { + public Either getLatestCertifiedByToscaResourceName(String toscaResourceName, + VertexTypeEnum vertexType, JsonParseFlagEnum parseFlag) { - Either result = null; Map props = new EnumMap<>(GraphPropertyEnum.class); props.put(GraphPropertyEnum.TOSCA_RESOURCE_NAME, toscaResourceName); props.put(GraphPropertyEnum.IS_HIGHEST_VERSION, true); @@ -367,25 +373,34 @@ public class ToscaOperationFacade { Either, JanusGraphOperationStatus> getLatestRes = janusGraphDao .getByCriteria(vertexType, props, parseFlag); - if (getLatestRes.isRight()) { - JanusGraphOperationStatus status = getLatestRes.right().value(); - CommonUtility.addRecordToLog(log, LogLevelEnum.DEBUG, "Failed to fetch {} with name {}. status={} ", vertexType, toscaResourceName, status); - result = Either.right(DaoStatusConverter.convertJanusGraphStatusToStorageStatus(status)); - } - if (result == null) { - List resources = getLatestRes.left().value(); - double version = 0.0; - GraphVertex highestResource = null; - for (GraphVertex resource : resources) { - double resourceVersion = Double.parseDouble((String) resource.getJsonMetadataField(JsonPresentationFields.VERSION)); - if (resourceVersion > version) { - version = resourceVersion; - highestResource = resource; + return getLatestRes + .right().map( + status -> { + CommonUtility.addRecordToLog(log, LogLevelEnum.DEBUG, "Failed to fetch {} with name {}. status={} ", + vertexType, toscaResourceName, status); + return DaoStatusConverter.convertJanusGraphStatusToStorageStatus(status); } - } - result = getToscaFullElement(highestResource.getUniqueId()); - } - return result; + ) + .left().bind( + resources -> { + double version = 0.0; + GraphVertex highestResource = null; + for (GraphVertex resource : resources) { + double resourceVersion = Double + .parseDouble((String) resource.getJsonMetadataField(JsonPresentationFields.VERSION)); + if (resourceVersion > version) { + version = resourceVersion; + highestResource = resource; + } + } + if (highestResource != null) { + return getToscaFullElement(highestResource.getUniqueId()); + } else { + log.debug("The vertex with the highest version could not be found for {}", toscaResourceName); + return Either.right(StorageOperationStatus.GENERAL_ERROR); + } + } + ); } public Either validateToscaResourceNameExists(String templateName) { @@ -540,12 +555,20 @@ public class ToscaOperationFacade { ToscaElementOperation toscaElementOperation = getToscaElementOperation(elementV); ToscaElement toscaElementToUpdate = ModelConverter.convertToToscaElement(componentToUpdate); - Either updateToscaElement = toscaElementOperation.updateToscaElement(toscaElementToUpdate, elementV, filterResult); - if (updateToscaElement.isRight()) { - log.debug("Failed to update tosca element {} error {}", componentId, updateToscaElement.right().value()); - return Either.right(updateToscaElement.right().value()); + Either updateToscaElement = null; + if (toscaElementOperation != null) { + updateToscaElement = toscaElementOperation.updateToscaElement(toscaElementToUpdate, elementV, filterResult); + } else { + log.debug("Null value returned by `getToscaElementOperation` with value {}", elementV); + updateToscaElement = Either.right(StorageOperationStatus.GENERAL_ERROR); } - return Either.left(ModelConverter.convertFromToscaElement(updateToscaElement.left().value())); + + return updateToscaElement.bimap( + ModelConverter::convertFromToscaElement, + status -> { + log.debug("Failed to update tosca element {} error {}", componentId, status); + return status; + }); } private Either getLatestByName(GraphPropertyEnum property, String nodeName, JsonParseFlagEnum parseFlag) { -- 2.16.6