diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java index fdcad88f10c..78752a5e127 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/ModelScanner.java @@ -441,6 +441,18 @@ class ModelScanner { providesMembershipInCompartments.add(name); } + /** + * In the base FHIR R4 specification, the Device resource is not a part of the Patient compartment. + * However, it is a patient-specific resource that most users expect to be, and several derivative + * specifications including g(10) testing expect it to be, and the fact that it is not has led to many + * bug reports in HAPI FHIR. As of HAPI FHIR 8.0.0 it is being manually added in response to those + * requests. + * See https://github.com/hapifhir/hapi-fhir/issues/6536 for more information. + */ + if (searchParam.name().equals("patient") && searchParam.path().equals("Device.patient")) { + providesMembershipInCompartments.add("Patient"); + } + List components = null; if (paramType == RestSearchParameterTypeEnum.COMPOSITE) { components = new ArrayList<>(); diff --git a/hapi-fhir-docs/src/main/java/ca/uhn/hapi/fhir/docs/AuthorizationInterceptors.java b/hapi-fhir-docs/src/main/java/ca/uhn/hapi/fhir/docs/AuthorizationInterceptors.java index 3fc5ca8d057..bf56f0a277d 100644 --- a/hapi-fhir-docs/src/main/java/ca/uhn/hapi/fhir/docs/AuthorizationInterceptors.java +++ b/hapi-fhir-docs/src/main/java/ca/uhn/hapi/fhir/docs/AuthorizationInterceptors.java @@ -262,7 +262,6 @@ public class AuthorizationInterceptors { } }; // END SNIPPET: advancedCompartment - } @SuppressWarnings("InnerClassMayBeStatic") diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6536-make-device-part-of-patient-compartment.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6536-make-device-part-of-patient-compartment.yaml new file mode 100644 index 00000000000..cb51d174f77 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6536-make-device-part-of-patient-compartment.yaml @@ -0,0 +1,14 @@ +--- +type: add +jira: SMILE-9260 +title: "The `patient` search parameter for the `Device` resource has been added to the Patient Compartment for the purposes of: + +- AuthorizationInterceptor +- SearchNarrowingInterceptor +- $everything Operation +- Patient/Group Bulk Export + +This means that a search for $everything on a patient will return devices associated to this patient. This used to be possible +via the [Advanced Compartment Authorization](/hapi-fhir/docs/security/authorization_interceptor.html#advanced-compartment-authorization), but that would only apply to authorization, +and not these other use cases. This solution adds this Search Parameter to all places where compartment membership would be checked. +" diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/upgrade.md b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/upgrade.md index b8fa6a5af5d..2cb6e887053 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/upgrade.md +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/upgrade.md @@ -1,4 +1,4 @@ -# Upgrade Notes +## Resource Provenance The JPA server stores values for the field `Resource.meta.source` in dedicated columns in its database so that they can be indexes and searched for as needed, using the `_source` Search Parameter. @@ -15,6 +15,19 @@ If you do have such data, you should follow the following steps: * When this reindex operation has successfully completed, the setting above can be disabled. Disabling this setting avoids an extra database round-trip when loading data, so this change will have a positive performance impact on your server. -# Fulltext Search with _lastUpdated Filter +## Device membership in Patient Compartment + +As of 8.0.0, versions of FHIR below R5 now consider the `Device` resource's `patient` Search Parameter to be in the Patient Compartment. The following features are affected: + +- Patient Search with `_revInclude=*` +- Patient instance-level `$everything` operation +- Patient type-level `$everything` operation +- Automatic Search Narrowing +- Bulk Export + +Previously, there were various shims in the code that permitted similar behaviour in these features. Those shims have been removed. The only remaining component is [Advanced Compartment Authorization](/hapi-fhir/docs/security/authorization_interceptor.html#advanced-compartment-authorization), which can still be used +to add other Search Parameters into a given compartment. + +## Fulltext Search with _lastUpdated Filter Fulltext searches have been updated to support `_lastUpdated` search parameter. If you are using Advanced Hibernate Search indexing and wish to use the `_lastUpdated` search parameetr with this feature, a full reindex of your repository is required. diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java index 542d390a2b3..4c2a3366319 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/bulk/BulkExportUseCaseTest.java @@ -113,8 +113,6 @@ public class BulkExportUseCaseTest extends BaseResourceProviderR4Test { public class SpecConformanceTests { - - @Test public void testBulkExportJobsAreMetaTaggedWithJobIdAndExportId() throws IOException { //Given a patient exists diff --git a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java index f9c28053da9..3f85f74c92b 100644 --- a/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java +++ b/hapi-fhir-storage-batch2-jobs/src/main/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProvider.java @@ -360,10 +360,6 @@ public class BulkDataExportProvider { if (myCompartmentResources == null) { myCompartmentResources = new HashSet<>(SearchParameterUtil.getAllResourceTypesThatAreInPatientCompartment(theFhirContext)); - if (isDeviceResourceSupportedForPatientCompartmentForFhirVersion( - theFhirContext.getVersion().getVersion())) { - myCompartmentResources.add("Device"); - } } return myCompartmentResources; } @@ -819,9 +815,4 @@ public class BulkDataExportProvider { throw new InvalidRequestException(Msg.code(513) + "Must request async processing for " + theOperationName); } } - - private static boolean isDeviceResourceSupportedForPatientCompartmentForFhirVersion( - FhirVersionEnum theFhirVersionEnum) { - return PATIENT_COMPARTMENT_FHIR_VERSIONS_SUPPORT_DEVICE.contains(theFhirVersionEnum); - } } diff --git a/hapi-fhir-storage-batch2-jobs/src/test/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProviderTest.java b/hapi-fhir-storage-batch2-jobs/src/test/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProviderTest.java index 2cac04148e1..7d9c6fc3ce4 100644 --- a/hapi-fhir-storage-batch2-jobs/src/test/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProviderTest.java +++ b/hapi-fhir-storage-batch2-jobs/src/test/java/ca/uhn/fhir/batch2/jobs/export/BulkDataExportProviderTest.java @@ -38,7 +38,7 @@ class BulkDataExportProviderTest { @MethodSource("fhirContexts") void checkDeviceIsSupportedInPatientCompartment(FhirContext theFhirContext) { Set resourceNames = new BulkDataExportProvider().getPatientCompartmentResources(theFhirContext); - if (PATIENT_COMPARTMENT_FHIR_VERSIONS_SUPPORT_DEVICE.contains(theFhirContext.getVersion().getVersion())) { + if (theFhirContext.getVersion().getVersion().isOlderThan(FhirVersionEnum.R5)) { assertThat(resourceNames).contains("Device"); } else { assertThat(resourceNames).doesNotContain("Device"); diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptorTest.java index 3807a4542b0..a9201404417 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptorTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/SearchNarrowingInterceptorTest.java @@ -39,6 +39,7 @@ import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.Device; import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Parameters; @@ -100,6 +101,7 @@ public class SearchNarrowingInterceptorTest { private RestfulServerExtension myRestfulServerExtension = new RestfulServerExtension(ourCtx) .registerProvider(new DummyObservationResourceProvider()) .registerProvider(new DummyPatientResourceProvider()) + .registerProvider(new DummyDeviceResourceProvider()) .registerProvider(new DummySystemProvider()) .withPagingProvider(new FifoMemoryPagingProvider(100)); @@ -283,6 +285,42 @@ public class SearchNarrowingInterceptorTest { assertNull(ourLastCodeParam); } + @Test + public void testNarrowCompartment_DevicesByPatientContext_ClientRequestedNoParams() { + ourNextAuthorizedList = new AuthorizedList() + .addCompartments("Patient/123", "Patient/456"); + + myClient + .search() + .forResource("Device") + .execute(); + + assertEquals("Device.search", ourLastHitMethod); + assertNull(ourLastIdParam); + assertNull(ourLastCodeParam); + assertNull(ourLastSubjectParam); + assertNull(ourLastPerformerParam); + assertThat(toStrings(ourLastPatientParam)).containsExactly("Patient/123,Patient/456"); + } + + @Test + public void testNarrowCompartment_DevicesByPatientContext_ClientRequestedWithParams() { + ourNextAuthorizedList = new AuthorizedList() + .addCompartments("Patient/123"); + + myClient + .search() + .byUrl("Device?patient=Patient/123") + .execute(); + + assertEquals("Device.search", ourLastHitMethod); + assertNull(ourLastIdParam); + assertNull(ourLastCodeParam); + assertNull(ourLastSubjectParam); + assertNull(ourLastPerformerParam); + assertThat(toStrings(ourLastPatientParam)).containsExactly("Patient/123"); + } + @Test public void testNarrowCompartment_ObservationsByPatientContext_ClientRequestedNoParams() { ourNextAuthorizedList = new AuthorizedList() @@ -950,7 +988,110 @@ public class SearchNarrowingInterceptorTest { return new MethodOutcome(new IdType("Patient/123"), true); } + @SuppressWarnings("unused") + public static class DummyObservationResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Observation.class; + } + + + @Search() + public List search( + @OptionalParam(name = "_id") TokenAndListParam theIdParam, + @OptionalParam(name = Observation.SP_SUBJECT) ReferenceAndListParam theSubjectParam, + @OptionalParam(name = Observation.SP_PATIENT) ReferenceAndListParam thePatientParam, + @OptionalParam(name = Observation.SP_PERFORMER) ReferenceAndListParam thePerformerParam, + @OptionalParam(name = Observation.SP_CODE) TokenAndListParam theCodeParam + ) { + ourLastHitMethod = "Observation.search"; + ourLastIdParam = theIdParam; + ourLastSubjectParam = theSubjectParam; + ourLastPatientParam = thePatientParam; + ourLastPerformerParam = thePerformerParam; + ourLastCodeParam = theCodeParam; + return ourReturn; + } + + @Create + public MethodOutcome create(@ResourceParam IBaseResource theResource, @ConditionalUrlParam String theConditionalUrl) { + ourLastHitMethod = "Observation.create"; + ourLastConditionalUrl = theConditionalUrl; + return new MethodOutcome(new IdType("Observation/123"), true); + } + + @Update + public MethodOutcome update(@ResourceParam IBaseResource theResource, @ConditionalUrlParam String theConditionalUrl) { + ourLastHitMethod = "Observation.update"; + ourLastConditionalUrl = theConditionalUrl; + return new MethodOutcome(new IdType("Observation/123"), true); + } + + @Delete + public MethodOutcome delete(@IdParam IIdType theId, @ConditionalUrlParam String theConditionalUrl) { + ourLastHitMethod = "Observation.delete"; + ourLastConditionalUrl = theConditionalUrl; + return new MethodOutcome(new IdType("Observation/123"), true); + } + + @Patch + public MethodOutcome patch(@IdParam IIdType theId, @ResourceParam IBaseResource theResource, @ConditionalUrlParam String theConditionalUrl, PatchTypeEnum thePatchType) { + ourLastHitMethod = "Observation.patch"; + ourLastConditionalUrl = theConditionalUrl; + return new MethodOutcome(new IdType("Observation/123"), true); + } + } } + @SuppressWarnings("unused") + public static class DummyDeviceResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Device.class; + } + + + @Search() + public List search( + @OptionalParam(name = "_id") TokenAndListParam theIdParam, + @OptionalParam(name = Device.SP_PATIENT) ReferenceAndListParam thePatientParam + ) { + ourLastHitMethod = "Device.search"; + ourLastIdParam = theIdParam; + ourLastPatientParam = thePatientParam; + return ourReturn; + } + + @Create + public MethodOutcome create(@ResourceParam IBaseResource theResource, @ConditionalUrlParam String theConditionalUrl) { + ourLastHitMethod = "Device.create"; + ourLastConditionalUrl = theConditionalUrl; + return new MethodOutcome(new IdType("Device/123"), true); + } + + @Update + public MethodOutcome update(@ResourceParam IBaseResource theResource, @ConditionalUrlParam String theConditionalUrl) { + ourLastHitMethod = "Device.update"; + ourLastConditionalUrl = theConditionalUrl; + return new MethodOutcome(new IdType("Device/123"), true); + } + + @Delete + public MethodOutcome delete(@IdParam IIdType theId, @ConditionalUrlParam String theConditionalUrl) { + ourLastHitMethod = "Device.delete"; + ourLastConditionalUrl = theConditionalUrl; + return new MethodOutcome(new IdType("Device/123"), true); + } + + @Patch + public MethodOutcome patch(@IdParam IIdType theId, @ResourceParam IBaseResource theResource, @ConditionalUrlParam String theConditionalUrl, PatchTypeEnum thePatchType) { + ourLastHitMethod = "Device.patch"; + ourLastConditionalUrl = theConditionalUrl; + return new MethodOutcome(new IdType("Device/123"), true); + } + } + @SuppressWarnings("unused") public static class DummyObservationResourceProvider implements IResourceProvider { diff --git a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java index ed4e8718913..a624a164e41 100644 --- a/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-validation/src/test/java/ca/uhn/fhir/rest/server/interceptor/auth/AuthorizationInterceptorR4Test.java @@ -415,23 +415,19 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline extractResponseAndClose(status); assertEquals(200, status.getStatusLine().getStatusCode()); assertTrue(ourHitMethod); - } @Test - public void testCustomCompartmentSpsOnMultipleInstances() throws Exception { + public void testDeviceIsNativelyInPatientCompartmentForAuthorizationPurposes() throws Exception { //Given ourServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { - AdditionalCompartmentSearchParameters additionalCompartmentSearchParameters = new AdditionalCompartmentSearchParameters(); - additionalCompartmentSearchParameters.addSearchParameters("Device:patient"); List relatedIds = new ArrayList<>(); relatedIds.add(new IdType("Patient/123")); - relatedIds.add(new IdType("Patient/456")); return new RuleBuilder() .allow().read().allResources() - .inCompartmentWithAdditionalSearchParams("Patient", relatedIds, additionalCompartmentSearchParameters) + .inCompartment("Patient", relatedIds) .andThen().denyAll() .build(); } @@ -459,19 +455,19 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline assertEquals(200, status.getStatusLine().getStatusCode()); } + @Test - public void testCustomSearchParamsDontOverPermit() throws Exception { + public void testCustomCompartmentSpsOnMultipleInstances() throws Exception { //Given ourServer.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { @Override public List buildRuleList(RequestDetails theRequestDetails) { - AdditionalCompartmentSearchParameters additionalCompartmentSearchParameters = new AdditionalCompartmentSearchParameters(); - additionalCompartmentSearchParameters.addSearchParameters("Encounter:patient"); List relatedIds = new ArrayList<>(); relatedIds.add(new IdType("Patient/123")); + relatedIds.add(new IdType("Patient/456")); return new RuleBuilder() .allow().read().allResources() - .inCompartmentWithAdditionalSearchParams("Patient", relatedIds, additionalCompartmentSearchParameters) + .inCompartment("Patient", relatedIds) .andThen().denyAll() .build(); } @@ -494,12 +490,11 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline status = ourClient.execute(httpGet); extractResponseAndClose(status); - //then - assertFalse(ourHitMethod); - assertEquals(403, status.getStatusLine().getStatusCode()); + //Then + assertTrue(ourHitMethod); + assertEquals(200, status.getStatusLine().getStatusCode()); } - @Test public void testNonsenseParametersThrowAtRuntime() throws Exception { //Given @@ -531,7 +526,7 @@ public class AuthorizationInterceptorR4Test extends BaseValidationTestWithInline ourReturn = Collections.singletonList(d); //When - httpGet = new HttpGet(ourServer.getBaseUrl() + "/Device/124456"); + httpGet = new HttpGet(ourServer.getBaseUrl() + "/Device/"); status = ourClient.execute(httpGet); extractResponseAndClose(status);