From 7a44fa3eb33f0cdaed5f73343a1810b9364068c9 Mon Sep 17 00:00:00 2001 From: Ibrohim Kholilul Islam Date: Wed, 8 Jul 2020 21:38:50 +0700 Subject: [PATCH] Fix GraphQL Array of Argument should be ORed (#1966) * provide IQueryParameterOr instead of IQueryParameterType to params.add * add tests --- .../fhir/jpa/graphql/JpaStorageServices.java | 99 ++++++++++++------- .../jpa/graphql/JpaStorageServicesTest.java | 27 +++++ .../provider/JpaGraphQLR4ProviderTest.java | 79 ++++++++++++--- 3 files changed, 160 insertions(+), 45 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/graphql/JpaStorageServices.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/graphql/JpaStorageServices.java index b422ad16233..27f6b8c2dfd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/graphql/JpaStorageServices.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/graphql/JpaStorageServices.java @@ -25,15 +25,24 @@ import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao; import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; +import ca.uhn.fhir.model.api.IQueryParameterOr; import ca.uhn.fhir.model.api.IQueryParameterType; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.param.DateOrListParam; import ca.uhn.fhir.rest.param.DateParam; +import ca.uhn.fhir.rest.param.NumberOrListParam; import ca.uhn.fhir.rest.param.NumberParam; +import ca.uhn.fhir.rest.param.QuantityOrListParam; import ca.uhn.fhir.rest.param.QuantityParam; +import ca.uhn.fhir.rest.param.ReferenceOrListParam; import ca.uhn.fhir.rest.param.ReferenceParam; +import ca.uhn.fhir.rest.param.SpecialOrListParam; import ca.uhn.fhir.rest.param.SpecialParam; +import ca.uhn.fhir.rest.param.StringAndListParam; +import ca.uhn.fhir.rest.param.StringOrListParam; import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.param.TokenOrListParam; import ca.uhn.fhir.rest.param.TokenParam; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.NotImplementedOperationException; @@ -56,6 +65,7 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; import java.util.stream.Collectors; +import java.util.stream.Stream; import static ca.uhn.fhir.rest.api.Constants.PARAM_FILTER; @@ -111,42 +121,63 @@ public class JpaStorageServices extends BaseHapiFhirDao implement throw new InvalidRequestException(msg); } - for (Value nextValue : nextArgument.getValues()) { - String value = nextValue.getValue(); + IQueryParameterOr queryParam; - IQueryParameterType param = null; - switch (searchParam.getParamType()) { - case NUMBER: - param = new NumberParam(value); - break; - case DATE: - param = new DateParam(value); - break; - case STRING: - param = new StringParam(value); - break; - case TOKEN: - param = new TokenParam(null, value); - break; - case REFERENCE: - param = new ReferenceParam(value); - break; - case COMPOSITE: - throw new InvalidRequestException("Composite parameters are not yet supported in GraphQL"); - case QUANTITY: - param = new QuantityParam(value); - break; - case SPECIAL: - param = new SpecialParam().setValue(value); - break; - case URI: - break; - case HAS: - break; - } - - params.add(searchParamName, param); + switch (searchParam.getParamType()) { + case NUMBER: + NumberOrListParam numberOrListParam = new NumberOrListParam(); + for (Value value: nextArgument.getValues()) { + numberOrListParam.addOr(new NumberParam(value.getValue())); + } + queryParam = numberOrListParam; + break; + case DATE: + DateOrListParam dateOrListParam = new DateOrListParam(); + for (Value value: nextArgument.getValues()) { + dateOrListParam.addOr(new DateParam(value.getValue())); + } + queryParam = dateOrListParam; + break; + case STRING: + StringOrListParam stringOrListParam = new StringOrListParam(); + for (Value value: nextArgument.getValues()) { + stringOrListParam.addOr(new StringParam(value.getValue())); + } + queryParam = stringOrListParam; + break; + case TOKEN: + TokenOrListParam tokenOrListParam = new TokenOrListParam(); + for (Value value: nextArgument.getValues()) { + tokenOrListParam.addOr(new TokenParam(value.getValue())); + } + queryParam = tokenOrListParam; + break; + case REFERENCE: + ReferenceOrListParam referenceOrListParam = new ReferenceOrListParam(); + for (Value value: nextArgument.getValues()) { + referenceOrListParam.addOr(new ReferenceParam(value.getValue())); + } + queryParam = referenceOrListParam; + break; + case QUANTITY: + QuantityOrListParam quantityOrListParam = new QuantityOrListParam(); + for (Value value: nextArgument.getValues()) { + quantityOrListParam.addOr(new QuantityParam(value.getValue())); + } + queryParam = quantityOrListParam; + break; + case SPECIAL: + SpecialOrListParam specialOrListParam = new SpecialOrListParam(); + for (Value value: nextArgument.getValues()) { + specialOrListParam.addOr(new SpecialParam().setValue(value.getValue())); + } + queryParam = specialOrListParam; + break; + default: + throw new InvalidRequestException(String.format("%s parameters are not yet supported in GraphQL", searchParam.getParamType())); } + + params.add(searchParamName, queryParam); } RequestDetails requestDetails = (RequestDetails) theAppInfo; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/graphql/JpaStorageServicesTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/graphql/JpaStorageServicesTest.java index 01bb7db42dd..dd0c5d8f626 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/graphql/JpaStorageServicesTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/graphql/JpaStorageServicesTest.java @@ -9,6 +9,7 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.model.Appointment; import org.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.Coding; +import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.utilities.graphql.Argument; import org.hl7.fhir.utilities.graphql.IGraphQLStorageServices; import org.hl7.fhir.utilities.graphql.StringValue; @@ -22,6 +23,7 @@ import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -94,4 +96,29 @@ public class JpaStorageServicesTest extends BaseJpaR4Test { assertEquals("Unknown GraphQL argument \"test\". Value GraphQL argument for this type are: [_id, _language, actor, appointment_type, based_on, date, identifier, location, part_status, patient, practitioner, reason_code, reason_reference, service_category, service_type, slot, specialty, status, supporting_info]", e.getMessage()); } } + + private void createSomePatientWithId(String id) { + Patient somePatient = new Patient(); + somePatient.setId(id); + myPatientDao.update(somePatient); + } + + @Test + public void testListResourceGraphqlArrayOfArgument() { + createSomePatientWithId("hapi-123"); + createSomePatientWithId("hapi-124"); + + Argument argument = new Argument(); + argument.setName("_id"); + argument.addValue(new StringValue("hapi-123")); + argument.addValue(new StringValue("hapi-124")); + + List result = new ArrayList<>(); + mySvc.listResources(mySrd, "Patient", Collections.singletonList(argument), result); + + assertFalse(result.isEmpty()); + + List expectedId = Arrays.asList("hapi-123", "hapi-124"); + assertTrue(result.stream().allMatch((it) -> expectedId.contains(it.getIdElement().getIdPart()))); + } } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/JpaGraphQLR4ProviderTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/JpaGraphQLR4ProviderTest.java index fb8ac5800c5..9c078cb6ae5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/JpaGraphQLR4ProviderTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/JpaGraphQLR4ProviderTest.java @@ -31,6 +31,7 @@ import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.Resource; import org.hl7.fhir.utilities.graphql.Argument; import org.hl7.fhir.utilities.graphql.IGraphQLStorageServices; +import org.hl7.fhir.utilities.graphql.Value; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -38,8 +39,11 @@ import org.junit.jupiter.api.Test; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedList; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.startsWith; @@ -158,7 +162,38 @@ public class JpaGraphQLR4ProviderTest { " }]\n" + " },{\n" + " \"name\":[{\n" + - " \"given\":[\"GivenOnlyB1\",\"GivenOnlyB2\"]\n" + + " \"given\":[\"pet\",\"GivenOnlyB1\",\"GivenOnlyB2\"]\n" + + " }]\n" + + " }]\n" + + "}" + DATA_SUFFIX), TestUtil.stripWhitespace(responseContent)); + assertThat(status.getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue(), startsWith("application/json")); + + } finally { + IOUtils.closeQuietly(status.getEntity().getContent()); + } + + } + + @Test + public void testGraphSystemArrayArgumentList() throws Exception { + String query = "{PatientList(id:[\"hapi-123\",\"hapi-124\"]){id,name{family}}}"; + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/$graphql?query=" + UrlUtil.escapeUrlParam(query)); + CloseableHttpResponse status = ourClient.execute(httpGet); + try { + String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(responseContent); + assertEquals(200, status.getStatusLine().getStatusCode()); + + assertEquals(TestUtil.stripWhitespace(DATA_PREFIX + "{\n" + + " \"PatientList\":[{\n" + + " \"id\":\"Patient/hapi-123/_history/2\",\n" + + " \"name\":[{\n" + + " \"family\":\"FAMILY 123\"\n" + + " }]\n" + + " },{\n" + + " \"id\":\"Patient/hapi-124/_history/1\",\n" + + " \"name\":[{\n" + + " \"family\":\"FAMILY 124\"\n" + " }]\n" + " }]\n" + "}" + DATA_SUFFIX), TestUtil.stripWhitespace(responseContent)); @@ -230,24 +265,46 @@ public class JpaGraphQLR4ProviderTest { ourLog.info("listResources of {} - {}", theType, theSearchParams); if (theSearchParams.size() == 1) { - String name = theSearchParams.get(0).getName(); - if ("name".equals(name)) { - Patient p = new Patient(); - p.addName() - .setFamily(theSearchParams.get(0).getValues().get(0).toString()) + Argument argument = theSearchParams.get(0); + + String name = argument.getName(); + List value = argument.getValues().stream() + .map((it) -> it.getValue()) + .collect(Collectors.toList()); + + if ("name".equals(name) && "pet".equals(value.get(0))) { + Patient patient1 = new Patient(); + patient1.addName() + .setFamily("pet") .addGiven("GIVEN1") .addGiven("GIVEN2"); - p.addName() + patient1.addName() .addGiven("GivenOnly1") .addGiven("GivenOnly2"); - theMatches.add(p); - p = new Patient(); - p.addName() + Patient patient2 = new Patient(); + patient2.addName() + .addGiven("pet") .addGiven("GivenOnlyB1") .addGiven("GivenOnlyB2"); - theMatches.add(p); + theMatches.add(patient1); + theMatches.add(patient2); + } + + if ("id".equals(name) && Arrays.asList("hapi-123", "hapi-124").containsAll(value)) { + Patient patient1 = new Patient(); + patient1.setId("Patient/hapi-123/_history/2"); + patient1.addName() + .setFamily("FAMILY 123"); + + Patient patient2 = new Patient(); + patient2.setId("Patient/hapi-124/_history/1"); + patient2.addName() + .setFamily("FAMILY 124"); + + theMatches.add(patient1); + theMatches.add(patient2); } } }