3547 Interceptor hook for client-assigned IDs (#3559)

* add pointcut

* draft test

* fix test, change name and location of pointcut

* add changelog

* change check condition, improve changelog wording

* improve changelog wording

* Update hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3547-interceptor-hook-for-client-assigned-ids.yaml

Co-authored-by: Tadgh <garygrantgraham@gmail.com>

* change pointcut name

* update test

* fix test

* fix changelog

* renaming variables in test

Co-authored-by: Justin_Dar <justin.dar@smilecdr.com>
Co-authored-by: Tadgh <garygrantgraham@gmail.com>

hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/api/Pointcut.java

@@ -1345,6 +1345,34 @@ public enum Pointcut implements IPointcut {
 		"ca.uhn.fhir.rest.api.server.storage.TransactionDetails"
 	),
 
+	/**
+	 * <b>Storage Hook:</b>
+	 * Invoked before client-assigned id is created.
+	 * <p>
+	 * Hooks will have access to the contents of the resource being created
+	 * so that client-assigned ids can be allowed/denied. These changes will
+	 * be reflected in permanent storage.
+	 * </p>
+	 * Hooks may accept the following parameters:
+	 * <ul>
+	 * <li>org.hl7.fhir.instance.model.api.IBaseResource</li>
+	 * <li>
+	 * ca.uhn.fhir.rest.api.server.RequestDetails - A bean containing details about the request that is about to be processed, including details such as the
+	 * resource type and logical ID (if any) and other FHIR-specific aspects of the request which have been
+	 * pulled out of the servlet request. Note that the bean
+	 * properties are not all guaranteed to be populated, depending on how early during processing the
+	 * exception occurred.
+	 * </li>
+	 * </ul>
+	 * <p>
+	 * Hooks should return <code>void</code>.
+	 * </p>
+	 */
+	STORAGE_PRESTORAGE_CLIENT_ASSIGNED_ID(void.class,
+		"org.hl7.fhir.instance.model.api.IBaseResource",
+		"ca.uhn.fhir.rest.api.server.RequestDetails"
+	),
+
 	/**
 	 * <b>Storage Hook:</b>
 	 * Invoked before a resource will be updated, immediately before the resource

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3547-interceptor-hook-for-client-assigned-ids.yaml

@@ -0,0 +1,4 @@
+type: add
+issue: 3547
+jira: SMILE-4141
+title: "Added a new pointcut: `STORAGE_PRESTORAGE_CLIENT_ASSIGNED_ID` which is invoked when a user attempts to create a resource with a client-assigned ID. "

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java

@@ -70,6 +70,7 @@ import ca.uhn.fhir.rest.api.EncodingEnum;
 import ca.uhn.fhir.rest.api.InterceptorInvocationTimingEnum;
 import ca.uhn.fhir.rest.api.MethodOutcome;
 import ca.uhn.fhir.rest.api.PatchTypeEnum;
+import ca.uhn.fhir.rest.api.RequestTypeEnum;
 import ca.uhn.fhir.rest.api.RestOperationTypeEnum;
 import ca.uhn.fhir.rest.api.SearchContainedModeEnum;
 import ca.uhn.fhir.rest.api.ValidationModeEnum;
@@ -310,17 +311,28 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 			notifyInterceptors(RestOperationTypeEnum.CREATE, requestDetails);
 		}
 
+		String resourceIdBeforeStorage = theResource.getIdElement().getIdPart();
+		boolean resourceHadIdBeforeStorage = isNotBlank(resourceIdBeforeStorage);
+		boolean resourceIdWasServerAssigned = theResource.getUserData(JpaConstants.RESOURCE_ID_SERVER_ASSIGNED) == Boolean.TRUE;
+
+		HookParams hookParams;
+
+		// Notify interceptor for accepting/rejecting client assigned ids
+		if (!resourceIdWasServerAssigned && resourceHadIdBeforeStorage) {
+			hookParams = new HookParams()
+				.add(IBaseResource.class, theResource)
+				.add(RequestDetails.class, theRequest);
+			doCallHooks(theTransactionDetails, theRequest, Pointcut.STORAGE_PRESTORAGE_CLIENT_ASSIGNED_ID, hookParams);
+		}
+
 		// Interceptor call: STORAGE_PRESTORAGE_RESOURCE_CREATED
-		HookParams hookParams = new HookParams()
+		hookParams = new HookParams()
 			.add(IBaseResource.class, theResource)
 			.add(RequestDetails.class, theRequest)
 			.addIfMatchesType(ServletRequestDetails.class, theRequest)
 			.add(TransactionDetails.class, theTransactionDetails);
 		doCallHooks(theTransactionDetails, theRequest, Pointcut.STORAGE_PRESTORAGE_RESOURCE_CREATED, hookParams);
 
-		String resourceIdBeforeStorage = theResource.getIdElement().getIdPart();
-		boolean resourceHadIdBeforeStorage = isNotBlank(resourceIdBeforeStorage);
-		boolean resourceIdWasServerAssigned = theResource.getUserData(JpaConstants.RESOURCE_ID_SERVER_ASSIGNED) == Boolean.TRUE;
 		if (resourceHadIdBeforeStorage && !resourceIdWasServerAssigned) {
 			validateResourceIdCreation(theResource, theRequest);
 		}

hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4InterceptorTest.java

@@ -6,6 +6,7 @@ import ca.uhn.fhir.jpa.api.config.DaoConfig;
 import ca.uhn.fhir.jpa.api.model.DeleteMethodOutcome;
 import ca.uhn.fhir.jpa.test.BaseJpaR4Test;
 import ca.uhn.fhir.rest.api.server.RequestDetails;
+import ca.uhn.fhir.rest.server.exceptions.ForbiddenOperationException;
 import ca.uhn.fhir.rest.server.interceptor.IServerOperationInterceptor;
 import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
 import org.hl7.fhir.instance.model.api.IBaseResource;
@@ -32,6 +33,7 @@ import static org.hamcrest.Matchers.containsString;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.inOrder;
@@ -511,6 +513,7 @@ public class FhirResourceDaoR4InterceptorTest extends BaseJpaR4Test {
 
 		Patient p = new Patient();
 		p.setActive(false);
+
 		IIdType id = myPatientDao.create(p, mySrd).getId().toUnqualifiedVersionless();
 
 		p = myPatientDao.read(id);
@@ -518,6 +521,37 @@ public class FhirResourceDaoR4InterceptorTest extends BaseJpaR4Test {
 
 	}
 
+	@Test
+	public void testPrestorageClientAssignedIdInterceptorCanDenyClientAssignedIds() {
+		Object interceptor = new Object() {
+			@Hook(Pointcut.STORAGE_PRESTORAGE_CLIENT_ASSIGNED_ID)
+			public void prestorageClientAssignedId(IBaseResource theResource, RequestDetails theRequest) {
+				throw new ForbiddenOperationException("Client assigned id rejected.");
+			}
+		};
+		mySrdInterceptorService.registerInterceptor(interceptor);
+
+		{//Ensure interceptor is not invoked on create.
+			Patient serverAssignedPatient = new Patient();
+			try {
+				myPatientDao.create(serverAssignedPatient, mySrd);
+			} catch (ForbiddenOperationException e) {
+				fail("Interceptor was invoked, and should not have been!");
+			}
+		}
+
+		{//Ensure attempting to set a client assigned id is rejected by our interceptor.
+			try {
+				Patient clientAssignedPatient = new Patient();
+				clientAssignedPatient.setId("Patient/custom-id");
+				myPatientDao.update(clientAssignedPatient, mySrd);
+				fail();
+			} catch (ForbiddenOperationException e) {
+				assertEquals("Client assigned id rejected.", e.getMessage());
+			}
+		}
+	}
+
 	/**
 	 * Make sure that both JPA interceptors and RestfulServer interceptors can
 	 * get called