From 1b2a537b6e9322e5f80dd749168a6e0670285d46 Mon Sep 17 00:00:00 2001 From: Lee Surprenant Date: Tue, 12 Jul 2022 14:10:18 -0500 Subject: [PATCH] issue #3773 - add guard to avoid NPE for conditional patch Signed-off-by: Lee Surprenant --- fhir-search/pom.xml | 10 +++ .../ibm/fhir/server/test/FHIRPatchTest.java | 63 +++++++++++++++++++ .../ibm/fhir/server/util/FHIRRestHelper.java | 25 ++++---- 3 files changed, 87 insertions(+), 11 deletions(-) diff --git a/fhir-search/pom.xml b/fhir-search/pom.xml index 1d54de9d4e1..bfbcb21e305 100644 --- a/fhir-search/pom.xml +++ b/fhir-search/pom.xml @@ -27,6 +27,16 @@ fhir-path ${project.version} + + ${project.groupId} + fhir-term + ${project.version} + + + ${project.groupId} + fhir-cache + ${project.version} + ${project.groupId} diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRPatchTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRPatchTest.java index 225e3f9d331..457c6f22be6 100644 --- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRPatchTest.java +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/FHIRPatchTest.java @@ -1152,6 +1152,69 @@ public void testJSONPatchDeletedResource() throws Exception { assertEquals("Resource 'Patient/" + patient.getId() + "' is deleted.", oo.getIssue().get(0).getDetails().getText().getValue()); } + @Test(groups = { "fhir-patch" }) + public void testConditionalFhirPatch() throws Exception { + WebTarget target = getWebTarget(); + + // Build a new Patient and then call the 'create' API. + Patient patient = buildPatient(); + + Entity entity = Entity.entity(patient, FHIRMediaType.APPLICATION_FHIR_JSON); + Response response = target.path("Patient/" + patient.getId()).request().put(entity, Response.class); + assertResponse(response, Response.Status.CREATED.getStatusCode()); + + // Get the patient's logical id value. + String patientId = getLocationLogicalId(response); + + // create a copy of the patient and update it using the model API + Patient.Builder patientBuilder = patient.toBuilder(); + List name = new ArrayList<>(patient.getName()); + patientBuilder.name(Collections.singletonList( + name.get(0).toBuilder() + .given(string("Jack")) + .build())); + + Parameters patch = Parameters.builder() + .parameter(Parameter.builder() + .name(string("operation")) + .part(Parameter.builder() + .name(string("type")) + .value(Code.of("add")) + .build()) + .part(Parameter.builder() + .name(string("path")) + .value(string("Patient.name[0]")) + .build()) + .part(Parameter.builder() + .name(string("name")) + .value(string("given")) + .build()) + .part(Parameter.builder() + .name(string("value")) + .value(string("Jack")) + .build()) + .build()) + .build(); + + Entity patchEntity = Entity.entity(patch, FHIRMediaType.APPLICATION_FHIR_JSON); + // perform the conditional patch + response = target.path("Patient") + .queryParam("_id", patientId) + .request(FHIRMediaType.APPLICATION_FHIR_JSON) + .method("PATCH", patchEntity, Response.class); + assertResponse(response, Response.Status.OK.getStatusCode()); + + // Next, call the 'read' API to retrieve the new patient and verify it. + response = target.path("Patient/" + patientId).request(FHIRMediaType.APPLICATION_FHIR_JSON).get(); + assertResponse(response, Response.Status.OK.getStatusCode()); + Patient responsePatient = response.readEntity(Patient.class); + + patientBuilder.meta(responsePatient.getMeta()); + Patient updatedPatient = patientBuilder.build(); + + Assert.assertEquals(updatedPatient, responsePatient); + } + private void assertGoodGetResponse(Bundle.Entry entry, int expectedStatusCode, HTTPReturnPreference returnPref) throws Exception { assertNotNull(entry); Bundle.Entry.Response response = entry.getResponse(); diff --git a/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java b/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java index ad17ffbfc2e..d198ede5730 100644 --- a/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java +++ b/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java @@ -616,17 +616,20 @@ public FHIRRestOperationResponse doUpdateMeta(FHIRPersistenceEvent event, String throw buildRestException(msg, IssueType.VALUE); } - if (newResource.getId() != null) { - // If the id of the input resource is provided, it MUST match the id of the previous resource - // found by the search - if (!newResource.getId().equals(id)) { - String msg = "Input resource 'id' attribute must match the id of the search result resource."; - throw buildRestException(msg, IssueType.VALUE); + // if patch is null then we have a "normal" update, so we need to check the newResource id + if (patch == null) { + if (newResource.getId() != null) { + // If the id of the input resource is provided, it MUST match the id of the previous resource + // found by the search + if (!newResource.getId().equals(id)) { + String msg = "Input resource 'id' attribute must match the id of the search result resource."; + throw buildRestException(msg, IssueType.VALUE); + } + } else { + // The new resource does not contain an id, so we set it using the id of the + // previous resource found by the search + newResource = newResource.toBuilder().id(id).build(); } - } else { - // The new resource does not contain an id, so we set it using the id of the - // previous resource found by the search - newResource = newResource.toBuilder().id(id).build(); } // Got a match, so definitely can't be deleted @@ -3346,7 +3349,7 @@ public List doRetrieveIndex(FHIROperationContext operationContext, String List indexIds = null; FHIRPersistenceContext context = null; - + FHIRTransactionHelper txn = null; try { txn = new FHIRTransactionHelper(getTransaction());