diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2958-procedure-patient-consistent-error-handling.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2958-procedure-patient-consistent-error-handling.yaml index f90ca63a917..4cee4993a52 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2958-procedure-patient-consistent-error-handling.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_6_0/2958-procedure-patient-consistent-error-handling.yaml @@ -3,4 +3,4 @@ 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." +Additionally, ensured that requests like Procedure?patient= cause HTTP 400 Bad Request instead of HTTP 500 Internal Error." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index 8c1303f1571..c03fc350699 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -1014,9 +1014,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { logged = true; ourLog.warn("Failed during search due to invalid request: {}", t.toString()); } - }else if (t instanceof java.lang.IllegalArgumentException && t.getMessage().contentEquals("The validated expression is false")) { - logged = true; - ourLog.warn("Failed during search due to invalid request: {}", t.toString()); } if (!logged) { @@ -1031,8 +1028,6 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { int failureCode = InternalErrorException.STATUS_CODE; if (t instanceof BaseServerResponseException) { failureCode = ((BaseServerResponseException) t).getStatusCode(); - }else if(t instanceof java.lang.IllegalArgumentException && t.getMessage().contentEquals("The validated expression is false")) { - failureCode = Constants.STATUS_HTTP_400_BAD_REQUEST; } if (System.getProperty(UNIT_TEST_CAPTURE_STACK) != null) { @@ -1212,6 +1207,9 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { } catch (IOException e) { ourLog.error("IO failure during database access", e); throw new InternalErrorException(e); + } catch (IllegalArgumentException e) { + ourLog.error("Illegal Argument during database access", e); + throw new InvalidRequestException("Parameter value missing in request", e); } } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4CacheTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4CacheTest.java index fb33fcf46da..eca83de0556 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4CacheTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4CacheTest.java @@ -17,6 +17,7 @@ import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Patient; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +32,7 @@ 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; @@ -233,24 +235,27 @@ public class ResourceProviderR4CacheTest extends BaseResourceProviderR4Test { } @Test - public void testProcedurePatient(){ - Bundle resp2 = myClient + 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();}); - assertEquals(Constants.STATUS_HTTP_400_BAD_REQUEST, exception.getStatusCode()); + // Then: We do not get a cache hit + assertNotEquals(Constants.STATUS_HTTP_200_OK, exception.getStatusCode()); } @Test - public void testPatient(){ + public void testReturn400ForParameterWithNoValue(){ BaseServerResponseException exception = assertThrows(BaseServerResponseException.class, () -> {myClient .search() .byUrl("Procedure?patient=") diff --git a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java index 061531f4cc1..1e88bb00673 100644 --- a/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java +++ b/hapi-fhir-jpaserver-searchparam/src/main/java/ca/uhn/fhir/jpa/searchparam/SearchParameterMap.java @@ -396,13 +396,6 @@ public class SearchParameterMap implements Serializable { for (List nextValuesAndIn : nextValuesAndsIn) { List nextValuesOrsOut = new ArrayList<>(); - /* - for (IQueryParameterType nextValueOrIn : nextValuesAndIn) { - if (nextValueOrIn.getMissing() != null || isNotBlank(nextValueOrIn.getValueAsQueryToken(theCtx))) { - nextValuesOrsOut.add(nextValueOrIn); - } - } - */ nextValuesOrsOut.addAll(nextValuesAndIn);