Merge branch 'master' into conditional-bug

This commit is contained in:
Tadgh 2021-09-14 08:51:58 -04:00 committed by GitHub
commit 9d982c5e08
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 54 additions and 6 deletions

View File

@ -0,0 +1,5 @@
---
type: perf
issue: 2457
title: "A regression in HAPI FHIR 5.3.0 resulted in concurrent searches being executed in a sequential
(and not parallel) fashion in some circumstances."

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 2958
jira: SMILE-643
title: "Fixed issue where the processing of queries like Procedure?patient= before a cache search would cause the parameter key to be removed.
Additionally, ensured that requests like Procedure?patient= cause HTTP 400 Bad Request instead of HTTP 500 Internal Error."

View File

@ -34,6 +34,7 @@ import ca.uhn.fhir.jpa.util.MemoryCacheService;
import ca.uhn.fhir.jpa.util.QueryChunker; import ca.uhn.fhir.jpa.util.QueryChunker;
import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId; import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import com.google.common.annotations.VisibleForTesting; import com.google.common.annotations.VisibleForTesting;
@ -204,7 +205,11 @@ public class IdHelperService {
*/ */
@Nonnull @Nonnull
public List<ResourcePersistentId> resolveResourcePersistentIdsWithCache(RequestPartitionId theRequestPartitionId, List<IIdType> theIds) { public List<ResourcePersistentId> resolveResourcePersistentIdsWithCache(RequestPartitionId theRequestPartitionId, List<IIdType> theIds) {
theIds.forEach(id -> Validate.isTrue(id.hasIdPart())); for (IIdType id : theIds) {
if (!id.hasIdPart()) {
throw new InvalidRequestException("Parameter value missing in request");
}
}
if (theIds.isEmpty()) { if (theIds.isEmpty()) {
return Collections.emptyList(); return Collections.emptyList();

View File

@ -7,6 +7,7 @@ import ca.uhn.fhir.parser.StrictErrorHandler;
import ca.uhn.fhir.rest.api.CacheControlDirective; import ca.uhn.fhir.rest.api.CacheControlDirective;
import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.client.interceptor.CapturingInterceptor; import ca.uhn.fhir.rest.client.interceptor.CapturingInterceptor;
import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.IdType;
@ -27,6 +28,8 @@ import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.core.IsNot.not; import static org.hamcrest.core.IsNot.not;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assertions.fail;
public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test { public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test {
@ -227,5 +230,37 @@ public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test {
assertEquals(1, resp2.getEntry().size()); assertEquals(1, resp2.getEntry().size());
} }
@Test
public void testParamWithNoValueIsConsideredForCacheResults(){
// Given: We populate the cache by searching
myClient
.search()
.byUrl("Procedure")
.returnBundle(Bundle.class)
.execute();
// When: We search Procedure?patient=
BaseServerResponseException exception = assertThrows(BaseServerResponseException.class, () -> {myClient
.search()
.byUrl("Procedure?patient=")
.returnBundle(Bundle.class)
.execute();});
// Then: We do not get a cache hit
assertNotEquals(Constants.STATUS_HTTP_200_OK, exception.getStatusCode());
}
@Test
public void testReturn400ForParameterWithNoValue(){
// When: We search Procedure?patient=
BaseServerResponseException exception = assertThrows(BaseServerResponseException.class, () -> {myClient
.search()
.byUrl("Procedure?patient=")
.returnBundle(Bundle.class)
.execute();});
// Then: We get a HTTP 400 Bad Request
assertEquals(Constants.STATUS_HTTP_400_BAD_REQUEST, exception.getStatusCode());
}
} }

View File

@ -396,11 +396,8 @@ public class SearchParameterMap implements Serializable {
for (List<? extends IQueryParameterType> nextValuesAndIn : nextValuesAndsIn) { for (List<? extends IQueryParameterType> nextValuesAndIn : nextValuesAndsIn) {
List<IQueryParameterType> nextValuesOrsOut = new ArrayList<>(); List<IQueryParameterType> nextValuesOrsOut = new ArrayList<>();
for (IQueryParameterType nextValueOrIn : nextValuesAndIn) {
if (nextValueOrIn.getMissing() != null || isNotBlank(nextValueOrIn.getValueAsQueryToken(theCtx))) { nextValuesOrsOut.addAll(nextValuesAndIn);
nextValuesOrsOut.add(nextValueOrIn);
}
}
nextValuesOrsOut.sort(new QueryParameterTypeComparator(theCtx)); nextValuesOrsOut.sort(new QueryParameterTypeComparator(theCtx));