Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OZ-689: Add functionality to remove transfer_encoding header and handle response status code in DelegatingController #2

Merged
merged 3 commits into from
Oct 11, 2024
Merged

OZ-689: Add functionality to remove transfer_encoding header and handle response status code in DelegatingController #2

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ecd9034
OZ-708: Exclude duplicate TRANSFER_ENCODING header from being include…
corneliouzbett Oct 9, 2024
68306c8
OZ-689: Add logic to handle client error exceptions and return the ap…
corneliouzbett Oct 9, 2024
2d5e0de
OZ-689: use LOG in favor of @Slf4j annotation
corneliouzbett Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions omod/src/main/java/org/openmrs/module/fhirproxy/web/controller/DelegatingController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,39 +9,39 @@
*/
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;
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.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.
*/
@Slf4j
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually don't like using lombok to generate loggers, because they are anti pattern, as in they are in lower case.

@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();
}
Expand All @@ -59,14 +59,30 @@ public Object delegate(HttpServletRequest request) throws IOException {
url += ("?" + queryString);
}

if (LOG.isDebugEnabled()) {
LOG.debug("Requesting resource at -> {}", url);
if (log.isDebugEnabled()) {
corneliouzbett marked this conversation as resolved.
Show resolved Hide resolved
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);

try {
ResponseEntity<?> responseEntity = restTemplate.exchange(urlBuilder.encode().toUriString(), GET,
new HttpEntity<>(headers), Object.class);
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());
}
catch (HttpClientErrorException clientErrorException) {
return new ResponseEntity<>(clientErrorException.getResponseBodyAsString(),
clientErrorException.getStatusCode());
}
}
}
62 changes: 50 additions & 12 deletions omod/src/test/java/org/openmrs/module/fhirproxy/web/controller/DelegatingControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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.*" })
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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<ResponseEntity<?>> 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<HttpEntity<?>> 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());
}
}