Fix parameter count limitation on MSSQL/Oracle large transactions (#3075)

* Fix parameter count limitation on MSSQL/Oracle large transactions

* Changelog fix

* Move changelog

* Force a change to trigger CI

* Test refactoring
This commit is contained in:
James Agnew 2022-02-20 11:52:42 -05:00 committed by GitHub
parent e55f9d510a
commit 1e2e17784a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 39 additions and 18 deletions

View File

@ -0,0 +1,5 @@
---
type: fix
issue: 3065
title: "A regression in HAPI FHIR 5.5.0 meant that very large transactions where the bundle contained over 1000 distinct client-assigned
resource IDs could fail on MSSQL and Oracle due to SQL parameter count limitations."

View File

@ -244,23 +244,29 @@ public class IdHelperService {
public List<ResourcePersistentId> resolveResourcePersistentIdsWithCache(RequestPartitionId theRequestPartitionId, List<IIdType> theIds) {
assert myDontCheckActiveTransactionForUnitTest || TransactionSynchronizationManager.isSynchronizationActive();
for (IIdType id : theIds) {
List<ResourcePersistentId> retVal = new ArrayList<>(theIds.size());
new QueryChunker<IIdType>().chunk(theIds, ids -> doResolveResourcePersistentIdsWithCache(theRequestPartitionId, ids, retVal));
return retVal;
}
private void doResolveResourcePersistentIdsWithCache(RequestPartitionId theRequestPartitionId, List<IIdType> theInputIds, List<ResourcePersistentId> theOutputListToPopulate) {
for (IIdType id : theInputIds) {
if (!id.hasIdPart()) {
throw new InvalidRequestException(Msg.code(1101) + "Parameter value missing in request");
}
}
if (theIds.isEmpty()) {
return Collections.emptyList();
if (theInputIds.isEmpty()) {
return;
}
List<ResourcePersistentId> retVal = new ArrayList<>(theIds.size());
Set<IIdType> idsToCheck = new HashSet<>(theIds.size());
for (IIdType nextId : theIds) {
Set<IIdType> idsToCheck = new HashSet<>(theInputIds.size());
for (IIdType nextId : theInputIds) {
if (myDaoConfig.getResourceClientIdStrategy() != DaoConfig.ClientIdStrategyEnum.ANY) {
if (nextId.isIdPartValidLong()) {
retVal.add(new ResourcePersistentId(nextId.getIdPartAsLong()).setAssociatedResourceId(nextId));
theOutputListToPopulate.add(new ResourcePersistentId(nextId.getIdPartAsLong()).setAssociatedResourceId(nextId));
continue;
}
}
@ -268,7 +274,7 @@ public class IdHelperService {
String key = toForcedIdToPidKey(theRequestPartitionId, nextId.getResourceType(), nextId.getIdPart());
ResourcePersistentId cachedId = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.FORCED_ID_TO_PID, key);
if (cachedId != null) {
retVal.add(cachedId);
theOutputListToPopulate.add(cachedId);
continue;
}
@ -321,7 +327,7 @@ public class IdHelperService {
if (nextId.getResourceId() != null) {
ResourcePersistentId persistentId = new ResourcePersistentId(nextId.getResourceId());
populateAssociatedResourceId(nextId.getResourceType(), nextId.getForcedId(), persistentId);
retVal.add(persistentId);
theOutputListToPopulate.add(persistentId);
String key = toForcedIdToPidKey(theRequestPartitionId, nextId.getResourceType(), nextId.getForcedId());
myMemoryCacheService.putAfterCommit(MemoryCacheService.CacheEnum.FORCED_ID_TO_PID, key, persistentId);
@ -330,7 +336,6 @@ public class IdHelperService {
}
return retVal;
}
private void populateAssociatedResourceId(String nextResourceType, String forcedId, ResourcePersistentId persistentId) {

View File

@ -24,6 +24,8 @@ import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import io.swagger.v3.oas.annotations.media.Schema;
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
import java.util.ArrayList;
import java.util.HashMap;
@ -63,4 +65,12 @@ public class PackageInstallOutcomeJson {
getResourcesInstalled().put(theResourceType, existing + 1);
}
}
@Override
public String toString() {
return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE)
.append("message", myMessage)
.append("resourcesInstalled", myResourcesInstalled)
.toString();
}
}

View File

@ -516,7 +516,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest {
}
@Nested
public class WithContainedIndexing {
public class WithContainedIndexingIT {
@BeforeEach
public void enableContains() {
// we don't support chained or contained yet, but turn it on to test we don't blow up.
@ -753,7 +753,7 @@ public class FhirResourceDaoR4SearchWithElasticSearchIT extends BaseJpaTest {
}
@Nested
public class DateSearchTests extends BaseDateSearchDaoTests {
public class DateSearchIT extends BaseDateSearchDaoTests {
@Override
protected Fixture getFixture() {

View File

@ -458,7 +458,7 @@ public class NpmR4Test extends BaseJpaR4Test {
spec.setInstallResourceTypes(resourceList);
try {
PackageInstallOutcomeJson outcome = myPackageInstallerSvc.install(spec);
fail();
fail(outcome.toString());
} catch (ImplementationGuideInstallationException theE) {
assertThat(theE.getMessage(), containsString("Resources in a package must have a url or identifier to be loaded by the package installer."));
}
@ -478,6 +478,7 @@ public class NpmR4Test extends BaseJpaR4Test {
PackageInstallationSpec spec = new PackageInstallationSpec().setName("test-ig").setVersion("1.0.0").setInstallMode(PackageInstallationSpec.InstallModeEnum.STORE_AND_INSTALL);
spec.setInstallResourceTypes(resourceList);
PackageInstallOutcomeJson outcome = myPackageInstallerSvc.install(spec);
ourLog.info("Outcome: {}", outcome);
assertEquals(1, outcome.getResourcesInstalled().get("ImplementationGuide"));
// Be sure no further communication with the server

View File

@ -6,6 +6,7 @@ import ca.uhn.fhir.jpa.subscription.BaseSubscriptionsR5Test;
import ca.uhn.fhir.rest.api.CacheControlDirective;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import ca.uhn.fhir.util.HapiExtensions;
import org.hl7.fhir.instance.model.api.IBaseBundle;
@ -18,6 +19,7 @@ import org.springframework.beans.factory.annotation.Autowired;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
@ -363,8 +365,8 @@ public class RestHookTestR5Test extends BaseSubscriptionsR5Test {
waitForSize(0, ourCreatedObservations);
waitForSize(2, ourUpdatedObservations);
Observation observation1 = ourUpdatedObservations.get(0);
Observation observation2 = ourUpdatedObservations.get(1);
Observation observation1 = ourUpdatedObservations.stream().filter(t->t.getIdElement().getVersionIdPart().equals("1")).findFirst().orElseThrow(()->new IllegalStateException());
Observation observation2 = ourUpdatedObservations.stream().filter(t->t.getIdElement().getVersionIdPart().equals("2")).findFirst().orElseThrow(()->new IllegalStateException());
assertEquals("1", observation1.getIdElement().getVersionIdPart());
assertNull(observation1.getNoteFirstRep().getText());

View File

@ -11,13 +11,11 @@
<description>An open-source implementation of the FHIR specification in Java.</description>
<url>https://hapifhir.io</url>
<organization>
<name>Smile CDR, Inc.</name>
<url>https://smilecdr.com</url>
</organization>
<inceptionYear>2014</inceptionYear>
<issueManagement>