From e9936a7b8855dddb93d9c152822b0482c309e93c Mon Sep 17 00:00:00 2001 From: "m.panagrosso" Date: Tue, 26 Sep 2023 18:18:57 +0200 Subject: [PATCH 1/5] ENG-5201: fix db backup on expired token --- .../aps/system/storage/CdsRemoteCaller.java | 32 +++++++++++++++---- .../aps/system/storage/CdsStorageManager.java | 4 ++- .../system/storage/CdsRemoteCallerTest.java | 18 +++++++---- .../system/storage/CdsStorageManagerTest.java | 21 ++++++++---- 4 files changed, 55 insertions(+), 20 deletions(-) diff --git a/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCaller.java b/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCaller.java index f017474496..467724f739 100644 --- a/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCaller.java +++ b/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCaller.java @@ -16,7 +16,9 @@ import static org.entando.entando.aps.system.services.tenants.ITenantManager.PRIMARY_CODE; import java.io.ByteArrayInputStream; +import java.io.IOException; import java.io.InputStream; +import java.net.SocketTimeoutException; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; @@ -24,6 +26,7 @@ import java.util.Map; import java.util.Optional; import java.util.WeakHashMap; +import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.entando.entando.aps.system.services.tenants.TenantConfig; import org.entando.entando.ent.exception.EntRuntimeException; @@ -45,6 +48,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestTemplate; import org.entando.entando.aps.system.services.storage.CdsActive; @@ -60,7 +64,7 @@ public class CdsRemoteCaller { private final RestTemplate restTemplateWithRedirect; private final CdsConfiguration configuration; - // used weak map to skip excessive growth + // used weak map to skip excessive growthclear private Map tenantsToken = new WeakHashMap<>(); @Autowired @@ -76,8 +80,8 @@ public CdsCreateResponseDto executePostCall(URI url, boolean isProtectedResource, Optional fileInputStream , Optional config, - boolean forceTokenRetrieve) { - + boolean forceTokenRetrieve, + int cdsRetry) { try { logger.debug("Trying to call POST on url:'{}' with isProtectedResource:'{}' forceTokenRetrieve:'{}' isFile:'{}' and is config tenant empty:'{}'", url, @@ -90,6 +94,7 @@ public CdsCreateResponseDto executePostCall(URI url, headers.setContentType(MediaType.MULTIPART_FORM_DATA); MultiValueMap body = fileInputStream + .map(this::cloneInputStream) .map(is -> buildFileBodyRequest(subPath, isProtectedResource, is)) .orElseGet(() -> buildDirectoryBodyRequest(subPath, isProtectedResource)); @@ -98,7 +103,8 @@ public CdsCreateResponseDto executePostCall(URI url, ResponseEntity> fullResponse = restTemplate.exchange(url, HttpMethod.POST, entity, - new ParameterizedTypeReference>(){}); + new ParameterizedTypeReference<>() { + }); List responseList = Optional .ofNullable(fullResponse.getBody()) @@ -115,15 +121,29 @@ public CdsCreateResponseDto executePostCall(URI url, return response; } catch (HttpClientErrorException e) { if (!forceTokenRetrieve && (e.getStatusCode().equals(HttpStatus.UNAUTHORIZED))) { - return this.executePostCall(url, subPath, isProtectedResource, fileInputStream, config, true); + return this.executePostCall(url, subPath, isProtectedResource, fileInputStream, config, true, + cdsRetry + 1); } else { - throw buildExceptionWithMessage("POST", e.getStatusCode() , url.toString()); + throw buildExceptionWithMessage("POST", e.getStatusCode(), url.toString()); } } catch(Exception ex){ + if (ex instanceof ResourceAccessException && ex.getCause() instanceof SocketTimeoutException + && cdsRetry < 2) { + return this.executePostCall(url, subPath, isProtectedResource, fileInputStream, config, true, + cdsRetry + 1); + } throw new EntRuntimeException(String.format(GENERIC_REST_ERROR_MSG, url.toString()), ex); } } + private InputStream cloneInputStream(InputStream is) { + try { + return IOUtils.toBufferedInputStream(is); + } catch (IOException e) { + logger.error("Error in cloning input stream", e); + throw new EntRuntimeException("Error in cloning input stream", e); + } + } private MultiValueMap buildDirectoryBodyRequest(String subPath, boolean isProtectedResource){ MultiValueMap body = new LinkedMultiValueMap<>(); body.add("path", subPath); diff --git a/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsStorageManager.java b/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsStorageManager.java index dd255c3ade..269c129312 100644 --- a/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsStorageManager.java +++ b/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsStorageManager.java @@ -80,12 +80,14 @@ private void create(String subPath, boolean isProtectedResource, Optional Date: Wed, 27 Sep 2023 18:00:50 +0200 Subject: [PATCH 2/5] ENG-5201: unit test (partial) --- .../aps/system/storage/CdsRemoteCaller.java | 1 - .../system/storage/CdsRemoteCallerTest.java | 48 ++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCaller.java b/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCaller.java index 467724f739..973f5a7a3b 100644 --- a/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCaller.java +++ b/cds-plugin/src/main/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCaller.java @@ -140,7 +140,6 @@ private InputStream cloneInputStream(InputStream is) { try { return IOUtils.toBufferedInputStream(is); } catch (IOException e) { - logger.error("Error in cloning input stream", e); throw new EntRuntimeException("Error in cloning input stream", e); } } diff --git a/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java b/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java index b7e999ddfd..8808efdd05 100644 --- a/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java +++ b/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java @@ -17,10 +17,13 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import com.agiletec.aps.util.ApsTenantApplicationUtils; import java.io.ByteArrayInputStream; +import java.io.FileInputStream; +import java.io.IOException; import java.io.InputStream; import java.net.URI; import java.nio.charset.StandardCharsets; @@ -39,6 +42,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.ParameterizedTypeReference; @@ -199,7 +203,6 @@ void shouldManageErrorInExecutePostCall() throws Exception { ex.getMessage()); } - @Test void shouldCreateFileForPrimary() throws Exception { Map configMap = Map.of("cdsPublicUrl","http://my-server/tenant1/cms-resources", @@ -551,4 +554,47 @@ void shouldRetrieveFileAttribute() { eq(new ParameterizedTypeReference>(){})); } + @Test + void shouldThrowExceptionIfAnErrorOccurInCloningFileInputStream() throws Exception { + + Map configMap = Map.of("cdsPublicUrl", "http://my-server/tenant1/cms-resources", + "cdsPrivateUrl", "http://cds-kube-service:8081/", + "cdsPath", "/mytenant/api/v1", + "kcAuthUrl", "http://tenant1.server.com/auth", + "kcRealm", "tenant1", + "kcClientId", "id", + "kcClientSecret", "secret", + "tenantCode", "my-tenant1"); + TenantConfig tc = new TenantConfig(configMap); + ApsTenantApplicationUtils.setTenant("my-tenant"); + + URI url = URI.create("http://cds-kube-service:8081/mytenant/api/v1/upload/"); + + Mockito.when(restTemplate.exchange(anyString(), + eq(HttpMethod.POST), + any(), + eq(new ParameterizedTypeReference>() { + }))).thenReturn( + ResponseEntity.status(HttpStatus.OK).body(Map.of(OAuth2AccessToken.ACCESS_TOKEN, "entando"))); + + try (MockedStatic utilities = Mockito.mockStatic(IOUtils.class)) { + utilities.when(() -> IOUtils.toBufferedInputStream(any())) + .thenThrow(new IOException()); + + Exception ex = assertThrows(EntRuntimeException.class, + () -> cdsRemoteCaller.executePostCall(url, + "/sub-path-testy", + false, + Optional.of(new ByteArrayInputStream("fake".getBytes(StandardCharsets.UTF_8))), + Optional.ofNullable(tc), + false, + 0) + ); + assertEquals("Generic error in a rest call for url:'http://cds-kube-service:8081/mytenant/api/v1/upload/'", + ex.getMessage()); + assertEquals("Error in cloning input stream", ex.getCause().getMessage()); + + } + + } } From 0dfbbeb0c0e9c17fbbc14de64a39ffe282c7f740 Mon Sep 17 00:00:00 2001 From: "m.panagrosso" Date: Thu, 28 Sep 2023 17:30:47 +0200 Subject: [PATCH 3/5] ENG-5201: more test coverage --- .../system/storage/CdsRemoteCallerTest.java | 72 +++++++++++++++++-- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java b/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java index 8808efdd05..826383e62a 100644 --- a/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java +++ b/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java @@ -25,9 +25,12 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.SocketException; +import java.net.SocketTimeoutException; import java.net.URI; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -46,11 +49,13 @@ import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.http.HttpEntity; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.oauth2.common.OAuth2AccessToken; import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestTemplate; @ExtendWith(MockitoExtension.class) @@ -556,7 +561,7 @@ void shouldRetrieveFileAttribute() { @Test void shouldThrowExceptionIfAnErrorOccurInCloningFileInputStream() throws Exception { - + // set the config map for tenant Map configMap = Map.of("cdsPublicUrl", "http://my-server/tenant1/cms-resources", "cdsPrivateUrl", "http://cds-kube-service:8081/", "cdsPath", "/mytenant/api/v1", @@ -566,9 +571,13 @@ void shouldThrowExceptionIfAnErrorOccurInCloningFileInputStream() throws Excepti "kcClientSecret", "secret", "tenantCode", "my-tenant1"); TenantConfig tc = new TenantConfig(configMap); + Optional tenantConfig = Optional.of(tc); + // set tenant ApsTenantApplicationUtils.setTenant("my-tenant"); - URI url = URI.create("http://cds-kube-service:8081/mytenant/api/v1/upload/"); + // the input stream used to execute the post call + Optional fileInputStream = Optional.of( + new ByteArrayInputStream("fake".getBytes(StandardCharsets.UTF_8))); Mockito.when(restTemplate.exchange(anyString(), eq(HttpMethod.POST), @@ -582,11 +591,12 @@ void shouldThrowExceptionIfAnErrorOccurInCloningFileInputStream() throws Excepti .thenThrow(new IOException()); Exception ex = assertThrows(EntRuntimeException.class, - () -> cdsRemoteCaller.executePostCall(url, + () -> cdsRemoteCaller.executePostCall( + URI.create("http://cds-kube-service:8081/mytenant/api/v1/upload/"), "/sub-path-testy", false, - Optional.of(new ByteArrayInputStream("fake".getBytes(StandardCharsets.UTF_8))), - Optional.ofNullable(tc), + fileInputStream, + tenantConfig, false, 0) ); @@ -595,6 +605,58 @@ void shouldThrowExceptionIfAnErrorOccurInCloningFileInputStream() throws Excepti assertEquals("Error in cloning input stream", ex.getCause().getMessage()); } + } + @Test + void shouldRetryOneTimeIfPostTimeOutOccurs() throws Exception { + // set the config map for tenant + Map configMap = Map.of("cdsPublicUrl", "http://my-server/tenant1/cms-resources", + "cdsPrivateUrl", "http://cds-kube-service:8081/", + "cdsPath", "/mytenant/api/v1", + "kcAuthUrl", "http://tenant1.server.com/auth", + "kcRealm", "tenant1", + "kcClientId", "id", + "kcClientSecret", "secret", + "tenantCode", "my-tenant1"); + TenantConfig tc = new TenantConfig(configMap); + Optional tenantConfig = Optional.of(tc); + // set tenant + ApsTenantApplicationUtils.setTenant("my-tenant"); + ArrayList responseDtoList = new ArrayList<>(); + responseDtoList.add(new CdsCreateRowResponseDto()); + // the input stream used to execute the post call + Optional fileInputStream = Optional.of( + new ByteArrayInputStream("fake".getBytes(StandardCharsets.UTF_8))); + + CdsCreateRowResponseDto resp = new CdsCreateRowResponseDto(); + resp.setStatus("OK"); + List list = new ArrayList<>(); + list.add(resp); + URI url = URI.create("http://cds-kube-service:8081/mytenant/api/v1/upload/"); + Mockito.when(restTemplate.exchange(eq(url), eq(HttpMethod.POST), any(), + eq(new ParameterizedTypeReference>() { + }))) + + .thenThrow(new HttpClientErrorException(HttpStatus.UNAUTHORIZED)) + .thenThrow(new ResourceAccessException("", new SocketTimeoutException(""))) + .thenReturn(ResponseEntity.ok(list)); + + URI auth = URI.create("http://tenant1.server.com/auth/realms/tenant1/protocol/openid-connect/token"); + Mockito.when(restTemplate.exchange(eq(auth.toString()), + eq(HttpMethod.POST), + any(), + eq(new ParameterizedTypeReference>() { + }))).thenReturn( + ResponseEntity.status(HttpStatus.OK).body(Map.of("access_token", "xxxxxx"))); + + CdsCreateResponseDto responseDto = cdsRemoteCaller.executePostCall( + URI.create("http://cds-kube-service:8081/mytenant/api/v1/upload/"), + "/sub-path-testy", + false, + fileInputStream, + tenantConfig, + false, + 0); + assertTrue(responseDto.isStatusOk()); } } From 2fba7b6bec7a53718cd62faa9653203ef034031f Mon Sep 17 00:00:00 2001 From: "m.panagrosso" Date: Fri, 29 Sep 2023 15:06:43 +0200 Subject: [PATCH 4/5] ENG-5201: fix code smell --- .../plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java b/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java index 826383e62a..2da52b5250 100644 --- a/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java +++ b/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java @@ -622,8 +622,6 @@ void shouldRetryOneTimeIfPostTimeOutOccurs() throws Exception { Optional tenantConfig = Optional.of(tc); // set tenant ApsTenantApplicationUtils.setTenant("my-tenant"); - ArrayList responseDtoList = new ArrayList<>(); - responseDtoList.add(new CdsCreateRowResponseDto()); // the input stream used to execute the post call Optional fileInputStream = Optional.of( new ByteArrayInputStream("fake".getBytes(StandardCharsets.UTF_8))); @@ -650,7 +648,7 @@ void shouldRetryOneTimeIfPostTimeOutOccurs() throws Exception { ResponseEntity.status(HttpStatus.OK).body(Map.of("access_token", "xxxxxx"))); CdsCreateResponseDto responseDto = cdsRemoteCaller.executePostCall( - URI.create("http://cds-kube-service:8081/mytenant/api/v1/upload/"), + url, "/sub-path-testy", false, fileInputStream, From 34e06a83ff0d1466cb79c982f74fc32923cacc36 Mon Sep 17 00:00:00 2001 From: "m.panagrosso" Date: Fri, 29 Sep 2023 16:39:21 +0200 Subject: [PATCH 5/5] ENG-5201: fix code smell --- .../plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java b/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java index 2da52b5250..a4f5acd8a0 100644 --- a/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java +++ b/cds-plugin/src/test/java/org/entando/entando/plugins/jpcds/aps/system/storage/CdsRemoteCallerTest.java @@ -590,9 +590,10 @@ void shouldThrowExceptionIfAnErrorOccurInCloningFileInputStream() throws Excepti utilities.when(() -> IOUtils.toBufferedInputStream(any())) .thenThrow(new IOException()); + URI url = URI.create("http://cds-kube-service:8081/mytenant/api/v1/upload/"); Exception ex = assertThrows(EntRuntimeException.class, () -> cdsRemoteCaller.executePostCall( - URI.create("http://cds-kube-service:8081/mytenant/api/v1/upload/"), + url, "/sub-path-testy", false, fileInputStream,