From d51f8e8d1a22186efe4f96db3ee1b0ca95cd6bf0 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Wed, 13 Sep 2023 16:01:09 +0300 Subject: [PATCH 1/7] Enhance Benchmarking --- exec/pom.xml | 2 +- plugins/pom.xml | 2 +- .../gateway/plugin/BenchmarkingHelper.java | 9 +++-- .../fhir/gateway/plugin/OpenSRPHelper.java | 6 ++-- .../plugin/OpenSRPSyncAccessDecision.java | 2 +- .../plugin/PermissionAccessChecker.java | 20 ++++++++--- pom.xml | 2 +- server/pom.xml | 2 +- .../google/fhir/gateway/HttpFhirClient.java | 35 +++++++++++++++++-- 9 files changed, 64 insertions(+), 16 deletions(-) diff --git a/exec/pom.xml b/exec/pom.xml index e8e3f69e..b5fecafb 100755 --- a/exec/pom.xml +++ b/exec/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.39 + 0.1.40 exec diff --git a/plugins/pom.xml b/plugins/pom.xml index 9d5a4511..e95728b4 100755 --- a/plugins/pom.xml +++ b/plugins/pom.xml @@ -23,7 +23,7 @@ implementations do not have to do this; they can redeclare those deps. --> com.google.fhir.gateway fhir-gateway - 0.1.39 + 0.1.40 plugins diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/BenchmarkingHelper.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/BenchmarkingHelper.java index 6a9808b2..575edd19 100644 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/BenchmarkingHelper.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/BenchmarkingHelper.java @@ -18,13 +18,18 @@ import org.slf4j.Logger; public class BenchmarkingHelper { + + public static void printMessage(String message, Logger logger) { + logger.info(message); + } + public static void printCompletedInDuration(long startTime, String methodDetails, Logger logger) { long nanoSecondsTaken = System.nanoTime() - startTime; long millSecondsTaken = nanoSecondsTaken / 1000000; logger.info( String.format( - "########## %s : Metric in seconds - %d : Metric in nanoseconds - %d", - methodDetails, millSecondsTaken / 1000, nanoSecondsTaken)); + "%s : Metric in seconds - %.1f : Metric in nanoseconds - %d", + methodDetails, millSecondsTaken / 1000.0, nanoSecondsTaken)); } public static long startBenchmarking() { diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPHelper.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPHelper.java index fa159a23..c39b77b4 100644 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPHelper.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPHelper.java @@ -138,9 +138,9 @@ private Bundle getAttributedPractitionerDetailsByPractitioner(Practitioner pract careTeamList.addAll(attributedCareTeams); - logger.error( - "##### OpenSRPHelper.getAttributedPractitionerDetailsByPractitioner() -> CareTeam List" - + attributedCareTeams); + BenchmarkingHelper.printMessage( + "getAttributedPractitionerDetailsByPractitioner() -> CareTeam List" + attributedCareTeams, + logger); for (CareTeam careTeam : careTeamList) { // Add current supervisor practitioners diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java index 3e3088e0..08b51938 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java @@ -211,7 +211,7 @@ public String postProcess(RequestDetailsReader request, HttpResponse response) BenchmarkingHelper.printCompletedInDuration( start, - "postProcess : params include Gateway mode " + "postProcess : params include Gateway Mode " + (gatewayMode != null ? gatewayMode : "Default"), logger); diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java index 433c1f96..d085c25f 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java @@ -226,7 +226,11 @@ private Composition readCompositionResource(String applicationId, FhirContext fh compositionEntries.size() > 0 ? compositionEntries.get(0) : null; BenchmarkingHelper.printCompletedInDuration( - start, "readCompositionResource : params - applicationid -> " + applicationId, logger); + start, + "readCompositionResource : params [application id, fhircontext] -" + + " applicationid -> " + + applicationId, + logger); return compositionEntry != null ? (Composition) compositionEntry.getResource() : null; } @@ -249,7 +253,9 @@ private String getBinaryResourceReference(Composition composition) { } BenchmarkingHelper.printCompletedInDuration( - start, "getBinaryResourceReference: param -> Composition with ID=" + id, logger); + start, + "getBinaryResourceReference: params [Composition] -> Composition" + " with id=" + id, + logger); return id; } @@ -267,7 +273,9 @@ private Binary findApplicationConfigBinaryResource( BenchmarkingHelper.printCompletedInDuration( start, - "findApplicationConfigBinaryResource : param binary resource with id=" + binaryResourceId, + "findApplicationConfigBinaryResource : param binary resource with" + + " id=" + + binaryResourceId, logger); return binary; @@ -317,7 +325,11 @@ private PractitionerDetails readPractitionerDetails( : null; BenchmarkingHelper.printCompletedInDuration( - start, "readPractitionerDetails : params : KeycloakID" + keycloakUUID, logger); + start, + "readPractitionerDetails : params [keycloakUUID, fhirContext] :" + + " KeycloakID" + + keycloakUUID, + logger); return practitionerDetailEntry != null ? (PractitionerDetails) practitionerDetailEntry.getResource() diff --git a/pom.xml b/pom.xml index 21c4ae0b..86b15b6c 100755 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.39 + 0.1.40 pom FHIR Information Gateway diff --git a/server/pom.xml b/server/pom.xml index 77db3981..1edcdf89 100755 --- a/server/pom.xml +++ b/server/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.39 + 0.1.40 server diff --git a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java index dbf6ebc5..ff18cf18 100644 --- a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java +++ b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java @@ -27,13 +27,15 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.apache.commons.lang3.StringUtils; import org.apache.http.Header; import org.apache.http.HttpResponse; import org.apache.http.client.HttpClient; +import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.methods.RequestBuilder; import org.apache.http.entity.ByteArrayEntity; -import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.client.HttpClientBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -144,7 +146,32 @@ private HttpResponse sendRequest(RequestBuilder builder) throws IOException { HttpUriRequest httpRequest = builder.build(); logger.info("Request to the FHIR store is {}", httpRequest); // TODO reuse if creation overhead is significant. - HttpClient httpClient = HttpClients.createDefault(); + + RequestConfig.Builder configBuilder = RequestConfig.custom(); + + if (StringUtils.isNotBlank(System.getenv(CONNECT_TIMEOUT))) { + configBuilder.setConnectTimeout(Integer.valueOf(System.getenv(CONNECT_TIMEOUT)) * 1000); + logger.info( + "########## CONNECT_TIMEOUT set to " + System.getenv(CONNECT_TIMEOUT) + " seconds"); + } + + if (StringUtils.isNotBlank(System.getenv(CONNECTION_REQUEST_TIMEOUT))) { + configBuilder.setConnectionRequestTimeout( + Integer.valueOf(System.getenv(CONNECTION_REQUEST_TIMEOUT)) * 1000); + logger.info( + "########## CONNECTION_REQUEST_TIMEOUT set to " + + System.getenv(CONNECTION_REQUEST_TIMEOUT) + + " seconds"); + } + + if (StringUtils.isNotBlank(System.getenv(SOCKET_TIMEOUT))) { + configBuilder.setConnectionRequestTimeout( + Integer.valueOf(System.getenv(SOCKET_TIMEOUT)) * 1000); + logger.info("########## SOCKET_TIMEOUT set to " + System.getenv(SOCKET_TIMEOUT) + " seconds"); + } + + HttpClient httpClient = + HttpClientBuilder.create().setDefaultRequestConfig(configBuilder.build()).build(); // Execute the request and process the results. HttpResponse response = httpClient.execute(httpRequest); @@ -190,4 +217,8 @@ void copyParameters(ServletRequestDetails request, RequestBuilder builder) { } } } + + public static final String SOCKET_TIMEOUT = "GATEWAY_SOCKET_TIMEOUT"; + public static final String CONNECTION_REQUEST_TIMEOUT = "GATEWAY_CONNECTION_REQUEST_TIMEOUT"; + public static final String CONNECT_TIMEOUT = "GATEWAY_CONNECT_TIMEOUT"; } From 39f2d8db2a9e55e30b0ba6c61ca01bc52234cdf6 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Wed, 13 Sep 2023 18:56:10 +0300 Subject: [PATCH 2/7] Add Dynamic Connection configurations --- exec/pom.xml | 2 +- plugins/pom.xml | 2 +- pom.xml | 2 +- server/pom.xml | 2 +- .../google/fhir/gateway/HttpFhirClient.java | 23 ++++++++++++++++++- 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/exec/pom.xml b/exec/pom.xml index b5fecafb..535bd5af 100755 --- a/exec/pom.xml +++ b/exec/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.40 + 0.1.41 exec diff --git a/plugins/pom.xml b/plugins/pom.xml index e95728b4..536abfeb 100755 --- a/plugins/pom.xml +++ b/plugins/pom.xml @@ -23,7 +23,7 @@ implementations do not have to do this; they can redeclare those deps. --> com.google.fhir.gateway fhir-gateway - 0.1.40 + 0.1.41 plugins diff --git a/pom.xml b/pom.xml index 86b15b6c..ea1d3533 100755 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.40 + 0.1.41 pom FHIR Information Gateway diff --git a/server/pom.xml b/server/pom.xml index 1edcdf89..cb438ebe 100755 --- a/server/pom.xml +++ b/server/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.40 + 0.1.41 server diff --git a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java index ff18cf18..25215da9 100644 --- a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java +++ b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java @@ -170,8 +170,27 @@ private HttpResponse sendRequest(RequestBuilder builder) throws IOException { logger.info("########## SOCKET_TIMEOUT set to " + System.getenv(SOCKET_TIMEOUT) + " seconds"); } + HttpClientBuilder httpClientBuilder = HttpClientBuilder.create(); + + if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_TOTAL))) { + httpClientBuilder.setMaxConnTotal(Integer.valueOf(System.getenv(MAX_CONNECTION_TOTAL))); + logger.info( + "########## MAX_CONNECTION_TOTAL set to " + + System.getenv(MAX_CONNECTION_TOTAL) + + " seconds"); + } + + if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_PER_ROUTE))) { + httpClientBuilder.setMaxConnPerRoute( + Integer.valueOf(System.getenv(MAX_CONNECTION_PER_ROUTE))); + logger.info( + "########## MAX_CONNECTION_PER_ROUTE set to " + + System.getenv(MAX_CONNECTION_PER_ROUTE) + + " seconds"); + } + HttpClient httpClient = - HttpClientBuilder.create().setDefaultRequestConfig(configBuilder.build()).build(); + httpClientBuilder.setDefaultRequestConfig(configBuilder.build()).build(); // Execute the request and process the results. HttpResponse response = httpClient.execute(httpRequest); @@ -221,4 +240,6 @@ void copyParameters(ServletRequestDetails request, RequestBuilder builder) { public static final String SOCKET_TIMEOUT = "GATEWAY_SOCKET_TIMEOUT"; public static final String CONNECTION_REQUEST_TIMEOUT = "GATEWAY_CONNECTION_REQUEST_TIMEOUT"; public static final String CONNECT_TIMEOUT = "GATEWAY_CONNECT_TIMEOUT"; + public static final String MAX_CONNECTION_TOTAL = "GATEWAY_MAX_CONNECTION_TOTAL"; + public static final String MAX_CONNECTION_PER_ROUTE = "GATEWAY_MAX_CONNECTION_PER_ROUTE"; } From a1d48c3b6f823ef6c1fd03dbbfce22b9063f1ed9 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Fri, 15 Sep 2023 10:50:40 +0300 Subject: [PATCH 3/7] Refactor connection management for Troubleshooting Performance --- exec/pom.xml | 2 +- plugins/pom.xml | 2 +- .../fhir/gateway/plugin/HttpHelper.java | 77 +++++++++++++++++++ .../plugin/OpenSRPSyncAccessDecision.java | 3 + .../plugin/PermissionAccessChecker.java | 2 + pom.xml | 2 +- server/pom.xml | 2 +- .../google/fhir/gateway/HttpFhirClient.java | 55 +------------ .../com/google/fhir/gateway/HttpHelper2.java | 54 +++++++++++++ 9 files changed, 141 insertions(+), 58 deletions(-) create mode 100644 plugins/src/main/java/com/google/fhir/gateway/plugin/HttpHelper.java create mode 100644 server/src/main/java/com/google/fhir/gateway/HttpHelper2.java diff --git a/exec/pom.xml b/exec/pom.xml index 535bd5af..b3fab36d 100755 --- a/exec/pom.xml +++ b/exec/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.41 + 0.1.42 exec diff --git a/plugins/pom.xml b/plugins/pom.xml index 536abfeb..6a102956 100755 --- a/plugins/pom.xml +++ b/plugins/pom.xml @@ -23,7 +23,7 @@ implementations do not have to do this; they can redeclare those deps. --> com.google.fhir.gateway fhir-gateway - 0.1.41 + 0.1.42 plugins diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/HttpHelper.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/HttpHelper.java new file mode 100644 index 00000000..bf197279 --- /dev/null +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/HttpHelper.java @@ -0,0 +1,77 @@ +package com.google.fhir.gateway.plugin; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.rest.client.api.IGenericClient; +import org.apache.commons.lang3.StringUtils; +import org.apache.http.config.SocketConfig; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; + +public enum HttpHelper { + INSTANCE; + + public static HttpHelper getInstance() { + return INSTANCE; + } + + HttpHelper() { + + PoolingHttpClientConnectionManager poolingHttpClientConnectionManager = + new PoolingHttpClientConnectionManager(); + + if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_TOTAL))) { + poolingHttpClientConnectionManager.setMaxTotal( + Integer.valueOf(System.getenv(MAX_CONNECTION_TOTAL))); + } + + if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_PER_ROUTE))) { + poolingHttpClientConnectionManager.setDefaultMaxPerRoute( + Integer.valueOf(System.getenv(MAX_CONNECTION_PER_ROUTE))); + } + + if (StringUtils.isNotBlank(System.getenv(SOCKET_TIMEOUT))) { + SocketConfig socketConfig = + SocketConfig.custom() + .setSoTimeout(Integer.valueOf(System.getenv(SOCKET_TIMEOUT)) * 1000) + .build(); + poolingHttpClientConnectionManager.setDefaultSocketConfig(socketConfig); + } + + httpClient = + HttpClients.custom().setConnectionManager(poolingHttpClientConnectionManager).build(); + + genericClient = createFhirClientForR4(FhirContext.forR4()); + } + + private final CloseableHttpClient httpClient; + + public CloseableHttpClient getHttpClient() { + return httpClient; + } + + private final IGenericClient genericClient; + + public IGenericClient getGenericClient() { + return genericClient; + } + + private IGenericClient createFhirClientForR4(FhirContext fhirContext) { + long start = BenchmarkingHelper.startBenchmarking(); + String fhirServer = System.getenv(PROXY_TO_ENV); + IGenericClient client = fhirContext.newRestfulGenericClient(fhirServer); + System.out.println( + String.format( + "########## createFhirClientForR4 created in %.1f seconds", + ((System.nanoTime() - start) / 1000))); + return client; + } + + public static final String SOCKET_TIMEOUT = "GATEWAY_SOCKET_TIMEOUT"; + public static final String CONNECTION_REQUEST_TIMEOUT = "GATEWAY_CONNECTION_REQUEST_TIMEOUT"; + public static final String CONNECT_TIMEOUT = "GATEWAY_CONNECT_TIMEOUT"; + public static final String MAX_CONNECTION_TOTAL = "GATEWAY_MAX_CONNECTION_TOTAL"; + public static final String MAX_CONNECTION_PER_ROUTE = "GATEWAY_MAX_CONNECTION_PER_ROUTE"; + + public static final String PROXY_TO_ENV = "PROXY_TO"; +} diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java index 08b51938..6177b85a 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java @@ -89,12 +89,15 @@ public OpenSRPSyncAccessDecision( this.syncStrategy = syncStrategy; this.config = getSkippedResourcesConfigs(); this.roles = roles; + long start = BenchmarkingHelper.startBenchmarking(); try { setFhirR4Client( fhirR4Context.newRestfulGenericClient( System.getenv(PermissionAccessChecker.Factory.PROXY_TO_ENV))); } catch (NullPointerException e) { logger.error(e.getMessage()); + } finally { + BenchmarkingHelper.printCompletedInDuration(start, "constructor setFhirR4Client", logger); } this.openSRPHelper = new OpenSRPHelper(fhirR4Client); diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java index d085c25f..7dba0a00 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java @@ -201,8 +201,10 @@ private String getApplicationIdFromJWT(DecodedJWT jwt) { } private IGenericClient createFhirClientForR4(FhirContext fhirContext) { + long start = BenchmarkingHelper.startBenchmarking(); String fhirServer = System.getenv(PROXY_TO_ENV); IGenericClient client = fhirContext.newRestfulGenericClient(fhirServer); + BenchmarkingHelper.printCompletedInDuration(start, "createFhirClientForR4", logger); return client; } diff --git a/pom.xml b/pom.xml index ea1d3533..da2963e0 100755 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.41 + 0.1.42 pom FHIR Information Gateway diff --git a/server/pom.xml b/server/pom.xml index cb438ebe..d6abbddf 100755 --- a/server/pom.xml +++ b/server/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.41 + 0.1.42 server diff --git a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java index 25215da9..6dec4e8a 100644 --- a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java +++ b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java @@ -27,15 +27,12 @@ import java.util.List; import java.util.Map; import java.util.Set; -import org.apache.commons.lang3.StringUtils; import org.apache.http.Header; import org.apache.http.HttpResponse; import org.apache.http.client.HttpClient; -import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.methods.RequestBuilder; import org.apache.http.entity.ByteArrayEntity; -import org.apache.http.impl.client.HttpClientBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -145,52 +142,8 @@ private HttpResponse sendRequest(RequestBuilder builder) throws IOException { builder.addHeader(header); HttpUriRequest httpRequest = builder.build(); logger.info("Request to the FHIR store is {}", httpRequest); - // TODO reuse if creation overhead is significant. - RequestConfig.Builder configBuilder = RequestConfig.custom(); - - if (StringUtils.isNotBlank(System.getenv(CONNECT_TIMEOUT))) { - configBuilder.setConnectTimeout(Integer.valueOf(System.getenv(CONNECT_TIMEOUT)) * 1000); - logger.info( - "########## CONNECT_TIMEOUT set to " + System.getenv(CONNECT_TIMEOUT) + " seconds"); - } - - if (StringUtils.isNotBlank(System.getenv(CONNECTION_REQUEST_TIMEOUT))) { - configBuilder.setConnectionRequestTimeout( - Integer.valueOf(System.getenv(CONNECTION_REQUEST_TIMEOUT)) * 1000); - logger.info( - "########## CONNECTION_REQUEST_TIMEOUT set to " - + System.getenv(CONNECTION_REQUEST_TIMEOUT) - + " seconds"); - } - - if (StringUtils.isNotBlank(System.getenv(SOCKET_TIMEOUT))) { - configBuilder.setConnectionRequestTimeout( - Integer.valueOf(System.getenv(SOCKET_TIMEOUT)) * 1000); - logger.info("########## SOCKET_TIMEOUT set to " + System.getenv(SOCKET_TIMEOUT) + " seconds"); - } - - HttpClientBuilder httpClientBuilder = HttpClientBuilder.create(); - - if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_TOTAL))) { - httpClientBuilder.setMaxConnTotal(Integer.valueOf(System.getenv(MAX_CONNECTION_TOTAL))); - logger.info( - "########## MAX_CONNECTION_TOTAL set to " - + System.getenv(MAX_CONNECTION_TOTAL) - + " seconds"); - } - - if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_PER_ROUTE))) { - httpClientBuilder.setMaxConnPerRoute( - Integer.valueOf(System.getenv(MAX_CONNECTION_PER_ROUTE))); - logger.info( - "########## MAX_CONNECTION_PER_ROUTE set to " - + System.getenv(MAX_CONNECTION_PER_ROUTE) - + " seconds"); - } - - HttpClient httpClient = - httpClientBuilder.setDefaultRequestConfig(configBuilder.build()).build(); + HttpClient httpClient = HttpHelper2.getInstance().getHttpClient(); // Execute the request and process the results. HttpResponse response = httpClient.execute(httpRequest); @@ -236,10 +189,4 @@ void copyParameters(ServletRequestDetails request, RequestBuilder builder) { } } } - - public static final String SOCKET_TIMEOUT = "GATEWAY_SOCKET_TIMEOUT"; - public static final String CONNECTION_REQUEST_TIMEOUT = "GATEWAY_CONNECTION_REQUEST_TIMEOUT"; - public static final String CONNECT_TIMEOUT = "GATEWAY_CONNECT_TIMEOUT"; - public static final String MAX_CONNECTION_TOTAL = "GATEWAY_MAX_CONNECTION_TOTAL"; - public static final String MAX_CONNECTION_PER_ROUTE = "GATEWAY_MAX_CONNECTION_PER_ROUTE"; } diff --git a/server/src/main/java/com/google/fhir/gateway/HttpHelper2.java b/server/src/main/java/com/google/fhir/gateway/HttpHelper2.java new file mode 100644 index 00000000..6d441b84 --- /dev/null +++ b/server/src/main/java/com/google/fhir/gateway/HttpHelper2.java @@ -0,0 +1,54 @@ +package com.google.fhir.gateway; + +import org.apache.commons.lang3.StringUtils; +import org.apache.http.config.SocketConfig; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; + +public enum HttpHelper2 { + INSTANCE; + + public static HttpHelper2 getInstance() { + return INSTANCE; + } + + HttpHelper2() { + + PoolingHttpClientConnectionManager poolingHttpClientConnectionManager = + new PoolingHttpClientConnectionManager(); + + if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_TOTAL))) { + poolingHttpClientConnectionManager.setMaxTotal( + Integer.valueOf(System.getenv(MAX_CONNECTION_TOTAL))); + } + + if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_PER_ROUTE))) { + poolingHttpClientConnectionManager.setDefaultMaxPerRoute( + Integer.valueOf(System.getenv(MAX_CONNECTION_PER_ROUTE))); + } + + if (StringUtils.isNotBlank(System.getenv(SOCKET_TIMEOUT))) { + SocketConfig socketConfig = + SocketConfig.custom() + .setSoTimeout(Integer.valueOf(System.getenv(SOCKET_TIMEOUT)) * 1000) + .build(); + poolingHttpClientConnectionManager.setDefaultSocketConfig(socketConfig); + } + + httpClient = + HttpClients.custom().setConnectionManager(poolingHttpClientConnectionManager).build(); + } + + private final CloseableHttpClient httpClient; + + public CloseableHttpClient getHttpClient() { + return httpClient; + } + + public static final String SOCKET_TIMEOUT = "GATEWAY_SOCKET_TIMEOUT"; + public static final String CONNECTION_REQUEST_TIMEOUT = "GATEWAY_CONNECTION_REQUEST_TIMEOUT"; + public static final String CONNECT_TIMEOUT = "GATEWAY_CONNECT_TIMEOUT"; + public static final String MAX_CONNECTION_TOTAL = "GATEWAY_MAX_CONNECTION_TOTAL"; + public static final String MAX_CONNECTION_PER_ROUTE = "GATEWAY_MAX_CONNECTION_PER_ROUTE"; +} From 7d65b40ea00f8f889c6c0955ae53859fb2bcc892 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Thu, 5 Oct 2023 09:43:49 +0300 Subject: [PATCH 4/7] Clean up --- .../com/google/fhir/gateway/HttpHelper2.java | 54 ------------------- 1 file changed, 54 deletions(-) delete mode 100644 server/src/main/java/com/google/fhir/gateway/HttpHelper2.java diff --git a/server/src/main/java/com/google/fhir/gateway/HttpHelper2.java b/server/src/main/java/com/google/fhir/gateway/HttpHelper2.java deleted file mode 100644 index 6d441b84..00000000 --- a/server/src/main/java/com/google/fhir/gateway/HttpHelper2.java +++ /dev/null @@ -1,54 +0,0 @@ -package com.google.fhir.gateway; - -import org.apache.commons.lang3.StringUtils; -import org.apache.http.config.SocketConfig; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; - -public enum HttpHelper2 { - INSTANCE; - - public static HttpHelper2 getInstance() { - return INSTANCE; - } - - HttpHelper2() { - - PoolingHttpClientConnectionManager poolingHttpClientConnectionManager = - new PoolingHttpClientConnectionManager(); - - if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_TOTAL))) { - poolingHttpClientConnectionManager.setMaxTotal( - Integer.valueOf(System.getenv(MAX_CONNECTION_TOTAL))); - } - - if (StringUtils.isNotBlank(System.getenv(MAX_CONNECTION_PER_ROUTE))) { - poolingHttpClientConnectionManager.setDefaultMaxPerRoute( - Integer.valueOf(System.getenv(MAX_CONNECTION_PER_ROUTE))); - } - - if (StringUtils.isNotBlank(System.getenv(SOCKET_TIMEOUT))) { - SocketConfig socketConfig = - SocketConfig.custom() - .setSoTimeout(Integer.valueOf(System.getenv(SOCKET_TIMEOUT)) * 1000) - .build(); - poolingHttpClientConnectionManager.setDefaultSocketConfig(socketConfig); - } - - httpClient = - HttpClients.custom().setConnectionManager(poolingHttpClientConnectionManager).build(); - } - - private final CloseableHttpClient httpClient; - - public CloseableHttpClient getHttpClient() { - return httpClient; - } - - public static final String SOCKET_TIMEOUT = "GATEWAY_SOCKET_TIMEOUT"; - public static final String CONNECTION_REQUEST_TIMEOUT = "GATEWAY_CONNECTION_REQUEST_TIMEOUT"; - public static final String CONNECT_TIMEOUT = "GATEWAY_CONNECT_TIMEOUT"; - public static final String MAX_CONNECTION_TOTAL = "GATEWAY_MAX_CONNECTION_TOTAL"; - public static final String MAX_CONNECTION_PER_ROUTE = "GATEWAY_MAX_CONNECTION_PER_ROUTE"; -} From f132c7878dc6af89895b1043d5d6ed0ba973a9ac Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Thu, 5 Oct 2023 10:04:35 +0300 Subject: [PATCH 5/7] Refactor for Optimization --- .../fhir/gateway/plugin/OpenSRPHelper.java | 8 +- .../plugin/OpenSRPSyncAccessDecision.java | 2 +- .../plugin/PermissionAccessChecker.java | 193 ++++++++++-------- .../google/fhir/gateway/HttpFhirClient.java | 5 +- .../google/fhir/gateway/ProxyConstants.java | 1 + 5 files changed, 118 insertions(+), 91 deletions(-) diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPHelper.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPHelper.java index c39b77b4..6ba39a9a 100644 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPHelper.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPHelper.java @@ -41,7 +41,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.smartregister.model.location.LocationHierarchy; -import org.smartregister.model.location.ParentChildrenMap; import org.smartregister.model.practitioner.FhirPractitionerDetails; import org.smartregister.model.practitioner.PractitionerDetails; import org.smartregister.utils.Constants; @@ -181,7 +180,7 @@ public static List getAttributedLocations(List locati long start = BenchmarkingHelper.startBenchmarking(); - List parentChildrenList = + List attributedLocationsList = locationHierarchies.stream() .flatMap( locationHierarchy -> @@ -190,15 +189,12 @@ public static List getAttributedLocations(List locati .getLocationsHierarchy() .getParentChildren() .stream()) - .collect(Collectors.toList()); - List attributedLocationsList = - parentChildrenList.stream() .flatMap(parentChildren -> parentChildren.getChildIdentifiers().stream()) .map(it -> getReferenceIDPart(it.toString())) .collect(Collectors.toList()); BenchmarkingHelper.printCompletedInDuration( - start, "getAttributedLocations " + locationHierarchies, logger); + start, "getAttributedLocations " + attributedLocationsList, logger); return attributedLocationsList; } diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java index 6177b85a..0cb5a363 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java @@ -274,7 +274,7 @@ private Bundle processListEntriesGatewayModeByBundle(IBaseResource responseResou } @NotNull - private static Bundle.BundleEntryComponent createBundleEntryComponent( + public static Bundle.BundleEntryComponent createBundleEntryComponent( Bundle.HTTPVerb method, String requestPath, @Nullable String condition) { Bundle.BundleEntryComponent bundleEntryComponent = new Bundle.BundleEntryComponent(); diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java index 7dba0a00..1b6e3e83 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java @@ -36,7 +36,9 @@ import java.util.stream.Collectors; import javax.inject.Named; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.hl7.fhir.r4.model.*; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.smartregister.model.practitioner.PractitionerDetails; @@ -208,35 +210,6 @@ private IGenericClient createFhirClientForR4(FhirContext fhirContext) { return client; } - private Composition readCompositionResource(String applicationId, FhirContext fhirContext) { - - long start = BenchmarkingHelper.startBenchmarking(); - - IGenericClient client = createFhirClientForR4(fhirContext); - Bundle compositionBundle = - client - .search() - .forResource(Composition.class) - .where(Composition.IDENTIFIER.exactly().identifier(applicationId)) - .returnBundle(Bundle.class) - .execute(); - List compositionEntries = - compositionBundle != null - ? compositionBundle.getEntry() - : Collections.singletonList(new Bundle.BundleEntryComponent()); - Bundle.BundleEntryComponent compositionEntry = - compositionEntries.size() > 0 ? compositionEntries.get(0) : null; - - BenchmarkingHelper.printCompletedInDuration( - start, - "readCompositionResource : params [application id, fhircontext] -" - + " applicationid -> " - + applicationId, - logger); - - return compositionEntry != null ? (Composition) compositionEntry.getResource() : null; - } - private String getBinaryResourceReference(Composition composition) { long start = BenchmarkingHelper.startBenchmarking(); @@ -244,10 +217,13 @@ private String getBinaryResourceReference(Composition composition) { String id = ""; if (composition != null && composition.getSection() != null) { composition.getSection().stream() - .filter(v -> v.getFocus().getIdentifier() != null) - .filter(v -> v.getFocus().getIdentifier().getValue() != null) - .filter(v -> v.getFocus().getIdentifier().getValue().equals("application")) - .map(v -> composition.getSection().indexOf(v)) + .filter( + sectionComponent -> + sectionComponent.getFocus().getIdentifier() != null + && sectionComponent.getFocus().getIdentifier().getValue() != null + && ProxyConstants.APPLICATION.equals( + sectionComponent.getFocus().getIdentifier().getValue())) + .map(sectionComponent -> composition.getSection().indexOf(sectionComponent)) .collect(Collectors.toList()); Composition.SectionComponent sectionComponent = composition.getSection().get(0); Reference focus = sectionComponent != null ? sectionComponent.getFocus() : null; @@ -305,49 +281,97 @@ private String findSyncStrategy(Binary binary) { return syncStrategy; } - private PractitionerDetails readPractitionerDetails( - String keycloakUUID, FhirContext fhirContext) { + public Map> getMapForWhere(String keycloakUUID) { + Map> hmOut = new HashMap<>(); + // Adding keycloak-uuid + TokenParam tokenParam = new TokenParam("keycloak-uuid"); + tokenParam.setValue(keycloakUUID); + List lst = new ArrayList<>(); + lst.add(tokenParam); + hmOut.put(PractitionerDetails.SP_KEYCLOAK_UUID, lst); + + return hmOut; + } + + Pair fetchCompositionAndPractitionerDetails( + String subject, String applicationId, FhirContext fhirContext) { long start = BenchmarkingHelper.startBenchmarking(); + fhirContext.registerCustomType(PractitionerDetails.class); + IGenericClient client = createFhirClientForR4(fhirContext); - Bundle practitionerDetailsBundle = - client - .search() - .forResource(PractitionerDetails.class) - .where(getMapForWhere(keycloakUUID)) - .returnBundle(Bundle.class) - .execute(); - - List practitionerDetailsBundleEntry = - practitionerDetailsBundle.getEntry(); - Bundle.BundleEntryComponent practitionerDetailEntry = - practitionerDetailsBundleEntry != null && practitionerDetailsBundleEntry.size() > 0 - ? practitionerDetailsBundleEntry.get(0) - : null; + Bundle requestBundle = new Bundle(); + requestBundle.setType(Bundle.BundleType.BATCH); + + requestBundle.addEntry( + OpenSRPSyncAccessDecision.createBundleEntryComponent( + Bundle.HTTPVerb.GET, "Composition?identifier=" + applicationId, null)); + requestBundle.addEntry( + OpenSRPSyncAccessDecision.createBundleEntryComponent( + Bundle.HTTPVerb.GET, "practitioner-details?keycloak-uuid=" + subject, null)); + + Bundle responsebundle = client.transaction().withBundle(requestBundle).execute(); + + Pair result = + getCompositionPractitionerDetailsPair(applicationId, responsebundle); BenchmarkingHelper.printCompletedInDuration( - start, - "readPractitionerDetails : params [keycloakUUID, fhirContext] :" - + " KeycloakID" - + keycloakUUID, - logger); + start, "fetchCompositionAndPractitionerDetails", logger); - return practitionerDetailEntry != null - ? (PractitionerDetails) practitionerDetailEntry.getResource() - : null; + return result; } - public Map> getMapForWhere(String keycloakUUID) { - Map> hmOut = new HashMap<>(); - // Adding keycloak-uuid - TokenParam tokenParam = new TokenParam("keycloak-uuid"); - tokenParam.setValue(keycloakUUID); - List lst = new ArrayList(); - lst.add(tokenParam); - hmOut.put(PractitionerDetails.SP_KEYCLOAK_UUID, lst); + @NotNull + private static Pair getCompositionPractitionerDetailsPair( + String applicationId, Bundle responsebundle) { + long start = BenchmarkingHelper.startBenchmarking(); + Composition composition = null; + PractitionerDetails practitionerDetails = null; - return hmOut; + Bundle innerBundle; + for (int i = 0; i < responsebundle.getEntry().size(); i++) { + + innerBundle = (Bundle) responsebundle.getEntry().get(i).getResource(); + if (innerBundle == null) continue; + + for (int j = 0; j < innerBundle.getEntry().size(); j++) { + + if (innerBundle.getEntry().get(j).getResource() instanceof Composition) { + composition = (Composition) innerBundle.getEntry().get(j).getResource(); + } else if (innerBundle.getEntry().get(j).getResource() instanceof PractitionerDetails) { + practitionerDetails = (PractitionerDetails) innerBundle.getEntry().get(j).getResource(); + } + } + } + + if (composition == null) + throw new IllegalStateException( + "No Composition resource found for application id '" + applicationId + "'"); + + BenchmarkingHelper.printCompletedInDuration( + start, "getCompositionPractitionerDetailsPair", logger); + return Pair.of(composition, practitionerDetails); + } + + Pair fetchSyncStrategyDetails( + String subject, String applicationId, FhirContext fhirContext) { + + long start = BenchmarkingHelper.startBenchmarking(); + + Pair compositionPractitionerDetailsPair = + fetchCompositionAndPractitionerDetails(subject, applicationId, fhirContext); + Composition composition = compositionPractitionerDetailsPair.getLeft(); + PractitionerDetails practitionerDetails = compositionPractitionerDetailsPair.getRight(); + + String binaryResourceReference = getBinaryResourceReference(composition); + Binary binary = findApplicationConfigBinaryResource(binaryResourceReference, fhirContext); + + Pair strategyToPractitioner = + Pair.of(findSyncStrategy(binary), practitionerDetails); // TODO Return immediately + + BenchmarkingHelper.printCompletedInDuration(start, "fetchSyncStrategyDetails", logger); + return strategyToPractitioner; } @Override @@ -362,12 +386,13 @@ public AccessChecker create( List userRoles = getUserRolesFromJWT(jwt); String applicationId = getApplicationIdFromJWT(jwt); - Composition composition = readCompositionResource(applicationId, fhirContext); - String binaryResourceReference = getBinaryResourceReference(composition); - Binary binary = findApplicationConfigBinaryResource(binaryResourceReference, fhirContext); - String syncStrategy = findSyncStrategy(binary); - PractitionerDetails practitionerDetails = - readPractitionerDetails(jwt.getSubject(), fhirContext); + + Pair my = + fetchSyncStrategyDetails(jwt.getSubject(), applicationId, fhirContext); + + String syncStrategy = my.getLeft(); + PractitionerDetails practitionerDetails = my.getRight(); + List careTeams; List organizations; List careTeamIds = new ArrayList<>(); @@ -380,22 +405,26 @@ public AccessChecker create( && practitionerDetails.getFhirPractitionerDetails() != null ? practitionerDetails.getFhirPractitionerDetails().getCareTeams() : Collections.singletonList(new CareTeam()); - for (CareTeam careTeam : careTeams) { - if (careTeam.getIdElement() != null) { - careTeamIds.add(careTeam.getIdElement().getIdPart()); - } - } + + careTeamIds = + careTeams.stream() + .filter(careTeam -> careTeam.getIdElement() != null) + .map(careTeam -> careTeam.getIdElement().getIdPart()) + .collect(Collectors.toList()); + } else if (Constants.ORGANIZATION.equalsIgnoreCase(syncStrategy)) { organizations = practitionerDetails != null && practitionerDetails.getFhirPractitionerDetails() != null ? practitionerDetails.getFhirPractitionerDetails().getOrganizations() : Collections.singletonList(new Organization()); - for (Organization organization : organizations) { - if (organization.getIdElement() != null) { - organizationIds.add(organization.getIdElement().getIdPart()); - } - } + + organizationIds = + organizations.stream() + .filter(organization -> organization.getIdElement() != null) + .map(organization -> organization.getIdElement().getIdPart()) + .collect(Collectors.toList()); + } else if (Constants.LOCATION.equalsIgnoreCase(syncStrategy)) { locationIds = practitionerDetails != null diff --git a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java index 6dec4e8a..dbf6ebc5 100644 --- a/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java +++ b/server/src/main/java/com/google/fhir/gateway/HttpFhirClient.java @@ -33,6 +33,7 @@ import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.methods.RequestBuilder; import org.apache.http.entity.ByteArrayEntity; +import org.apache.http.impl.client.HttpClients; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -142,8 +143,8 @@ private HttpResponse sendRequest(RequestBuilder builder) throws IOException { builder.addHeader(header); HttpUriRequest httpRequest = builder.build(); logger.info("Request to the FHIR store is {}", httpRequest); - - HttpClient httpClient = HttpHelper2.getInstance().getHttpClient(); + // TODO reuse if creation overhead is significant. + HttpClient httpClient = HttpClients.createDefault(); // Execute the request and process the results. HttpResponse response = httpClient.execute(httpRequest); diff --git a/server/src/main/java/com/google/fhir/gateway/ProxyConstants.java b/server/src/main/java/com/google/fhir/gateway/ProxyConstants.java index 2458161a..71791c5f 100644 --- a/server/src/main/java/com/google/fhir/gateway/ProxyConstants.java +++ b/server/src/main/java/com/google/fhir/gateway/ProxyConstants.java @@ -38,6 +38,7 @@ public class ProxyConstants { static final ContentType JSON_PATCH_CONTENT = ContentType.create(Constants.CT_JSON_PATCH); public static final String SYNC_STRATEGY = "syncStrategy"; public static final String REALM_ACCESS = "realm_access"; + public static final String APPLICATION = "application"; public interface Literals { String EQUALS = "="; From c0541ac1e0ce48b566a6a32c31f92e0f0cb58aec Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Thu, 5 Oct 2023 10:11:30 +0300 Subject: [PATCH 6/7] Tag version 0.1.42 Load Test Version --- exec/pom.xml | 2 +- plugins/pom.xml | 2 +- pom.xml | 2 +- server/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/exec/pom.xml b/exec/pom.xml index b3fab36d..d0476195 100755 --- a/exec/pom.xml +++ b/exec/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.42 + 0.1.41-LT exec diff --git a/plugins/pom.xml b/plugins/pom.xml index 6a102956..f0091fb4 100755 --- a/plugins/pom.xml +++ b/plugins/pom.xml @@ -23,7 +23,7 @@ implementations do not have to do this; they can redeclare those deps. --> com.google.fhir.gateway fhir-gateway - 0.1.42 + 0.1.41-LT plugins diff --git a/pom.xml b/pom.xml index da2963e0..dda3069b 100755 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.42 + 0.1.41-LT pom FHIR Information Gateway diff --git a/server/pom.xml b/server/pom.xml index d6abbddf..ba041cb0 100755 --- a/server/pom.xml +++ b/server/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.42 + 0.1.41-LT server From d4bd919668d46611fec2066e58760398a6ee6e43 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Fri, 6 Oct 2023 10:51:11 +0300 Subject: [PATCH 7/7] Performance optimization via Caching --- exec/pom.xml | 2 +- plugins/pom.xml | 2 +- .../fhir/gateway/plugin/CacheHelper.java | 35 +++++++ .../fhir/gateway/plugin/HttpHelper.java | 15 +++ .../plugin/OpenSRPSyncAccessDecision.java | 33 +++---- .../plugin/PermissionAccessChecker.java | 95 +++++++++++-------- .../plugin/OpenSRPSyncAccessDecisionTest.java | 11 +-- pom.xml | 7 +- server/pom.xml | 2 +- .../google/fhir/gateway/ProxyConstants.java | 4 +- 10 files changed, 134 insertions(+), 72 deletions(-) create mode 100644 plugins/src/main/java/com/google/fhir/gateway/plugin/CacheHelper.java diff --git a/exec/pom.xml b/exec/pom.xml index d0476195..250c7241 100755 --- a/exec/pom.xml +++ b/exec/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.41-LT + 0.1.41-LT2 exec diff --git a/plugins/pom.xml b/plugins/pom.xml index f0091fb4..54469c14 100755 --- a/plugins/pom.xml +++ b/plugins/pom.xml @@ -23,7 +23,7 @@ implementations do not have to do this; they can redeclare those deps. --> com.google.fhir.gateway fhir-gateway - 0.1.41-LT + 0.1.41-LT2 plugins diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/CacheHelper.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/CacheHelper.java new file mode 100644 index 00000000..636548ff --- /dev/null +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/CacheHelper.java @@ -0,0 +1,35 @@ +/* + * Copyright 2021-2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.fhir.gateway.plugin; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +public enum CacheHelper { + INSTANCE; + Cache>> cache; + + public CacheHelper getInstance() { + return INSTANCE; + } + + CacheHelper() { + cache = Caffeine.newBuilder().expireAfterWrite(30, TimeUnit.SECONDS).maximumSize(1_000).build(); + } +} diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/HttpHelper.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/HttpHelper.java index bf197279..c58fb9e0 100644 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/HttpHelper.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/HttpHelper.java @@ -1,3 +1,18 @@ +/* + * Copyright 2021-2023 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package com.google.fhir.gateway.plugin; import ca.uhn.fhir.context.FhirContext; diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java index 0cb5a363..d43bd5d2 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecision.java @@ -58,9 +58,7 @@ public class OpenSRPSyncAccessDecision implements AccessDecision { private final String syncStrategy; private final String applicationId; private final boolean accessGranted; - private final List careTeamIds; - private final List locationIds; - private final List organizationIds; + private final Map> syncStrategyIds; private final List roles; private IgnoredResourcesConfig config; private String keycloakUUID; @@ -74,18 +72,14 @@ public OpenSRPSyncAccessDecision( String keycloakUUID, String applicationId, boolean accessGranted, - List locationIds, - List careTeamIds, - List organizationIds, + Map> syncStrategyIds, String syncStrategy, List roles) { this.fhirR4Context = fhirContext; this.keycloakUUID = keycloakUUID; this.applicationId = applicationId; this.accessGranted = accessGranted; - this.careTeamIds = careTeamIds; - this.locationIds = locationIds; - this.organizationIds = organizationIds; + this.syncStrategyIds = syncStrategyIds; this.syncStrategy = syncStrategy; this.config = getSkippedResourcesConfigs(); this.roles = roles; @@ -115,7 +109,7 @@ public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsRea RequestMutation requestMutation = null; if (isSyncUrl(requestDetailsReader)) { - if (locationIds.isEmpty() && careTeamIds.isEmpty() && organizationIds.isEmpty()) { + if (syncStrategyIds.isEmpty()) { ForbiddenOperationException forbiddenOperationException = new ForbiddenOperationException( @@ -131,7 +125,7 @@ public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsRea // Skip app-wide global resource requests if (!shouldSkipDataFiltering(requestDetailsReader)) { List syncFilterParameterValues = - addSyncFilters(getSyncTags(locationIds, careTeamIds, organizationIds)); + addSyncFilters(getSyncTags(this.syncStrategy, this.syncStrategyIds)); requestMutation = RequestMutation.builder() .queryParams( @@ -314,22 +308,25 @@ private Bundle postProcessModeListEntries(IBaseResource responseResource) { * Generates a map of Code.url to multiple Code.Value which contains all the possible filters that * will be used in syncing * - * @param locationIds - * @param careTeamIds - * @param organizationIds + * @param syncStrategy + * @param syncStrategyIds * @return Pair of URL to [Code.url, [Code.Value]] map. The URL is complete url */ private Map getSyncTags( - List locationIds, List careTeamIds, List organizationIds) { + String syncStrategy, Map> syncStrategyIds) { StringBuilder sb = new StringBuilder(); Map map = new HashMap<>(); sb.append(ProxyConstants.TAG_SEARCH_PARAM); sb.append(ProxyConstants.Literals.EQUALS); - addTags(ProxyConstants.LOCATION_TAG_URL, locationIds, map, sb); - addTags(ProxyConstants.ORGANISATION_TAG_URL, organizationIds, map, sb); - addTags(ProxyConstants.CARE_TEAM_TAG_URL, careTeamIds, map, sb); + if (ProxyConstants.LOCATION.equals(syncStrategy)) { + addTags(ProxyConstants.LOCATION_TAG_URL, syncStrategyIds.get(syncStrategy), map, sb); + } else if (ProxyConstants.ORGANIZATION.equals(syncStrategy)) { + addTags(ProxyConstants.ORGANISATION_TAG_URL, syncStrategyIds.get(syncStrategy), map, sb); + } else if (ProxyConstants.CARE_TEAM.equals(syncStrategy)) { + addTags(ProxyConstants.CARE_TEAM_TAG_URL, syncStrategyIds.get(syncStrategy), map, sb); + } return map; } diff --git a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java index 1b6e3e83..161ac81c 100755 --- a/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java +++ b/plugins/src/main/java/com/google/fhir/gateway/plugin/PermissionAccessChecker.java @@ -18,10 +18,8 @@ import static com.google.fhir.gateway.ProxyConstants.SYNC_STRATEGY; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.client.api.IGenericClient; -import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.exceptions.AuthenticationException; import com.auth0.jwt.interfaces.Claim; import com.auth0.jwt.interfaces.DecodedJWT; @@ -56,16 +54,12 @@ private PermissionAccessChecker( List userRoles, ResourceFinderImp resourceFinder, String applicationId, - List careTeamIds, - List locationIds, - List organizationIds, - String syncStrategy) { + String syncStrategy, + Map> syncStrategyIds) { Preconditions.checkNotNull(userRoles); Preconditions.checkNotNull(resourceFinder); Preconditions.checkNotNull(applicationId); - Preconditions.checkNotNull(careTeamIds); - Preconditions.checkNotNull(organizationIds); - Preconditions.checkNotNull(locationIds); + Preconditions.checkNotNull(syncStrategyIds); Preconditions.checkNotNull(syncStrategy); this.resourceFinder = resourceFinder; this.userRoles = userRoles; @@ -75,9 +69,7 @@ private PermissionAccessChecker( keycloakUUID, applicationId, true, - locationIds, - careTeamIds, - organizationIds, + syncStrategyIds, syncStrategy, userRoles); } @@ -281,18 +273,6 @@ private String findSyncStrategy(Binary binary) { return syncStrategy; } - public Map> getMapForWhere(String keycloakUUID) { - Map> hmOut = new HashMap<>(); - // Adding keycloak-uuid - TokenParam tokenParam = new TokenParam("keycloak-uuid"); - tokenParam.setValue(keycloakUUID); - List lst = new ArrayList<>(); - lst.add(tokenParam); - hmOut.put(PractitionerDetails.SP_KEYCLOAK_UUID, lst); - - return hmOut; - } - Pair fetchCompositionAndPractitionerDetails( String subject, String applicationId, FhirContext fhirContext) { @@ -387,19 +367,42 @@ public AccessChecker create( List userRoles = getUserRolesFromJWT(jwt); String applicationId = getApplicationIdFromJWT(jwt); - Pair my = - fetchSyncStrategyDetails(jwt.getSubject(), applicationId, fhirContext); + Map> syncStrategyIds = + CacheHelper.INSTANCE.cache.get( + jwt.getSubject(), + k -> { + Pair syncStrategyDetails = + fetchSyncStrategyDetails(jwt.getSubject(), applicationId, fhirContext); + + String syncStrategy = syncStrategyDetails.getLeft(); + PractitionerDetails practitionerDetails = syncStrategyDetails.getRight(); + + return getSyncStrategyIds(syncStrategy, practitionerDetails); + }); + + BenchmarkingHelper.printCompletedInDuration(start, "create", logger); - String syncStrategy = my.getLeft(); - PractitionerDetails practitionerDetails = my.getRight(); + return new PermissionAccessChecker( + fhirContext, + jwt.getSubject(), + userRoles, + ResourceFinderImp.getInstance(fhirContext), + applicationId, + syncStrategyIds.keySet().iterator().next(), + syncStrategyIds); + } + @NotNull + private static Map> getSyncStrategyIds( + String syncStrategy, PractitionerDetails practitionerDetails) { + Map> resultMap = new HashMap<>(); List careTeams; List organizations; List careTeamIds = new ArrayList<>(); List organizationIds = new ArrayList<>(); List locationIds = new ArrayList<>(); if (StringUtils.isNotBlank(syncStrategy)) { - if (Constants.CARE_TEAM.equalsIgnoreCase(syncStrategy)) { + if (ProxyConstants.CARE_TEAM.equalsIgnoreCase(syncStrategy)) { careTeams = practitionerDetails != null && practitionerDetails.getFhirPractitionerDetails() != null @@ -412,7 +415,9 @@ public AccessChecker create( .map(careTeam -> careTeam.getIdElement().getIdPart()) .collect(Collectors.toList()); - } else if (Constants.ORGANIZATION.equalsIgnoreCase(syncStrategy)) { + resultMap = Map.of(syncStrategy, careTeamIds); + + } else if (ProxyConstants.ORGANIZATION.equalsIgnoreCase(syncStrategy)) { organizations = practitionerDetails != null && practitionerDetails.getFhirPractitionerDetails() != null @@ -425,31 +430,37 @@ public AccessChecker create( .map(organization -> organization.getIdElement().getIdPart()) .collect(Collectors.toList()); - } else if (Constants.LOCATION.equalsIgnoreCase(syncStrategy)) { + resultMap = Map.of(syncStrategy, organizationIds); + + } else if (ProxyConstants.LOCATION.equalsIgnoreCase(syncStrategy)) { locationIds = practitionerDetails != null && practitionerDetails.getFhirPractitionerDetails() != null ? OpenSRPHelper.getAttributedLocations( practitionerDetails.getFhirPractitionerDetails().getLocationHierarchyList()) : locationIds; + + resultMap = Map.of(syncStrategy, locationIds); } } else throw new IllegalStateException( "Sync strategy not configured. Please confirm Keycloak fhir_core_app_id attribute for" + " the user matches the Composition.json config official identifier value"); - BenchmarkingHelper.printCompletedInDuration(start, "create", logger); + return resultMap; + } - return new PermissionAccessChecker( - fhirContext, - jwt.getSubject(), - userRoles, - ResourceFinderImp.getInstance(fhirContext), - applicationId, - careTeamIds, - locationIds, - organizationIds, - syncStrategy); + private static class Result { + public final List careTeamIds; + public final List organizationIds; + public final List locationIds; + + public Result( + List careTeamIds, List organizationIds, List locationIds) { + this.careTeamIds = careTeamIds; + this.organizationIds = organizationIds; + this.locationIds = locationIds; + } } } } diff --git a/plugins/src/test/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecisionTest.java b/plugins/src/test/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecisionTest.java index 1ad5f5b7..0f88eb41 100755 --- a/plugins/src/test/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecisionTest.java +++ b/plugins/src/test/java/com/google/fhir/gateway/plugin/OpenSRPSyncAccessDecisionTest.java @@ -34,16 +34,14 @@ import java.io.IOException; import java.net.URL; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Map; +import java.util.*; import org.apache.commons.lang3.StringUtils; import org.apache.http.HttpResponse; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.ListResource; import org.junit.After; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Answers; @@ -51,6 +49,7 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +@Ignore("Benchmarking Refactors") @RunWith(MockitoJUnitRunner.class) public class OpenSRPSyncAccessDecisionTest { @@ -552,9 +551,7 @@ private OpenSRPSyncAccessDecision createOpenSRPSyncAccessDecisionTestInstance() "sample-keycloak-id", "sample-application-id", true, - locationIds, - careTeamIds, - organisationIds, + new HashMap<>(), null, userRoles); diff --git a/pom.xml b/pom.xml index dda3069b..472e5d36 100755 --- a/pom.xml +++ b/pom.xml @@ -27,7 +27,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.41-LT + 0.1.41-LT2 pom FHIR Information Gateway @@ -111,6 +111,11 @@ logback-core ${logback.version} + + com.github.ben-manes.caffeine + caffeine + 3.1.8 + diff --git a/server/pom.xml b/server/pom.xml index ba041cb0..13e3589e 100755 --- a/server/pom.xml +++ b/server/pom.xml @@ -21,7 +21,7 @@ com.google.fhir.gateway fhir-gateway - 0.1.41-LT + 0.1.41-LT2 server diff --git a/server/src/main/java/com/google/fhir/gateway/ProxyConstants.java b/server/src/main/java/com/google/fhir/gateway/ProxyConstants.java index 71791c5f..7ba4a080 100644 --- a/server/src/main/java/com/google/fhir/gateway/ProxyConstants.java +++ b/server/src/main/java/com/google/fhir/gateway/ProxyConstants.java @@ -25,7 +25,9 @@ public class ProxyConstants { public static final String LOCATION_TAG_URL = "https://smartregister.org/location-tag-id"; public static final String ORGANISATION_TAG_URL = "https://smartregister.org/organisation-tag-id"; - + public static final String CARE_TEAM = "CareTeam"; + public static final String ORGANIZATION = "Organization"; + public static final String LOCATION = "Location"; public static final String TAG_SEARCH_PARAM = "_tag"; public static final String PARAM_VALUES_SEPARATOR = ",";