From ecd9034a333b2bf136f02370ef986a430cacb9d9 Mon Sep 17 00:00:00 2001 From: "Kipchumba C. Bett" Date: Wed, 9 Oct 2024 13:57:11 +0300 Subject: [PATCH 1/3] OZ-708: Exclude duplicate TRANSFER_ENCODING header from being included in the response --- .../web/controller/DelegatingController.java | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java b/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java index 7cb684a..9440501 100644 --- a/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java +++ b/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java @@ -17,13 +17,13 @@ import javax.servlet.http.HttpServletRequest; +import lombok.extern.slf4j.Slf4j; import org.openmrs.module.fhirproxy.Config; import org.openmrs.module.fhirproxy.FhirProxyUtils; import org.openmrs.module.fhirproxy.web.ProxyWebConstants; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; +import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.RestTemplate; @@ -33,15 +33,15 @@ * Provides a proxy mechanism for all GET requests for ChargeItemDefinition and InventoryItem FHIR * resources by delegating to a configured external API to process the request. */ +@Slf4j @RestController("delegatingController") public class DelegatingController { - private static final Logger LOG = LoggerFactory.getLogger(DelegatingController.class); - private RestTemplate restTemplate; @GetMapping(ProxyWebConstants.PATH_DELEGATE) - public Object delegate(HttpServletRequest request) throws IOException { + public ResponseEntity delegate(HttpServletRequest request) throws IOException { + log.debug("Delegating to external API to process FHIR request -> {}", request.getRequestURI()); if (restTemplate == null) { restTemplate = new RestTemplate(); } @@ -59,14 +59,27 @@ public Object delegate(HttpServletRequest request) throws IOException { url += ("?" + queryString); } - if (LOG.isDebugEnabled()) { - LOG.debug("Requesting resource at -> {}", url); + if (log.isDebugEnabled()) { + log.debug("Requesting resource at -> {}", url); } UriComponentsBuilder urlBuilder = UriComponentsBuilder.fromHttpUrl(url); final String auth = getEncoder().encodeToString((cfg.getUsername() + ":" + cfg.getPassword()).getBytes(UTF_8)); HttpHeaders headers = new HttpHeaders(); headers.add(HttpHeaders.AUTHORIZATION, "Basic " + auth); - return restTemplate.exchange(urlBuilder.encode().toUriString(), GET, new HttpEntity<>(headers), Object.class); + + ResponseEntity responseEntity = restTemplate.exchange(urlBuilder.encode().toUriString(), GET, + new HttpEntity<>(headers), Object.class); + + // Create new HttpHeaders and copy headers, excluding the ones to remove + HttpHeaders newHeaders = new HttpHeaders(); + responseEntity.getHeaders().forEach((key, value) -> { + if (!HttpHeaders.TRANSFER_ENCODING.equalsIgnoreCase(key) && !HttpHeaders.CONNECTION.equalsIgnoreCase(key)) { + newHeaders.put(key, value); + } + }); + + return new ResponseEntity<>(responseEntity.getBody(), newHeaders, responseEntity.getStatusCode()); + } } From 68306c8496bc7f350f97fedf9fb86caebe6b2164 Mon Sep 17 00:00:00 2001 From: "Kipchumba C. Bett" Date: Wed, 9 Oct 2024 15:40:25 +0300 Subject: [PATCH 2/3] OZ-689: Add logic to handle client error exceptions and return the appropriate response status code --- .../web/controller/DelegatingController.java | 27 ++++---- .../controller/DelegatingControllerTest.java | 62 +++++++++++++++---- 2 files changed, 65 insertions(+), 24 deletions(-) diff --git a/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java b/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java index 9440501..1d1bfda 100644 --- a/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java +++ b/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java @@ -9,14 +9,6 @@ */ package org.openmrs.module.fhirproxy.web.controller; -import static java.nio.charset.StandardCharsets.UTF_8; -import static java.util.Base64.getEncoder; -import static org.springframework.http.HttpMethod.GET; - -import java.io.IOException; - -import javax.servlet.http.HttpServletRequest; - import lombok.extern.slf4j.Slf4j; import org.openmrs.module.fhirproxy.Config; import org.openmrs.module.fhirproxy.FhirProxyUtils; @@ -26,9 +18,17 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; import org.springframework.web.util.UriComponentsBuilder; +import javax.servlet.http.HttpServletRequest; +import java.io.IOException; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Base64.getEncoder; +import static org.springframework.http.HttpMethod.GET; + /** * Provides a proxy mechanism for all GET requests for ChargeItemDefinition and InventoryItem FHIR * resources by delegating to a configured external API to process the request. @@ -67,11 +67,10 @@ public ResponseEntity delegate(HttpServletRequest request) throws IOException final String auth = getEncoder().encodeToString((cfg.getUsername() + ":" + cfg.getPassword()).getBytes(UTF_8)); HttpHeaders headers = new HttpHeaders(); headers.add(HttpHeaders.AUTHORIZATION, "Basic " + auth); - + + try { ResponseEntity responseEntity = restTemplate.exchange(urlBuilder.encode().toUriString(), GET, new HttpEntity<>(headers), Object.class); - - // Create new HttpHeaders and copy headers, excluding the ones to remove HttpHeaders newHeaders = new HttpHeaders(); responseEntity.getHeaders().forEach((key, value) -> { if (!HttpHeaders.TRANSFER_ENCODING.equalsIgnoreCase(key) && !HttpHeaders.CONNECTION.equalsIgnoreCase(key)) { @@ -80,6 +79,10 @@ public ResponseEntity delegate(HttpServletRequest request) throws IOException }); return new ResponseEntity<>(responseEntity.getBody(), newHeaders, responseEntity.getStatusCode()); - + } + catch (HttpClientErrorException clientErrorException) { + return new ResponseEntity<>(clientErrorException.getResponseBodyAsString(), + clientErrorException.getStatusCode()); + } } } diff --git a/omod/src/test/java/org/openmrs/module/fhirproxy/web/controller/DelegatingControllerTest.java b/omod/src/test/java/org/openmrs/module/fhirproxy/web/controller/DelegatingControllerTest.java index 0bab8a1..b4f16fb 100644 --- a/omod/src/test/java/org/openmrs/module/fhirproxy/web/controller/DelegatingControllerTest.java +++ b/omod/src/test/java/org/openmrs/module/fhirproxy/web/controller/DelegatingControllerTest.java @@ -1,14 +1,5 @@ package org.openmrs.module.fhirproxy.web.controller; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.when; -import static org.springframework.http.HttpMethod.GET; - -import java.io.IOException; - -import javax.servlet.http.HttpServletRequest; - -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -23,9 +14,23 @@ import org.powermock.modules.junit4.PowerMockRunner; import org.powermock.reflect.Whitebox; import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; +import javax.servlet.http.HttpServletRequest; +import java.io.IOException; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; +import static org.springframework.http.HttpMethod.GET; + @RunWith(PowerMockRunner.class) @PrepareForTest(FhirProxyUtils.class) @PowerMockIgnore({ "javax.management.*", "javax.script.*" }) @@ -69,7 +74,7 @@ public void delegate_shouldGetTheResourceFromTheExternalApi() throws Exception { when(mockTemplate.exchange(eq(url), eq(GET), httpEntityCaptor.capture(), eq(Object.class))) .thenReturn(ResponseEntity.ok(expectedJson)); when(mockRequest.getAttribute(ProxyWebConstants.ATTRIB_RESOURCE_NAME)).thenReturn(RESOURCE); - Assert.assertEquals(expectedJson, ((HttpEntity) controller.delegate(mockRequest)).getBody()); + assertEquals(expectedJson, ((HttpEntity) controller.delegate(mockRequest)).getBody()); } @Test @@ -82,7 +87,7 @@ public void delegate_shouldGetTheResourceByIdFromTheExternalApi() throws Excepti .thenReturn(ResponseEntity.ok(expectedJson)); when(mockRequest.getAttribute(ProxyWebConstants.ATTRIB_RESOURCE_NAME)).thenReturn(RESOURCE); when(mockRequest.getAttribute(ProxyWebConstants.ATTRIB_RESOURCE_ID)).thenReturn(resId); - Assert.assertEquals(expectedJson, ((HttpEntity) controller.delegate(mockRequest)).getBody()); + assertEquals(expectedJson, ((HttpEntity) controller.delegate(mockRequest)).getBody()); } @Test @@ -95,7 +100,40 @@ public void delegate_shouldIncludeTheQueryString() throws Exception { .thenReturn(ResponseEntity.ok(expectedJson)); when(mockRequest.getAttribute(ProxyWebConstants.ATTRIB_RESOURCE_NAME)).thenReturn(RESOURCE); when(mockRequest.getAttribute(ProxyWebConstants.ATTRIB_QUERY_STR)).thenReturn(queryString); - Assert.assertEquals(expectedJson, ((HttpEntity) controller.delegate(mockRequest)).getBody()); + assertEquals(expectedJson, ((HttpEntity) controller.delegate(mockRequest)).getBody()); } + @Test + public void delegate_shouldRemoveTransferEncodingHeader() throws Exception { + final String expectedJson = "{}"; + final String url = BASE_URL + "/" + RESOURCE; + ArgumentCaptor> httpEntityCaptor = ArgumentCaptor.forClass(ResponseEntity.class); + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.add(HttpHeaders.TRANSFER_ENCODING, "chunked"); + responseHeaders.add(HttpHeaders.CONTENT_TYPE, "application/json"); + when(mockTemplate.exchange(eq(url), eq(GET), httpEntityCaptor.capture(), eq(Object.class))) + .thenReturn(new ResponseEntity<>(expectedJson, responseHeaders, HttpStatus.OK)); + when(mockRequest.getAttribute(ProxyWebConstants.ATTRIB_RESOURCE_NAME)).thenReturn(RESOURCE); + + ResponseEntity response = controller.delegate(mockRequest); + + assertEquals(HttpStatus.OK, response.getStatusCode()); + assertEquals(expectedJson, response.getBody()); + assertFalse(response.getHeaders().containsKey(HttpHeaders.TRANSFER_ENCODING)); + assertTrue(response.getHeaders().containsKey(HttpHeaders.CONTENT_TYPE)); + } + + @Test + public void delegate_shouldReturnClientErrorStatusCode() throws Exception { + final String url = BASE_URL + "/" + RESOURCE; + ArgumentCaptor> httpEntityCaptor = ArgumentCaptor.forClass(HttpEntity.class); + when(mockTemplate.exchange(eq(url), eq(GET), httpEntityCaptor.capture(), eq(Object.class))).thenThrow( + new HttpClientErrorException(HttpStatus.BAD_REQUEST, "Bad Request", "Bad Request".getBytes(), UTF_8)); + when(mockRequest.getAttribute(ProxyWebConstants.ATTRIB_RESOURCE_NAME)).thenReturn(RESOURCE); + + ResponseEntity response = controller.delegate(mockRequest); + + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode()); + assertEquals("Bad Request", response.getBody()); + } } From 2d5e0de625161950f3ad25c2a4307d0998c9a9b6 Mon Sep 17 00:00:00 2001 From: "Kipchumba C. Bett" Date: Thu, 10 Oct 2024 12:00:39 +0300 Subject: [PATCH 3/3] OZ-689: use LOG in favor of @Slf4j annotation --- .../web/controller/DelegatingController.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java b/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java index 1d1bfda..7688c80 100644 --- a/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java +++ b/omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java @@ -9,10 +9,11 @@ */ package org.openmrs.module.fhirproxy.web.controller; -import lombok.extern.slf4j.Slf4j; import org.openmrs.module.fhirproxy.Config; import org.openmrs.module.fhirproxy.FhirProxyUtils; import org.openmrs.module.fhirproxy.web.ProxyWebConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseEntity; @@ -33,15 +34,16 @@ * Provides a proxy mechanism for all GET requests for ChargeItemDefinition and InventoryItem FHIR * resources by delegating to a configured external API to process the request. */ -@Slf4j @RestController("delegatingController") public class DelegatingController { private RestTemplate restTemplate; + private static final Logger LOG = LoggerFactory.getLogger(DelegatingController.class); + @GetMapping(ProxyWebConstants.PATH_DELEGATE) public ResponseEntity delegate(HttpServletRequest request) throws IOException { - log.debug("Delegating to external API to process FHIR request -> {}", request.getRequestURI()); + LOG.debug("Delegating to external API to process FHIR request -> {}", request.getRequestURI()); if (restTemplate == null) { restTemplate = new RestTemplate(); } @@ -59,8 +61,8 @@ public ResponseEntity delegate(HttpServletRequest request) throws IOException url += ("?" + queryString); } - if (log.isDebugEnabled()) { - log.debug("Requesting resource at -> {}", url); + if (LOG.isDebugEnabled()) { + LOG.debug("Requesting resource at -> {}", url); } UriComponentsBuilder urlBuilder = UriComponentsBuilder.fromHttpUrl(url);