From 60457e41054465e9fa10c8e6bfdbfa62a45d1f0a Mon Sep 17 00:00:00 2001 From: "mark.j.leonard" Date: Fri, 29 Mar 2019 10:50:04 +0000 Subject: [PATCH] Additional null checks and comments Add defensive coding to check for possible null pointer values, thus preventing certain potential runtime exceptions. Fix some minor spelling inconsistencies. Refactor some method signatures and update the Javadoc. Reformat Resource toString() method to avoid lengthy String concatenation and duplicated separators. Change-Id: I4b01eb844700e00d0c909bcc5fa2dbc91d5149e4 Issue-ID: AAI-2281 Signed-off-by: mark.j.leonard --- .../onap/aai/babel/csar/CsarToXmlConverter.java | 2 +- .../aai/babel/csar/extractor/YamlExtractor.java | 6 ++-- .../babel/parser/ArtifactGeneratorToscaParser.java | 36 +++++++++++++--------- .../xml/generator/api/AaiArtifactGenerator.java | 31 +++++++++---------- .../aai/babel/xml/generator/model/Resource.java | 19 ++++++++++-- .../babel/csar/extractor/YamlExtractorTest.java | 8 ++--- .../service/TestGenerateArtifactsServiceImpl.java | 3 +- .../org/onap/aai/babel/util/ArtifactTestUtils.java | 8 +++-- 8 files changed, 68 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/onap/aai/babel/csar/CsarToXmlConverter.java b/src/main/java/org/onap/aai/babel/csar/CsarToXmlConverter.java index 9e1ff6e..6115f8f 100644 --- a/src/main/java/org/onap/aai/babel/csar/CsarToXmlConverter.java +++ b/src/main/java/org/onap/aai/babel/csar/CsarToXmlConverter.java @@ -75,7 +75,7 @@ public class CsarToXmlConverter { logger.debug(xmlArtifacts.size() + " XML artifact(s) have been generated"); } catch (InvalidArchiveException e) { throw new CsarConverterException( - "An error occurred trying to extract the YMAL files from the csar file : " + e); + "An error occurred trying to extract the YAML files from the CSAR file : " + e); } catch (XmlArtifactGenerationException e) { throw new CsarConverterException( "An error occurred trying to generate XML files from a collection of YAML files : " + e); diff --git a/src/main/java/org/onap/aai/babel/csar/extractor/YamlExtractor.java b/src/main/java/org/onap/aai/babel/csar/extractor/YamlExtractor.java index 15d77a1..424e661 100644 --- a/src/main/java/org/onap/aai/babel/csar/extractor/YamlExtractor.java +++ b/src/main/java/org/onap/aai/babel/csar/extractor/YamlExtractor.java @@ -75,11 +75,11 @@ public class YamlExtractor { } } if (ymlFiles.isEmpty()) { - throw new InvalidArchiveException("No valid YAML files were found in the csar file."); + throw new InvalidArchiveException("No valid YAML files were found in the CSAR file."); } } catch (IOException e) { throw new InvalidArchiveException( - "An error occurred trying to create a ZipFile. Is the content being converted really a csar file?", + "An error occurred trying to create a ZipFile. Is the content being converted really a CSAR file?", e); } @@ -90,7 +90,7 @@ public class YamlExtractor { /** * Throw an error if the supplied parameters are not valid. - * + * * @param archive * @param name * @param version diff --git a/src/main/java/org/onap/aai/babel/parser/ArtifactGeneratorToscaParser.java b/src/main/java/org/onap/aai/babel/parser/ArtifactGeneratorToscaParser.java index 0777e51..2816cb5 100644 --- a/src/main/java/org/onap/aai/babel/parser/ArtifactGeneratorToscaParser.java +++ b/src/main/java/org/onap/aai/babel/parser/ArtifactGeneratorToscaParser.java @@ -351,10 +351,13 @@ public class ArtifactGeneratorToscaParser { */ private void processVfModule(List resources, Model vfModel, Group groupDefinition, NodeTemplate serviceNode, Resource groupModel) throws XmlArtifactGenerationException { - groupModel.populateModelIdentificationInformation( - mergeProperties(groupDefinition.getMetadata().getAllProperties(), groupDefinition.getProperties())); + Metadata metadata = groupDefinition.getMetadata(); + Map mergedProperties = + mergeProperties(metadata == null ? Collections.emptyMap() : metadata.getAllProperties(), + groupDefinition.getProperties()); + groupModel.populateModelIdentificationInformation(mergedProperties); SubstitutionMappings substitutionMappings = serviceNode.getSubMappingToscaTemplate(); if (substitutionMappings != null) { processVfModuleGroup(groupModel, getVfModuleMembers(substitutionMappings, @@ -457,29 +460,34 @@ public class ArtifactGeneratorToscaParser { * parent Resource * @param metaData * for populating the Resource IDs - * @param resourceNode - * any Model (will be ignored if not a Resource) + * @param childResource + * a child Resource (will be ignored if this is a Widget type) * @param nodeProperties * the node properties * @return whether or not a ProvidingService was processed */ - private boolean processModel(Model resourceModel, Metadata metaData, Resource resourceNode, + private boolean processModel(Model resourceModel, Metadata metaData, Resource childResource, Map nodeProperties) { - boolean foundProvidingService = resourceNode != null - && (boolean) Optional.ofNullable(resourceNode.getProperties().get("providingService")).orElse(false); + boolean isProvidingService = childResource != null + && (boolean) Optional.ofNullable(childResource.getProperties().get("providingService")).orElse(false); - if (foundProvidingService) { - processProvidingService(resourceModel, resourceNode, nodeProperties); - } else if (resourceNode != null && resourceNode.getModelType() == ModelType.RESOURCE - && !resourceNode.hasWidgetType("L3_NET")) { + if (isProvidingService) { + processProvidingService(resourceModel, childResource, nodeProperties); + } else if (childResource != null && childResource.getModelType() == ModelType.RESOURCE + && !childResource.hasWidgetType("L3_NET")) { if (metaData != null) { - resourceNode.populateModelIdentificationInformation(metaData.getAllProperties()); + childResource.populateModelIdentificationInformation(metaData.getAllProperties()); } - resourceModel.addResource(resourceNode); + resourceModel.addResource(childResource); } - return foundProvidingService; + return isProvidingService; } + /** + * @param resourceModel + * @param resourceNode + * @param nodeProperties + */ private void processProvidingService(Model resourceModel, Resource resourceNode, Map nodeProperties) { if (nodeProperties == null || nodeProperties.get("providing_service_uuid") == null diff --git a/src/main/java/org/onap/aai/babel/xml/generator/api/AaiArtifactGenerator.java b/src/main/java/org/onap/aai/babel/xml/generator/api/AaiArtifactGenerator.java index 32237b5..424b1a6 100644 --- a/src/main/java/org/onap/aai/babel/xml/generator/api/AaiArtifactGenerator.java +++ b/src/main/java/org/onap/aai/babel/xml/generator/api/AaiArtifactGenerator.java @@ -105,7 +105,7 @@ public class AaiArtifactGenerator implements ArtifactGenerator { ISdcCsarHelper csarHelper = SdcToscaParserFactory.getInstance().getSdcCsarHelper(csarPath.toAbsolutePath().toString()); return generateAllArtifacts(validateServiceVersion(additionalParams), csarHelper); - } catch (SdcToscaParserException | XmlArtifactGenerationException e) { + } catch (SdcToscaParserException | ClassCastException | XmlArtifactGenerationException e) { log.error(ApplicationMsgs.INVALID_CSAR_FILE, e); return createErrorData(e); } finally { @@ -131,17 +131,11 @@ public class AaiArtifactGenerator implements ArtifactGenerator { */ public GenerationData generateAllArtifacts(final String serviceVersion, ISdcCsarHelper csarHelper) throws XmlArtifactGenerationException { - List serviceNodeTemplates = - ToscaParser.getServiceNodeTemplates(csarHelper).collect(Collectors.toList()); - if (serviceNodeTemplates == null) { - throw new IllegalArgumentException(GENERATOR_AAI_ERROR_MISSING_SERVICE_TOSCA); - } - Service serviceModel = createServiceModel(serviceVersion, csarHelper.getServiceMetadataAllProperties()); MDC.put(MDC_PARAM_MODEL_INFO, serviceModel.getModelName() + "," + getArtifactLabel(serviceModel)); - List resources = generateResourceModels(csarHelper, serviceNodeTemplates, serviceModel); + List resources = generateResourceModels(csarHelper, serviceModel); // Generate the A&AI XML model for the Service. final String serviceArtifact = modelGenerator.generateModelFor(serviceModel); @@ -183,18 +177,21 @@ public class AaiArtifactGenerator implements ArtifactGenerator { /** * @param csarHelper - * @param serviceNodeTemplates * @param serviceModel * @return the generated Models * @throws XmlArtifactGenerationException * if the configured widget mappings do not support processed widget type(s) */ - private List generateResourceModels(ISdcCsarHelper csarHelper, List serviceNodeTemplates, - Service serviceModel) throws XmlArtifactGenerationException { - final ArtifactGeneratorToscaParser parser = new ArtifactGeneratorToscaParser(csarHelper); + private List generateResourceModels(ISdcCsarHelper csarHelper, Service serviceModel) + throws XmlArtifactGenerationException { + List serviceNodeTemplates = + ToscaParser.getServiceNodeTemplates(csarHelper).collect(Collectors.toList()); + if (serviceNodeTemplates == null) { + throw new IllegalArgumentException(GENERATOR_AAI_ERROR_MISSING_SERVICE_TOSCA); + } + final ArtifactGeneratorToscaParser parser = new ArtifactGeneratorToscaParser(csarHelper); List resources = new ArrayList<>(); - final List serviceGroups = ToscaParser.getServiceLevelGroups(csarHelper); for (NodeTemplate nodeTemplate : serviceNodeTemplates) { if (nodeTemplate.getMetaData() != null) { @@ -436,15 +433,17 @@ public class AaiArtifactGenerator implements ArtifactGenerator { private int hashCodeUuId(String uuId) { int hashcode = 0; - for (int i = 0; i < uuId.length(); i++) { - hashcode = 31 * hashcode + uuId.charAt(i); + if (uuId != null) { + for (int i = 0; i < uuId.length(); i++) { + hashcode = 31 * hashcode + uuId.charAt(i); + } } return hashcode; } private String truncateName(String name) { String truncatedName = name; - if (name.length() >= 200) { + if (name != null && name.length() >= 200) { truncatedName = name.substring(0, 199); } return truncatedName; diff --git a/src/main/java/org/onap/aai/babel/xml/generator/model/Resource.java b/src/main/java/org/onap/aai/babel/xml/generator/model/Resource.java index 04c69bb..8c01a50 100644 --- a/src/main/java/org/onap/aai/babel/xml/generator/model/Resource.java +++ b/src/main/java/org/onap/aai/babel/xml/generator/model/Resource.java @@ -21,9 +21,12 @@ package org.onap.aai.babel.xml.generator.model; +import com.google.common.collect.ImmutableMap; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; import org.onap.aai.babel.xml.generator.XmlArtifactGenerationException; import org.onap.aai.babel.xml.generator.types.ModelType; @@ -135,9 +138,19 @@ public class Resource extends Model { @Override public String toString() { - return "Resource [widget type=" + getWidgetType() + ", deleteFlag=" + deleteFlag + ", modelType=" + modelType - + ", properties=" + properties + ", vserver=" + vserver + ", addlintf=" + addlintf + ", addvolume=" - + addvolume + ", members=" + members + "]"; + return ImmutableMap.builder() // + .put("Resource", Optional.ofNullable(getModelId()).orElse("null")) // + .put("widget type", getWidgetType().toString()) // + .put("deleteFlag", Boolean.toString(deleteFlag)) // + .put("modelType", modelType.toString()) // + .put("properties", properties.toString()) // + .put("vserver", Optional.ofNullable(vserver).map(Widget::toString).orElse("null")) // + .put("addlintf", Boolean.toString(addlintf)) // + .put("addvolume", Boolean.toString(addvolume)) // + .put("members", Optional.ofNullable(members).map(List::toString).orElse("null")) // + .build().entrySet().stream() // + .map(e -> e.getKey() + "=" + e.getValue()) // + .collect(Collectors.joining(", ")); } private void addVolumeWidget(Widget widget) { diff --git a/src/test/java/org/onap/aai/babel/csar/extractor/YamlExtractorTest.java b/src/test/java/org/onap/aai/babel/csar/extractor/YamlExtractorTest.java index 5bb7763..20c3434 100644 --- a/src/test/java/org/onap/aai/babel/csar/extractor/YamlExtractorTest.java +++ b/src/test/java/org/onap/aai/babel/csar/extractor/YamlExtractorTest.java @@ -2,8 +2,8 @@ * ============LICENSE_START======================================================= * org.onap.aai * ================================================================================ - * Copyright © 2017-2018 AT&T Intellectual Property. All rights reserved. - * Copyright © 2017-2018 European Software Marketing Ltd. + * Copyright © 2017-2019 AT&T Intellectual Property. All rights reserved. + * Copyright © 2017-2019 European Software Marketing Ltd. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -91,7 +91,7 @@ public class YamlExtractorTest { @Test public void testInvalidContentSupplied() { invalidArgumentsTest("This is a piece of nonsense and not a zip file".getBytes(), FOO, FOO, - "An error occurred trying to create a ZipFile. Is the content being converted really a csar file?"); + "An error occurred trying to create a ZipFile. Is the content being converted really a CSAR file?"); } @Test @@ -102,7 +102,7 @@ public class YamlExtractorTest { } catch (Exception e) { assertTrue("An instance of InvalidArchiveException should have been thrown.", e instanceof InvalidArchiveException); - assertEquals("Incorrect message was returned", "No valid YAML files were found in the csar file.", + assertEquals("Incorrect message was returned", "No valid YAML files were found in the CSAR file.", e.getMessage()); } } diff --git a/src/test/java/org/onap/aai/babel/service/TestGenerateArtifactsServiceImpl.java b/src/test/java/org/onap/aai/babel/service/TestGenerateArtifactsServiceImpl.java index 0673d04..55311a6 100644 --- a/src/test/java/org/onap/aai/babel/service/TestGenerateArtifactsServiceImpl.java +++ b/src/test/java/org/onap/aai/babel/service/TestGenerateArtifactsServiceImpl.java @@ -152,8 +152,7 @@ public class TestGenerateArtifactsServiceImpl { * if the resource cannot be loaded */ private Response processJsonRequest(CsarTest csar) throws IOException, URISyntaxException { - String jsonString = csar.getJsonRequest(); - return invokeService(jsonString); + return invokeService(csar.getJsonRequest()); } /** diff --git a/src/test/java/org/onap/aai/babel/util/ArtifactTestUtils.java b/src/test/java/org/onap/aai/babel/util/ArtifactTestUtils.java index 066911e..a98b7c2 100644 --- a/src/test/java/org/onap/aai/babel/util/ArtifactTestUtils.java +++ b/src/test/java/org/onap/aai/babel/util/ArtifactTestUtils.java @@ -65,7 +65,7 @@ public class ArtifactTestUtils { /** * Load the Widget type mappings (resource). - * + * * @throws IOException * if the configuration file is not loaded */ @@ -127,7 +127,11 @@ public class ArtifactTestUtils { } public String loadResourceAsString(String resourceName) throws IOException { + try { return IOUtils.toString(getResource(resourceName), Charset.defaultCharset()); + } catch (NullPointerException e) { + throw new IllegalArgumentException("No such resource " + resourceName); + } } public String getRequestJson(String resource) throws IOException { @@ -144,7 +148,7 @@ public class ArtifactTestUtils { /** * Create Properties from the content of the named resource (e.g. a file on the classpath). - * + * * @param resourceName * the resource name * @return Properties loaded from the named resource -- 2.16.6