Clean up references in IPS documents (#4509)

* Clean up references in IPS documents

* Add changelog

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4509-fix-refs-in-ips.yaml

@@ -0,0 +1,6 @@
+---
+type: fix
+issue: 4509
+title: "References to the patient/subject in IPS documents generated using the $summary
+  operation were not replaced with the bundle-local UUID assigned to the patient. Also,
+  some dangling references were left in the generated bundle. This has been corrected."

hapi-fhir-jpaserver-ips/src/main/java/ca/uhn/fhir/jpa/ips/generator/IpsGeneratorSvcImpl.java

@@ -114,16 +114,18 @@ public class IpsGeneratorSvcImpl implements IIpsGeneratorSvc {
 	}
 
 	private IBaseBundle generateIpsForPatient(RequestDetails theRequestDetails, IBaseResource thePatient) {
-		IIdType originalSubjectId = myFhirContext.getVersion().newIdType().setValue(thePatient.getIdElement().getValue());
+		IIdType originalSubjectId = myFhirContext.getVersion().newIdType().setValue(thePatient.getIdElement().getValue()).toUnqualifiedVersionless();
 		massageResourceId(null, thePatient);
 		IpsContext context = new IpsContext(thePatient, originalSubjectId);
 
+		ResourceInclusionCollection globalResourcesToInclude = new ResourceInclusionCollection();
+		globalResourcesToInclude.addResourceIfNotAlreadyPresent(thePatient, originalSubjectId.getValue());
+
 		IBaseResource author = myGenerationStrategy.createAuthor();
 		massageResourceId(context, author);
 
 		CompositionBuilder compositionBuilder = createComposition(thePatient, context, author);
-
+		determineInclusions(theRequestDetails, originalSubjectId, context, compositionBuilder, globalResourcesToInclude);
-		ResourceInclusionCollection globalResourcesToInclude = determineInclusions(theRequestDetails, originalSubjectId, context, compositionBuilder);
 
 		IBaseResource composition = compositionBuilder.getComposition();
 
@@ -131,10 +133,10 @@ public class IpsGeneratorSvcImpl implements IIpsGeneratorSvc {
 		CustomThymeleafNarrativeGenerator generator = newNarrativeGenerator(globalResourcesToInclude);
 		generator.populateResourceNarrative(myFhirContext, composition);
 
-		return createCompositionDocument(thePatient, author, composition, globalResourcesToInclude);
+		return createCompositionDocument(author, composition, globalResourcesToInclude);
 	}
 
-	private IBaseBundle createCompositionDocument(IBaseResource thePatient, IBaseResource author, IBaseResource composition, ResourceInclusionCollection theResourcesToInclude) {
+	private IBaseBundle createCompositionDocument(IBaseResource author, IBaseResource composition, ResourceInclusionCollection theResourcesToInclude) {
 		BundleBuilder bundleBuilder = new BundleBuilder(myFhirContext);
 		bundleBuilder.setType(Bundle.BundleType.DOCUMENT.toCode());
 		bundleBuilder.setIdentifier("urn:ietf:rfc:4122", UUID.randomUUID().toString());
@@ -143,9 +145,6 @@ public class IpsGeneratorSvcImpl implements IIpsGeneratorSvc {
 		// Add composition to document
 		bundleBuilder.addDocumentEntry(composition);
 
-		// Add subject to document
-		bundleBuilder.addDocumentEntry(thePatient);
-
 		// Add inclusion candidates
 		for (IBaseResource next : theResourcesToInclude.getResources()) {
 			bundleBuilder.addDocumentEntry(next);
@@ -158,13 +157,12 @@ public class IpsGeneratorSvcImpl implements IIpsGeneratorSvc {
 	}
 
 	@Nonnull
-	private ResourceInclusionCollection determineInclusions(RequestDetails theRequestDetails, IIdType originalSubjectId, IpsContext context, CompositionBuilder theCompositionBuilder) {
+	private ResourceInclusionCollection determineInclusions(RequestDetails theRequestDetails, IIdType originalSubjectId, IpsContext context, CompositionBuilder theCompositionBuilder, ResourceInclusionCollection theGlobalResourcesToInclude) {
-		ResourceInclusionCollection globalResourcesToInclude = new ResourceInclusionCollection();
 		SectionRegistry sectionRegistry = myGenerationStrategy.getSectionRegistry();
 		for (SectionRegistry.Section nextSection : sectionRegistry.getSections()) {
-			determineInclusionsForSection(theRequestDetails, originalSubjectId, context, theCompositionBuilder, globalResourcesToInclude, nextSection);
+			determineInclusionsForSection(theRequestDetails, originalSubjectId, context, theCompositionBuilder, theGlobalResourcesToInclude, nextSection);
 		}
-		return globalResourcesToInclude;
+		return theGlobalResourcesToInclude;
 	}
 
 	private void determineInclusionsForSection(RequestDetails theRequestDetails, IIdType theOriginalSubjectId, IpsContext theIpsContext, CompositionBuilder theCompositionBuilder, ResourceInclusionCollection theGlobalResourcesToInclude, SectionRegistry.Section theSection) {
@@ -241,8 +239,15 @@ public class IpsGeneratorSvcImpl implements IIpsGeneratorSvc {
 					if (isNotBlank(existingReference)) {
 						existingReference = new IdType(existingReference).toUnqualifiedVersionless().getValue();
 						String replacement = theGlobalResourcesToInclude.getIdSubstitution(existingReference);
-						if (isNotBlank(replacement) && !replacement.equals(existingReference)) {
+						if (isNotBlank(replacement)) {
-							nextReference.getResourceReference().setReference(replacement);
+							if (!replacement.equals(existingReference)) {
+								nextReference.getResourceReference().setReference(replacement);
+							}
+						} else if (theGlobalResourcesToInclude.getResourceById(existingReference) == null) {
+							// If this reference doesn't point to something we have actually
+							// included in the bundle, clear the reference.
+							nextReference.getResourceReference().setReference(null);
+							nextReference.getResourceReference().setResource(null);
 						}
 					}
 				}
@@ -541,7 +546,11 @@ public class IpsGeneratorSvcImpl implements IIpsGeneratorSvc {
 		}
 
 		public IBaseResource getResourceById(IIdType theReference) {
-			return myIdToResource.get(theReference.toUnqualifiedVersionless().getValue());
+			return getResourceById(theReference.toUnqualifiedVersionless().getValue());
+		}
+
+		public IBaseResource getResourceById(String theReference) {
+			return myIdToResource.get(theReference);
 		}
 
 		@Nullable

hapi-fhir-jpaserver-ips/src/test/java/ca/uhn/fhir/jpa/ips/generator/IpsGenerationTest.java

@@ -1,5 +1,6 @@
 package ca.uhn.fhir.jpa.ips.generator;
 
+import ca.uhn.fhir.batch2.jobs.models.BatchResourceId;
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
 import ca.uhn.fhir.jpa.ips.api.IIpsGenerationStrategy;
@@ -8,8 +9,11 @@ import ca.uhn.fhir.jpa.ips.strategy.DefaultIpsGenerationStrategy;
 import ca.uhn.fhir.jpa.model.util.JpaConstants;
 import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test;
 import ca.uhn.fhir.util.ClasspathUtil;
+import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.r4.model.Bundle;
+import org.hl7.fhir.r4.model.MedicationStatement;
 import org.hl7.fhir.r4.model.Parameters;
+import org.hl7.fhir.r4.model.Patient;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -18,7 +22,10 @@ import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.test.context.ContextConfiguration;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.matchesPattern;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
 @ContextConfiguration(classes = {IpsGenerationTest.IpsConfig.class})
 public class IpsGenerationTest extends BaseResourceProviderR4Test {
@@ -28,12 +35,12 @@ public class IpsGenerationTest extends BaseResourceProviderR4Test {
 
 	@BeforeEach
 	public void beforeEach() {
-		myServer.withServer(t->t.registerProvider(myIpsOperationProvider));
+		myServer.withServer(t -> t.registerProvider(myIpsOperationProvider));
 	}
 
 	@AfterEach
 	public void afterEach() {
-		myServer.withServer(t->t.unregisterProvider(myIpsOperationProvider));
+		myServer.withServer(t -> t.unregisterProvider(myIpsOperationProvider));
 	}
 
 
@@ -55,10 +62,27 @@ public class IpsGenerationTest extends BaseResourceProviderR4Test {
 			.withNoParameters(Parameters.class)
 			.returnResourceType(Bundle.class)
 			.execute();
-
 		ourLog.info("Output: {}", myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(output));
 
+		// Verify
+
 		assertEquals(37, output.getEntry().size());
+		String patientId = findFirstEntryResource(output, Patient.class).getId();
+		assertThat(patientId, matchesPattern("urn:uuid:.*"));
+		MedicationStatement medicationStatement = findFirstEntryResource(output, MedicationStatement.class);
+		assertEquals(patientId, medicationStatement.getSubject().getReference());
+		assertNull(medicationStatement.getInformationSource().getReference());
+	}
+
+	@SuppressWarnings("unchecked")
+	private <T extends IBaseResource> T findFirstEntryResource(Bundle theBundle, Class<T> theType) {
+		return (T) theBundle
+			.getEntry()
+			.stream()
+			.filter(t -> theType.isAssignableFrom(t.getResource().getClass()))
+			.findFirst()
+			.orElseThrow()
+			.getResource();
 	}
 
 

hapi-fhir-jpaserver-test-r5/src/test/java/ca/uhn/fhir/jpa/dao/r5/FhirResourceDaoR5ValueSetTest.java

@@ -7,11 +7,14 @@ import ca.uhn.fhir.context.support.ValidationSupportContext;
 import ca.uhn.fhir.context.support.ValueSetExpansionOptions;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.jpa.api.config.DaoConfig;
+import ca.uhn.fhir.jpa.api.model.ExpungeOptions;
+import ca.uhn.fhir.jpa.model.entity.ResourceTable;
 import ca.uhn.fhir.jpa.term.api.ITermDeferredStorageSvc;
 import ca.uhn.fhir.jpa.term.custom.CustomTerminologySet;
 import ca.uhn.fhir.jpa.util.ValueSetTestUtil;
 import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
+import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
 import org.hl7.fhir.common.hapi.validation.support.CachingValidationSupport;
 import org.hl7.fhir.instance.model.api.IIdType;
 import org.hl7.fhir.instance.model.api.IPrimitiveType;
@@ -19,6 +22,7 @@ import org.hl7.fhir.r5.model.CodeSystem;
 import org.hl7.fhir.r5.model.CodeType;
 import org.hl7.fhir.r5.model.CodeableConcept;
 import org.hl7.fhir.r5.model.Coding;
+import org.hl7.fhir.r5.model.Enumerations;
 import org.hl7.fhir.r5.model.IdType;
 import org.hl7.fhir.r5.model.StringType;
 import org.hl7.fhir.r5.model.UriType;
@@ -30,6 +34,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.transaction.annotation.Transactional;
 
 import java.io.IOException;
+import java.util.Optional;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
@@ -38,6 +43,7 @@ import static org.hamcrest.Matchers.stringContainsInOrder;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -56,6 +62,7 @@ public class FhirResourceDaoR5ValueSetTest extends BaseJpaR5Test {
 	public void after() {
 		myDaoConfig.setPreExpandValueSets(new DaoConfig().isPreExpandValueSets());
 		myDaoConfig.setMaximumExpansionSize(new DaoConfig().getMaximumExpansionSize());
+		myDaoConfig.setExpungeEnabled(new DaoConfig().isExpungeEnabled());
 	}
 
 
@@ -248,6 +255,49 @@ public class FhirResourceDaoR5ValueSetTest extends BaseJpaR5Test {
 
 	}
 
+
+	/**
+	 * See #4305
+	 */
+	@Test
+	public void testDeleteExpungePreExpandedValueSet() {
+		myDaoConfig.setExpungeEnabled(true);
+
+		// Create valueset
+		ValueSet vs = myValidationSupport.fetchResource(ValueSet.class, "http://hl7.org/fhir/ValueSet/address-use");
+		assertNotNull(vs);
+		IIdType id = myValueSetDao.create(vs).getId().toUnqualifiedVersionless();
+
+		// Update valueset
+		vs.setName("Hello");
+		assertEquals("2", myValueSetDao.update(vs, mySrd).getId().getVersionIdPart());
+		runInTransaction(()->{
+			Optional<ResourceTable> resource = myResourceTableDao.findById(id.getIdPartAsLong());
+			assertTrue(resource.isPresent());
+		});
+
+		// Precalculate
+		myTerminologyDeferredStorageSvc.saveAllDeferred();
+		myTermSvc.preExpandDeferredValueSetsToTerminologyTables();
+		logAllValueSets();
+
+		// Delete
+		myValueSetDao.delete(id, mySrd);
+
+		// Verify it's deleted
+		assertThrows(ResourceGoneException.class, ()-> myValueSetDao.read(id, mySrd));
+
+		// Expunge
+		myValueSetDao.expunge(id, new ExpungeOptions().setExpungeDeletedResources(true).setExpungeOldVersions(true), mySrd);
+
+		// Verify expunged
+		runInTransaction(()->{
+			Optional<ResourceTable> resource = myResourceTableDao.findById(id.getIdPartAsLong());
+			assertFalse(resource.isPresent());
+		});
+	}
+
+
 	@Test
 	public void testExpandByValueSet_ExceedsMaxSize() {
 		// Add a bunch of codes

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

@@ -814,7 +814,7 @@ public class ResponseHighlighterInterceptor {
 			writeLength(theServletResponse, outputBuffer.length());
 			theServletResponse.getWriter().append(" total including HTML)");
 
-			theServletResponse.getWriter().append(" in estimated ");
+			theServletResponse.getWriter().append(" in approximately ");
 			theServletResponse.getWriter().append(writeSw.toString());
 			theServletResponse.getWriter().append("</div>");