issue #3773 - add guard to avoid NPE for conditional patch

Signed-off-by: Lee Surprenant <lmsurpre@merative.com>
LinuxForHealth · Jul 12, 2022 · 1b2a537 · 1b2a537
1 parent 5b87fad
commit 1b2a537
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 11 deletions.
diff --git a/fhir-search/pom.xml b/fhir-search/pom.xml
@@ -27,6 +27,16 @@
             <artifactId>fhir-path</artifactId>
             <version>${project.version}</version>
         </dependency>
+        <dependency>
+            <groupId>${project.groupId}</groupId>
+            <artifactId>fhir-term</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>${project.groupId}</groupId>
+            <artifactId>fhir-cache</artifactId>
+            <version>${project.version}</version>
+        </dependency>
         <dependency>
             <!-- Not strictly required but, without it, you won't have any search parameters or compartment definitions -->
             <groupId>${project.groupId}</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
@@ -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<Patient> 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<HumanName> 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<Parameters> 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
@@ -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<Long> doRetrieveIndex(FHIROperationContext operationContext, String
         List<Long> indexIds = null;
 
         FHIRPersistenceContext context = null;
-        
+
         FHIRTransactionHelper txn = null;
         try {
             txn = new FHIRTransactionHelper(getTransaction());