handle non OK response from SDC while getting model

Issue-ID: VID-378
Signed-off-by: Eylon Malin <eylon.malin@intl.att.com>
Change-Id: Idc6e587abb24fbec65ed159db7008e50abee2581

8 files changed, 175 insertions, 44 deletions

diff --git a/vid-app-common/src/main/java/org/onap/vid/asdc/rest/SdcRestClient.java b/vid-app-common/src/main/java/org/onap/vid/asdc/rest/SdcRestClient.java
index decf446d6..1de9715ee 100644
--- a/vid-app-common/src/main/java/org/onap/vid/asdc/rest/SdcRestClient.java
+++ b/vid-app-common/src/main/java/org/onap/vid/asdc/rest/SdcRestClient.java
@@ -28,6 +28,7 @@ import static org.onap.vid.asdc.AsdcClient.URIS.TOSCA_MODEL_URL_TEMPLATE;
 import static org.onap.vid.client.SyncRestClientInterface.HEADERS.AUTHORIZATION;
 import static org.onap.vid.client.SyncRestClientInterface.HEADERS.CONTENT_TYPE;
 import static org.onap.vid.client.SyncRestClientInterface.HEADERS.X_ECOMP_INSTANCE_ID;
+import static org.onap.vid.client.UnirestPatchKt.extractRawAsString;
 import static org.onap.vid.utils.Logging.REQUEST_ID_HEADER_KEY;
 import static org.onap.vid.utils.Logging.logRequest;
 
@@ -43,6 +44,8 @@ import java.nio.file.StandardCopyOption;
 import java.util.Collections;
 import java.util.Map;
 import java.util.UUID;
+import javax.ws.rs.ProcessingException;
+import javax.ws.rs.client.ResponseProcessingException;
 import org.onap.portalsdk.core.util.SystemProperties;
 import org.onap.vid.aai.ExceptionWithRequestInfo;
 import org.onap.vid.aai.HttpResponseWithRequestInfo;
@@ -87,13 +90,35 @@ public class SdcRestClient implements AsdcClient {
 
     @Override
     public Path getServiceToscaModel(UUID uuid) throws AsdcCatalogException {
-        InputStream inputStream = Try
-                .of(() -> getServiceInputStream(uuid, false))
-                .getOrElseThrow(AsdcCatalogException::new)
-                .getResponse()
-                .getBody();
-
-        return createTmpFile(inputStream);
+        try {
+            HttpResponseWithRequestInfo<InputStream> responseWithRequestInfo = getServiceInputStream(uuid, false);
+
+            if (responseWithRequestInfo.getResponse().getStatus()>399) {
+                Logging.logResponse(LOGGER, HttpMethod.GET,
+                    responseWithRequestInfo.getRequestUrl(), responseWithRequestInfo.getResponse());
+
+                String body = extractRawAsString(responseWithRequestInfo.getResponse());
+                throw new AsdcCatalogException(String.format("Http bad status code: %s, body: %s",
+                    responseWithRequestInfo.getResponse().getStatus(),
+                    body));
+            }
+
+            final InputStream csarInputStream = responseWithRequestInfo.getResponse().getBody();
+            Path toscaFilePath = createTmpFile(csarInputStream);
+            LOGGER.debug("Received {} {} . Tosca file was saved at: {}",
+                responseWithRequestInfo.getRequestHttpMethod().name(),
+                responseWithRequestInfo.getRequestUrl(),
+                toscaFilePath.toAbsolutePath());
+            return toscaFilePath;
+        } catch (ResponseProcessingException e) {
+            //Couldn't convert response to Java type
+            throw new AsdcCatalogException("ASDC response could not be processed", e);
+        } catch (ProcessingException e) {
+            //IO problems during request
+            throw new AsdcCatalogException("Failed to get a response from ASDC service. Cause: " + e.getMessage(), e);
+        } catch (RuntimeException e) {
+            throw new AsdcCatalogException(e);
+        }
     }
 
     @Override
diff --git a/vid-app-common/src/main/java/org/onap/vid/client/UnirestPatch.kt b/vid-app-common/src/main/java/org/onap/vid/client/UnirestPatch.kt
index 750646621..5730c11f1 100644
--- a/vid-app-common/src/main/java/org/onap/vid/client/UnirestPatch.kt
+++ b/vid-app-common/src/main/java/org/onap/vid/client/UnirestPatch.kt
@@ -22,9 +22,11 @@ package org.onap.vid.client
 
 import io.joshworks.restclient.http.Headers
 import io.joshworks.restclient.http.HttpResponse
+import org.apache.commons.io.IOUtils
 import org.apache.http.HttpVersion
 import org.apache.http.message.BasicHttpResponse
 import java.io.InputStream
+import java.nio.charset.StandardCharsets
 
 /// Patch NPE in joshworks's Unirest HttpResponse::getBody when getRawBody is null
 fun <T> patched(httpResponse: HttpResponse<T>) =
@@ -35,6 +37,17 @@ private fun <T> willGetBodyTriggerNPE(httpResponse: HttpResponse<T>) =
 
 private val dummyHttpResponse = BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "ok")
 
+fun extractRawAsString(response: HttpResponse<*>?): String {
+    try {
+        if (response == null || response.rawBody==null) return ""
+        response.rawBody.reset()
+        return IOUtils.toString(response.rawBody, StandardCharsets.UTF_8.name())
+    } catch (e: Exception) {
+        //Nothing to do here
+    }
+
+    return ""
+}
 /**
  * This class inherits HttpResponse to have compatible interface,
  * but implementation is done through delegation to another
diff --git a/vid-app-common/src/main/java/org/onap/vid/model/probes/HttpRequestMetadata.java b/vid-app-common/src/main/java/org/onap/vid/model/probes/HttpRequestMetadata.java
index 984c0d766..0e7e914f1 100644
--- a/vid-app-common/src/main/java/org/onap/vid/model/probes/HttpRequestMetadata.java
+++ b/vid-app-common/src/main/java/org/onap/vid/model/probes/HttpRequestMetadata.java
@@ -22,10 +22,9 @@
 package org.onap.vid.model.probes;
 
 import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
+import static org.onap.vid.client.UnirestPatchKt.extractRawAsString;
 
 import com.google.common.base.MoreObjects;
-import java.nio.charset.StandardCharsets;
-import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.onap.vid.aai.ExceptionWithRequestInfo;
 import org.onap.vid.aai.HttpResponseWithRequestInfo;
@@ -89,16 +88,10 @@ public class HttpRequestMetadata extends StatusMetadata {
         this.url = response.getRequestUrl();
         this.httpCode = response.getResponse().getStatus();
         if (readRawData) {
-            try {
-                response.getResponse().getRawBody().reset();
-                this.rawData = IOUtils.toString(response.getResponse().getRawBody(), StandardCharsets.UTF_8.name());
-            } catch (Exception e) {
-                //Nothing to do here
-            }
+            this.rawData = extractRawAsString(response.getResponse());
         }
     }
 
-
     public HttpMethod getHttpMethod() {
         return httpMethod;
     }
diff --git a/vid-app-common/src/test/java/org/onap/vid/asdc/rest/SdcRestClientTest.java b/vid-app-common/src/test/java/org/onap/vid/asdc/rest/SdcRestClientTest.java
index 5eaaf9335..13fe761c6 100644
--- a/vid-app-common/src/test/java/org/onap/vid/asdc/rest/SdcRestClientTest.java
+++ b/vid-app-common/src/test/java/org/onap/vid/asdc/rest/SdcRestClientTest.java
@@ -21,6 +21,7 @@
 
 package org.onap.vid.asdc.rest;
 
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -33,6 +34,7 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.matches;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.onap.vid.testUtils.TestUtils.mockGetRawBodyWithStringBody;
 import static org.testng.AssertJUnit.fail;
 
 import io.joshworks.restclient.http.HttpResponse;
@@ -178,10 +180,10 @@ public class SdcRestClientTest {
     public void whenJavaxClientThrowException_then_getServiceToscaModelRethrowException(Class<? extends Throwable> expectedType, Consumer<SyncRestClient> setupMocks) throws Exception {
         /*
         Call chain is like:
-            this test -> RestfulAsdcClient ->  javax's Client
+            this test -> SdcRestClient ->  SdcRestClient -> joshworks client which return  joshworks HttpResponse
 
-        In this test, *RestfulAsdcClient* is under test (actual implementation is used), while javax's Client is
-        mocked to return pseudo-responses or - better - throw exceptions.
+        In this test, *SdcRestClient* is under test (actual implementation is used), while SdcRestClient is
+        mocked to return pseudo joshworks HttpResponse or - better - throw exceptions.
          */
 
         /// TEST:
@@ -198,4 +200,55 @@ public class SdcRestClientTest {
         fail("exception shall rethrown by getServiceToscaModel once javax client throw exception ");
     }
 
+    @DataProvider
+    public static Object[][] badResponses() {
+        return new Object[][] {
+            {(Consumer<HttpResponse>) response -> {
+                when(response.getStatus()).thenReturn(404);
+                mockGetRawBodyWithStringBody(response,"");},
+                ""
+            },
+            {(Consumer<HttpResponse>) response -> {
+                when(response.getStatus()).thenReturn(405);
+                when(response.getRawBody()).thenThrow(ClassCastException.class);},
+                ""
+            },
+            {(Consumer<HttpResponse>) response -> {
+                when(response.getStatus()).thenReturn(500);
+                mockGetRawBodyWithStringBody(response,"some message");},
+                "some message"
+            },
+        };
+    }
+
+    @Test(dataProvider = "badResponses")
+    public void whenJavaxClientReturnBadCode_then_getServiceToscaModelThrowException(Consumer<HttpResponse> setupMocks, String exceptedBody) throws Exception {
+        /*
+        Call chain is like:
+            this test -> SdcRestClient ->  SdcRestClient -> joshworks client which return  joshworks HttpResponse
+
+        In this test, *SdcRestClient* is under test (actual implementation is used), while SdcRestClient is
+        mocked to return pseudo joshworks HttpResponse
+         */
+
+        HttpResponse<InputStream> mockResponse = mock(HttpResponse.class);
+        SyncRestClient syncRestClient = mock(SyncRestClient.class);
+        when(syncRestClient.getStream(anyString(), anyMap(), anyMap())).thenReturn(mockResponse);
+
+        // prepare real RestfulAsdcClient (Under test)
+
+        setupMocks.accept(mockResponse);
+
+        try {
+            new SdcRestClient(SAMPLE_BASE_URL, SAMPLE_AUTH, syncRestClient).getServiceToscaModel(UUID.randomUUID());
+        } catch (AsdcCatalogException e) {
+            assertThat(e.getMessage(), containsString(String.valueOf(mockResponse.getStatus())));
+            assertThat(e.getMessage(), containsString(exceptedBody));
+            return; //OK
+        }
+
+        fail("exception shall be thrown by getServiceToscaModel once response contains error code ");
+    }
+
+
 }
diff --git a/vid-app-common/src/test/java/org/onap/vid/client/UnirestPatchKtTest.kt b/vid-app-common/src/test/java/org/onap/vid/client/UnirestPatchKtTest.kt
new file mode 100644
index 000000000..8948ee22c
--- /dev/null
+++ b/vid-app-common/src/test/java/org/onap/vid/client/UnirestPatchKtTest.kt
@@ -0,0 +1,36 @@
+package org.onap.vid.client
+
+import org.onap.vid.testUtils.TestUtils
+import org.testng.Assert.assertEquals
+import org.testng.annotations.Test
+
+class UnirestPatchKtTest {
+
+    @Test
+    fun givenHttpResponseIsNull_whenExtractRawAsString_emptyStringIsReturn() {
+        val result = extractRawAsString(null)
+        assertEquals(result, "")
+    }
+
+    @Test
+    fun givenHttpResponseBodyIsNull_whenExtractRawAsString_emptyStringIsReturn() {
+        val result = extractRawAsString(TestUtils.createTestHttpResponse(200, null))
+        assertEquals(result, "")
+    }
+
+    @Test
+    fun givenHttpResponseBodyIsString_whenExtractRawAsString_sameStringIsReturn() {
+        val body = "someBody"
+        val result = extractRawAsString(TestUtils.createTestHttpResponse(200, body))
+        assertEquals(result, body)
+    }
+
+    @Test
+    fun givenHttpResponseBodyIsString_whenReadBodyAndExtractRawAsString_sameStringIsReturn() {
+        val body = "someBody"
+        val httpResponse = TestUtils.createTestHttpResponse(200, body);
+        assertEquals(httpResponse.body, body) //read body once
+        val result = extractRawAsString(httpResponse)
+        assertEquals(result, body)
+    }
+}
diff --git a/vid-app-common/src/test/java/org/onap/vid/mso/MsoBusinessLogicImplTest.java b/vid-app-common/src/test/java/org/onap/vid/mso/MsoBusinessLogicImplTest.java
index ffabc18a2..c0683425b 100644
--- a/vid-app-common/src/test/java/org/onap/vid/mso/MsoBusinessLogicImplTest.java
+++ b/vid-app-common/src/test/java/org/onap/vid/mso/MsoBusinessLogicImplTest.java
@@ -55,7 +55,6 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import io.joshworks.restclient.http.HttpResponse;
 import java.io.IOException;
 import java.net.URL;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
@@ -97,6 +96,7 @@ import org.onap.vid.mso.rest.Request;
 import org.onap.vid.mso.rest.RequestDetails;
 import org.onap.vid.mso.rest.RequestDetailsWrapper;
 import org.onap.vid.mso.rest.Task;
+import org.onap.vid.testUtils.TestUtils;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.test.context.ContextConfiguration;
@@ -1442,11 +1442,7 @@ public class MsoBusinessLogicImplTest extends AbstractTestNGSpringContextTests {
         HttpResponse<String> httpResponse = mockForGetOrchestrationRequest();
         when(httpResponse.getStatus()).thenReturn(statusCode);
         when(httpResponse.getBody()).thenReturn(body);
-        try {
-            when(httpResponse.getRawBody()).thenReturn(IOUtils.toInputStream(body, StandardCharsets.UTF_8.name()));
-        } catch (IOException e) {
-            throw new RuntimeException(e);
-        }
+        TestUtils.mockGetRawBodyWithStringBody(httpResponse, body);
         return httpResponse;
     }
 
diff --git a/vid-app-common/src/test/java/org/onap/vid/mso/MsoUtilTest.java b/vid-app-common/src/test/java/org/onap/vid/mso/MsoUtilTest.java
index 650bda13b..10456bebf 100644
--- a/vid-app-common/src/test/java/org/onap/vid/mso/MsoUtilTest.java
+++ b/vid-app-common/src/test/java/org/onap/vid/mso/MsoUtilTest.java
@@ -21,17 +21,13 @@
 
 package org.onap.vid.mso;
 
-import io.joshworks.restclient.http.HttpResponse;
-import org.apache.http.HttpResponseFactory;
-import org.apache.http.entity.StringEntity;
-import org.apache.http.impl.DefaultHttpResponseFactory;
-import org.apache.http.message.BasicStatusLine;
-import org.testng.annotations.Test;
-
 import static org.apache.http.HttpStatus.SC_OK;
-import static org.apache.http.HttpVersion.HTTP_1_1;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import io.joshworks.restclient.http.HttpResponse;
+import org.onap.vid.testUtils.TestUtils;
+import org.testng.annotations.Test;
+
 public class MsoUtilTest {
 
     @Test
@@ -51,7 +47,7 @@ public class MsoUtilTest {
     @Test
     public void shouldWrapHttpResponse() throws Exception {
         // given
-        HttpResponse<String> httpResponse = createTestHttpResponse(SC_OK, null);
+        HttpResponse<String> httpResponse = TestUtils.createTestHttpResponse(SC_OK, null);
         // when
         MsoResponseWrapper result = MsoUtil.wrapResponse(httpResponse);
         // then
@@ -63,7 +59,7 @@ public class MsoUtilTest {
     public void shouldWrapHttpResponseWithEntity() throws Exception {
         // given
         String entity = "entity";
-        HttpResponse<String> httpResponse = createTestHttpResponse(SC_OK, entity);
+        HttpResponse<String> httpResponse = TestUtils.createTestHttpResponse(SC_OK, entity);
         // when
         MsoResponseWrapper result = MsoUtil.wrapResponse(httpResponse);
         // then
@@ -71,12 +67,4 @@ public class MsoUtilTest {
         assertThat(result.getStatus()).isEqualTo(SC_OK);
     }
 
-    private HttpResponse<String> createTestHttpResponse(int statusCode, String entity) throws Exception {
-        HttpResponseFactory factory = new DefaultHttpResponseFactory();
-        org.apache.http.HttpResponse response = factory.newHttpResponse(new BasicStatusLine(HTTP_1_1, statusCode, null), null);
-        if (entity != null) {
-            response.setEntity(new StringEntity(entity));
-        }
-        return new HttpResponse<>(response, String.class, null);
-    }
 }
diff --git a/vid-app-common/src/test/java/org/onap/vid/testUtils/TestUtils.java b/vid-app-common/src/test/java/org/onap/vid/testUtils/TestUtils.java
index 58ee2d38d..399274d69 100644
--- a/vid-app-common/src/test/java/org/onap/vid/testUtils/TestUtils.java
+++ b/vid-app-common/src/test/java/org/onap/vid/testUtils/TestUtils.java
@@ -28,7 +28,8 @@ import static org.apache.commons.beanutils.PropertyUtils.getPropertyDescriptors;
 import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic;
 import static org.apache.commons.text.CharacterPredicates.DIGITS;
 import static org.apache.commons.text.CharacterPredicates.LETTERS;
-import static org.mockito.Matchers.any;
+import static org.apache.http.HttpVersion.HTTP_1_1;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.RETURNS_DEFAULTS;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -39,12 +40,14 @@ import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.code.beanmatchers.BeanMatchers;
 import com.google.common.collect.ImmutableList;
+import io.joshworks.restclient.http.HttpResponse;
 import java.beans.PropertyDescriptor;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Serializable;
 import java.net.URI;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
@@ -56,8 +59,13 @@ import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import org.apache.commons.beanutils.BeanUtils;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.apache.commons.lang3.reflect.MethodUtils;
 import org.apache.commons.text.RandomStringGenerator;
+import org.apache.http.HttpResponseFactory;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.DefaultHttpResponseFactory;
+import org.apache.http.message.BasicStatusLine;
 import org.apache.log4j.LogManager;
 import org.apache.log4j.Logger;
 import org.json.JSONArray;
@@ -67,6 +75,7 @@ import org.mockito.MockSettings;
 import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
+import org.mockito.stubbing.OngoingStubbing;
 import org.onap.portalsdk.core.util.SystemProperties;
 import org.onap.vid.asdc.AsdcCatalogException;
 import org.onap.vid.asdc.beans.Service;
@@ -193,6 +202,24 @@ public class TestUtils {
             ), CloudConfiguration.class);
     }
 
+    public static OngoingStubbing<InputStream> mockGetRawBodyWithStringBody(HttpResponse<String> httpResponse, String body) {
+        try {
+            return when(httpResponse.getRawBody()).thenReturn(IOUtils.toInputStream(body, StandardCharsets.UTF_8.name()));
+        } catch (IOException e) {
+            ExceptionUtils.rethrow(e);
+        }
+        return null; //never shall get here
+    }
+
+    public static HttpResponse<String> createTestHttpResponse(int statusCode, String entity) throws Exception {
+        HttpResponseFactory factory = new DefaultHttpResponseFactory();
+        org.apache.http.HttpResponse response = factory.newHttpResponse(new BasicStatusLine(HTTP_1_1, statusCode, null), null);
+        if (entity != null) {
+            response.setEntity(new StringEntity(entity));
+        }
+        return new HttpResponse<>(response, String.class, null);
+    }
+
 
     public static class JavaxRsClientMocks {
         private final javax.ws.rs.client.Client fakeClient;