From 472984ac65cbf3e9076652cd9dabccf66e1655a9 Mon Sep 17 00:00:00 2001
From: Martha Mitran <martha.mitran@smiledigitalhealth.com>
Date: Tue, 18 Jun 2024 14:57:04 -0700
Subject: [PATCH] Document pointcut to use to modify request parameters (#6019)

* Document pointcut to use to modify request parameters. Add a test to exemplify.

* Add another alternate pointcut to use
---
 .../ca/uhn/fhir/interceptor/api/Pointcut.java | 17 ++++----
 .../provider/r4/ResourceProviderR4Test.java   | 43 +++++++++++++++++--
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java
index 016512e537d..cac6a1725a6 100644
--- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java
+++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java
@@ -321,9 +321,12 @@ public enum Pointcut implements IPointcut {
 	 * This hook is invoked before an incoming request is processed. Note that this method is called
 	 * after the server has begun preparing the response to the incoming client request.
 	 * As such, it is not able to supply a response to the incoming request in the way that
-	 * SERVER_INCOMING_REQUEST_PRE_PROCESSED and
-	 * {@link #SERVER_INCOMING_REQUEST_POST_PROCESSED}
-	 * are.
+	 * SERVER_INCOMING_REQUEST_PRE_PROCESSED and {@link #SERVER_INCOMING_REQUEST_POST_PROCESSED} are.
+	 * At this point the request has already been passed to the handler so any changes
+	 * (e.g. adding parameters) will not be considered.
+	 * If you'd like to modify request parameters before they are passed to the handler,
+	 * use {@link Pointcut#SERVER_INCOMING_REQUEST_PRE_HANDLER_SELECTED} or {@link Pointcut#SERVER_INCOMING_REQUEST_POST_PROCESSED}.
+	 * If you are attempting to modify a search before it occurs, use {@link Pointcut#STORAGE_PRESEARCH_REGISTERED}.
 	 * <p>
 	 * Hooks may accept the following parameters:
 	 * <ul>
@@ -902,7 +905,6 @@ public enum Pointcut implements IPointcut {
 	 * canonical subscription such as adding headers, modifying the channel
 	 * endpoint, etc.
 	 * Furthermore, you may modify the outgoing message wrapper, for example adding headers via ResourceModifiedJsonMessage field.
-	 *
 	 * </p>
 	 * Hooks may accept the following parameters:
 	 * <ul>
@@ -1122,7 +1124,6 @@ public enum Pointcut implements IPointcut {
 	 * <b>Storage Hook:</b>
 	 * Invoked when a Bulk Export job is being kicked off. Hook methods may modify
 	 * the request, or raise an exception to prevent it from being initiated.
-	 *
 	 * This hook is not guaranteed to be called before permission checks, and so
 	 * anu implementers should be cautious of changing the options in ways that would
 	 * affect permissions.
@@ -1192,7 +1193,7 @@ public enum Pointcut implements IPointcut {
 
 	/**
 	 * <b>Storage Hook:</b>
-	 * Invoked when a set of resources are about to be deleted and expunged via url like http://localhost/Patient?active=false&_expunge=true
+	 * Invoked when a set of resources are about to be deleted and expunged via url like {@code http://localhost/Patient?active=false&_expunge=true}.
 	 * <p>
 	 * Hooks may accept the following parameters:
 	 * </p>
@@ -1228,7 +1229,7 @@ public enum Pointcut implements IPointcut {
 
 	/**
 	 * <b>Storage Hook:</b>
-	 * Invoked when a batch of resource pids are about to be deleted and expunged via url like http://localhost/Patient?active=false&_expunge=true
+	 * Invoked when a batch of resource pids are about to be deleted and expunged via url like {@code http://localhost/Patient?active=false&_expunge=true}.
 	 * <p>
 	 * Hooks may accept the following parameters:
 	 * </p>
@@ -2957,7 +2958,6 @@ public enum Pointcut implements IPointcut {
 			"ca.uhn.fhir.rest.server.servlet.ServletRequestDetails",
 			"ca.uhn.fhir.jpa.util.SqlQueryList"),
 
-	@Deprecated(since = "7.2.0 - Use STORAGE_BINARY_ASSIGN_BINARY_CONTENT_ID_PREFIX instead.")
 	/**
 	 * <b> Deprecated but still supported.  Will eventually be removed.  <code>Please use Pointcut.STORAGE_BINARY_ASSIGN_BINARY_CONTENT_ID_PREFIX</code>  </b>
 	 * <b> Binary Blob Prefix Assigning Hook:</b>
@@ -2980,6 +2980,7 @@ public enum Pointcut implements IPointcut {
 	 * Hooks should return <code>String</code>, which represents the full prefix to be applied to the blob.
 	 * </p>
 	 */
+	@Deprecated(since = "7.2.0 - Use STORAGE_BINARY_ASSIGN_BINARY_CONTENT_ID_PREFIX instead.")
 	STORAGE_BINARY_ASSIGN_BLOB_ID_PREFIX(
 			String.class,
 			"ca.uhn.fhir.rest.api.server.RequestDetails",
diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java
index f33bbcd2c28..14fb2bb0d7e 100644
--- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java
+++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java
@@ -3,6 +3,8 @@ package ca.uhn.fhir.jpa.provider.r4;
 import ca.uhn.fhir.context.RuntimeSearchParam;
 import ca.uhn.fhir.i18n.HapiLocalizer;
 import ca.uhn.fhir.i18n.Msg;
+import ca.uhn.fhir.interceptor.api.Hook;
+import ca.uhn.fhir.interceptor.api.Pointcut;
 import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
 import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
 import ca.uhn.fhir.jpa.dao.data.ISearchDao;
@@ -44,12 +46,16 @@ import ca.uhn.fhir.rest.gclient.NumberClientParam;
 import ca.uhn.fhir.rest.gclient.StringClientParam;
 import ca.uhn.fhir.rest.param.DateRangeParam;
 import ca.uhn.fhir.rest.param.ParamPrefixEnum;
+import ca.uhn.fhir.rest.param.TokenParam;
+import ca.uhn.fhir.rest.param.TokenParamModifier;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
 import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
 import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
 import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor;
+import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
+import ca.uhn.fhir.rest.server.util.ICachedSearchDetails;
 import ca.uhn.fhir.util.ClasspathUtil;
 import ca.uhn.fhir.util.StopWatch;
 import ca.uhn.fhir.util.TestUtil;
@@ -198,15 +204,12 @@ import static ca.uhn.fhir.util.TestUtil.sleepAtLeast;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
-import static org.junit.jupiter.api.Assertions.fail;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
-
 import static org.mockito.Mockito.when;
 
 @SuppressWarnings("Duplicates")
@@ -5324,6 +5327,40 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
 
 	}
 
+	@Test
+	public void testSearchWithParameterAddedInInterceptor() {
+		Object interceptor = new Object() {
+			@Hook(Pointcut.STORAGE_PRESEARCH_REGISTERED)
+			public void storagePreSearchRegistered(
+					ICachedSearchDetails theCachedSearchDetails,
+					RequestDetails theRequestDetails,
+					ServletRequestDetails theServletRequestDetails,
+					SearchParameterMap theSearchParameterMap) {
+				theSearchParameterMap.add("_security", new TokenParam("http://system", "security1").setModifier(TokenParamModifier.NOT));
+			}
+		};
+		myInterceptorRegistry.registerInterceptor(interceptor);
+
+		try {
+			final Patient patient1 = new Patient().setActive(true);
+			patient1.getMeta().addSecurity("http://system", "security1", "Tag 1");
+			MethodOutcome outcome1 = myPatientDao.create(patient1, mySrd);
+			assertTrue(outcome1.getCreated());
+
+			final Patient patient2 = new Patient().setActive(true);
+			patient2.getMeta().addSecurity("http://system", "security2", "Tag 2");
+			MethodOutcome outcome2 = myPatientDao.create(patient2, mySrd);
+			assertTrue(outcome2.getCreated());
+			String idForPatient2 = outcome2.getId().toUnqualifiedVersionless().getValue();
+
+			IBaseBundle bundle = myClient.search().forResource("Patient").execute();
+			List<String> ids = toUnqualifiedVersionlessIdValues(bundle);
+			assertThat(ids).containsExactly(idForPatient2);
+		} finally {
+			myInterceptorRegistry.unregisterInterceptor(interceptor);
+		}
+	}
+
 	@Test
 	public void testSelfReferentialInclude() {
 		Location loc1 = new Location();