Refactor babel-related code to not update parameter values

- return processed Artifact's as List<Artifact> in BabelArtifactService and ArtifactDownloadManager

Issue-ID: AAI-3840
Change-Id: Ibc78517e87ebf1bfc9c6336555c96e6d1506cf5c
Signed-off-by: Fiete Ostkamp <Fiete.Ostkamp@telekom.de>

8 files changed, 127 insertions, 105 deletions

diff --git a/src/main/java/org/onap/aai/modelloader/babel/BabelArtifactService.java b/src/main/java/org/onap/aai/modelloader/babel/BabelArtifactService.java
index 1221861..029814a 100644
--- a/src/main/java/org/onap/aai/modelloader/babel/BabelArtifactService.java
+++ b/src/main/java/org/onap/aai/modelloader/babel/BabelArtifactService.java
@@ -19,13 +19,12 @@
  */
 package org.onap.aai.modelloader.babel;
 
+import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
 
 import org.onap.aai.babel.service.data.BabelArtifact;
-import org.onap.aai.babel.service.data.BabelRequest;
 import org.onap.aai.babel.service.data.BabelArtifact.ArtifactType;
+import org.onap.aai.babel.service.data.BabelRequest;
 import org.onap.aai.cl.api.Logger;
 import org.onap.aai.cl.eelf.LoggerFactory;
 import org.onap.aai.modelloader.entity.Artifact;
@@ -39,62 +38,65 @@ import org.springframework.stereotype.Service;
 @Service
 public class BabelArtifactService {
 
-  private static Logger logger = LoggerFactory.getInstance().getLogger(BabelArtifactService.class);
+    private static Logger logger = LoggerFactory.getInstance().getLogger(BabelArtifactService.class);
+
+    private final BabelServiceClient babelServiceClient;
+    private final BabelArtifactConverter babelArtifactConverter;
+
+    public BabelArtifactService(BabelServiceClient babelServiceClient, BabelArtifactConverter babelArtifactConverter) {
+        this.babelServiceClient = babelServiceClient;
+        this.babelArtifactConverter = babelArtifactConverter;
+    }
+
+    public List<Artifact> invokeBabelService(BabelRequest babelRequest, String distributionId)
+            throws ProcessToscaArtifactsException {
+        try {
+            logger.info(ModelLoaderMsgs.DISTRIBUTION_EVENT,
+                    "Posting artifact: " + babelRequest.getArtifactName() + ", service version: "
+                            + babelRequest.getArtifactVersion()
+                            + ", artifact version: " + babelRequest.getArtifactVersion());
+
+            List<BabelArtifact> babelArtifacts = babelServiceClient.postArtifact(babelRequest, distributionId);
+
+            List<Artifact> convertedArtifacts = new ArrayList<>();
+            for(BabelArtifact babelArtifact : babelArtifacts) {
+                if(!isUnknownType(babelArtifact)) {
+                    if(babelArtifact.getType() == ArtifactType.MODEL) {
+                        convertedArtifacts.addAll(babelArtifactConverter.convertToModel(babelArtifact));
+                    } else {
+                        convertedArtifacts.add(babelArtifactConverter.convertToCatalog(babelArtifact));
+                    }
+                }
+            }
+
+            return convertedArtifacts;
 
-  private final BabelServiceClient babelServiceClient;
-  private final BabelArtifactConverter babelArtifactConverter;
+        } catch (BabelArtifactParsingException e) {
+            logger.error(ModelLoaderMsgs.ARTIFACT_PARSE_ERROR,
+                    "Error for artifact " + babelRequest.getArtifactName() + " " + babelRequest.getArtifactVersion()
+                            + " " + e);
+            throw new ProcessToscaArtifactsException(
+                    "An error occurred while trying to parse the Babel artifacts: " + e.getLocalizedMessage());
+        } catch (Exception e) {
+            logger.error(ModelLoaderMsgs.BABEL_REST_REQUEST_ERROR, e, "POST",
+                    "Error posting artifact " + babelRequest.getArtifactName() + " " + babelRequest.getArtifactVersion()
+                            + " to Babel: "
+                            + e.getLocalizedMessage());
+            throw new ProcessToscaArtifactsException(
+                    "An error occurred while calling the Babel service: " + e.getLocalizedMessage());
+        }
+    }
 
-  public BabelArtifactService(BabelServiceClient babelServiceClient, BabelArtifactConverter babelArtifactConverter) {
-    this.babelServiceClient = babelServiceClient;
-    this.babelArtifactConverter = babelArtifactConverter;
-  }
+    private boolean isUnknownType(BabelArtifact babelArtifact) {
+        if (babelArtifact.getType() == ArtifactType.MODEL || babelArtifact.getType() == ArtifactType.VNFCATALOG) {
+            return false;
+        } else {
+            logger.warn(ModelLoaderMsgs.ARTIFACT_PARSE_ERROR,
+                    babelArtifact.getName() + " " + babelArtifact.getType()
+                            + ". Unexpected artifact types returned by the babel service: "
+                            + babelArtifact.getPayload());
+            return true;
+        }
+    }
 
-  public void invokeBabelService(List<Artifact> modelArtifacts, List<Artifact> catalogArtifacts, BabelRequest babelRequest, String distributionId)
-          throws ProcessToscaArtifactsException {
-      try {
-          logger.info(ModelLoaderMsgs.DISTRIBUTION_EVENT,
-                  "Posting artifact: " + babelRequest.getArtifactName() + ", service version: " + babelRequest.getArtifactVersion()
-                          + ", artifact version: " + babelRequest.getArtifactVersion());
-  
-          List<BabelArtifact> babelArtifacts =
-                babelServiceClient.postArtifact(babelRequest, distributionId);
-  
-          // Sort Babel artifacts based on type
-          Map<ArtifactType, List<BabelArtifact>> artifactMap =
-                  babelArtifacts.stream().collect(Collectors.groupingBy(BabelArtifact::getType));
-  
-          if (artifactMap.containsKey(BabelArtifact.ArtifactType.MODEL)) {
-              modelArtifacts.addAll(
-                      babelArtifactConverter.convertToModel(artifactMap.get(BabelArtifact.ArtifactType.MODEL)));
-              artifactMap.remove(BabelArtifact.ArtifactType.MODEL);
-          }
-  
-          if (artifactMap.containsKey(BabelArtifact.ArtifactType.VNFCATALOG)) {
-              catalogArtifacts.addAll(babelArtifactConverter
-                      .convertToCatalog(artifactMap.get(BabelArtifact.ArtifactType.VNFCATALOG)));
-              artifactMap.remove(BabelArtifact.ArtifactType.VNFCATALOG);
-          }
-  
-          // Log unexpected artifact types
-          if (!artifactMap.isEmpty()) {
-              logger.warn(ModelLoaderMsgs.ARTIFACT_PARSE_ERROR,
-                      babelRequest.getArtifactName() + " " + babelRequest.getArtifactVersion()
-                              + ". Unexpected artifact types returned by the babel service: "
-                              + artifactMap.keySet().toString());
-          }
-  
-      } catch (BabelArtifactParsingException e) {
-          logger.error(ModelLoaderMsgs.ARTIFACT_PARSE_ERROR,
-                  "Error for artifact " + babelRequest.getArtifactName() + " " + babelRequest.getArtifactVersion() + " " + e);
-          throw new ProcessToscaArtifactsException(
-                  "An error occurred while trying to parse the Babel artifacts: " + e.getLocalizedMessage());
-      } catch (Exception e) {
-          logger.error(ModelLoaderMsgs.BABEL_REST_REQUEST_ERROR, e, "POST",
-                  "Error posting artifact " + babelRequest.getArtifactName() + " " + babelRequest.getArtifactVersion() + " to Babel: "
-                          + e.getLocalizedMessage());
-          throw new ProcessToscaArtifactsException(
-                  "An error occurred while calling the Babel service: " + e.getLocalizedMessage());
-      }
-  }
-  
-}
+}
\ No newline at end of file
diff --git a/src/main/java/org/onap/aai/modelloader/entity/model/AbstractModelArtifactParser.java b/src/main/java/org/onap/aai/modelloader/entity/model/AbstractModelArtifactParser.java
index 35405e6..14f46f3 100644
--- a/src/main/java/org/onap/aai/modelloader/entity/model/AbstractModelArtifactParser.java
+++ b/src/main/java/org/onap/aai/modelloader/entity/model/AbstractModelArtifactParser.java
@@ -82,7 +82,8 @@ public abstract class AbstractModelArtifactParser implements IModelParser {
 
             IModelArtifact model = parseModel(doc.getDocumentElement(), artifactPayload);
 
-            if (!processParsedModel(modelList, artifactName, model)) {
+            boolean success = processParsedModel(modelList, artifactName, model);
+            if (!success) {
                 modelList = null;
             }
         } catch (Exception ex) {
diff --git a/src/main/java/org/onap/aai/modelloader/entity/model/ModelArtifactParser.java b/src/main/java/org/onap/aai/modelloader/entity/model/ModelArtifactParser.java
index de99880..367b212 100644
--- a/src/main/java/org/onap/aai/modelloader/entity/model/ModelArtifactParser.java
+++ b/src/main/java/org/onap/aai/modelloader/entity/model/ModelArtifactParser.java
@@ -37,10 +37,12 @@ import org.onap.aai.cl.api.Logger;
 import org.onap.aai.cl.eelf.LoggerFactory;
 import org.onap.aai.modelloader.entity.Artifact;
 import org.onap.aai.modelloader.service.ModelLoaderMsgs;
+import org.springframework.stereotype.Component;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
+@Component
 public class ModelArtifactParser extends AbstractModelArtifactParser {
 
     public static final String MODEL_VER = "model-ver";
diff --git a/src/main/java/org/onap/aai/modelloader/notification/ArtifactDownloadManager.java b/src/main/java/org/onap/aai/modelloader/notification/ArtifactDownloadManager.java
index f0c96bd..90e20bd 100644
--- a/src/main/java/org/onap/aai/modelloader/notification/ArtifactDownloadManager.java
+++ b/src/main/java/org/onap/aai/modelloader/notification/ArtifactDownloadManager.java
@@ -24,6 +24,9 @@ import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.Base64;
 import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
 import org.onap.aai.babel.service.data.BabelRequest;
 import org.onap.aai.cl.api.Logger;
 import org.onap.aai.cl.eelf.LoggerFactory;
@@ -31,6 +34,8 @@ import org.onap.aai.cl.mdc.MdcContext;
 import org.onap.aai.cl.mdc.MdcOverride;
 import org.onap.aai.modelloader.babel.BabelArtifactService;
 import org.onap.aai.modelloader.entity.Artifact;
+import org.onap.aai.modelloader.entity.ArtifactType;
+import org.onap.aai.modelloader.entity.catalog.VnfCatalogArtifact;
 import org.onap.aai.modelloader.entity.model.BabelArtifactParsingException;
 import org.onap.aai.modelloader.entity.model.IModelParser;
 import org.onap.aai.modelloader.entity.model.NamedQueryArtifactParser;
@@ -132,9 +137,11 @@ public class ArtifactDownloadManager {
     private void processDownloadedArtifacts(List<Artifact> modelArtifacts, List<Artifact> catalogArtifacts,
             IArtifactInfo artifactInfo, IDistributionClientDownloadResult downloadResult, INotificationData data)
             throws ProcessToscaArtifactsException, InvalidArchiveException, BabelArtifactParsingException {
+        List<Artifact> artifacts = null;
         if ("TOSCA_CSAR".equalsIgnoreCase(artifactInfo.getArtifactType())) {
-            processToscaArtifacts(modelArtifacts, catalogArtifacts, downloadResult.getArtifactPayload(), artifactInfo,
+            artifacts = processToscaArtifacts(downloadResult.getArtifactPayload(), artifactInfo,
                     data.getDistributionID(), data.getServiceVersion());
+            
         } else if (ArtifactTypeEnum.MODEL_QUERY_SPEC.toString().equalsIgnoreCase(artifactInfo.getArtifactType())) {
             processModelQuerySpecArtifact(modelArtifacts, downloadResult);
         } else {
@@ -142,29 +149,46 @@ public class ArtifactDownloadManager {
                     artifactInfo.getArtifactType());
             throw new InvalidArchiveException("Unsupported artifact type: " + artifactInfo.getArtifactType());
         }
+        if(artifacts != null) {
+            for(Artifact artifact : artifacts) {
+                if(artifact.getType() == ArtifactType.VNF_CATALOG || artifact.getType() == ArtifactType.VNF_CATALOG_XML) {
+                    catalogArtifacts.add(artifact);
+                } else {
+                    modelArtifacts.add(artifact);
+                }
+            }
+        }
     }
 
-    public void processToscaArtifacts(List<Artifact> modelArtifacts, List<Artifact> catalogArtifacts, byte[] payload,
-            IArtifactInfo artifactInfo, String distributionId, String serviceVersion)
+    public List<Artifact> processToscaArtifacts(byte[] payload, IArtifactInfo artifactInfo, String distributionId, String serviceVersion)
             throws ProcessToscaArtifactsException, InvalidArchiveException {
         // Get translated artifacts from Babel Service
         BabelRequest babelRequest = new BabelRequest();
         babelRequest.setArtifactName(artifactInfo.getArtifactName());
         babelRequest.setCsar(Base64.getEncoder().encodeToString(payload));
         babelRequest.setArtifactVersion(serviceVersion);
-        babelArtifactService.invokeBabelService(modelArtifacts, catalogArtifacts, babelRequest, distributionId);
+        List<Artifact> artifacts = babelArtifactService.invokeBabelService(babelRequest, distributionId);
 
         // Get VNF Catalog artifacts directly from CSAR
         List<Artifact> csarCatalogArtifacts = vnfCatalogExtractor.extract(payload, artifactInfo.getArtifactName());
 
         // Throw an error if VNF Catalog data is present in the Babel payload and directly in the CSAR
-        if (!catalogArtifacts.isEmpty() && !csarCatalogArtifacts.isEmpty()) {
+        if (isDuplicateVnfCatalogData(artifacts, csarCatalogArtifacts)) {
             logger.error(ModelLoaderMsgs.DUPLICATE_VNFC_DATA_ERROR, artifactInfo.getArtifactName());
             throw new InvalidArchiveException("CSAR: " + artifactInfo.getArtifactName()
                     + " contains VNF Catalog data in the format of both TOSCA and XML files. Only one format can be used for each CSAR file.");
-        } else if (!csarCatalogArtifacts.isEmpty()) {
-            catalogArtifacts.addAll(csarCatalogArtifacts);
         }
+        return Stream
+                .concat(artifacts.stream(), csarCatalogArtifacts.stream())
+                .collect(Collectors.toList());
+
+    }
+
+    private boolean isDuplicateVnfCatalogData(List<Artifact> babelArtifacts, List<Artifact> csarCatalogArtifacts) {
+        boolean babelIsEmpty = babelArtifacts.stream()
+            .filter(VnfCatalogArtifact.class::isInstance)
+            .findAny().isEmpty();
+        return !csarCatalogArtifacts.isEmpty() && !babelIsEmpty;
     }
 
     private void processModelQuerySpecArtifact(List<Artifact> modelArtifacts,
diff --git a/src/main/java/org/onap/aai/modelloader/notification/BabelArtifactConverter.java b/src/main/java/org/onap/aai/modelloader/notification/BabelArtifactConverter.java
index 5118652..59d3348 100644
--- a/src/main/java/org/onap/aai/modelloader/notification/BabelArtifactConverter.java
+++ b/src/main/java/org/onap/aai/modelloader/notification/BabelArtifactConverter.java
@@ -20,7 +20,6 @@
  */
 package org.onap.aai.modelloader.notification;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 import org.onap.aai.babel.service.data.BabelArtifact;
@@ -38,6 +37,12 @@ import org.springframework.stereotype.Component;
 @Component
 public class BabelArtifactConverter {
 
+    private final ModelArtifactParser modelArtifactParser;
+
+    public BabelArtifactConverter(ModelArtifactParser modelArtifactParser) {
+        this.modelArtifactParser = modelArtifactParser;
+    }
+
     /**
      * This method converts BabelArtifacts into instances of {@link ModelArtifact}.
      *
@@ -46,24 +51,15 @@ public class BabelArtifactConverter {
      * @throws BabelArtifactParsingException if an error occurs trying to parse the generated XML files that were
      *         converted from tosca artifacts
      */
-    public List<Artifact> convertToModel(List<BabelArtifact> xmlArtifacts) throws BabelArtifactParsingException {
-        Objects.requireNonNull(xmlArtifacts);
-        List<Artifact> modelArtifacts = new ArrayList<>();
-        ModelArtifactParser modelArtParser = new ModelArtifactParser();
-
-        // Parse TOSCA payloads
-        for (BabelArtifact xmlArtifact : xmlArtifacts) {
-
-            List<Artifact> parsedArtifacts = modelArtParser.parse(xmlArtifact.getPayload(), xmlArtifact.getName());
-
-            if (parsedArtifacts == null || parsedArtifacts.isEmpty()) {
-                throw new BabelArtifactParsingException("Could not parse generated XML: " + xmlArtifact.getPayload());
-            }
-
-            modelArtifacts.addAll(parsedArtifacts);
+    public List<Artifact> convertToModel(BabelArtifact babelArtifact) throws BabelArtifactParsingException {
+        Objects.requireNonNull(babelArtifact);
+        List<Artifact> parsedArtifacts = modelArtifactParser.parse(babelArtifact.getPayload(), babelArtifact.getName());
+        
+        if (parsedArtifacts == null || parsedArtifacts.isEmpty()) {
+            throw new BabelArtifactParsingException("Could not parse generated XML: " + babelArtifact.getPayload());
         }
 
-        return modelArtifacts;
+        return parsedArtifacts;
     }
 
     /**
@@ -72,14 +68,7 @@ public class BabelArtifactConverter {
      * @param xmlArtifacts xml artifacts to be parsed
      * @return List<org.openecomp.modelloader.entity.Artifact> list of converted catalog artifacts
      */
-    public List<Artifact> convertToCatalog(List<BabelArtifact> xmlArtifacts) {
-        Objects.requireNonNull(xmlArtifacts);
-        List<Artifact> catalogArtifacts = new ArrayList<>();
-
-        for (BabelArtifact xmlArtifact : xmlArtifacts) {
-            catalogArtifacts.add(new VnfCatalogArtifact(xmlArtifact.getPayload()));
-        }
-
-        return catalogArtifacts;
+    public Artifact convertToCatalog(BabelArtifact babelArtifact) {
+        return new VnfCatalogArtifact(babelArtifact.getPayload());
     }
-}
+}
\ No newline at end of file
diff --git a/src/main/java/org/onap/aai/modelloader/notification/EventCallback.java b/src/main/java/org/onap/aai/modelloader/notification/EventCallback.java
index 754eaff..d047bf9 100644
--- a/src/main/java/org/onap/aai/modelloader/notification/EventCallback.java
+++ b/src/main/java/org/onap/aai/modelloader/notification/EventCallback.java
@@ -67,7 +67,7 @@ public class EventCallback implements INotificationCallback {
                 artifactDownloadManager.downloadArtifacts(data, artifacts, modelArtifacts, catalogArtifacts);
 
         if (success) {
-            success = artifactDeploymentManager.deploy(data, modelArtifacts, catalogArtifacts);
+            success = artifactDeploymentManager.deploy(data.getDistributionID(), modelArtifacts, catalogArtifacts);
         }
 
         String statusString = success ? "SUCCESS" : "FAILURE";
diff --git a/src/main/java/org/onap/aai/modelloader/service/ArtifactDeploymentManager.java b/src/main/java/org/onap/aai/modelloader/service/ArtifactDeploymentManager.java
index 222ae34..e4535be 100644
--- a/src/main/java/org/onap/aai/modelloader/service/ArtifactDeploymentManager.java
+++ b/src/main/java/org/onap/aai/modelloader/service/ArtifactDeploymentManager.java
@@ -22,11 +22,11 @@ package org.onap.aai.modelloader.service;
 
 import java.util.ArrayList;
 import java.util.List;
+
 import org.onap.aai.modelloader.entity.Artifact;
 import org.onap.aai.modelloader.entity.catalog.VnfCatalogArtifactHandler;
 import org.onap.aai.modelloader.entity.model.ModelArtifactHandler;
 import org.onap.aai.modelloader.restclient.AaiRestClient;
-import org.onap.sdc.api.notification.INotificationData;
 import org.springframework.stereotype.Component;
 
 /**
@@ -48,17 +48,15 @@ public class ArtifactDeploymentManager {
     /**
      * Deploys model and catalog artifacts to A&AI.
      *
-     * @param data data about the notification that is being processed
+     * @param distributionId data about the notification that is being processed
      * @param modelArtifacts collection of artifacts that represent yml files found in a TOSCA_CSAR file that have been
      *        converted to XML and also those for model query specs
      * @param catalogArtifacts collection of artifacts that represent vnf catalog files
      * @return boolean <code>true</code> if all deployments were successful otherwise <code>false</code>
      */
-    public boolean deploy(final INotificationData data, final List<Artifact> modelArtifacts,
+    public boolean deploy(final String distributionId, final List<Artifact> modelArtifacts,
             final List<Artifact> catalogArtifacts) {
 
-        String distributionId = data.getDistributionID();
-
         List<Artifact> completedArtifacts = new ArrayList<>();
         boolean deploySuccess =
                 modelArtifactHandler.pushArtifacts(modelArtifacts, distributionId, completedArtifacts, aaiClient);
@@ -77,4 +75,4 @@ public class ArtifactDeploymentManager {
 
         return deploySuccess;
     }
-}
+}
\ No newline at end of file
diff --git a/src/main/java/org/onap/aai/modelloader/service/ModelController.java b/src/main/java/org/onap/aai/modelloader/service/ModelController.java
index 4e883aa..233f2ab 100644
--- a/src/main/java/org/onap/aai/modelloader/service/ModelController.java
+++ b/src/main/java/org/onap/aai/modelloader/service/ModelController.java
@@ -29,6 +29,7 @@ import org.onap.aai.cl.api.Logger;
 import org.onap.aai.cl.eelf.LoggerFactory;
 import org.onap.aai.modelloader.config.ModelLoaderConfig;
 import org.onap.aai.modelloader.entity.Artifact;
+import org.onap.aai.modelloader.entity.ArtifactType;
 import org.onap.aai.modelloader.notification.ArtifactDownloadManager;
 import org.onap.aai.modelloader.notification.NotificationDataImpl;
 import org.onap.aai.modelloader.notification.NotificationPublisher;
@@ -103,11 +104,16 @@ public class ModelController {
 
             logger.info(ModelLoaderMsgs.DISTRIBUTION_EVENT, "Generating XML models from test artifact");
 
+            List<Artifact> artifacts = artifactDownloadManager.processToscaArtifacts(csarFile, artifactInfo, "test-transaction-id", modelVersion);
             List<Artifact> modelArtifacts = new ArrayList<>();
             List<Artifact> catalogArtifacts = new ArrayList<>();
-
-            artifactDownloadManager.processToscaArtifacts(modelArtifacts,
-                    catalogArtifacts, csarFile, artifactInfo, "test-transaction-id", modelVersion);
+            for(Artifact artifact : artifacts) {
+                if(artifact.getType().equals(ArtifactType.MODEL)) {
+                    modelArtifacts.add(artifact);
+                } else {
+                    catalogArtifacts.add(artifact);
+                }
+            }
 
             logger.info(ModelLoaderMsgs.DISTRIBUTION_EVENT, "Loading xml models from test artifacts: "
                     + modelArtifacts.size() + " model(s) and " + catalogArtifacts.size() + " catalog(s)");
@@ -115,7 +121,7 @@ public class ModelController {
             NotificationDataImpl notificationData = new NotificationDataImpl();
             notificationData.setDistributionID("TestDistributionID");
             boolean success =
-                    artifactDeploymentManager.deploy(notificationData, modelArtifacts, catalogArtifacts);
+                    artifactDeploymentManager.deploy(notificationData.getDistributionID(), modelArtifacts, catalogArtifacts);
             logger.info(ModelLoaderMsgs.DISTRIBUTION_EVENT, "Deployment success was " + success);
             response = success ? ResponseEntity.ok().build() : ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).build();
         } catch (Exception e) {