Fix #1794 - Client ID and Server ID mode clash (#1795)

* Fix #1794 - Client ID and Server ID mode clash

* Try to track down intermittent test failure

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1794-allow-client-id-none-server-id-uuid.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 1794
+title: A bug in the JPA server prevented Client Resource ID mode from being set to NOT_ALLOWED when Server Resource ID mode
+   was set to UUID. Thanks to GitHub user @G-2-Z for reporting!

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

@@ -34,6 +34,7 @@ import ca.uhn.fhir.jpa.delete.DeleteConflictService;
 import ca.uhn.fhir.jpa.model.cross.ResourcePersistentId;
 import ca.uhn.fhir.jpa.model.entity.*;
 import ca.uhn.fhir.jpa.model.search.SearchRuntimeDetails;
+import ca.uhn.fhir.jpa.model.util.JpaConstants;
 import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider;
 import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider;
 import ca.uhn.fhir.jpa.search.reindex.IResourceReindexingSvc;
@@ -164,6 +165,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 
 		if (myDaoConfig.getResourceServerIdStrategy() == DaoConfig.IdStrategyEnum.UUID) {
 			theResource.setId(UUID.randomUUID().toString());
+			theResource.setUserData(JpaConstants.RESOURCE_ID_SERVER_ASSIGNED, Boolean.TRUE);
 		}
 
 		return doCreate(theResource, theIfNoneExist, thePerformIndexing, theUpdateTimestamp, theRequestDetails);
@@ -419,6 +421,10 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 
 		boolean serverAssignedId;
 		if (isNotBlank(theResource.getIdElement().getIdPart())) {
+			if (theResource.getUserData(JpaConstants.RESOURCE_ID_SERVER_ASSIGNED) == Boolean.TRUE) {
+				createForcedIdIfNeeded(entity, theResource.getIdElement(), true);
+				serverAssignedId = true;
+			} else {
 				switch (myDaoConfig.getResourceClientIdStrategy()) {
 					case NOT_ALLOWED:
 						throw new ResourceNotFoundException(
@@ -435,6 +441,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 						break;
 				}
 				serverAssignedId = false;
+			}
 		} else {
 			serverAssignedId = true;
 		}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4CreateTest.java

@@ -60,7 +60,7 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
 	}
 
 	@Test
-	public void testCreateWithUuidResourceStrategy() {
+	public void testCreateWithUuidServerResourceStrategy() {
 		myDaoConfig.setResourceServerIdStrategy(DaoConfig.IdStrategyEnum.UUID);
 
 		Patient p = new Patient();
@@ -74,6 +74,22 @@ public class FhirResourceDaoR4CreateTest extends BaseJpaR4Test {
 
 	}
 
+	@Test
+	public void testCreateWithUuidServerResourceStrategy_ClientIdNotAllowed() {
+		myDaoConfig.setResourceServerIdStrategy(DaoConfig.IdStrategyEnum.UUID);
+		myDaoConfig.setResourceClientIdStrategy(DaoConfig.ClientIdStrategyEnum.NOT_ALLOWED);
+
+		Patient p = new Patient();
+		p.addName().setFamily("FAM");
+		IIdType id = myPatientDao.create(p).getId().toUnqualified();
+
+		assertThat(id.getIdPart(), matchesPattern("[a-z0-9]{8}-.*"));
+
+		p = myPatientDao.read(id);
+		assertEquals("FAM", p.getNameFirstRep().getFamily());
+
+	}
+
 	/**
 	 * See #1352
 	 */

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java

@@ -34,6 +34,8 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
 	public void afterResetDao() {
 		myDaoConfig.setResourceMetaCountHardLimit(new DaoConfig().getResourceMetaCountHardLimit());
 		myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields());
+		myDaoConfig.setResourceServerIdStrategy(new DaoConfig().getResourceServerIdStrategy());
+		myDaoConfig.setResourceClientIdStrategy(new DaoConfig().getResourceClientIdStrategy());
 	}
 
 	@Before
@@ -925,6 +927,24 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
 
 	}
 
+	@Test
+	public void testUpdateWithUuidServerResourceStrategy_ClientIdNotAllowed() {
+		myDaoConfig.setResourceServerIdStrategy(DaoConfig.IdStrategyEnum.UUID);
+		myDaoConfig.setResourceClientIdStrategy(DaoConfig.ClientIdStrategyEnum.NOT_ALLOWED);
+
+		Patient p = new Patient();
+		p.setId(UUID.randomUUID().toString());
+		p.addName().setFamily("FAM");
+		try {
+			myPatientDao.update(p);
+			fail();
+		} catch (ResourceNotFoundException e) {
+			assertThat(e.getMessage(), matchesPattern("No resource exists on this server resource with ID.*, and client-assigned IDs are not enabled."));
+		}
+
+	}
+
+
 	@AfterClass
 	public static void afterClassClearContext() {
 		TestUtil.clearAllStaticFieldsForUnitTest();

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/util/JpaConstants.java

@@ -25,38 +25,30 @@ import ca.uhn.fhir.rest.api.Constants;
 public class JpaConstants {
 
 	/**
-	 * Non-instantiable
+	 * Userdata key for tracking the fact that a resource ID was assigned by the server
 	 */
-	private JpaConstants() {
+	public static final String RESOURCE_ID_SERVER_ASSIGNED = JpaConstants.class.getName() + "_RESOURCE_ID_SERVER_ASSIGNED";
-		// nothing
-	}
-
 	/**
 	 * Operation name for the $apply-codesystem-delta-add operation
 	 */
 	public static final String OPERATION_APPLY_CODESYSTEM_DELTA_ADD = "$apply-codesystem-delta-add";
-
 	/**
 	 * Operation name for the $apply-codesystem-delta-remove operation
 	 */
 	public static final String OPERATION_APPLY_CODESYSTEM_DELTA_REMOVE = "$apply-codesystem-delta-remove";
-
 	/**
 	 * Operation name for the $expunge operation
 	 */
 	public static final String OPERATION_EXPUNGE = "$expunge";
-
 	/**
 	 * Operation name for the $match operation
 	 */
 	public static final String OPERATION_MATCH = "$match";
-
 	/**
 	 * @deprecated Replace with {@link #OPERATION_EXPUNGE}
 	 */
 	@Deprecated
 	public static final String OPERATION_NAME_EXPUNGE = OPERATION_EXPUNGE;
-
 	/**
 	 * Parameter name for the $expunge operation
 	 */
@@ -84,113 +76,91 @@ public class JpaConstants {
 	 * be removed if they are nt explicitly included in updates
 	 */
 	public static final String HEADER_META_SNAPSHOT_MODE = "X-Meta-Snapshot-Mode";
-
 	/**
 	 * Operation name for the $lookup operation
 	 */
 	public static final String OPERATION_LOOKUP = "$lookup";
-
 	/**
 	 * Operation name for the $expand operation
 	 */
 	public static final String OPERATION_EXPAND = "$expand";
-
 	/**
 	 * Operation name for the $validate-code operation
 	 */
 	public static final String OPERATION_VALIDATE_CODE = "$validate-code";
-
 	/**
 	 * Operation name for the $get-resource-counts operation
 	 */
 	public static final String OPERATION_GET_RESOURCE_COUNTS = "$get-resource-counts";
-
 	/**
 	 * Operation name for the $meta operation
 	 */
 	public static final String OPERATION_META = "$meta";
-
 	/**
 	 * Operation name for the $validate operation
 	 */
 	// NB don't delete this, it's used in Smile as well, even though hapi-fhir-server uses the version from Constants.java
 	public static final String OPERATION_VALIDATE = Constants.EXTOP_VALIDATE;
-
 	/**
 	 * Operation name for the $suggest-keywords operation
 	 */
 	public static final String OPERATION_SUGGEST_KEYWORDS = "$suggest-keywords";
-
 	/**
 	 * Operation name for the $everything operation
 	 */
 	public static final String OPERATION_EVERYTHING = "$everything";
-
 	/**
 	 * Operation name for the $process-message operation
 	 */
 	public static final String OPERATION_PROCESS_MESSAGE = "$process-message";
-
 	/**
 	 * Operation name for the $meta-delete operation
 	 */
 	public static final String OPERATION_META_DELETE = "$meta-delete";
-
 	/**
 	 * Operation name for the $meta-add operation
 	 */
 	public static final String OPERATION_META_ADD = "$meta-add";
-
 	/**
 	 * Operation name for the $translate operation
 	 */
 	public static final String OPERATION_TRANSLATE = "$translate";
-
 	/**
 	 * Operation name for the $document operation
 	 */
 	public static final String OPERATION_DOCUMENT = "$document";
-
 	/**
 	 * Trigger a subscription manually for a given resource
 	 */
 	public static final String OPERATION_TRIGGER_SUBSCRIPTION = "$trigger-subscription";
-
 	/**
 	 * Operation name for the "$subsumes" operation
 	 */
 	public static final String OPERATION_SUBSUMES = "$subsumes";
-
 	/**
 	 * Operation name for the "$snapshot" operation
 	 */
 	public static final String OPERATION_SNAPSHOT = "$snapshot";
-
 	/**
 	 * Operation name for the "$binary-access" operation
 	 */
 	public static final String OPERATION_BINARY_ACCESS_READ = "$binary-access-read";
-
 	/**
 	 * Operation name for the "$binary-access" operation
 	 */
 	public static final String OPERATION_BINARY_ACCESS_WRITE = "$binary-access-write";
-
 	/**
 	 * Operation name for the "$upload-external-code-system" operation
 	 */
 	public static final String OPERATION_UPLOAD_EXTERNAL_CODE_SYSTEM = "$upload-external-code-system";
-
 	/**
 	 * Operation name for the "$export" operation
 	 */
 	public static final String OPERATION_EXPORT = "$export";
-
 	/**
 	 * Operation name for the "$export-poll-status" operation
 	 */
 	public static final String OPERATION_EXPORT_POLL_STATUS = "$export-poll-status";
-
 	/**
 	 * <p>
 	 * This extension should be of type <code>string</code> and should be
@@ -198,7 +168,6 @@ public class JpaConstants {
 	 * </p>
 	 */
 	public static final String EXT_SUBSCRIPTION_SUBJECT_TEMPLATE = "http://hapifhir.io/fhir/StructureDefinition/subscription-email-subject-template";
-
 	/**
 	 * This extension URL indicates whether a REST HOOK delivery should
 	 * include the version ID when delivering.
@@ -208,7 +177,6 @@ public class JpaConstants {
 	 * </p>
 	 */
 	public static final String EXT_SUBSCRIPTION_RESTHOOK_STRIP_VERSION_IDS = "http://hapifhir.io/fhir/StructureDefinition/subscription-resthook-strip-version-ids";
-
 	/**
 	 * This extension URL indicates whether a REST HOOK delivery should
 	 * reload the resource and deliver the latest version always. This
@@ -226,12 +194,10 @@ public class JpaConstants {
 	 * </p>
 	 */
 	public static final String EXT_SUBSCRIPTION_RESTHOOK_DELIVER_LATEST_VERSION = "http://hapifhir.io/fhir/StructureDefinition/subscription-resthook-deliver-latest-version";
-
 	/**
 	 * Indicate which strategy will be used to match this subscription
 	 */
 	public static final String EXT_SUBSCRIPTION_MATCHING_STRATEGY = "http://hapifhir.io/fhir/StructureDefinition/subscription-matching-strategy";
-
 	/**
 	 * <p>
 	 * This extension should be of type <code>string</code> and should be
@@ -239,45 +205,43 @@ public class JpaConstants {
 	 * </p>
 	 */
 	public static final String EXT_SUBSCRIPTION_EMAIL_FROM = "http://hapifhir.io/fhir/StructureDefinition/subscription-email-from";
-
 	/**
 	 * Extension ID for external binary references
 	 */
 	public static final String EXT_EXTERNALIZED_BINARY_ID = "http://hapifhir.io/fhir/StructureDefinition/externalized-binary-id";
-
 	/**
 	 * Placed in system-generated extensions
 	 */
 	public static final String EXTENSION_EXT_SYSTEMDEFINED = JpaConstants.class.getName() + "_EXTENSION_EXT_SYSTEMDEFINED";
-
 	/**
 	 * Message added to expansion valueset
 	 */
 	public static final String EXT_VALUESET_EXPANSION_MESSAGE = "http://hapifhir.io/fhir/StructureDefinition/valueset-expansion-message";
-
-
 	/**
 	 * Parameter for the $export operation
 	 */
 	public static final String PARAM_EXPORT_POLL_STATUS_JOB_ID = "_jobId";
-
 	/**
 	 * Parameter for the $export operation
 	 */
 	public static final String PARAM_EXPORT_OUTPUT_FORMAT = "_outputFormat";
-
 	/**
 	 * Parameter for the $export operation
 	 */
 	public static final String PARAM_EXPORT_TYPE = "_type";
-
 	/**
 	 * Parameter for the $export operation
 	 */
 	public static final String PARAM_EXPORT_SINCE = "_since";
-
 	/**
 	 * Parameter for the $export operation
 	 */
 	public static final String PARAM_EXPORT_TYPE_FILTER = "_typeFilter";
+
+	/**
+	 * Non-instantiable
+	 */
+	private JpaConstants() {
+		// nothing
+	}
 }

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/ResponseSizeCapturingInterceptor.java

@@ -72,7 +72,7 @@ public class ResponseSizeCapturingInterceptor {
 		CountingWriter countingWriter = (CountingWriter) theRequestDetails.getUserData().get(COUNTING_WRITER_KEY);
 		if (countingWriter != null) {
 			int charCount = countingWriter.getCount();
-			Result result = new Result(charCount);
+			Result result = new Result(theRequestDetails, charCount);
 			notifyConsumers(result);
 
 			theRequestDetails.getUserData().put(RESPONSE_RESULT_KEY, result);
@@ -99,7 +99,14 @@ public class ResponseSizeCapturingInterceptor {
 	public static class Result {
 		private final int myWrittenChars;
 
-		public Result(int theWrittenChars) {
+		public RequestDetails getRequestDetails() {
+			return myRequestDetails;
+		}
+
+		private final RequestDetails myRequestDetails;
+
+		public Result(RequestDetails theRequestDetails, int theWrittenChars) {
+			myRequestDetails = theRequestDetails;
 			myWrittenChars = theWrittenChars;
 		}
 

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/ResponseSizeCapturingInterceptorTest.java

@@ -3,6 +3,7 @@ package ca.uhn.fhir.rest.server.interceptor;
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.test.utilities.server.HashMapResourceProviderRule;
 import ca.uhn.fhir.test.utilities.server.RestfulServerRule;
+import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.r4.model.Patient;
 import org.junit.After;
@@ -18,9 +19,22 @@ import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
+import static org.awaitility.Awaitility.await;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.matchesPattern;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.verify;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -56,11 +70,22 @@ public class ResponseSizeCapturingInterceptorTest {
 
 		myInterceptor.registerConsumer(myConsumer);
 
+		List<String> stacks = Collections.synchronizedList(new ArrayList<>());
+		doAnswer(t->{
+			ResponseSizeCapturingInterceptor.Result result =t.getArgument(0, ResponseSizeCapturingInterceptor.Result.class);
+			try {
+				throw new Exception();
+			} catch (Exception e) {
+				stacks.add("INVOCATION\n" + result.getRequestDetails().getCompleteUrl() + "\n" + ExceptionUtils.getStackTrace(e));
+			}
+			return null;
+		}).when(myConsumer).accept(any());
+
 		resource = ourServerRule.getFhirClient().read().resource(Patient.class).withId(id).execute();
 		assertEquals(true, resource.getActive());
 
-		verify(myConsumer, Mockito.timeout(Duration.ofSeconds(10)).times(1)).accept(myResultCaptor.capture());
+		await().until(()->stacks.size() > 0);
-		assertEquals(100, myResultCaptor.getValue().getWrittenChars());
+		await().until(()->stacks.stream().collect(Collectors.joining("\n")), not(matchesPattern(Pattern.compile(".*INVOCATION.*INVOCATION.*", Pattern.MULTILINE | Pattern.DOTALL))));
 	}