Return List<Artifact> in ArtifactDownloadManager

- return processed artifacts as List<Artifact> instead of updating the parameters in ArtifactDownloadManager
- use exceptions to signal exceptional behaviour instead of boolean success = false;

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

10 files changed, 137 insertions, 258 deletions

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 90e20bd..e2c5e27 100644
--- a/src/main/java/org/onap/aai/modelloader/notification/ArtifactDownloadManager.java
+++ b/src/main/java/org/onap/aai/modelloader/notification/ArtifactDownloadManager.java
@@ -22,6 +22,7 @@ package org.onap.aai.modelloader.notification;
 
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
+import java.util.ArrayList;
 import java.util.Base64;
 import java.util.List;
 import java.util.stream.Collectors;
@@ -86,29 +87,25 @@ public class ArtifactDownloadManager {
      * @param modelArtifacts collection of artifacts for model query specs
      * @param catalogArtifacts collection of artifacts that represent vnf catalog files
      * @return boolean <code>true</code> if the download process was successful otherwise <code>false</code>
+     * @throws Exception 
      */
-    boolean downloadArtifacts(INotificationData data, List<IArtifactInfo> artifacts, List<Artifact> modelArtifacts,
-            List<Artifact> catalogArtifacts) {
-        boolean success = true;
+    List<Artifact> downloadArtifacts(INotificationData data, List<IArtifactInfo> artifacts) throws Exception {
 
+        List<Artifact> allArtifacts = new ArrayList<>();
         for (IArtifactInfo artifact : artifacts) {
             try {
                 IDistributionClientDownloadResult downloadResult = downloadIndividualArtifacts(data, artifact);
-                processDownloadedArtifacts(modelArtifacts, catalogArtifacts, artifact, downloadResult, data);
+                List<Artifact> processedArtifacts = processDownloadedArtifacts(artifact, downloadResult, data);
+                allArtifacts.addAll(processedArtifacts);
             } catch (DownloadFailureException e) {
                 notificationPublisher.publishDownloadFailure(client, data, artifact, e.getMessage());
-                success = false;
-            } catch (Exception e) {
+                throw e;
+            } catch (ProcessToscaArtifactsException | InvalidArchiveException | BabelArtifactParsingException e) {
                 notificationPublisher.publishDeployFailure(client, data, artifact);
-                success = false;
-            }
-
-            if (!success) {
-                break;
+                throw e;
             }
         }
-
-        return success;
+        return allArtifacts;
     }
 
     private IDistributionClientDownloadResult downloadIndividualArtifacts(INotificationData data,
@@ -134,30 +131,25 @@ public class ArtifactDownloadManager {
         return downloadResult;
     }
 
-    private void processDownloadedArtifacts(List<Artifact> modelArtifacts, List<Artifact> catalogArtifacts,
+    private List<Artifact> processDownloadedArtifacts(
             IArtifactInfo artifactInfo, IDistributionClientDownloadResult downloadResult, INotificationData data)
             throws ProcessToscaArtifactsException, InvalidArchiveException, BabelArtifactParsingException {
-        List<Artifact> artifacts = null;
+        List<Artifact> artifacts = new ArrayList<>();
+        List<Artifact> querySpecArtifacts = new ArrayList<>();
         if ("TOSCA_CSAR".equalsIgnoreCase(artifactInfo.getArtifactType())) {
             artifacts = processToscaArtifacts(downloadResult.getArtifactPayload(), artifactInfo,
                     data.getDistributionID(), data.getServiceVersion());
             
         } else if (ArtifactTypeEnum.MODEL_QUERY_SPEC.toString().equalsIgnoreCase(artifactInfo.getArtifactType())) {
-            processModelQuerySpecArtifact(modelArtifacts, downloadResult);
+            querySpecArtifacts = processModelQuerySpecArtifact(downloadResult);
         } else {
             logger.info(ModelLoaderMsgs.UNSUPPORTED_ARTIFACT_TYPE, artifactInfo.getArtifactName(),
                     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);
-                }
-            }
-        }
+        return Stream
+            .concat(artifacts.stream(), querySpecArtifacts.stream())
+            .collect(Collectors.toList());
     }
 
     public List<Artifact> processToscaArtifacts(byte[] payload, IArtifactInfo artifactInfo, String distributionId, String serviceVersion)
@@ -191,8 +183,7 @@ public class ArtifactDownloadManager {
         return !csarCatalogArtifacts.isEmpty() && !babelIsEmpty;
     }
 
-    private void processModelQuerySpecArtifact(List<Artifact> modelArtifacts,
-            IDistributionClientDownloadResult downloadResult) throws BabelArtifactParsingException {
+    private List<Artifact> processModelQuerySpecArtifact(IDistributionClientDownloadResult downloadResult) throws BabelArtifactParsingException {
         logger.debug(ModelLoaderMsgs.DISTRIBUTION_EVENT, "Processing named query artifact.");
 
         IModelParser parser = new NamedQueryArtifactParser();
@@ -200,15 +191,11 @@ public class ArtifactDownloadManager {
         List<Artifact> parsedArtifacts =
                 parser.parse(new String(downloadResult.getArtifactPayload()), downloadResult.getArtifactFilename());
 
-        if (parsedArtifactsExist(parsedArtifacts)) {
-            modelArtifacts.addAll(parsedArtifacts);
+        if (parsedArtifacts != null && !parsedArtifacts.isEmpty()) {
+            return parsedArtifacts;
         } else {
             throw new BabelArtifactParsingException(
                     "Could not parse generated XML: " + new String(downloadResult.getArtifactPayload()));
         }
     }
-
-    private boolean parsedArtifactsExist(List<Artifact> parsedArtifacts) {
-        return parsedArtifacts != null && !parsedArtifacts.isEmpty();
-    }
 }
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 d047bf9..6993948 100644
--- a/src/main/java/org/onap/aai/modelloader/notification/EventCallback.java
+++ b/src/main/java/org/onap/aai/modelloader/notification/EventCallback.java
@@ -27,6 +27,7 @@ import org.onap.aai.cl.api.Logger;
 import org.onap.aai.cl.eelf.LoggerFactory;
 import org.onap.aai.cl.mdc.MdcContext;
 import org.onap.aai.modelloader.entity.Artifact;
+import org.onap.aai.modelloader.entity.ArtifactType;
 import org.onap.aai.modelloader.extraction.ArtifactInfoExtractor;
 import org.onap.aai.modelloader.service.ArtifactDeploymentManager;
 import org.onap.aai.modelloader.service.ModelLoaderMsgs;
@@ -60,11 +61,26 @@ public class EventCallback implements INotificationCallback {
         logger.info(ModelLoaderMsgs.DISTRIBUTION_EVENT, "Received distribution " + data.getDistributionID());
 
         List<IArtifactInfo> artifacts = new ArtifactInfoExtractor().extract(data);
+        boolean success = true;
+        List<Artifact> downloadedArtifacts = new ArrayList<>();
+        try {
+            downloadedArtifacts =
+                    artifactDownloadManager.downloadArtifacts(data, artifacts);
+        } catch (Exception e) {
+            success = false;
+        }
+
         List<Artifact> catalogArtifacts = new ArrayList<>();
         List<Artifact> modelArtifacts = new ArrayList<>();
-
-        boolean success =
-                artifactDownloadManager.downloadArtifacts(data, artifacts, modelArtifacts, catalogArtifacts);
+        if(downloadedArtifacts != null) {
+            for(Artifact artifact : downloadedArtifacts) {
+                if(artifact.getType() == ArtifactType.VNF_CATALOG || artifact.getType() == ArtifactType.VNF_CATALOG_XML) {
+                    catalogArtifacts.add(artifact);
+                } else {
+                    modelArtifacts.add(artifact);
+                }
+            }
+        }
 
         if (success) {
             success = artifactDeploymentManager.deploy(data.getDistributionID(), modelArtifacts, catalogArtifacts);
diff --git a/src/main/java/org/onap/aai/modelloader/service/BabelServiceClientFactory.java b/src/main/java/org/onap/aai/modelloader/service/BabelServiceClientFactory.java
deleted file mode 100644
index ce690c5..0000000
--- a/src/main/java/org/onap/aai/modelloader/service/BabelServiceClientFactory.java
+++ /dev/null
@@ -1,30 +0,0 @@
-/**
- * ============LICENSE_START=======================================================
- * org.onap.aai
- * ================================================================================
- * Copyright © 2017-2018 AT&T Intellectual Property. All rights reserved.
- * Copyright © 2017-2018 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.
- * You may obtain a copy of the License at
- *
- *       http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- * ============LICENSE_END=========================================================
- */
-
-package org.onap.aai.modelloader.service;
-
-import org.onap.aai.modelloader.config.ModelLoaderConfig;
-import org.onap.aai.modelloader.restclient.BabelServiceClient;
-import org.onap.aai.modelloader.restclient.BabelServiceClientException;
-
-public interface BabelServiceClientFactory {
-    public BabelServiceClient create(ModelLoaderConfig config) throws BabelServiceClientException;
-}
diff --git a/src/test/java/org/onap/aai/modelloader/notification/ArtifactDownloadManagerTest.java b/src/test/java/org/onap/aai/modelloader/notification/ArtifactDownloadManagerTest.java
index eb31688..f1bbd31 100644
--- a/src/test/java/org/onap/aai/modelloader/notification/ArtifactDownloadManagerTest.java
+++ b/src/test/java/org/onap/aai/modelloader/notification/ArtifactDownloadManagerTest.java
@@ -28,15 +28,16 @@ import static com.github.tomakehurst.wiremock.client.WireMock.post;
 import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
 import org.junit.jupiter.api.Test;
 import org.onap.aai.modelloader.DistributionClientTestConfiguration;
 import org.onap.aai.modelloader.entity.Artifact;
 import org.onap.aai.modelloader.entity.ArtifactType;
+import org.onap.aai.modelloader.entity.model.ModelArtifact;
 import org.onap.aai.modelloader.service.ArtifactInfoImpl;
 import org.onap.sdc.api.notification.IArtifactInfo;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -47,8 +48,6 @@ import org.springframework.http.MediaType;
 import org.springframework.kafka.test.context.EmbeddedKafka;
 import org.springframework.test.annotation.DirtiesContext;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
-
 @DirtiesContext
 @AutoConfigureWireMock(port = 0)
 @EmbeddedKafka(partitions = 1, ports = 9092, topics = {"${topics.distribution.notification}"})
@@ -59,7 +58,7 @@ public class ArtifactDownloadManagerTest {
   @Autowired ArtifactDownloadManager artifactDownloadManager;
 
   @Test
-  public void downloadArtifacts() throws JsonProcessingException {
+  public void downloadArtifacts() throws Exception {
     NotificationDataImpl notificationData = new NotificationDataImpl();
     notificationData.setDistributionID("distributionID");
     notificationData.setServiceVersion("2.0");
@@ -94,13 +93,16 @@ public class ArtifactDownloadManagerTest {
     artifactInfo.setArtifactUUID("f6f907f1-3f45-4fb4-8cbe-15a4c6ee16db");
     List<IArtifactInfo> artifacts = new ArrayList<>();
     artifacts.add(artifactInfo);
-    List<Artifact> modelArtifacts = new ArrayList<>(); // processed artifacts will be written to this list
-    List<Artifact> catalogArtifacts = new ArrayList<>(); // processed artifacts will be written to this list
-    boolean result = artifactDownloadManager.downloadArtifacts(notificationData, artifacts, modelArtifacts, catalogArtifacts);
+    List<Artifact> result = artifactDownloadManager.downloadArtifacts(notificationData, artifacts);
     
-    assertEquals(1, modelArtifacts.size());
-    assertEquals(ArtifactType.MODEL, modelArtifacts.get(0).getType());
-    assertTrue(result);
+    assertEquals(1, result.size());
+    ModelArtifact modelArtifact = (ModelArtifact) result.get(0);
+    assertEquals(ArtifactType.MODEL, modelArtifact.getType());
+    assertEquals("3c8bc8e7-e387-46ed-8616-70e99e2206dc", modelArtifact.getModelInvariantId());
+    assertEquals("http://org.onap.aai.inventory/v28", modelArtifact.getModelNamespace());
+    assertEquals("v28", modelArtifact.getModelNamespaceVersion());
+    assertEquals("71f47717-b100-4eac-940b-7d4e86a4cbb7", modelArtifact.getModelVerId());
+    assertEquals(Set.of("82194af1-3c2c-485a-8f44-420e22a9eaa4|46b92144-923a-4d20-b85a-3cbd847668a9"), modelArtifact.getDependentModelIds());
   }
 
 }
diff --git a/src/test/java/org/onap/aai/modelloader/notification/ArtifactDownloadManagerVnfcTest.java b/src/test/java/org/onap/aai/modelloader/notification/ArtifactDownloadManagerVnfcTest.java
index 0e8a733..acd250b 100644
--- a/src/test/java/org/onap/aai/modelloader/notification/ArtifactDownloadManagerVnfcTest.java
+++ b/src/test/java/org/onap/aai/modelloader/notification/ArtifactDownloadManagerVnfcTest.java
@@ -20,10 +20,8 @@
  */
 package org.onap.aai.modelloader.notification;
 
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.when;
@@ -33,6 +31,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Properties;
+import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -51,7 +50,6 @@ import org.onap.aai.modelloader.extraction.InvalidArchiveException;
 import org.onap.aai.modelloader.extraction.VnfCatalogExtractor;
 import org.onap.aai.modelloader.restclient.BabelServiceClient;
 import org.onap.aai.modelloader.restclient.BabelServiceClientException;
-import org.onap.aai.modelloader.service.BabelServiceClientFactory;
 import org.onap.aai.modelloader.util.ArtifactTestUtils;
 import org.onap.sdc.api.IDistributionClient;
 import org.onap.sdc.api.notification.IArtifactInfo;
@@ -70,14 +68,12 @@ public class ArtifactDownloadManagerVnfcTest {
     @Mock private IDistributionClient mockDistributionClient;
     @Mock private NotificationPublisher mockNotificationPublisher;
     @Mock private BabelArtifactConverter mockBabelArtifactConverter;
-    @Mock private BabelServiceClientFactory mockClientFactory;
     @Mock private VnfCatalogExtractor mockVnfCatalogExtractor;
     @InjectMocks private BabelArtifactService babelArtifactService;
 
     @BeforeEach
     public void setup() throws Exception {
         MockitoAnnotations.openMocks(this);
-        when(mockClientFactory.create(Mockito.any())).thenReturn(mockBabelClient);
 
         Properties configProperties = new Properties();
         configProperties.load(this.getClass().getClassLoader().getResourceAsStream("model-loader.properties"));
@@ -87,7 +83,7 @@ public class ArtifactDownloadManagerVnfcTest {
 
     @Test
     public void downloadArtifacts_validToscaVnfcCsarFile()
-            throws IOException, BabelServiceClientException, BabelArtifactParsingException, InvalidArchiveException {
+            throws Exception {
         INotificationData data = getNotificationDataWithToscaCsarFile();
         IArtifactInfo artifactInfo = data.getServiceArtifacts().get(0);
 
@@ -95,17 +91,20 @@ public class ArtifactDownloadManagerVnfcTest {
         when(mockBabelClient.postArtifact(any(), any())).thenReturn(createBabelArtifacts());
         when(mockVnfCatalogExtractor.extract(any(), any())).thenReturn(new ArrayList<>());
 
-        List<Artifact> modelArtifacts = new ArrayList<>();
-        List<Artifact> catalogFiles = new ArrayList<>();
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), modelArtifacts, catalogFiles),
-                is(true));
-
-        assertEquals(1, catalogFiles.size(), "There should be a catalog file");
+        List<Artifact> artifacts = downloadManager.downloadArtifacts(data, data.getServiceArtifacts());
+        List<Artifact> catalogArtifacts = artifacts.stream()
+                .filter(VnfCatalogArtifact.class::isInstance)
+                .collect(Collectors.toList());
+        List<Artifact> modelArtifacts = artifacts.stream()
+                .filter(ModelArtifact.class::isInstance)
+                .collect(Collectors.toList());
+        assertEquals(1, catalogArtifacts.size(), "There should be a catalog artifact");
+        assertEquals(1, modelArtifacts.size(), "There should be a model artifact");
     }
 
     @Test
     public void downloadArtifacts_validXmlVnfcCsarFile()
-            throws IOException, BabelServiceClientException, BabelArtifactParsingException, InvalidArchiveException {
+            throws Exception {
         INotificationData data = getNotificationDataWithToscaCsarFile();
         IArtifactInfo artifactInfo = data.getServiceArtifacts().get(0);
 
@@ -113,17 +112,21 @@ public class ArtifactDownloadManagerVnfcTest {
         when(mockBabelClient.postArtifact(any(), any())).thenReturn(createBabelArtifactsNoVnfc());
         when(mockVnfCatalogExtractor.extract(any(), any())).thenReturn(createXmlVnfcArtifacts());
 
-        List<Artifact> modelArtifacts = new ArrayList<>();
-        List<Artifact> catalogFiles = new ArrayList<>();
-        assertTrue(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), modelArtifacts, catalogFiles));
+        List<Artifact> artifacts = downloadManager.downloadArtifacts(data, data.getServiceArtifacts());
 
-        assertEquals(3, catalogFiles.size(), "There should be three catalog artifacts");
+        List<Artifact> catalogArtifacts = artifacts.stream()
+                .filter(VnfCatalogArtifact.class::isInstance)
+                .collect(Collectors.toList());
+        List<Artifact> modelArtifacts = artifacts.stream()
+                .filter(ModelArtifact.class::isInstance)
+                .collect(Collectors.toList());
+        assertEquals(3, catalogArtifacts.size(), "There should be three catalog artifacts");
         assertEquals(1, modelArtifacts.size(), "There should be a model artifact");
     }
 
     @Test
     public void downloadArtifacts_validNoVnfcCsarFile()
-            throws IOException, BabelServiceClientException, BabelArtifactParsingException, InvalidArchiveException {
+            throws Exception {
         INotificationData data = getNotificationDataWithToscaCsarFile();
         IArtifactInfo artifactInfo = data.getServiceArtifacts().get(0);
 
@@ -131,17 +134,17 @@ public class ArtifactDownloadManagerVnfcTest {
         when(mockBabelClient.postArtifact(any(), any())).thenReturn(createBabelArtifactsNoVnfc());
         when(mockVnfCatalogExtractor.extract(any(), any())).thenReturn(new ArrayList<>());
 
-        List<Artifact> modelArtifacts = new ArrayList<>();
-        List<Artifact> catalogFiles = new ArrayList<>();
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), modelArtifacts, catalogFiles),
-                is(true));
+        List<Artifact> artifacts = downloadManager.downloadArtifacts(data, data.getServiceArtifacts());
+        List<Artifact> catalogArtifacts = artifacts.stream()
+                .filter(VnfCatalogArtifact.class::isInstance)
+                .collect(Collectors.toList());
 
-        assertEquals(0, catalogFiles.size(), "There should not have been any catalog files");
+        assertEquals(0, catalogArtifacts.size(), "There should not have been any catalog files");
     }
 
     @Test
     public void downloadArtifacts_invalidXmlAndToscaVnfcCsarFile()
-            throws IOException, BabelServiceClientException, BabelArtifactParsingException, InvalidArchiveException {
+            throws Exception {
         INotificationData data = getNotificationDataWithToscaCsarFile();
         IArtifactInfo artifactInfo = data.getServiceArtifacts().get(0);
 
@@ -150,10 +153,8 @@ public class ArtifactDownloadManagerVnfcTest {
         when(mockVnfCatalogExtractor.extract(any(), any())).thenReturn(createXmlVnfcArtifacts());
         doNothing().when(mockNotificationPublisher).publishDeployFailure(mockDistributionClient, data, artifactInfo);
 
-        List<Artifact> modelArtifacts = new ArrayList<>();
-        List<Artifact> catalogFiles = new ArrayList<>();
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), modelArtifacts, catalogFiles),
-                is(false));
+        InvalidArchiveException invalidArchiveException = assertThrows(InvalidArchiveException.class,
+                () -> downloadManager.downloadArtifacts(data, data.getServiceArtifacts()));
 
         Mockito.verify(mockNotificationPublisher).publishDeployFailure(mockDistributionClient, data, artifactInfo);
     }
diff --git a/src/test/java/org/onap/aai/modelloader/notification/TestArtifactDownloadManager.java b/src/test/java/org/onap/aai/modelloader/notification/TestArtifactDownloadManager.java
index 0661583..79eb94d 100644
--- a/src/test/java/org/onap/aai/modelloader/notification/TestArtifactDownloadManager.java
+++ b/src/test/java/org/onap/aai/modelloader/notification/TestArtifactDownloadManager.java
@@ -21,10 +21,9 @@
 package org.onap.aai.modelloader.notification;
 
 import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.collection.IsEmptyCollection.empty;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.when;
@@ -38,8 +37,8 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Properties;
+import java.util.stream.Collectors;
 
-import org.hamcrest.collection.IsEmptyCollection;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -53,10 +52,11 @@ import org.onap.aai.modelloader.entity.Artifact;
 import org.onap.aai.modelloader.entity.catalog.VnfCatalogArtifact;
 import org.onap.aai.modelloader.entity.model.BabelArtifactParsingException;
 import org.onap.aai.modelloader.entity.model.ModelArtifact;
+import org.onap.aai.modelloader.entity.model.NamedQueryArtifact;
+import org.onap.aai.modelloader.extraction.InvalidArchiveException;
 import org.onap.aai.modelloader.extraction.VnfCatalogExtractor;
 import org.onap.aai.modelloader.restclient.BabelServiceClient;
 import org.onap.aai.modelloader.restclient.BabelServiceClientException;
-import org.onap.aai.modelloader.service.BabelServiceClientFactory;
 import org.onap.aai.modelloader.util.ArtifactTestUtils;
 import org.onap.sdc.api.IDistributionClient;
 import org.onap.sdc.api.notification.IArtifactInfo;
@@ -75,7 +75,6 @@ public class TestArtifactDownloadManager {
     @Mock private IDistributionClient mockDistributionClient;
     @Mock private NotificationPublisher mockNotificationPublisher;
     @Mock private BabelArtifactConverter mockBabelArtifactConverter;
-    @Mock private BabelServiceClientFactory mockClientFactory;
     @InjectMocks BabelArtifactService babelArtifactService;
     private VnfCatalogExtractor vnfCatalogExtractor;
 
@@ -83,7 +82,6 @@ public class TestArtifactDownloadManager {
     public void setup() throws Exception {
         MockitoAnnotations.openMocks(this);
         vnfCatalogExtractor = new VnfCatalogExtractor();
-        when(mockClientFactory.create(any())).thenReturn(mockBabelClient);
 
         Properties configProperties = new Properties();
         configProperties.load(this.getClass().getClassLoader().getResourceAsStream("model-loader.properties"));
@@ -100,15 +98,12 @@ public class TestArtifactDownloadManager {
 
     /**
      * Test downloading zero artifacts from SDC.
+ * @throws Exception 
      */
     @Test
-    public void testDownloadWithZeroArtifacts() {
-        List<Artifact> modelFiles = new ArrayList<>();
-        List<Artifact> catalogFiles = new ArrayList<>();
-        assertThat(downloadManager.downloadArtifacts(getNotificationDataWithOneService(), new ArrayList<>(), modelFiles,
-                catalogFiles), is(true));
-        assertThat(modelFiles, is(empty()));
-        assertThat(catalogFiles, is(empty()));
+    public void testDownloadWithZeroArtifacts() throws Exception {
+        List<Artifact> artifacts = downloadManager.downloadArtifacts(getNotificationDataWithOneService(), new ArrayList<>());
+        assertThat(artifacts, is(empty()));
         Mockito.verifyNoInteractions(mockBabelClient, mockDistributionClient, mockNotificationPublisher,
                 mockBabelArtifactConverter);
     }
@@ -123,7 +118,8 @@ public class TestArtifactDownloadManager {
         doNothing().when(mockNotificationPublisher).publishDownloadFailure(mockDistributionClient, data, artifact,
                 errorMessage);
 
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), null, null), is(false));
+        assertThrows(DownloadFailureException.class,
+                () -> downloadManager.downloadArtifacts(data, data.getServiceArtifacts()));
 
         Mockito.verify(mockDistributionClient).download(artifact);
         Mockito.verify(mockNotificationPublisher).publishDownloadFailure(mockDistributionClient, data, artifact,
@@ -133,22 +129,6 @@ public class TestArtifactDownloadManager {
     }
 
     @Test
-    public void testErrorCreatingBabelClient() throws Exception {
-        when(mockClientFactory.create(any())).thenThrow(new BabelServiceClientException(new Exception()));
-
-        INotificationData data = getNotificationDataWithToscaCsarFile();
-        IArtifactInfo artifactInfo = data.getServiceArtifacts().get(0);
-        setupValidDownloadCsarMocks(data, artifactInfo, new ArtifactTestUtils());
-        doNothing().when(mockNotificationPublisher).publishDeployFailure(mockDistributionClient, data, artifactInfo);
-
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), null, null), is(false));
-
-        Mockito.verify(mockDistributionClient).download(artifactInfo);
-        Mockito.verify(mockNotificationPublisher).publishDownloadSuccess(mockDistributionClient, data, artifactInfo);
-        Mockito.verify(mockNotificationPublisher).publishDeployFailure(mockDistributionClient, data, artifactInfo);
-    }
-
-    @Test
     public void downloadArtifacts_invalidToscaCsarFile() throws IOException, BabelServiceClientException {
         INotificationData data = getNotificationDataWithToscaCsarFile();
         IArtifactInfo artifact = data.getServiceArtifacts().get(0);
@@ -158,7 +138,8 @@ public class TestArtifactDownloadManager {
         when(mockBabelClient.postArtifact(any(), any())).thenThrow(new BabelServiceClientException(""));
         doNothing().when(mockNotificationPublisher).publishDeployFailure(mockDistributionClient, data, artifact);
 
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), null, null), is(false));
+        assertThrows(ProcessToscaArtifactsException.class,
+                () -> downloadManager.downloadArtifacts(data, data.getServiceArtifacts()));
 
         Mockito.verify(mockDistributionClient).download(artifact);
         Mockito.verify(mockNotificationPublisher).publishDownloadSuccess(mockDistributionClient, data, artifact);
@@ -180,8 +161,8 @@ public class TestArtifactDownloadManager {
                 DistributionActionResultEnum.SUCCESS, null, "This is not a valid Model Query Spec".getBytes()));
         doNothing().when(mockNotificationPublisher).publishDownloadSuccess(mockDistributionClient, data, artifact);
 
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), modelArtifacts, null),
-                is(false));
+        assertThrows(BabelArtifactParsingException.class,
+                () -> downloadManager.downloadArtifacts(data, data.getServiceArtifacts()));
 
         Mockito.verify(mockDistributionClient).download(artifact);
         Mockito.verify(mockNotificationPublisher).publishDownloadSuccess(mockDistributionClient, data, artifact);
@@ -192,18 +173,17 @@ public class TestArtifactDownloadManager {
 
     @Test
     public void downloadArtifacts_validToscaCsarFile()
-            throws IOException, BabelServiceClientException, BabelArtifactParsingException {
+            throws Exception {
         INotificationData data = getNotificationDataWithToscaCsarFile();
         IArtifactInfo artifactInfo = data.getServiceArtifacts().get(0);
 
         setupValidDownloadCsarMocks(data, artifactInfo, new ArtifactTestUtils());
 
-        when(mockBabelArtifactConverter.convertToModel(any(BabelArtifact.class))).thenReturn(List.of(new ModelArtifact()));
-        when(mockBabelArtifactConverter.convertToCatalog(any(BabelArtifact.class))).thenReturn(new VnfCatalogArtifact(""));
-
-        List<org.onap.aai.modelloader.entity.Artifact> modelArtifacts = new ArrayList<>();
-        List<org.onap.aai.modelloader.entity.Artifact> catalogArtifacts = new ArrayList<>();
-        assertTrue(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), modelArtifacts, catalogArtifacts));
+        List<Artifact> artifacts = downloadManager.downloadArtifacts(data, data.getServiceArtifacts());
+        List<Artifact> modelArtifacts = artifacts.stream().filter(ModelArtifact.class::isInstance)
+                .collect(Collectors.toList());
+        List<Artifact> catalogArtifacts = artifacts.stream().filter(VnfCatalogArtifact.class::isInstance)
+                .collect(Collectors.toList());
         assertThat(modelArtifacts.size(), is(1));
         assertThat(catalogArtifacts.size(), is(1));
 
@@ -215,11 +195,14 @@ public class TestArtifactDownloadManager {
     }
 
     private void setupValidDownloadCsarMocks(INotificationData data, IArtifactInfo artifactInfo,
-            ArtifactTestUtils artifactTestUtils) throws IOException, BabelServiceClientException {
+            ArtifactTestUtils artifactTestUtils) throws IOException, BabelServiceClientException, BabelArtifactParsingException {
         when(mockDistributionClient.download(artifactInfo))
                 .thenReturn(createDistributionClientDownloadResult(DistributionActionResultEnum.SUCCESS, null,
                         artifactTestUtils.loadResource("compressedArtifacts/service-VscpaasTest-csar.csar")));
         when(mockBabelClient.postArtifact(any(), any())).thenReturn(createBabelArtifacts());
+
+        when(mockBabelArtifactConverter.convertToModel(any(BabelArtifact.class))).thenReturn(List.of(new ModelArtifact()));
+        when(mockBabelArtifactConverter.convertToCatalog(any(BabelArtifact.class))).thenReturn(new VnfCatalogArtifact(""));
     }
 
     private List<BabelArtifact> createBabelArtifacts() {
@@ -231,17 +214,19 @@ public class TestArtifactDownloadManager {
 
     @Test
     public void downloadArtifactsWithValidModelQuerySpec()
-            throws IOException, BabelServiceClientException, BabelArtifactParsingException {
+            throws Exception {
         INotificationData data = getNotificationDataWithModelQuerySpec();
         IArtifactInfo artifact = data.getServiceArtifacts().get(0);
         setupValidModelQuerySpecMocks(new ArtifactTestUtils(), data, artifact);
 
-        List<org.onap.aai.modelloader.entity.Artifact> modelFiles = new ArrayList<>();
-        List<org.onap.aai.modelloader.entity.Artifact> catalogFiles = new ArrayList<>();
-        assertTrue(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), modelFiles, catalogFiles));
+        List<Artifact> artifacts = downloadManager.downloadArtifacts(data, data.getServiceArtifacts());
+        List<Artifact> namedQueryArtifacts = artifacts.stream().filter(NamedQueryArtifact.class::isInstance)
+                .collect(Collectors.toList());
+        List<Artifact> catalogArtifacts = artifacts.stream().filter(VnfCatalogArtifact.class::isInstance)
+                .collect(Collectors.toList());
 
-        assertThat(modelFiles, is(not(IsEmptyCollection.empty())));
-        assertThat(catalogFiles, is(empty()));
+        assertThat(namedQueryArtifacts.size(), is(1));
+        assertThat(catalogArtifacts, is(empty()));
 
         Mockito.verify(mockDistributionClient).download(artifact);
         Mockito.verify(mockNotificationPublisher).publishDownloadSuccess(mockDistributionClient, data, artifact);
@@ -259,26 +244,25 @@ public class TestArtifactDownloadManager {
 
     @Test
     public void downloadArtifacts_validCsarAndModelFiles()
-            throws IOException, BabelServiceClientException, BabelArtifactParsingException {
+            throws Exception {
         ArtifactTestUtils artifactTestUtils = new ArtifactTestUtils();
         INotificationData data = getNotificationDataWithOneOfEach();
-        List<IArtifactInfo> artifacts = new ArrayList<>();
+        List<IArtifactInfo> artifactInfos = new ArrayList<>();
 
         IArtifactInfo serviceArtifact = data.getServiceArtifacts().get(0);
         IArtifactInfo modelSpecArtifact = data.getResources().get(1).getArtifacts().get(0);
 
-        artifacts.add(serviceArtifact);
-        artifacts.add(modelSpecArtifact);
+        artifactInfos.add(serviceArtifact);
+        artifactInfos.add(modelSpecArtifact);
 
         setupValidDownloadCsarMocks(data, serviceArtifact, artifactTestUtils);
         setupValidModelQuerySpecMocks(artifactTestUtils, data, modelSpecArtifact);
-        when(mockBabelArtifactConverter.convertToModel(any(BabelArtifact.class))).thenReturn(List.of(new ModelArtifact()));
-        when(mockBabelArtifactConverter.convertToCatalog(any(BabelArtifact.class))).thenReturn(new VnfCatalogArtifact(""));
 
-
-        List<org.onap.aai.modelloader.entity.Artifact> modelArtifacts = new ArrayList<>();
-        List<org.onap.aai.modelloader.entity.Artifact> catalogArtifacts = new ArrayList<>();
-        assertTrue(downloadManager.downloadArtifacts(data, artifacts, modelArtifacts, catalogArtifacts));
+        List<Artifact> artifacts = downloadManager.downloadArtifacts(data, artifactInfos);
+        List<Artifact> modelArtifacts = artifacts.stream().filter(artifact -> !(artifact instanceof VnfCatalogArtifact))
+                .collect(Collectors.toList());
+        List<Artifact> catalogArtifacts = artifacts.stream().filter(VnfCatalogArtifact.class::isInstance)
+                .collect(Collectors.toList());
         assertThat(modelArtifacts.size(), is(2));
         assertThat(catalogArtifacts.size(), is(1));
 
@@ -311,8 +295,9 @@ public class TestArtifactDownloadManager {
 
         List<Artifact> modelArtifacts = new ArrayList<>();
         List<Artifact> catalogFiles = new ArrayList<>();
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), modelArtifacts, catalogFiles),
-                is(false));
+        assertThrows(ProcessToscaArtifactsException.class,
+                () -> downloadManager.downloadArtifacts(data, data.getServiceArtifacts()));
+
         assertThat(modelArtifacts, is(empty()));
         assertThat(catalogFiles, is(empty()));
 
@@ -332,8 +317,9 @@ public class TestArtifactDownloadManager {
                 DistributionActionResultEnum.SUCCESS, null, "This content does not matter.".getBytes()));
         doNothing().when(mockNotificationPublisher).publishDownloadSuccess(mockDistributionClient, data, artifact);
 
-        assertThat(downloadManager.downloadArtifacts(data, data.getServiceArtifacts(), null, new ArrayList<>()),
-                is(false));
+        assertThrows(InvalidArchiveException.class,
+                () -> downloadManager.downloadArtifacts(data, data.getServiceArtifacts()));
+
         Mockito.verify(mockDistributionClient).download(artifact);
         Mockito.verify(mockNotificationPublisher).publishDownloadSuccess(mockDistributionClient, data, artifact);
         Mockito.verify(mockNotificationPublisher).publishDeployFailure(mockDistributionClient, data, artifact);
diff --git a/src/test/java/org/onap/aai/modelloader/notification/TestEventCallback.java b/src/test/java/org/onap/aai/modelloader/notification/TestEventCallback.java
index b5d7fd3..ef346db 100644
--- a/src/test/java/org/onap/aai/modelloader/notification/TestEventCallback.java
+++ b/src/test/java/org/onap/aai/modelloader/notification/TestEventCallback.java
@@ -25,6 +25,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.List;
 import java.util.Properties;
 
@@ -34,7 +35,6 @@ import org.junit.jupiter.api.Test;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
-import org.onap.aai.modelloader.entity.model.BabelArtifactParsingException;
 import org.onap.aai.modelloader.fixture.NotificationDataFixtureBuilder;
 import org.onap.aai.modelloader.service.ArtifactDeploymentManager;
 import org.onap.sdc.api.IDistributionClient;
@@ -75,34 +75,32 @@ public class TestEventCallback {
 
     @Test
     @SuppressWarnings("unchecked")
-    public void activateCallback_downloadFails() {
+    public void activateCallback_downloadFails() throws Exception {
         INotificationData data = NotificationDataFixtureBuilder.getNotificationDataWithToscaCsarFile();
 
-        when(mockArtifactDownloadManager.downloadArtifacts(any(INotificationData.class), any(List.class),
-                any(List.class), any(List.class))).thenReturn(false);
+        when(mockArtifactDownloadManager.downloadArtifacts(any(INotificationData.class), any(List.class)
+                )).thenThrow(DownloadFailureException.class);
 
         eventCallback.activateCallback(data);
 
-        verify(mockArtifactDownloadManager).downloadArtifacts(any(INotificationData.class), any(List.class),
-                any(List.class), any(List.class));
+        verify(mockArtifactDownloadManager).downloadArtifacts(any(INotificationData.class), any(List.class));
         Mockito.verifyNoInteractions(mockArtifactDeploymentManager);
     }
 
     @SuppressWarnings("unchecked")
     @Test
-    public void activateCallback() throws BabelArtifactParsingException {
+    public void activateCallback() throws Exception {
         INotificationData data = NotificationDataFixtureBuilder.getNotificationDataWithToscaCsarFile();
 
-        when(mockArtifactDownloadManager.downloadArtifacts(any(INotificationData.class), any(List.class),
-                any(List.class), any(List.class))).thenReturn(true);
+        when(mockArtifactDownloadManager.downloadArtifacts(any(INotificationData.class), any(List.class)))
+            .thenReturn(Collections.emptyList());
 
         when(mockArtifactDeploymentManager.deploy(any(String.class), any(List.class), any(List.class)))
                 .thenReturn(true);
 
         eventCallback.activateCallback(data);
 
-        verify(mockArtifactDownloadManager).downloadArtifacts(any(INotificationData.class), any(List.class),
-                any(List.class), any(List.class));
+        verify(mockArtifactDownloadManager).downloadArtifacts(any(INotificationData.class), any(List.class));
         verify(mockArtifactDeploymentManager).deploy(any(String.class), any(List.class), any(List.class));
     }
 }
diff --git a/src/test/java/org/onap/aai/modelloader/restclient/MockBabelServiceClient.java b/src/test/java/org/onap/aai/modelloader/restclient/MockBabelServiceClient.java
deleted file mode 100644
index 783ad2e..0000000
--- a/src/test/java/org/onap/aai/modelloader/restclient/MockBabelServiceClient.java
+++ /dev/null
@@ -1,42 +0,0 @@
-/**
- * ============LICENSE_START=======================================================
- * org.onap.aai
- * ================================================================================
- * Copyright © 2017-2018 AT&T Intellectual Property. All rights reserved.
- * Copyright © 2017-2018 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.
- * You may obtain a copy of the License at
- *
- *       http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- * ============LICENSE_END=========================================================
- */
-package org.onap.aai.modelloader.restclient;
-
-import java.util.Collections;
-import java.util.List;
-import org.onap.aai.babel.service.data.BabelArtifact;
-import org.onap.aai.babel.service.data.BabelRequest;
-import org.onap.aai.modelloader.config.ModelLoaderConfig;
-
-/**
- * Mocked Client for interfacing with Babel.
- *
- */
-public class MockBabelServiceClient implements BabelServiceClient {
-
-    public MockBabelServiceClient(ModelLoaderConfig config) throws BabelServiceClientException {}
-
-    @Override
-    public List<BabelArtifact> postArtifact(BabelRequest babelRequest, String transactionId)
-            throws BabelServiceClientException {
-        return Collections.emptyList();
-    }
-}
diff --git a/src/test/java/org/onap/aai/modelloader/service/MockBabelServiceClientFactory.java b/src/test/java/org/onap/aai/modelloader/service/MockBabelServiceClientFactory.java
deleted file mode 100644
index fa369ce..0000000
--- a/src/test/java/org/onap/aai/modelloader/service/MockBabelServiceClientFactory.java
+++ /dev/null
@@ -1,37 +0,0 @@
-/**
- * ============LICENSE_START=======================================================
- * org.onap.aai
- * ================================================================================
- * Copyright © 2017-2018 AT&T Intellectual Property. All rights reserved.
- * Copyright © 2017-2018 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.
- * You may obtain a copy of the License at
- *
- *       http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- * ============LICENSE_END=========================================================
- */
-package org.onap.aai.modelloader.service;
-
-import org.onap.aai.modelloader.config.ModelLoaderConfig;
-import org.onap.aai.modelloader.restclient.BabelServiceClient;
-import org.onap.aai.modelloader.restclient.BabelServiceClientException;
-import org.onap.aai.modelloader.restclient.MockBabelServiceClient;
-import org.springframework.stereotype.Service;
-
-@Service
-public class MockBabelServiceClientFactory implements BabelServiceClientFactory {
-
-    @Override
-    public BabelServiceClient create(ModelLoaderConfig config) throws BabelServiceClientException {
-        return new MockBabelServiceClient(config);
-    }
-
-}
diff --git a/src/test/java/org/onap/aai/modelloader/service/TestModelController.java b/src/test/java/org/onap/aai/modelloader/service/TestModelController.java
index 6cfeb65..3387b32 100644
--- a/src/test/java/org/onap/aai/modelloader/service/TestModelController.java
+++ b/src/test/java/org/onap/aai/modelloader/service/TestModelController.java
@@ -64,7 +64,6 @@ public class TestModelController {
     @Autowired NotificationPublisher notificationPublisher;
     @Autowired VnfCatalogExtractor vnfCatalogExtractor;
 
-    @Mock BabelServiceClientFactory clientFactory;
     @Mock BabelServiceClient babelServiceClient;
     @InjectMocks BabelArtifactService babelArtifactService;
     
@@ -72,7 +71,6 @@ public class TestModelController {
 
     @BeforeEach
     public void init() throws BabelServiceClientException {
-        when(clientFactory.create(any())).thenReturn(babelServiceClient);
         when(babelServiceClient.postArtifact(any(), any())).thenReturn(Collections.emptyList());
         ArtifactDownloadManager artifactDownloadManager = new ArtifactDownloadManager(iDistributionClient, notificationPublisher, vnfCatalogExtractor, babelArtifactService);
         this.modelController = new ModelController(iDistributionClient, modelLoaderConfig, artifactDeploymentManager, artifactDownloadManager);