From 5509bd053d69ee1a4b4b84fa2b32fc85eabdeaba Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 29 Aug 2022 09:42:10 -0400 Subject: [PATCH] Move partition interceptor (#3907) * Add license headers * Move partition interceptor * More cleanup * One more tweak * Headers * Add changelog * Test logging enhancement * Test fix * Test fixes * Add test method --- .../ca/uhn/fhir/i18n/hapi-messages.properties | 7 +- .../3907-move-partition-interceptor.yaml | 5 + .../partition/RequestPartitionHelperSvc.java | 263 +-------------- .../ca/uhn/fhir/jpa/test/Batch2JobHelper.java | 28 +- .../PatientIdPartitionInterceptorTest.java | 27 ++ .../stresstest/GiantTransactionPerfTest.java | 5 + .../PatientIdPartitionInterceptor.java | 2 +- .../BaseRequestPartitionHelperSvc.java | 303 ++++++++++++++++++ .../partition/IRequestPartitionHelperSvc.java | 2 + 9 files changed, 375 insertions(+), 267 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3907-move-partition-interceptor.yaml create mode 100644 hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/BaseRequestPartitionHelperSvc.java diff --git a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties index 017c0a5650b..122e9f5970f 100644 --- a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties +++ b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties @@ -160,10 +160,9 @@ ca.uhn.fhir.jpa.patch.JsonPatchUtils.failedToApplyPatch=Failed to apply JSON pat ca.uhn.fhir.jpa.graphql.DaoRegistryGraphQLStorageServices.invalidGraphqlArgument=Unknown GraphQL argument "{0}". Value GraphQL argument for this type are: {1} ca.uhn.fhir.jpa.graphql.DaoRegistryGraphQLStorageServices.invalidGraphqlCursorArgument=GraphQL Cursor "{0}" does not exist and may have expired -ca.uhn.fhir.jpa.partition.RequestPartitionHelperSvc.nonDefaultPartitionSelectedForNonPartitionable=Resource type {0} can not be partitioned -ca.uhn.fhir.jpa.partition.RequestPartitionHelperSvc.unknownPartitionId=Unknown partition ID: {0} -ca.uhn.fhir.jpa.partition.RequestPartitionHelperSvc.unknownPartitionName=Unknown partition name: {0} - +ca.uhn.fhir.jpa.partition.BaseRequestPartitionHelperSvc.nonDefaultPartitionSelectedForNonPartitionable=Resource type {0} can not be partitioned +ca.uhn.fhir.jpa.partition.BaseRequestPartitionHelperSvc.unknownPartitionId=Unknown partition ID: {0} +ca.uhn.fhir.jpa.partition.BaseRequestPartitionHelperSvc.unknownPartitionName=Unknown partition name: {0} ca.uhn.fhir.jpa.search.builder.predicate.ResourceLinkPredicateBuilder.invalidTargetTypeForChain=Resource type "{0}" is not a valid target type for reference search parameter: {1} ca.uhn.fhir.jpa.search.SearchCoordinatorSvcImpl.invalidResourceType=Invalid/unsupported resource type: "{0}" diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3907-move-partition-interceptor.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3907-move-partition-interceptor.yaml new file mode 100644 index 00000000000..d5ac9cf3aa7 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/3907-move-partition-interceptor.yaml @@ -0,0 +1,5 @@ +--- +type: change +issue: 3907 +title: "The Partition Interceptor has been refactored out of the JPA module in order to facilitate + future use in other modules. No functional changes have been made." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java index 07c7dccc5f6..b06af32648a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/partition/RequestPartitionHelperSvc.java @@ -37,11 +37,9 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; -import ca.uhn.fhir.util.StringUtil; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBaseResource; -import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Autowired; import javax.annotation.Nonnull; @@ -57,233 +55,14 @@ import static ca.uhn.fhir.jpa.model.util.JpaConstants.ALL_PARTITIONS_NAME; import static ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster.doCallHooks; import static ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster.doCallHooksAndReturnObject; import static ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster.hasHooks; -import static org.slf4j.LoggerFactory.getLogger; -public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { - private static final Logger ourLog = getLogger(RequestPartitionHelperSvc.class); +public class RequestPartitionHelperSvc extends BaseRequestPartitionHelperSvc { - private final HashSet myNonPartitionableResourceNames; - - @Autowired - private IInterceptorBroadcaster myInterceptorBroadcaster; @Autowired private IPartitionLookupSvc myPartitionConfigSvc; - @Autowired - private FhirContext myFhirContext; - @Autowired - private PartitionSettings myPartitionSettings; - public RequestPartitionHelperSvc() { - myNonPartitionableResourceNames = new HashSet<>(); - - // Infrastructure - myNonPartitionableResourceNames.add("SearchParameter"); - - // Validation and Conformance - myNonPartitionableResourceNames.add("StructureDefinition"); - myNonPartitionableResourceNames.add("Questionnaire"); - myNonPartitionableResourceNames.add("CapabilityStatement"); - myNonPartitionableResourceNames.add("CompartmentDefinition"); - myNonPartitionableResourceNames.add("OperationDefinition"); - - myNonPartitionableResourceNames.add("Library"); - - // Terminology - myNonPartitionableResourceNames.add("ConceptMap"); - myNonPartitionableResourceNames.add("CodeSystem"); - myNonPartitionableResourceNames.add("ValueSet"); - myNonPartitionableResourceNames.add("NamingSystem"); - myNonPartitionableResourceNames.add("StructureMap"); - - } - - /** - * Invoke the {@link Pointcut#STORAGE_PARTITION_IDENTIFY_READ} interceptor pointcut to determine the tenant for a read request. - *

- * If no interceptors are registered with a hook for {@link Pointcut#STORAGE_PARTITION_IDENTIFY_READ}, return - * {@link RequestPartitionId#allPartitions()} instead. - */ - @Nonnull @Override - public RequestPartitionId determineReadPartitionForRequest(@Nullable RequestDetails theRequest, String theResourceType, ReadPartitionIdRequestDetails theDetails) { - RequestPartitionId requestPartitionId; - - boolean nonPartitionableResource = myNonPartitionableResourceNames.contains(theResourceType); - if (myPartitionSettings.isPartitioningEnabled()) { - // Handle system requests - //TODO GGG eventually, theRequest will not be allowed to be null here, and we will pass through SystemRequestDetails instead. - if ((theRequest == null || theRequest instanceof SystemRequestDetails) && nonPartitionableResource) { - return RequestPartitionId.defaultPartition(); - } - - if (theRequest instanceof SystemRequestDetails && systemRequestHasExplicitPartition((SystemRequestDetails) theRequest)) { - requestPartitionId = getSystemRequestPartitionId((SystemRequestDetails) theRequest, nonPartitionableResource); - } else if (hasHooks(Pointcut.STORAGE_PARTITION_IDENTIFY_READ, myInterceptorBroadcaster, theRequest)) { - // Interceptor call: STORAGE_PARTITION_IDENTIFY_READ - HookParams params = new HookParams() - .add(RequestDetails.class, theRequest) - .addIfMatchesType(ServletRequestDetails.class, theRequest) - .add(ReadPartitionIdRequestDetails.class, theDetails); - requestPartitionId = (RequestPartitionId) doCallHooksAndReturnObject(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PARTITION_IDENTIFY_READ, params); - } else { - requestPartitionId = null; - } - - validateRequestPartitionNotNull(requestPartitionId, Pointcut.STORAGE_PARTITION_IDENTIFY_READ); - - return validateNormalizeAndNotifyHooksForRead(requestPartitionId, theRequest, theResourceType); - } - - return RequestPartitionId.allPartitions(); - } - - /** - * For system requests, read partition from tenant ID if present, otherwise set to DEFAULT. If the resource they are attempting to partition - * is non-partitionable scream in the logs and set the partition to DEFAULT. - * - */ - private RequestPartitionId getSystemRequestPartitionId(SystemRequestDetails theRequest, boolean theNonPartitionableResource) { - RequestPartitionId requestPartitionId; - requestPartitionId = getSystemRequestPartitionId(theRequest); - if (theNonPartitionableResource && !requestPartitionId.isDefaultPartition()) { - throw new InternalErrorException(Msg.code(1315) + "System call is attempting to write a non-partitionable resource to a partition! This is a bug!"); - } - return requestPartitionId; - } - - /** - * Determine the partition for a System Call (defined by the fact that the request is of type SystemRequestDetails) - *

- * 1. If the tenant ID is set to the constant for all partitions, return all partitions - * 2. If there is a tenant ID set in the request, use it. - * 3. Otherwise, return the Default Partition. - * - * @param theRequest The {@link SystemRequestDetails} - * @return the {@link RequestPartitionId} to be used for this request. - */ - @Nonnull - private RequestPartitionId getSystemRequestPartitionId(@Nonnull SystemRequestDetails theRequest) { - if (theRequest.getRequestPartitionId() != null) { - return theRequest.getRequestPartitionId(); - } - if (theRequest.getTenantId() != null) { - if (theRequest.getTenantId().equals(ALL_PARTITIONS_NAME)) { - return RequestPartitionId.allPartitions(); - } else { - return RequestPartitionId.fromPartitionName(theRequest.getTenantId()); - } - } else { - return RequestPartitionId.defaultPartition(); - } - } - - /** - * Invoke the {@link Pointcut#STORAGE_PARTITION_IDENTIFY_CREATE} interceptor pointcut to determine the tenant for a create request. - */ - @Nonnull - @Override - public RequestPartitionId determineCreatePartitionForRequest(@Nullable RequestDetails theRequest, @Nonnull IBaseResource theResource, @Nonnull String theResourceType) { - RequestPartitionId requestPartitionId; - - if (myPartitionSettings.isPartitioningEnabled()) { - boolean nonPartitionableResource = myNonPartitionableResourceNames.contains(theResourceType); - - //TODO GGG eventually, theRequest will not be allowed to be null here, and we will pass through SystemRequestDetails instead. - if ((theRequest == null || theRequest instanceof SystemRequestDetails) && nonPartitionableResource) { - return RequestPartitionId.defaultPartition(); - } - - if (theRequest instanceof SystemRequestDetails && systemRequestHasExplicitPartition((SystemRequestDetails) theRequest)) { - requestPartitionId = getSystemRequestPartitionId((SystemRequestDetails) theRequest, nonPartitionableResource); - } else { - //This is an external Request (e.g. ServletRequestDetails) so we want to figure out the partition via interceptor. - // Interceptor call: STORAGE_PARTITION_IDENTIFY_CREATE - HookParams params = new HookParams() - .add(IBaseResource.class, theResource) - .add(RequestDetails.class, theRequest) - .addIfMatchesType(ServletRequestDetails.class, theRequest); - requestPartitionId = (RequestPartitionId) doCallHooksAndReturnObject(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PARTITION_IDENTIFY_CREATE, params); - - //If the interceptors haven't selected a partition, and its a non-partitionable resource anyhow, send to DEFAULT - if (nonPartitionableResource && requestPartitionId == null) { - requestPartitionId = RequestPartitionId.defaultPartition(); - } - } - - String resourceName = myFhirContext.getResourceType(theResource); - validateSinglePartitionForCreate(requestPartitionId, resourceName, Pointcut.STORAGE_PARTITION_IDENTIFY_CREATE); - - return validateNormalizeAndNotifyHooksForRead(requestPartitionId, theRequest, theResourceType); - } - - return RequestPartitionId.allPartitions(); - } - - private boolean systemRequestHasExplicitPartition(@Nonnull SystemRequestDetails theRequest) { - return theRequest.getRequestPartitionId() != null || theRequest.getTenantId() != null; - } - - @Nonnull - @Override - public PartitionablePartitionId toStoragePartition(@Nonnull RequestPartitionId theRequestPartitionId) { - Integer partitionId = theRequestPartitionId.getFirstPartitionIdOrNull(); - if (partitionId == null) { - partitionId = myPartitionSettings.getDefaultPartitionId(); - } - return new PartitionablePartitionId(partitionId, theRequestPartitionId.getPartitionDate()); - } - - @Nonnull - @Override - public Set toReadPartitions(@Nonnull RequestPartitionId theRequestPartitionId) { - return theRequestPartitionId - .getPartitionIds() - .stream() - .map(t->t == null ? myPartitionSettings.getDefaultPartitionId() : t) - .collect(Collectors.toSet()); - } - - /** - * If the partition only has a name but not an ID, this method resolves the ID. - *

- * If the partition has an ID but not a name, the name is resolved. - *

- * If the partition has both, they are validated to ensure that they correspond. - */ - @Nonnull - private RequestPartitionId validateNormalizeAndNotifyHooksForRead(@Nonnull RequestPartitionId theRequestPartitionId, RequestDetails theRequest, @Nonnull String theResourceType) { - RequestPartitionId retVal = theRequestPartitionId; - - if (retVal.getPartitionNames() != null) { - retVal = validateAndNormalizePartitionNames(retVal); - } else if (retVal.hasPartitionIds()) { - retVal = validateAndNormalizePartitionIds(retVal); - } - - // Note: It's still possible that the partition only has a date but no name/id - - if (StringUtils.isNotBlank(theResourceType)) { - validateHasPartitionPermissions(theRequest, theResourceType, retVal); - } - - return retVal; - - } - - public void validateHasPartitionPermissions(RequestDetails theRequest, String theResourceType, RequestPartitionId theRequestPartitionId) { - if (myInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_PARTITION_SELECTED)) { - RuntimeResourceDefinition runtimeResourceDefinition; - runtimeResourceDefinition = myFhirContext.getResourceDefinition(theResourceType); - HookParams params = new HookParams() - .add(RequestPartitionId.class, theRequestPartitionId) - .add(RequestDetails.class, theRequest) - .addIfMatchesType(ServletRequestDetails.class, theRequest) - .add(RuntimeResourceDefinition.class, runtimeResourceDefinition); - doCallHooks(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PARTITION_SELECTED, params); - } - } - - private RequestPartitionId validateAndNormalizePartitionIds(RequestPartitionId theRequestPartitionId) { + protected RequestPartitionId validateAndNormalizePartitionIds(RequestPartitionId theRequestPartitionId) { List names = null; for (int i = 0; i < theRequestPartitionId.getPartitionIds().size(); i++) { @@ -295,7 +74,7 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { try { partition = myPartitionConfigSvc.getPartitionById(id); } catch (IllegalArgumentException e) { - String msg = myFhirContext.getLocalizer().getMessage(RequestPartitionHelperSvc.class, "unknownPartitionId", theRequestPartitionId.getPartitionIds().get(i)); + String msg = myFhirContext.getLocalizer().getMessage(BaseRequestPartitionHelperSvc.class, "unknownPartitionId", theRequestPartitionId.getPartitionIds().get(i)); throw new ResourceNotFoundException(Msg.code(1316) + msg); } } @@ -326,7 +105,8 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { return theRequestPartitionId; } - private RequestPartitionId validateAndNormalizePartitionNames(RequestPartitionId theRequestPartitionId) { + @Override + protected RequestPartitionId validateAndNormalizePartitionNames(RequestPartitionId theRequestPartitionId) { List ids = null; for (int i = 0; i < theRequestPartitionId.getPartitionNames().size(); i++) { @@ -334,7 +114,7 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { try { partition = myPartitionConfigSvc.getPartitionByName(theRequestPartitionId.getPartitionNames().get(i)); } catch (IllegalArgumentException e) { - String msg = myFhirContext.getLocalizer().getMessage(RequestPartitionHelperSvc.class, "unknownPartitionName", theRequestPartitionId.getPartitionNames().get(i)); + String msg = myFhirContext.getLocalizer().getMessage(BaseRequestPartitionHelperSvc.class, "unknownPartitionName", theRequestPartitionId.getPartitionNames().get(i)); throw new ResourceNotFoundException(Msg.code(1317) + msg); } @@ -364,36 +144,5 @@ public class RequestPartitionHelperSvc implements IRequestPartitionHelperSvc { return theRequestPartitionId; } - private void validateSinglePartitionForCreate(RequestPartitionId theRequestPartitionId, @Nonnull String theResourceName, Pointcut thePointcut) { - validateRequestPartitionNotNull(theRequestPartitionId, thePointcut); - if (theRequestPartitionId.hasPartitionIds()) { - validateSinglePartitionIdOrNameForCreate(theRequestPartitionId.getPartitionIds()); - } - validateSinglePartitionIdOrNameForCreate(theRequestPartitionId.getPartitionNames()); - - // Make sure we're not using one of the conformance resources in a non-default partition - if ((theRequestPartitionId.hasPartitionIds() && !theRequestPartitionId.getPartitionIds().contains(null)) || - (theRequestPartitionId.hasPartitionNames() && !theRequestPartitionId.getPartitionNames().contains(JpaConstants.DEFAULT_PARTITION_NAME))) { - - if (myNonPartitionableResourceNames.contains(theResourceName)) { - String msg = myFhirContext.getLocalizer().getMessageSanitized(RequestPartitionHelperSvc.class, "nonDefaultPartitionSelectedForNonPartitionable", theResourceName); - throw new UnprocessableEntityException(Msg.code(1318) + msg); - } - - } - - } - - private void validateRequestPartitionNotNull(RequestPartitionId theRequestPartitionId, Pointcut theThePointcut) { - if (theRequestPartitionId == null) { - throw new InternalErrorException(Msg.code(1319) + "No interceptor provided a value for pointcut: " + theThePointcut); - } - } - - private void validateSinglePartitionIdOrNameForCreate(@Nullable List thePartitionIds) { - if (thePartitionIds != null && thePartitionIds.size() != 1) { - throw new InternalErrorException(Msg.code(1320) + "RequestPartitionId must contain a single partition for create operations, found: " + thePartitionIds); - } - } } diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/Batch2JobHelper.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/Batch2JobHelper.java index 6d446446c5d..464cf716c3c 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/Batch2JobHelper.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/Batch2JobHelper.java @@ -22,12 +22,15 @@ package ca.uhn.fhir.jpa.test; import ca.uhn.fhir.batch2.api.IJobCoordinator; import ca.uhn.fhir.batch2.api.IJobMaintenanceService; +import ca.uhn.fhir.batch2.api.IJobPersistence; import ca.uhn.fhir.batch2.model.JobInstance; import ca.uhn.fhir.batch2.model.StatusEnum; +import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.jpa.batch.models.Batch2JobStartResponse; import org.awaitility.core.ConditionTimeoutException; import org.hamcrest.Matchers; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.transaction.support.TransactionSynchronizationManager; import java.time.Duration; import java.util.ArrayList; @@ -36,6 +39,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import static org.awaitility.Awaitility.await; import static org.hamcrest.Matchers.equalTo; @@ -49,6 +53,9 @@ public class Batch2JobHelper { @Autowired private IJobMaintenanceService myJobMaintenanceService; + @Autowired + private IJobPersistence myJobPersistence; + @Autowired private IJobCoordinator myJobCoordinator; @@ -61,12 +68,23 @@ public class Batch2JobHelper { } public JobInstance awaitJobCompletion(String theId, int theSecondsToWait) { - await() + assert !TransactionSynchronizationManager.isActualTransactionActive(); + + try { + await() .atMost(theSecondsToWait, TimeUnit.SECONDS) - .until(() -> { - myJobMaintenanceService.runMaintenancePass(); - return myJobCoordinator.getInstance(theId).getStatus(); - }, equalTo(StatusEnum.COMPLETED)); + .until(() -> { + myJobMaintenanceService.runMaintenancePass(); + return myJobCoordinator.getInstance(theId).getStatus(); + }, equalTo(StatusEnum.COMPLETED)); + } catch (ConditionTimeoutException e) { + String statuses = myJobPersistence.fetchInstances(100, 0) + .stream() + .map(t -> t.getJobDefinitionId() + "/" + t.getStatus().name()) + .collect(Collectors.joining("\n")); + String currentStatus = myJobCoordinator.getInstance(theId).getStatus().name(); + fail("Job still has status " + currentStatus + " - All statuses:\n" + statuses); + } return myJobCoordinator.getInstance(theId); } diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptorTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptorTest.java index 73721a2e19e..32a4e6b6381 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptorTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptorTest.java @@ -15,6 +15,7 @@ import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; +import ca.uhn.fhir.util.BundleBuilder; import ca.uhn.fhir.util.MultimapCollector; import com.google.common.collect.ListMultimap; import com.google.common.collect.Multimap; @@ -26,6 +27,7 @@ import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Reference; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -34,6 +36,7 @@ import org.springframework.beans.factory.annotation.Autowired; import java.io.IOException; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; @@ -431,6 +434,30 @@ public class PatientIdPartitionInterceptorTest extends BaseJpaR4SystemTest { } + @Test + public void testTransaction_ConditionallyCreatedPatientAndConditionallyCreatedObservation() { + + BundleBuilder tx = new BundleBuilder(myFhirContext); + + Patient p = new Patient(); + p.setId(IdType.newRandomUuid()); + p.addIdentifier().setSystem("http://ids").setValue("A"); + tx.addTransactionCreateEntry(p).conditional("Patient?identifier=http://ids|A"); + + Observation o = new Observation(); + o.addIdentifier().setSystem("http://ids").setValue("B"); + o.setSubject(new Reference(p.getId())); + tx.addTransactionCreateEntry(o).conditional("Observation?identifier=http://ids|B"); + + try { + mySystemDao.transaction(mySrd, (Bundle) tx.getBundle()); + fail(); + } catch (MethodNotAllowedException e) { + assertEquals("HAPI-1321: Patient resource IDs must be client-assigned in patient compartment mode", e.getMessage()); + } + } + + @Test public void testSearch() throws IOException { diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/stresstest/GiantTransactionPerfTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/stresstest/GiantTransactionPerfTest.java index e3ab8f63cc0..0c6da76d851 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/stresstest/GiantTransactionPerfTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/stresstest/GiantTransactionPerfTest.java @@ -881,6 +881,11 @@ public class GiantTransactionPerfTest { return Collections.singleton(theRequestPartitionId.getFirstPartitionIdOrNull()); } + @Override + public boolean isResourcePartitionable(String theResourceType) { + return true; + } + } diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptor.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptor.java index ea19ba083e0..3634d091cc4 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptor.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/PatientIdPartitionInterceptor.java @@ -224,7 +224,7 @@ public class PatientIdPartitionInterceptor { @Nonnull protected RequestPartitionId provideCompartmentMemberInstanceResponse(RequestDetails theRequestDetails, String theResourceIdPart) { int partitionId = providePartitionIdForPatientId(theRequestDetails, theResourceIdPart); - return RequestPartitionId.fromPartitionId(partitionId); + return RequestPartitionId.fromPartitionIdAndName(partitionId, theResourceIdPart); } /** diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/BaseRequestPartitionHelperSvc.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/BaseRequestPartitionHelperSvc.java new file mode 100644 index 00000000000..d33c417f65d --- /dev/null +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/BaseRequestPartitionHelperSvc.java @@ -0,0 +1,303 @@ +package ca.uhn.fhir.jpa.partition; + +/*- + * #%L + * HAPI FHIR Storage api + * %% + * Copyright (C) 2014 - 2022 Smile CDR, Inc. + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.interceptor.api.HookParams; +import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster; +import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.interceptor.model.ReadPartitionIdRequestDetails; +import ca.uhn.fhir.interceptor.model.RequestPartitionId; +import ca.uhn.fhir.jpa.model.config.PartitionSettings; +import ca.uhn.fhir.jpa.model.entity.PartitionablePartitionId; +import ca.uhn.fhir.jpa.model.util.JpaConstants; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; +import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; +import org.apache.commons.lang3.StringUtils; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.springframework.beans.factory.annotation.Autowired; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +import static ca.uhn.fhir.jpa.model.util.JpaConstants.ALL_PARTITIONS_NAME; +import static ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster.doCallHooks; +import static ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster.doCallHooksAndReturnObject; +import static ca.uhn.fhir.rest.server.util.CompositeInterceptorBroadcaster.hasHooks; + +public abstract class BaseRequestPartitionHelperSvc implements IRequestPartitionHelperSvc{ + + private final HashSet myNonPartitionableResourceNames; + + @Autowired + private IInterceptorBroadcaster myInterceptorBroadcaster; + @Autowired + protected FhirContext myFhirContext; + @Autowired + private PartitionSettings myPartitionSettings; + + public BaseRequestPartitionHelperSvc() { + myNonPartitionableResourceNames = new HashSet<>(); + + // Infrastructure + myNonPartitionableResourceNames.add("SearchParameter"); + + // Validation and Conformance + myNonPartitionableResourceNames.add("StructureDefinition"); + myNonPartitionableResourceNames.add("Questionnaire"); + myNonPartitionableResourceNames.add("CapabilityStatement"); + myNonPartitionableResourceNames.add("CompartmentDefinition"); + myNonPartitionableResourceNames.add("OperationDefinition"); + + myNonPartitionableResourceNames.add("Library"); + + // Terminology + myNonPartitionableResourceNames.add("ConceptMap"); + myNonPartitionableResourceNames.add("CodeSystem"); + myNonPartitionableResourceNames.add("ValueSet"); + myNonPartitionableResourceNames.add("NamingSystem"); + myNonPartitionableResourceNames.add("StructureMap"); + + } + + /** + * Invoke the {@link Pointcut#STORAGE_PARTITION_IDENTIFY_READ} interceptor pointcut to determine the tenant for a read request. + *

+ * If no interceptors are registered with a hook for {@link Pointcut#STORAGE_PARTITION_IDENTIFY_READ}, return + * {@link RequestPartitionId#allPartitions()} instead. + */ + @Nonnull + @Override + public RequestPartitionId determineReadPartitionForRequest(@Nullable RequestDetails theRequest, String theResourceType, ReadPartitionIdRequestDetails theDetails) { + RequestPartitionId requestPartitionId; + + boolean nonPartitionableResource = !isResourcePartitionable(theResourceType); + if (myPartitionSettings.isPartitioningEnabled()) { + // Handle system requests + //TODO GGG eventually, theRequest will not be allowed to be null here, and we will pass through SystemRequestDetails instead. + if ((theRequest == null || theRequest instanceof SystemRequestDetails) && nonPartitionableResource) { + return RequestPartitionId.defaultPartition(); + } + + if (theRequest instanceof SystemRequestDetails && systemRequestHasExplicitPartition((SystemRequestDetails) theRequest)) { + requestPartitionId = getSystemRequestPartitionId((SystemRequestDetails) theRequest, nonPartitionableResource); + } else if (hasHooks(Pointcut.STORAGE_PARTITION_IDENTIFY_READ, myInterceptorBroadcaster, theRequest)) { + // Interceptor call: STORAGE_PARTITION_IDENTIFY_READ + HookParams params = new HookParams().add(RequestDetails.class, theRequest).addIfMatchesType(ServletRequestDetails.class, theRequest).add(ReadPartitionIdRequestDetails.class, theDetails); + requestPartitionId = (RequestPartitionId) doCallHooksAndReturnObject(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PARTITION_IDENTIFY_READ, params); + } else { + requestPartitionId = null; + } + + validateRequestPartitionNotNull(requestPartitionId, Pointcut.STORAGE_PARTITION_IDENTIFY_READ); + + return validateNormalizeAndNotifyHooksForRead(requestPartitionId, theRequest, theResourceType); + } + + return RequestPartitionId.allPartitions(); + } + + /** + * For system requests, read partition from tenant ID if present, otherwise set to DEFAULT. If the resource they are attempting to partition + * is non-partitionable scream in the logs and set the partition to DEFAULT. + */ + private RequestPartitionId getSystemRequestPartitionId(SystemRequestDetails theRequest, boolean theNonPartitionableResource) { + RequestPartitionId requestPartitionId; + requestPartitionId = getSystemRequestPartitionId(theRequest); + if (theNonPartitionableResource && !requestPartitionId.isDefaultPartition()) { + throw new InternalErrorException(Msg.code(1315) + "System call is attempting to write a non-partitionable resource to a partition! This is a bug!"); + } + return requestPartitionId; + } + + /** + * Determine the partition for a System Call (defined by the fact that the request is of type SystemRequestDetails) + *

+ * 1. If the tenant ID is set to the constant for all partitions, return all partitions + * 2. If there is a tenant ID set in the request, use it. + * 3. Otherwise, return the Default Partition. + * + * @param theRequest The {@link SystemRequestDetails} + * @return the {@link RequestPartitionId} to be used for this request. + */ + @Nonnull + private RequestPartitionId getSystemRequestPartitionId(@Nonnull SystemRequestDetails theRequest) { + if (theRequest.getRequestPartitionId() != null) { + return theRequest.getRequestPartitionId(); + } + if (theRequest.getTenantId() != null) { + if (theRequest.getTenantId().equals(ALL_PARTITIONS_NAME)) { + return RequestPartitionId.allPartitions(); + } else { + return RequestPartitionId.fromPartitionName(theRequest.getTenantId()); + } + } else { + return RequestPartitionId.defaultPartition(); + } + } + + /** + * Invoke the {@link Pointcut#STORAGE_PARTITION_IDENTIFY_CREATE} interceptor pointcut to determine the tenant for a create request. + */ + @Nonnull + @Override + public RequestPartitionId determineCreatePartitionForRequest(@Nullable RequestDetails theRequest, @Nonnull IBaseResource theResource, @Nonnull String theResourceType) { + RequestPartitionId requestPartitionId; + + if (myPartitionSettings.isPartitioningEnabled()) { + boolean nonPartitionableResource = myNonPartitionableResourceNames.contains(theResourceType); + + //TODO GGG eventually, theRequest will not be allowed to be null here, and we will pass through SystemRequestDetails instead. + if ((theRequest == null || theRequest instanceof SystemRequestDetails) && nonPartitionableResource) { + return RequestPartitionId.defaultPartition(); + } + + if (theRequest instanceof SystemRequestDetails && systemRequestHasExplicitPartition((SystemRequestDetails) theRequest)) { + requestPartitionId = getSystemRequestPartitionId((SystemRequestDetails) theRequest, nonPartitionableResource); + } else { + //This is an external Request (e.g. ServletRequestDetails) so we want to figure out the partition via interceptor. + // Interceptor call: STORAGE_PARTITION_IDENTIFY_CREATE + HookParams params = new HookParams().add(IBaseResource.class, theResource).add(RequestDetails.class, theRequest).addIfMatchesType(ServletRequestDetails.class, theRequest); + requestPartitionId = (RequestPartitionId) doCallHooksAndReturnObject(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PARTITION_IDENTIFY_CREATE, params); + + //If the interceptors haven't selected a partition, and its a non-partitionable resource anyhow, send to DEFAULT + if (nonPartitionableResource && requestPartitionId == null) { + requestPartitionId = RequestPartitionId.defaultPartition(); + } + } + + String resourceName = myFhirContext.getResourceType(theResource); + validateSinglePartitionForCreate(requestPartitionId, resourceName, Pointcut.STORAGE_PARTITION_IDENTIFY_CREATE); + + return validateNormalizeAndNotifyHooksForRead(requestPartitionId, theRequest, theResourceType); + } + + return RequestPartitionId.allPartitions(); + } + + private boolean systemRequestHasExplicitPartition(@Nonnull SystemRequestDetails theRequest) { + return theRequest.getRequestPartitionId() != null || theRequest.getTenantId() != null; + } + + @Nonnull + @Override + public PartitionablePartitionId toStoragePartition(@Nonnull RequestPartitionId theRequestPartitionId) { + Integer partitionId = theRequestPartitionId.getFirstPartitionIdOrNull(); + if (partitionId == null) { + partitionId = myPartitionSettings.getDefaultPartitionId(); + } + return new PartitionablePartitionId(partitionId, theRequestPartitionId.getPartitionDate()); + } + + @Nonnull + @Override + public Set toReadPartitions(@Nonnull RequestPartitionId theRequestPartitionId) { + return theRequestPartitionId.getPartitionIds().stream().map(t -> t == null ? myPartitionSettings.getDefaultPartitionId() : t).collect(Collectors.toSet()); + } + + /** + * If the partition only has a name but not an ID, this method resolves the ID. + *

+ * If the partition has an ID but not a name, the name is resolved. + *

+ * If the partition has both, they are validated to ensure that they correspond. + */ + @Nonnull + private RequestPartitionId validateNormalizeAndNotifyHooksForRead(@Nonnull RequestPartitionId theRequestPartitionId, RequestDetails theRequest, @Nonnull String theResourceType) { + RequestPartitionId retVal = theRequestPartitionId; + + if (!myPartitionSettings.isUnnamedPartitionMode()) { + if (retVal.getPartitionNames() != null) { + retVal = validateAndNormalizePartitionNames(retVal); + } else if (retVal.hasPartitionIds()) { + retVal = validateAndNormalizePartitionIds(retVal); + } + } + + // Note: It's still possible that the partition only has a date but no name/id + + if (StringUtils.isNotBlank(theResourceType)) { + validateHasPartitionPermissions(theRequest, theResourceType, retVal); + } + + return retVal; + + } + + @Override + public void validateHasPartitionPermissions(RequestDetails theRequest, String theResourceType, RequestPartitionId theRequestPartitionId) { + if (myInterceptorBroadcaster.hasHooks(Pointcut.STORAGE_PARTITION_SELECTED)) { + RuntimeResourceDefinition runtimeResourceDefinition; + runtimeResourceDefinition = myFhirContext.getResourceDefinition(theResourceType); + HookParams params = new HookParams().add(RequestPartitionId.class, theRequestPartitionId).add(RequestDetails.class, theRequest).addIfMatchesType(ServletRequestDetails.class, theRequest).add(RuntimeResourceDefinition.class, runtimeResourceDefinition); + doCallHooks(myInterceptorBroadcaster, theRequest, Pointcut.STORAGE_PARTITION_SELECTED, params); + } + } + + @Override + public boolean isResourcePartitionable(String theResourceType) { + return !myNonPartitionableResourceNames.contains(theResourceType); + } + + protected abstract RequestPartitionId validateAndNormalizePartitionIds(RequestPartitionId theRequestPartitionId); + + protected abstract RequestPartitionId validateAndNormalizePartitionNames(RequestPartitionId theRequestPartitionId); + + private void validateSinglePartitionForCreate(RequestPartitionId theRequestPartitionId, @Nonnull String theResourceName, Pointcut thePointcut) { + validateRequestPartitionNotNull(theRequestPartitionId, thePointcut); + + if (theRequestPartitionId.hasPartitionIds()) { + validateSinglePartitionIdOrNameForCreate(theRequestPartitionId.getPartitionIds()); + } + validateSinglePartitionIdOrNameForCreate(theRequestPartitionId.getPartitionNames()); + + // Make sure we're not using one of the conformance resources in a non-default partition + if ((theRequestPartitionId.hasPartitionIds() && !theRequestPartitionId.getPartitionIds().contains(null)) || (theRequestPartitionId.hasPartitionNames() && !theRequestPartitionId.getPartitionNames().contains(JpaConstants.DEFAULT_PARTITION_NAME))) { + + if (!isResourcePartitionable(theResourceName)) { + String msg = myFhirContext.getLocalizer().getMessageSanitized(BaseRequestPartitionHelperSvc.class, "nonDefaultPartitionSelectedForNonPartitionable", theResourceName); + throw new UnprocessableEntityException(Msg.code(1318) + msg); + } + + } + + } + + private void validateRequestPartitionNotNull(RequestPartitionId theRequestPartitionId, Pointcut theThePointcut) { + if (theRequestPartitionId == null) { + throw new InternalErrorException(Msg.code(1319) + "No interceptor provided a value for pointcut: " + theThePointcut); + } + } + + private void validateSinglePartitionIdOrNameForCreate(@Nullable List thePartitionIds) { + if (thePartitionIds != null && thePartitionIds.size() != 1) { + throw new InternalErrorException(Msg.code(1320) + "RequestPartitionId must contain a single partition for create operations, found: " + thePartitionIds); + } + } +} diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/IRequestPartitionHelperSvc.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/IRequestPartitionHelperSvc.java index d17a004d232..fc4d3ca626d 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/IRequestPartitionHelperSvc.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/partition/IRequestPartitionHelperSvc.java @@ -66,4 +66,6 @@ public interface IRequestPartitionHelperSvc { @Nonnull Set toReadPartitions(@Nonnull RequestPartitionId theRequestPartitionId); + + boolean isResourcePartitionable(String theResourceType); }