From 75e3375f0d52aedc0ead3d4e4cea0966fbadf555 Mon Sep 17 00:00:00 2001 From: Parshad Patel Date: Thu, 22 Aug 2019 16:30:16 +0900 Subject: Fix issue in so-bpmn-tasks Either log or rethrow this exception Define a constant instead of duplicating this literal Move the string literal on the left side of this string comparison Issue-ID: SO-1841 Change-Id: I828e744be9e71b84d85850d56373e921c174d26b Signed-off-by: Parshad Patel --- .../workflow/tasks/WorkflowAction.java | 66 ++++++++++++---------- .../workflow/tasks/WorkflowActionBBTasks.java | 44 ++++++++------- 2 files changed, 61 insertions(+), 49 deletions(-) diff --git a/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/workflow/tasks/WorkflowAction.java b/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/workflow/tasks/WorkflowAction.java index e6dd38ee1e..1f07166b60 100644 --- a/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/workflow/tasks/WorkflowAction.java +++ b/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/workflow/tasks/WorkflowAction.java @@ -93,6 +93,7 @@ public class WorkflowAction { private static final String WORKFLOW_ACTION_ERROR_MESSAGE = "WorkflowActionErrorMessage"; private static final String SERVICE_INSTANCES = "serviceInstances"; + private static final String SERVICE_INSTANCE = "serviceInstance"; private static final String VF_MODULES = "vfModules"; private static final String WORKFLOW_ACTION_WAS_UNABLE_TO_VERIFY_IF_THE_INSTANCE_NAME_ALREADY_EXIST_IN_AAI = "WorkflowAction was unable to verify if the instance name already exist in AAI."; @@ -107,7 +108,7 @@ public class WorkflowAction { private static final String ASSIGNINSTANCE = "assignInstance"; private static final String CREATEINSTANCE = "createInstance"; private static final String USERPARAMSERVICE = "service"; - private static final String supportedTypes = + private static final String SUPPORTEDTYPES = "vnfs|vfModules|networks|networkCollections|volumeGroups|serviceInstances|instanceGroups"; private static final String HOMINGSOLUTION = "Homing_Solution"; private static final String FABRIC_CONFIGURATION = "FabricConfiguration"; @@ -123,6 +124,8 @@ public class WorkflowAction { private static final String NAME_EXISTS_WITH_DIFF_CUSTOMIZATION_ID = "(%s), same parent and different customization id (%s)"; private static final String NAME_EXISTS_WITH_DIFF_PARENT = "(%s) id (%s) and different parent relationship"; + private static final String CREATENETWORKBB = "CreateNetworkBB"; + private static final String ACTIVATENETWORKBB = "ActivateNetworkBB"; @Autowired protected BBInputSetup bbInputSetup; @@ -183,12 +186,14 @@ public class WorkflowAction { try { cloudOwner = requestDetails.getCloudConfiguration().getCloudOwner(); } catch (Exception ex) { + logger.error("Exception in getCloundOwner", ex); cloudOwner = environment.getProperty(defaultCloudOwner); } boolean suppressRollback = false; try { suppressRollback = requestDetails.getRequestInfo().getSuppressRollback(); } catch (Exception ex) { + logger.error("Exception in getSuppressRollback", ex); suppressRollback = false; } execution.setVariable("suppressRollback", suppressRollback); @@ -209,7 +214,7 @@ public class WorkflowAction { } else { resourceId = resource.getResourceId(); } - if ((serviceInstanceId == null || serviceInstanceId.equals("")) && resourceType == WorkflowType.SERVICE) { + if ((serviceInstanceId == null || serviceInstanceId.isEmpty()) && resourceType == WorkflowType.SERVICE) { serviceInstanceId = resourceId; } execution.setVariable("resourceId", resourceId); @@ -300,9 +305,9 @@ public class WorkflowAction { traverseCatalogDbService(execution, sIRequest, resourceCounter, aaiResourceIds); } } else if (resourceType == WorkflowType.SERVICE - && (requestAction.equalsIgnoreCase("activateInstance") - || requestAction.equalsIgnoreCase("unassignInstance") - || requestAction.equalsIgnoreCase("deleteInstance") + && ("activateInstance".equalsIgnoreCase(requestAction) + || "unassignInstance".equalsIgnoreCase(requestAction) + || "deleteInstance".equalsIgnoreCase(requestAction) || requestAction.equalsIgnoreCase("activate" + FABRIC_CONFIGURATION))) { // SERVICE-MACRO-ACTIVATE, SERVICE-MACRO-UNASSIGN, and // SERVICE-MACRO-DELETE @@ -310,10 +315,10 @@ public class WorkflowAction { // to query the SI in AAI to find related instances. traverseAAIService(execution, resourceCounter, resourceId, aaiResourceIds); } else if (resourceType == WorkflowType.SERVICE - && requestAction.equalsIgnoreCase("deactivateInstance")) { + && "deactivateInstance".equalsIgnoreCase(requestAction)) { resourceCounter.add(new Resource(WorkflowType.SERVICE, "", false)); - } else if (resourceType == WorkflowType.VNF && (requestAction.equalsIgnoreCase("replaceInstance") - || (requestAction.equalsIgnoreCase("recreateInstance")))) { + } else if (resourceType == WorkflowType.VNF && ("replaceInstance".equalsIgnoreCase(requestAction) + || ("recreateInstance".equalsIgnoreCase(requestAction)))) { traverseAAIVnf(execution, resourceCounter, workflowResourceIds.getServiceInstanceId(), workflowResourceIds.getVnfId(), aaiResourceIds); } else { @@ -363,7 +368,7 @@ public class WorkflowAction { sIRequest.getRequestDetails().getRequestParameters().getUserParams(); for (Map params : userParams) { if (params.containsKey(HOMINGSOLUTION)) { - if (params.get(HOMINGSOLUTION).equals("none")) { + if ("none".equals(params.get(HOMINGSOLUTION))) { execution.setVariable("homing", false); } else { execution.setVariable("homing", true); @@ -447,7 +452,7 @@ public class WorkflowAction { protected boolean isConfiguration(List orchFlows) { for (OrchestrationFlow flow : orchFlows) { - if (flow.getFlowName().contains("Configuration") && !flow.getFlowName().equals("ConfigurationScaleOutBB")) { + if (flow.getFlowName().contains(CONFIGURATION) && !"ConfigurationScaleOutBB".equals(flow.getFlowName())) { return true; } } @@ -715,7 +720,7 @@ public class WorkflowAction { protected boolean vrfConfigurationAlreadyExists(RelatedInstance relatedVpnBinding, Configuration vrfConfiguration, AAIResultWrapper configWrapper) throws VrfBondingServiceException { - if (vrfConfiguration.getConfigurationType().equalsIgnoreCase("VRF-ENTRY")) { + if ("VRF-ENTRY".equalsIgnoreCase(vrfConfiguration.getConfigurationType())) { Optional relationshipsConfigOp = configWrapper.getRelationships(); if (relationshipsConfigOp.isPresent()) { Optional relatedInfraVpnBindingOp = @@ -752,7 +757,7 @@ public class WorkflowAction { if (collectionResourceCustomization.getCollectionResource().getInstanceGroup() != null) { String toscaNodeType = collectionResourceCustomization.getCollectionResource() .getInstanceGroup().getToscaNodeType(); - if (toscaNodeType != null && toscaNodeType.contains("NetworkCollection")) { + if (toscaNodeType != null && toscaNodeType.contains(NETWORKCOLLECTION)) { int minNetworks = 0; org.onap.so.db.catalog.beans.InstanceGroup instanceGroup = collectionResourceCustomization.getCollectionResource().getInstanceGroup(); @@ -885,6 +890,7 @@ public class WorkflowAction { } } } catch (Exception ex) { + logger.error("Exception in traverseAAIService", ex); buildAndThrowException(execution, "Could not find existing Service Instance or related Instances to execute the request on."); } @@ -926,6 +932,7 @@ public class WorkflowAction { } } } catch (Exception ex) { + logger.error("Exception in traverseAAIVnf", ex); buildAndThrowException(execution, "Could not find existing Vnf or related Instances to execute the request on."); } @@ -953,6 +960,7 @@ public class WorkflowAction { } } } catch (Exception ex) { + logger.error("Exception in findConfigurationsInsideVfModule", ex); buildAndThrowException(execution, "Failed to find Configuration object from the vfModule."); } } @@ -1015,8 +1023,8 @@ public class WorkflowAction { vfModuleCustomizationUUID = vfModule.getModelInfo().getModelCustomizationUuid(); } - if (!vnfCustomizationUUID.equals("") - && !vfModuleCustomizationUUID.equals("")) { + if (!vnfCustomizationUUID.isEmpty() + && !vfModuleCustomizationUUID.isEmpty()) { List configs = traverseCatalogDbForConfiguration( validate.getModelInfo().getModelVersionId(), @@ -1110,7 +1118,7 @@ public class WorkflowAction { protected Resource extractResourceIdAndTypeFromUri(String uri) { Pattern patt = Pattern.compile( - "[vV]\\d+.*?(?:(?:/(?" + supportedTypes + ")(?:/(?[^/]+))?)(?:/(?[^/]+))?)?$"); + "[vV]\\d+.*?(?:(?:/(?" + SUPPORTEDTYPES + ")(?:/(?[^/]+))?)(?:/(?[^/]+))?)?$"); Matcher m = patt.matcher(uri); Boolean generated = false; @@ -1123,15 +1131,15 @@ public class WorkflowAction { throw new IllegalArgumentException("Uri could not be parsed. No type found. " + uri); } if (action == null) { - if (type.equals(SERVICE_INSTANCES) && (id == null || id.equals("assign"))) { + if (type.equals(SERVICE_INSTANCES) && (id == null || "assign".equals(id))) { id = UUID.randomUUID().toString(); generated = true; - } else if (type.equals(VF_MODULES) && id.equals("scaleOut")) { + } else if (type.equals(VF_MODULES) && "scaleOut".equals(id)) { id = UUID.randomUUID().toString(); generated = true; } } else { - if (action.matches(supportedTypes)) { + if (action.matches(SUPPORTEDTYPES)) { id = UUID.randomUUID().toString(); generated = true; type = action; @@ -1159,7 +1167,7 @@ public class WorkflowAction { .equalsIgnoreCase(reqDetails.getModelInfo().getModelVersionId())) { return serviceInstanceAAI.get().getServiceInstanceId(); } else { - throw new DuplicateNameException("serviceInstance", + throw new DuplicateNameException(SERVICE_INSTANCE, String.format(NAME_EXISTS_WITH_DIFF_VERSION_ID, instanceName, reqDetails.getModelInfo().getModelVersionId())); } @@ -1170,7 +1178,7 @@ public class WorkflowAction { if (aaiServiceInstances.getServiceInstance() != null && !aaiServiceInstances.getServiceInstance().isEmpty()) { if (aaiServiceInstances.getServiceInstance().size() > 1) { - throw new DuplicateNameException("serviceInstance", + throw new DuplicateNameException(SERVICE_INSTANCE, String.format(NAME_EXISTS_MULTIPLE, instanceName)); } else { ServiceInstance si = @@ -1178,7 +1186,7 @@ public class WorkflowAction { Map keys = bbInputSetupUtils.getURIKeysFromServiceInstance(si.getServiceInstanceId()); - throw new DuplicateNameException("serviceInstance", + throw new DuplicateNameException(SERVICE_INSTANCE, String.format(NAME_EXISTS_WITH_DIFF_COMBINATION, instanceName, keys.get("global-customer-id"), keys.get("service-type"), si.getModelVersionId())); @@ -1295,7 +1303,7 @@ public class WorkflowAction { } protected String convertTypeFromPlural(String type) { - if (!type.matches(supportedTypes)) { + if (!type.matches(SUPPORTEDTYPES)) { return type; } else { if (type.equals(SERVICE_INSTANCES)) { @@ -1317,31 +1325,31 @@ public class WorkflowAction { String virtualLinkKey = ebb.getBuildingBlock().getVirtualLinkKey(); sortedOrchFlows.add(ebb); for (ExecuteBuildingBlock ebb2 : orchFlows) { - if (!isVirtualLink && ebb2.getBuildingBlock().getBpmnFlowName().equals("CreateNetworkBB") + if (!isVirtualLink && ebb2.getBuildingBlock().getBpmnFlowName().equals(CREATENETWORKBB) && ebb2.getBuildingBlock().getKey().equalsIgnoreCase(key)) { sortedOrchFlows.add(ebb2); break; } - if (isVirtualLink && ebb2.getBuildingBlock().getBpmnFlowName().equals("CreateNetworkBB") + if (isVirtualLink && ebb2.getBuildingBlock().getBpmnFlowName().equals(CREATENETWORKBB) && ebb2.getBuildingBlock().getVirtualLinkKey().equalsIgnoreCase(virtualLinkKey)) { sortedOrchFlows.add(ebb2); break; } } for (ExecuteBuildingBlock ebb2 : orchFlows) { - if (!isVirtualLink && ebb2.getBuildingBlock().getBpmnFlowName().equals("ActivateNetworkBB") + if (!isVirtualLink && ebb2.getBuildingBlock().getBpmnFlowName().equals(ACTIVATENETWORKBB) && ebb2.getBuildingBlock().getKey().equalsIgnoreCase(key)) { sortedOrchFlows.add(ebb2); break; } - if (isVirtualLink && ebb2.getBuildingBlock().getBpmnFlowName().equals("ActivateNetworkBB") + if (isVirtualLink && ebb2.getBuildingBlock().getBpmnFlowName().equals(ACTIVATENETWORKBB) && ebb2.getBuildingBlock().getVirtualLinkKey().equalsIgnoreCase(virtualLinkKey)) { sortedOrchFlows.add(ebb2); break; } } - } else if (ebb.getBuildingBlock().getBpmnFlowName().equals("CreateNetworkBB") - || ebb.getBuildingBlock().getBpmnFlowName().equals("ActivateNetworkBB")) { + } else if (ebb.getBuildingBlock().getBpmnFlowName().equals(CREATENETWORKBB) + || ebb.getBuildingBlock().getBpmnFlowName().equals(ACTIVATENETWORKBB)) { continue; } else if (!ebb.getBuildingBlock().getBpmnFlowName().equals("")) { sortedOrchFlows.add(ebb); @@ -1424,7 +1432,7 @@ public class WorkflowAction { } } else if (orchFlow.getFlowName().contains(VFMODULE)) { List vfModuleResourcesSorted = null; - if (requestAction.equals("createInstance") || requestAction.equals("assignInstance") + if (requestAction.equals(CREATEINSTANCE) || requestAction.equals(ASSIGNINSTANCE) || requestAction.equals("activateInstance")) { vfModuleResourcesSorted = sortVfModulesByBaseFirst(resourceCounter.stream() .filter(x -> WorkflowType.VFMODULE == x.getResourceType()).collect(Collectors.toList())); diff --git a/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/workflow/tasks/WorkflowActionBBTasks.java b/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/workflow/tasks/WorkflowActionBBTasks.java index 073dead8b3..100f81a0c7 100644 --- a/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/workflow/tasks/WorkflowActionBBTasks.java +++ b/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/workflow/tasks/WorkflowActionBBTasks.java @@ -40,7 +40,6 @@ import org.onap.so.bpmn.servicedecomposition.tasks.BBInputSetupUtils; import org.onap.so.client.aai.AAIObjectType; import org.onap.so.client.exception.ExceptionBuilder; import org.onap.so.db.catalog.beans.CvnfcConfigurationCustomization; -import org.onap.so.db.catalog.beans.VnfResourceCustomization; import org.onap.so.db.catalog.client.CatalogDbClient; import org.onap.so.db.request.beans.InfraActiveRequests; import org.onap.so.db.request.client.RequestsDbClient; @@ -65,6 +64,9 @@ public class WorkflowActionBBTasks { private static final String FABRIC_CONFIGURATION = "FabricConfiguration"; private static final String ASSIGN_FABRIC_CONFIGURATION_BB = "AssignFabricConfigurationBB"; private static final String ACTIVATE_FABRIC_CONFIGURATION_BB = "ActivateFabricConfigurationBB"; + private static final String COMPLETED = "completed"; + private static final String HANDLINGCODE = "handlingCode"; + private static final String ROLLBACKTOCREATED = "RollbackToCreated"; protected String maxRetries = "mso.rainyDay.maxRetries"; private static final Logger logger = LoggerFactory.getLogger(WorkflowActionBBTasks.class); @@ -98,9 +100,9 @@ public class WorkflowActionBBTasks { execution.setVariable("buildingBlock", ebb); currentSequence++; if (currentSequence >= flowsToExecute.size()) { - execution.setVariable("completed", true); + execution.setVariable(COMPLETED, true); } else { - execution.setVariable("completed", false); + execution.setVariable(COMPLETED, false); } execution.setVariable(G_CURRENT_SEQUENCE, currentSequence); } @@ -114,7 +116,8 @@ public class WorkflowActionBBTasks { } } catch (Exception ex) { logger.warn( - "Bpmn Flow Statistics was unable to update Request Db with the new completion percentage. Competion percentage may be invalid."); + "Bpmn Flow Statistics was unable to update Request Db with the new completion percentage. Competion percentage may be invalid.", + ex); } } @@ -236,7 +239,7 @@ public class WorkflowActionBBTasks { } public void checkRetryStatus(DelegateExecution execution) { - String handlingCode = (String) execution.getVariable("handlingCode"); + String handlingCode = (String) execution.getVariable(HANDLINGCODE); String requestId = (String) execution.getVariable(G_REQUEST_ID); String retryDuration = (String) execution.getVariable("RetryDuration"); int retryCount = (int) execution.getVariable(RETRY_COUNT); @@ -244,11 +247,11 @@ public class WorkflowActionBBTasks { try { envMaxRetries = Integer.parseInt(this.environment.getProperty(maxRetries)); } catch (Exception ex) { - logger.error("Could not read maxRetries from config file. Setting max to 5 retries"); + logger.error("Could not read maxRetries from config file. Setting max to 5 retries", ex); envMaxRetries = 5; } int nextCount = retryCount + 1; - if (handlingCode.equals("Retry")) { + if ("Retry".equals(handlingCode)) { workflowActionBBFailure.updateRequestErrorStatusMessage(execution); try { InfraActiveRequests request = requestDbclient.getInfraActiveRequestbyRequestId(requestId); @@ -259,8 +262,8 @@ public class WorkflowActionBBTasks { logger.warn("Failed to update Request Db Infra Active Requests with Retry Status", ex); } if (retryCount < envMaxRetries) { - int currSequence = (int) execution.getVariable("gCurrentSequence"); - execution.setVariable("gCurrentSequence", currSequence - 1); + int currSequence = (int) execution.getVariable(G_CURRENT_SEQUENCE); + execution.setVariable(G_CURRENT_SEQUENCE, currSequence - 1); execution.setVariable(RETRY_COUNT, nextCount); } else { workflowAction.buildAndThrowException(execution, @@ -301,15 +304,15 @@ public class WorkflowActionBBTasks { } } - String handlingCode = (String) execution.getVariable("handlingCode"); + String handlingCode = (String) execution.getVariable(HANDLINGCODE); List rollbackFlowsFiltered = new ArrayList<>(); rollbackFlowsFiltered.addAll(rollbackFlows); - if (handlingCode.equals("RollbackToAssigned") || handlingCode.equals("RollbackToCreated")) { + if ("RollbackToAssigned".equals(handlingCode) || ROLLBACKTOCREATED.equals(handlingCode)) { for (int i = 0; i < rollbackFlows.size(); i++) { if (rollbackFlows.get(i).getBuildingBlock().getBpmnFlowName().contains("Unassign")) { rollbackFlowsFiltered.remove(rollbackFlows.get(i)); } else if (rollbackFlows.get(i).getBuildingBlock().getBpmnFlowName().contains("Delete") - && handlingCode.equals("RollbackToCreated")) { + && ROLLBACKTOCREATED.equals(handlingCode)) { rollbackFlowsFiltered.remove(rollbackFlows.get(i)); } } @@ -321,9 +324,9 @@ public class WorkflowActionBBTasks { else execution.setVariable("isRollbackNeeded", true); execution.setVariable("flowsToExecute", rollbackFlowsFiltered); - execution.setVariable("handlingCode", "PreformingRollback"); + execution.setVariable(HANDLINGCODE, "PreformingRollback"); execution.setVariable("isRollback", true); - execution.setVariable("gCurrentSequence", 0); + execution.setVariable(G_CURRENT_SEQUENCE, 0); execution.setVariable(RETRY_COUNT, 0); } else { workflowAction.buildAndThrowException(execution, @@ -354,6 +357,7 @@ public class WorkflowActionBBTasks { } requestDbclient.updateInfraActiveRequests(request); } catch (Exception ex) { + logger.error("Exception in updateInstanceId", ex); workflowAction.buildAndThrowException(execution, "Failed to update Request db with instanceId"); } } @@ -361,12 +365,12 @@ public class WorkflowActionBBTasks { public void postProcessingExecuteBB(DelegateExecution execution) { List flowsToExecute = (List) execution.getVariable("flowsToExecute"); - String handlingCode = (String) execution.getVariable("handlingCode"); + String handlingCode = (String) execution.getVariable(HANDLINGCODE); final boolean aLaCarte = (boolean) execution.getVariable(G_ALACARTE); int currentSequence = (int) execution.getVariable(G_CURRENT_SEQUENCE); ExecuteBuildingBlock ebb = flowsToExecute.get(currentSequence - 1); String bbFlowName = ebb.getBuildingBlock().getBpmnFlowName(); - if (bbFlowName.equalsIgnoreCase("ActivateVfModuleBB") && aLaCarte && handlingCode.equalsIgnoreCase("Success")) { + if ("ActivateVfModuleBB".equalsIgnoreCase(bbFlowName) && aLaCarte && "Success".equalsIgnoreCase(handlingCode)) { postProcessingExecuteBBActivateVfModule(execution, ebb, flowsToExecute); } } @@ -410,16 +414,16 @@ public class WorkflowActionBBTasks { .forEach(executeBB -> logger.info("Flows to Execute After Post Processing: {}", executeBB.getBuildingBlock().getBpmnFlowName())); execution.setVariable("flowsToExecute", flowsToExecute); - execution.setVariable("completed", false); + execution.setVariable(COMPLETED, false); } else { - logger.debug("No cvnfcCustomization found for customizationId: " + modelCustomizationId); + logger.debug("No cvnfcCustomization found for customizationId: {}", modelCustomizationId); } } } catch (EntityNotFoundException e) { - logger.debug(e.getMessage() + " Will not be running Fabric Config Building Blocks"); + logger.debug("Will not be running Fabric Config Building Blocks", e); } catch (Exception e) { String errorMessage = "Error occurred in post processing of Vf Module create"; - execution.setVariable("handlingCode", "RollbackToCreated"); + execution.setVariable(HANDLINGCODE, ROLLBACKTOCREATED); execution.setVariable("WorkflowActionErrorMessage", errorMessage); logger.error(errorMessage, e); } -- cgit 1.2.3-korg