Merge pull request #2982 from hapifhir/2958-consistent-error-handling
2958 Fix for consistent error handling
This commit is contained in:
commit
b2d6f75964
|
@ -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."
|
|
@ -34,6 +34,7 @@ import ca.uhn.fhir.jpa.util.MemoryCacheService;
|
|||
import ca.uhn.fhir.jpa.util.QueryChunker;
|
||||
import ca.uhn.fhir.model.primitive.IdDt;
|
||||
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.ResourceNotFoundException;
|
||||
import com.google.common.annotations.VisibleForTesting;
|
||||
|
@ -204,7 +205,11 @@ public class IdHelperService {
|
|||
*/
|
||||
@Nonnull
|
||||
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()) {
|
||||
return Collections.emptyList();
|
||||
|
|
|
@ -7,6 +7,7 @@ import ca.uhn.fhir.parser.StrictErrorHandler;
|
|||
import ca.uhn.fhir.rest.api.CacheControlDirective;
|
||||
import ca.uhn.fhir.rest.api.Constants;
|
||||
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 org.hl7.fhir.r4.model.Bundle;
|
||||
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.junit.jupiter.api.Assertions.assertEquals;
|
||||
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;
|
||||
|
||||
public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test {
|
||||
|
@ -227,5 +230,37 @@ public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test {
|
|||
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());
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -396,11 +396,8 @@ public class SearchParameterMap implements Serializable {
|
|||
for (List<? extends IQueryParameterType> nextValuesAndIn : nextValuesAndsIn) {
|
||||
|
||||
List<IQueryParameterType> nextValuesOrsOut = new ArrayList<>();
|
||||
for (IQueryParameterType nextValueOrIn : nextValuesAndIn) {
|
||||
if (nextValueOrIn.getMissing() != null || isNotBlank(nextValueOrIn.getValueAsQueryToken(theCtx))) {
|
||||
nextValuesOrsOut.add(nextValueOrIn);
|
||||
}
|
||||
}
|
||||
|
||||
nextValuesOrsOut.addAll(nextValuesAndIn);
|
||||
|
||||
nextValuesOrsOut.sort(new QueryParameterTypeComparator(theCtx));
|
||||
|
||||
|
|
Loading…
Reference in New Issue