diff options
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; |