Documentation for request mutation change. Added test cases.

plugins/src/main/java/com/google/fhir/gateway/plugin/AccessGrantedAndUpdateList.java

@@ -65,7 +65,7 @@ private AccessGrantedAndUpdateList(
   }
 
   @Override
-  public RequestMutation preprocess(RequestDetailsReader requestDetailsReader) {
+  public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) {
     return null;
   }
 

server/src/main/java/com/google/fhir/gateway/BearerAuthorizationInterceptor.java

@@ -272,7 +272,7 @@ public boolean authorizeRequest(RequestDetails requestDetails) {
       return false;
     }
     AccessDecision outcome = checkAuthorization(requestDetails);
-    preprocessRequest(servletDetails, outcome);
+    mutateRequest(requestDetails, outcome);
     logger.debug("Authorized request path " + requestPath);
     try {
       HttpResponse response = fhirClient.handleRequest(servletDetails);
@@ -394,20 +394,16 @@ private void serveWellKnown(ServletRequestDetails request) {
     }
   }
 
-  private void preprocessRequest(
-      ServletRequestDetails servletRequestDetails, AccessDecision accessDecision) {
+  @VisibleForTesting
+  static void mutateRequest(RequestDetails requestDetails, AccessDecision accessDecision) {
     RequestMutation mutation =
-        accessDecision.preprocess(new RequestDetailsToReader(servletRequestDetails));
+        accessDecision.getRequestMutation(new RequestDetailsToReader(requestDetails));
     if (mutation == null || CollectionUtils.isEmpty(mutation.getQueryParams())) {
-      return ;
+      return;
     }
 
     mutation
         .getQueryParams()
-        .forEach((key, value) -> servletRequestDetails.addParameter(
-            key, value.toArray(new String[0])));
-
-    // TODO update the query params in search by Post
-
+        .forEach((key, value) -> requestDetails.addParameter(key, value.toArray(new String[0])));
   }
 }

server/src/main/java/com/google/fhir/gateway/CapabilityPostProcessor.java

@@ -55,7 +55,7 @@ static synchronized CapabilityPostProcessor getInstance(FhirContext fhirContext)
   }
 
   @Override
-  public RequestMutation preprocess(RequestDetailsReader requestDetailsReader) {
+  public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) {
     return null;
   }
 

server/src/main/java/com/google/fhir/gateway/interfaces/AccessDecision.java

@@ -16,22 +16,27 @@
 package com.google.fhir.gateway.interfaces;
 
 import java.io.IOException;
+import javax.annotation.Nullable;
 import org.apache.http.HttpResponse;
 
 public interface AccessDecision {
 
+  /** @return true iff access was granted. */
+  boolean canAccess();
+
   /**
    * Allows the incoming request mutation based on the access decision.
    *
+   * <p>Response is used to mutate the incoming request before executing the FHIR operation. We
+   * currently only support query parameters update for GET Http method. This is expected to be
+   * called after checking the access using @canAccess method. Mutating the request before checking
+   * access can have side effect of wrong access check.
+   *
    * @param requestDetailsReader details about the resource and operation requested
-   * @return the mutation to be applied on the incoming request
+   * @return mutation to be applied on the incoming request or null if no mutation required
    */
-  RequestMutation preprocess(RequestDetailsReader requestDetailsReader);
-
-  /**
-   * @return true iff access was granted.
-   */
-  boolean canAccess();
+  @Nullable
+  RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader);
 
   /**
    * Depending on the outcome of the FHIR operations, this does any post-processing operations that

server/src/main/java/com/google/fhir/gateway/interfaces/NoOpAccessDecision.java

@@ -26,7 +26,7 @@ public NoOpAccessDecision(boolean accessGranted) {
   }
 
   @Override
-  public RequestMutation preprocess(RequestDetailsReader requestDetailsReader) {
+  public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) {
     return null;
   }
 

server/src/main/java/com/google/fhir/gateway/interfaces/RequestMutation.java

@@ -11,6 +11,10 @@
 @Getter
 public class RequestMutation {
 
-  // Additional query parameters that should be added to the outgoing FHIR request
+  // Additional query parameters and list of values for a parameter that should be added to the
+  // outgoing FHIR request.
+  // New values overwrites the old one if there is a conflict for a request param (i.e. a returned
+  // parameter in RequestMutation is already present in the original request).
+  // Old parameter values should be explicitly retained while mutating values for that parameter.
   @Builder.Default Map<String, List<String>> queryParams = new HashMap<>();
 }

server/src/test/java/com/google/fhir/gateway/BearerAuthorizationInterceptorTest.java

@@ -16,6 +16,7 @@
 package com.google.fhir.gateway;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.mockito.ArgumentMatchers.any;
@@ -42,6 +43,7 @@
 import com.google.fhir.gateway.interfaces.AccessDecision;
 import com.google.fhir.gateway.interfaces.NoOpAccessDecision;
 import com.google.fhir.gateway.interfaces.RequestDetailsReader;
+import com.google.fhir.gateway.interfaces.RequestMutation;
 import com.google.gson.Gson;
 import java.io.IOException;
 import java.io.StringWriter;
@@ -56,6 +58,8 @@
 import java.security.interfaces.RSAPrivateKey;
 import java.security.interfaces.RSAPublicKey;
 import java.util.Base64;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import javax.servlet.http.HttpServletRequest;
 import org.apache.http.HttpResponse;
@@ -352,4 +356,39 @@ public void deniedRequest() throws IOException {
 
     testInstance.authorizeRequest(requestMock);
   }
+
+  @Test
+  public void mutateRequest() {
+    ServletRequestDetails requestDetails = new ServletRequestDetails();
+    requestDetails.addParameter("param1", new String[] {"param1-value1"});
+    requestDetails.addParameter("param2", new String[] {"param2-value1"});
+
+    HashMap<String, List<String>> paramMutations = new HashMap<>();
+    paramMutations.put("param1", List.of("param1-value2"));
+    paramMutations.put("param3", List.of("param3-value1", "param3-value2"));
+    AccessDecision mutableAccessDecision =
+        new AccessDecision() {
+          public boolean canAccess() {
+            return true;
+          }
+
+          public RequestMutation getRequestMutation(RequestDetailsReader requestDetailsReader) {
+            return RequestMutation.builder().queryParams(paramMutations).build();
+          }
+
+          public String postProcess(HttpResponse response) throws IOException {
+            return null;
+          }
+        };
+
+    BearerAuthorizationInterceptor.mutateRequest(requestDetails, mutableAccessDecision);
+
+    assertThat(
+        requestDetails.getParameters().get("param1"), arrayContainingInAnyOrder("param1-value2"));
+    assertThat(
+        requestDetails.getParameters().get("param2"), arrayContainingInAnyOrder("param2-value1"));
+    assertThat(
+        requestDetails.getParameters().get("param3"),
+        arrayContainingInAnyOrder("param3-value2", "param3-value1"));
+  }
 }