diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/derby/DerbyResourceDAO.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/derby/DerbyResourceDAO.java index ab9c635f639..a02444b9826 100644 --- a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/derby/DerbyResourceDAO.java +++ b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/derby/DerbyResourceDAO.java @@ -203,9 +203,9 @@ public Resource insert(Resource resource, List paramet * @return the resource_id for the entry we created * @throws Exception */ - public long storeResource(String tablePrefix, List parameters, + public long storeResource(String tablePrefix, List parameters, String p_logical_id, InputStream p_payload, Timestamp p_last_updated, boolean p_is_deleted, - String p_source_key, Integer p_version, String p_parameterHashB64, Connection conn, + String p_source_key, Integer p_version, String p_parameterHashB64, Connection conn, ParameterDAO parameterDao, Integer ifNoneMatch, String resourcePayloadKey, AtomicInteger outInteractionStatus, AtomicInteger outIfNoneMatchVersion) throws Exception { @@ -452,7 +452,7 @@ public long storeResource(String tablePrefix, List para stmt.setLong(1, v_resource_id); stmt.setLong(2, v_logical_resource_id); stmt.setInt(3, p_version); - + if (p_payload != null) { stmt.setBinaryStream(4, p_payload); } else { @@ -521,7 +521,7 @@ identityCache, getResourceReferenceDAO(), getTransactionData())) { // Finally, write a record to RESOURCE_CHANGE_LOG which records each event // related to resources changes (issue-1955) - String changeType = p_is_deleted ? "D" : v_new_resource ? "C" : "U"; + String changeType = p_is_deleted ? "D" : (v_new_resource || v_currently_deleted) ? "C" : "U"; String INSERT_CHANGE_LOG = "INSERT INTO resource_change_log(resource_id, change_tstamp, resource_type_id, logical_resource_id, version_id, change_type)" + " VALUES (?,?,?,?,?,?)"; try (PreparedStatement ps = conn.prepareStatement(INSERT_CHANGE_LOG)) { diff --git a/fhir-persistence-schema/src/main/resources/db2/add_any_resource.sql b/fhir-persistence-schema/src/main/resources/db2/add_any_resource.sql index c22d69611ad..7f15ea4fa8f 100644 --- a/fhir-persistence-schema/src/main/resources/db2/add_any_resource.sql +++ b/fhir-persistence-schema/src/main/resources/db2/add_any_resource.sql @@ -168,11 +168,11 @@ BEGIN -- check the current vs new parameter hash to see if we can bypass the delete/insert IF o_current_parameter_hash IS NULL OR o_current_parameter_hash != p_parameter_hash_b64 THEN - -- existing resource, so need to delete all its parameters. - -- TODO patch parameter sets instead of all delete/all insert. - PREPARE stmt FROM 'CALL ' || v_schema_name || '.delete_resource_parameters(?,?)'; - EXECUTE stmt USING p_resource_type, v_logical_resource_id; - END IF; -- end if parameter hash is different + -- existing resource, so need to delete all its parameters. + -- TODO patch parameter sets instead of all delete/all insert. + PREPARE stmt FROM 'CALL ' || v_schema_name || '.delete_resource_parameters(?,?)'; + EXECUTE stmt USING p_resource_type, v_logical_resource_id; + END IF; -- end if parameter hash is different END IF; -- end if existing resource PREPARE stmt FROM @@ -191,18 +191,18 @@ BEGIN PREPARE stmt FROM 'UPDATE ' || v_schema_name || '.logical_resources SET is_deleted = ?, last_updated = ?, parameter_hash = ? WHERE logical_resource_id = ?'; EXECUTE stmt USING p_is_deleted, p_last_updated, p_parameter_hash_b64, v_logical_resource_id; END IF; - + -- DB2 doesn't support user defined array types in dynamic SQL UNNEST/CAST statements, -- so we can no longer insert the parameters here - instead we have to use individual -- JDBC statements. - + -- Finally, write a record to RESOURCE_CHANGE_LOG which records each event -- related to resources changes (issue-1955) IF p_is_deleted = 'Y' THEN SET v_change_type = 'D'; ELSE - IF v_new_resource = 0 + IF v_new_resource = 0 AND v_currently_deleted = 'N' THEN SET v_change_type = 'U'; ELSE diff --git a/fhir-persistence-schema/src/main/resources/postgres/add_any_resource.sql b/fhir-persistence-schema/src/main/resources/postgres/add_any_resource.sql index 3be025c145c..0130d2054e0 100644 --- a/fhir-persistence-schema/src/main/resources/postgres/add_any_resource.sql +++ b/fhir-persistence-schema/src/main/resources/postgres/add_any_resource.sql @@ -165,7 +165,7 @@ BEGIN || ' VALUES ($1, $2, $3, $4, $5, $6, $7)' USING v_resource_id, v_logical_resource_id, p_version, p_payload, p_last_updated, p_is_deleted, p_resource_payload_key; - + IF v_new_resource = 0 THEN -- As this is an existing logical resource, we need to update the xx_logical_resource values to match -- the values of the current resource. For new resources, these are added by the insert so we don't @@ -184,7 +184,7 @@ BEGIN THEN v_change_type := 'D'; ELSE - IF v_new_resource = 0 + IF v_new_resource = 0 AND v_currently_deleted = 'N' THEN v_change_type := 'U'; ELSE diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/BundleTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/BundleTest.java index 3df654b5c1f..c00b841eee4 100644 --- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/BundleTest.java +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/BundleTest.java @@ -1,5 +1,5 @@ /* - * (C) Copyright IBM Corp. 2017, 2021 + * (C) Copyright IBM Corp. 2017, 2022 * * SPDX-License-Identifier: Apache-2.0 */ @@ -16,10 +16,6 @@ import java.util.List; import java.util.UUID; -import jakarta.json.Json; -import jakarta.json.JsonArray; -import jakarta.json.JsonObject; -import jakarta.json.JsonObjectBuilder; import javax.ws.rs.client.Entity; import javax.ws.rs.client.WebTarget; import javax.ws.rs.core.Response; @@ -54,6 +50,11 @@ import com.ibm.fhir.model.type.code.HTTPVerb; import com.ibm.fhir.model.type.code.IssueType; +import jakarta.json.Json; +import jakarta.json.JsonArray; +import jakarta.json.JsonObject; +import jakarta.json.JsonObjectBuilder; + /** * This class tests 'batch' and 'transaction' interactions. */ @@ -787,7 +788,7 @@ public void testBatchHistory() throws Exception { if(entry.getResponse() != null){ String returnedStatus = entry.getResponse().getStatus().getValue(); assertNotNull(returnedStatus); - assertTrue(returnedStatus.startsWith("200")); + assertTrue(returnedStatus.startsWith("200") || returnedStatus.startsWith("201")); result = true; } } @@ -1431,7 +1432,7 @@ public void testTransactionUpdatesError() throws Exception { assertHistoryResults(target, "Patient/" + patientT1.getId() + "/_history", 2); assertHistoryResults(target, "Patient/" + patientT2.getId() + "/_history", 2); } - + @Test(groups = { "transaction" }, dependsOnMethods = { "testTransactionUpdates" }) public void testTransactionInvalidRequestError() throws Exception { String method = "testTransactionInvalidRequestError"; @@ -1451,8 +1452,8 @@ public void testTransactionInvalidRequestError() throws Exception { patientT1); bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, "Patient/" + patientT2.getId(), null, patientT2); - - + + // the URL does not have an id and nor is it a conditional update, so this is an error // which should be picked up when translating the bundle entry into an interaction bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, "Observation", null, @@ -2041,7 +2042,7 @@ public void testTransactionLocalRefs2() throws Exception { Resource patientResource = patientEntry.getResource(); assertTrue(patientResource instanceof Patient); patient = (Patient) patientResource; - + // Verify the Patient.managingOrganization field. assertNotNull(patient.getManagingOrganization()); assertNotNull(patient.getManagingOrganization().getReference()); @@ -2055,7 +2056,7 @@ public void testTransactionLocalRefs2() throws Exception { actualReference = patient.getGeneralPractitioner().get(0).getReference().getValue(); assertNotNull(actualReference); assertEquals(actualReference, expectedPractitionerReference); - + // Next, check each observation to make sure their local references were // processed correctly. for (int i = 3; i < 5; i++) { @@ -2382,10 +2383,10 @@ public void testBatchConditionalUpdates() throws Exception { Bundle bundle = buildBundle(BundleType.BATCH); bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, urlString, null, patient); - + // Removed for 1869. We no longer support bundles with multiple updates for the same resource. // bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, urlString, null, patient.toBuilder().id(null).build()); - + bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, multipleMatches, null, patient); bundle = addRequestToBundle(null, bundle, HTTPVerb.PUT, badSearch, null, patient); @@ -2641,7 +2642,7 @@ public void testTransactionBundleWithSkippableUpdates() throws Exception { assertEquals(entry1.getResponse().getLocation().getValue(), "Patient/"+randomId+"/_history/1"); Patient responsePatient = entry1.getResource().as(Patient.class); - + // Transaction 2. PUT the same patient again. Should be skipped because the resource matches Bundle.Entry bundleEntry2 = Bundle.Entry.builder() .fullUrl(Uri.of("urn:2")) @@ -2715,7 +2716,7 @@ public void testTransactionBundleWithSkippableUpdates() throws Exception { } /** - * To test If-None-Match (conditional create-on-update) we must use + * To test If-None-Match (conditional create-on-update) we must use * multiple requests because: * "A resource can only appear in a transaction once (by identity)." * Requests: @@ -2767,7 +2768,7 @@ public void testTransactionBundleWithIfNoneMatch() throws Exception { assertEquals(entry1.getResponse().getStatus().getValue(), "201"); assertEquals(entry1.getResponse().getLocation().getValue(), "Patient/"+randomId+"/_history/1"); - + // Interaction 2. PUT the same patient again. Should be skipped because IfNoneMatch - because the // processing error for the entry is 412, this should fail the bundle with a 400 Bundle.Entry bundleEntry2 = Bundle.Entry.builder() @@ -2794,7 +2795,7 @@ public void testTransactionBundleWithIfNoneMatch() throws Exception { FHIRResponse response3 = client.delete(Patient.class.getSimpleName(), randomId); assertNotNull(response3); assertResponse(response3.getResponse(), Response.Status.OK.getStatusCode()); - + // Interaction 4. Undelete Bundle.Entry bundleEntry4 = Bundle.Entry.builder() .fullUrl(Uri.of("urn:2")) @@ -2821,13 +2822,13 @@ public void testTransactionBundleWithIfNoneMatch() throws Exception { // Undelete uses 201 Created to pass Touchstone assertEquals(entry4.getResponse().getStatus().getValue(), "201"); - + // Version 2 is the delete marker, so should be version 3 after undelete assertEquals(entry4.getResponse().getLocation().getValue(), "Patient/"+randomId+"/_history/3"); } /** - * To test If-None-Match (conditional create-on-update) we must use + * To test If-None-Match (conditional create-on-update) we must use * multiple requests because: * "A resource can only appear in a transaction once (by identity)." * Requests: @@ -2877,7 +2878,7 @@ public void testBatchBundleWithIfNoneMatch() throws Exception { assertEquals(entry1.getResponse().getStatus().getValue(), "201"); assertEquals(entry1.getResponse().getLocation().getValue(), "Patient/"+randomId+"/_history/1"); - + // Interaction 2. PUT the same patient again. Should be skipped because IfNoneMatch Bundle.Entry bundleEntry2 = Bundle.Entry.builder() .fullUrl(Uri.of("urn:2")) @@ -2907,7 +2908,7 @@ public void testBatchBundleWithIfNoneMatch() throws Exception { FHIRResponse response3 = client.delete(Patient.class.getSimpleName(), randomId); assertNotNull(response3); assertResponse(response3.getResponse(), Response.Status.OK.getStatusCode()); - + // Interaction 4. Undelete Bundle.Entry bundleEntry4 = Bundle.Entry.builder() .fullUrl(Uri.of("urn:2")) @@ -2934,7 +2935,7 @@ public void testBatchBundleWithIfNoneMatch() throws Exception { // Undelete uses 201 Created to pass Touchstone assertEquals(entry4.getResponse().getStatus().getValue(), "201"); - + // Version 2 is the delete marker, so should be version 3 after undelete assertEquals(entry4.getResponse().getLocation().getValue(), "Patient/"+randomId+"/_history/3"); } @@ -3005,7 +3006,7 @@ private void assertGoodGetResponse(Bundle.Entry entry, int expectedStatusCode, H assertNotNull(response); assertNotNull(response.getStatus()); - + // TestNG: ACTUAL, EXPECTED assertEquals(response.getStatus().getValue(), Integer.toString(expectedStatusCode)); diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SystemHistoryTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SystemHistoryTest.java index 2a16ee085fa..6887383f0c7 100644 --- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SystemHistoryTest.java +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/SystemHistoryTest.java @@ -6,6 +6,7 @@ package com.ibm.fhir.server.test; +import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertTrue; import static org.testng.AssertJUnit.assertNotNull; @@ -38,7 +39,7 @@ * [base]/_history?_count=10 * [base]/_history?_count=10&_since=2021-02-21T03:00:53.878052Z" * [base]/_history?_count=10&_before=2021-02-21T03:00:53.878052Z" - * + * * Error case. Don't mix paging styles * [base]/_history?_count=10&_since=2021-02-21T03:00:53.878052Z&_changeIdMarker=4" */ @@ -47,7 +48,7 @@ public class SystemHistoryTest extends FHIRServerTestBase { // Create some resources, update, delete and undelete private boolean deleteSupported = false; - + // the id of the test patient we create private String patientId; @@ -80,29 +81,26 @@ public void populateResourcesForHistory() throws Exception { Patient patient = TestUtil.readLocalResource("patient-example-a.json"); patient = patient.toBuilder().gender(AdministrativeGender.MALE).build(); - Entity entity = - Entity.entity(patient, FHIRMediaType.APPLICATION_FHIR_JSON); - Response response = - target.path("Patient").request() - .post(entity, Response.class); + Entity entity = Entity.entity(patient, FHIRMediaType.APPLICATION_FHIR_JSON); + Response response = target.path("Patient").request().post(entity, Response.class); assertResponse(response, Response.Status.CREATED.getStatusCode()); // Get the patient's logical id value. this.patientId = getLocationLogicalId(response); // Next, call the 'read' API to retrieve the new patient and verify it. - response = target.path("Patient/" - + patientId).request(FHIRMediaType.APPLICATION_FHIR_JSON) + response = target.path("Patient/" + patientId) + .request(FHIRMediaType.APPLICATION_FHIR_JSON) .get(); assertResponse(response, Response.Status.OK.getStatusCode()); Patient responsePatient = response.readEntity(Patient.class); TestUtil.assertResourceEquals(patient, responsePatient); - updateResource(responsePatient); // 1 updateResource(responsePatient); // 2 - deleteResource("Patient", responsePatient.getId()); // 3 - undeleteResource(responsePatient); // 4 - updateResource(responsePatient); // 5 + updateResource(responsePatient); // 3 + deleteResource("Patient", responsePatient.getId()); // 4 + undeleteResource(responsePatient); // 5 + updateResource(responsePatient); // 6 // Create an Observation, so that we can check the type filtering works Observation observation = @@ -116,6 +114,45 @@ public void populateResourcesForHistory() throws Exception { assertResponse(observationResponse, Response.Status.CREATED.getStatusCode()); } + @Test(dependsOnMethods = {"populateResourcesForHistory"}) + public void testInstanceHistory() throws Exception { + if (!deleteSupported) { + return; + } + WebTarget target = getWebTarget(); + + assertNotNull(this.patientId); + + // number of versions created in populateResourcesForHistory + int totalVersions = 6; + + // Array for storing actual results + String[] statusCodes = new String[totalVersions]; + + String requestPath = "Patient/" + patientId + "/_history"; + int found = 0; + Response historyResponse = target.path(requestPath).request().get(Response.class); + assertResponse(historyResponse, Response.Status.OK.getStatusCode()); + + Bundle bundle = historyResponse.readEntity(Bundle.class); + + // Check the bundle to see if we found the patient + for (Bundle.Entry be: bundle.getEntry()) { + // simple way to see if our patient has appeared + String fullUrl = be.getFullUrl().getValue(); + if (fullUrl.contains("Patient/" + patientId)) { + found++; + String locationURL = be.getResponse().getLocation().getValue(); + // example: "https://example.com/fhir/Patient/id/_history/5" + int versionId = Integer.parseInt(locationURL.substring(locationURL.lastIndexOf("/") + 1)); + statusCodes[versionId - 1] = be.getResponse().getStatus().getValue(); + } + } + + assertEquals(found, totalVersions, "All versions of the resource should have a single entry"); + // the 5th instance should be a 201 but is a 200 due to https://github.com/IBM/FHIR/issues/3507#issuecomment-1081157116 + assertEquals(statusCodes, new String[]{"201", "200", "200", "200", "200", "200"}); + } @Test(dependsOnMethods = {"populateResourcesForHistory"}) public void testSystemHistoryWithTypePatient() throws Exception { @@ -130,39 +167,44 @@ public void testSystemHistoryWithTypePatient() throws Exception { // _since filter, we'll probably get back data which has been created by // other tests, so the only assertion we can make is we get back at least // the number of change records we expect. - - // Follow the next links until we get + + // number of versions created in populateResourcesForHistory + int totalVersions = 6; + + // Array for storing actual results + String[] statusCodes = new String[totalVersions]; + + // Follow the next links until we get all the versions for the target Patient resource String requestPath = "Patient/_history"; - boolean found = false; - int count; + int found = 0; + int count = 10; do { Response historyResponse = target.path(requestPath) - .queryParam("_count", "2") + .queryParam("_count", Integer.toString(count)) .request().get(Response.class); assertResponse(historyResponse, Response.Status.OK.getStatusCode()); Bundle bundle = historyResponse.readEntity(Bundle.class); - + // Check the bundle to see if we found the patient for (Bundle.Entry be: bundle.getEntry()) { // simple way to see if our patient has appeared String fullUrl = be.getFullUrl().getValue(); if (fullUrl.contains("Patient/" + patientId)) { - found = true; + found++; + String locationURL = be.getResponse().getLocation().getValue(); + // example: "https://example.com/fhir/Patient/id/_history/5" + int versionId = Integer.parseInt(locationURL.substring(locationURL.lastIndexOf("/") + 1)); + statusCodes[versionId - 1] = be.getResponse().getStatus().getValue(); } } - - // See if there's more work to do - count = bundle.getEntry().size(); - if (!found && count > 0) { - // set up to follow the next link - requestPath = getNextLink(bundle); - assertNotNull(requestPath); - } - } while (count > 0 && !found); - - assertTrue(found, "Patient id in history"); + + requestPath = getNextLink(bundle); + } while (found < totalVersions && requestPath != null); + + assertEquals(found, totalVersions, "All versions of the resource should have a single entry"); + assertEquals(statusCodes, new String[]{"201", "200", "200", "200", "201", "200"}); } @Test(dependsOnMethods = {"populateResourcesForHistory"}) @@ -181,7 +223,7 @@ public void testSystemHistoryWithTypeObservation() throws Exception { assertNotNull(bundle.getEntry()); assertTrue(bundle.getEntry().size() >= 1); } - + @Test(dependsOnMethods = {"populateResourcesForHistory"}) public void testSystemHistoryWithMultipleTypes() throws Exception { if (!deleteSupported) { @@ -200,7 +242,7 @@ public void testSystemHistoryWithMultipleTypes() throws Exception { assertNotNull(bundle); assertNotNull(bundle.getEntry()); assertTrue(bundle.getEntry().size() >= 5); - + // Check that the next link was composed correctly: String nextLink = getNextLink(bundle); assertNotNull(nextLink); @@ -236,7 +278,7 @@ public void testSystemHistoryWithBadSort() throws Exception { .queryParam("_sort", "bogus") .request().get(Response.class); assertResponse(historyResponse, Response.Status.BAD_REQUEST.getStatusCode()); - } + } @Test(dependsOnMethods = {"populateResourcesForHistory"}) public void testSystemHistoryWithTypePatientAndOrderNone() throws Exception { @@ -251,8 +293,8 @@ public void testSystemHistoryWithTypePatientAndOrderNone() throws Exception { // _since filter, we'll probably get back data which has been created by // other tests, so the only assertion we can make is we get back at least // the number of change records we expect. - - // Follow the next links until we + + // Follow the next links until we String requestPath = "Patient/_history"; boolean found = false; int count; @@ -272,7 +314,7 @@ public void testSystemHistoryWithTypePatientAndOrderNone() throws Exception { assertResponse(historyResponse, Response.Status.OK.getStatusCode()); Bundle bundle = historyResponse.readEntity(Bundle.class); - + // Check the bundle to see if we found the patient for (Bundle.Entry be: bundle.getEntry()) { // simple way to see if our patient has appeared @@ -280,7 +322,7 @@ public void testSystemHistoryWithTypePatientAndOrderNone() throws Exception { if (fullUrl.contains("Patient/" + patientId)) { found = true; } - + // Check that the resourceId value for the resource is always increasing. For // whole system history interactions, this id goes in the bundle.entry.id field long resourceId = Long.parseLong(be.getId()); @@ -296,7 +338,7 @@ public void testSystemHistoryWithTypePatientAndOrderNone() throws Exception { assertNotNull(nextPath); } } while (count > 0); - + assertTrue(found, "Patient id in history"); } @@ -324,7 +366,7 @@ public void testSystemHistoryWithTypePatientAndOrderASC() throws Exception { int count; Instant prevLastUpdated = null; String nextPath = null; - String prevDigest = null; // to help see if the response changed + String prevDigest = null; // to help see if the response changed Instant since = Instant.now().minus(1, ChronoUnit.HOURS); Instant before = Instant.now().plus(1, ChronoUnit.HOURS); do { @@ -349,7 +391,7 @@ public void testSystemHistoryWithTypePatientAndOrderASC() throws Exception { assertResponse(historyResponse, Response.Status.OK.getStatusCode()); Bundle bundle = historyResponse.readEntity(Bundle.class); - + // Check the bundle to see if we found the patient MessageDigest digest = MessageDigest.getInstance("SHA-256"); for (Bundle.Entry be: bundle.getEntry()) { @@ -387,7 +429,7 @@ public void testSystemHistoryWithTypePatientAndOrderASC() throws Exception { assertNotNull(nextPath); } } while (count > 0); - + assertTrue(found, "Patient id in history"); } @@ -404,14 +446,14 @@ public void testSystemHistoryWithTypePatientAndOrderDESC() throws Exception { // _since filter, we'll probably get back data which has been created by // other tests, so the only assertion we can make is we get back at least // the number of change records we expect. - - // Follow the next links until we + + // Follow the next links until we String requestPath = "Patient/_history"; boolean found = false; int count; Instant prevLastUpdated = null; String nextPath = null; - String prevDigest = null; // to help see if the response changed + String prevDigest = null; // to help see if the response changed do { final Response historyResponse; if (nextPath == null) { @@ -426,7 +468,7 @@ public void testSystemHistoryWithTypePatientAndOrderDESC() throws Exception { assertResponse(historyResponse, Response.Status.OK.getStatusCode()); Bundle bundle = historyResponse.readEntity(Bundle.class); - + // Check the bundle to see if we found the patient MessageDigest digest = MessageDigest.getInstance("SHA-256"); for (Bundle.Entry be: bundle.getEntry()) { @@ -464,7 +506,7 @@ public void testSystemHistoryWithTypePatientAndOrderDESC() throws Exception { assertNotNull(nextPath); } } while (count > 0); - + assertTrue(found, "Patient id in history"); } diff --git a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/operation/EraseOperationTest.java b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/operation/EraseOperationTest.java index 1c6b6c0dd64..b8548b9e807 100644 --- a/fhir-server-test/src/test/java/com/ibm/fhir/server/test/operation/EraseOperationTest.java +++ b/fhir-server-test/src/test/java/com/ibm/fhir/server/test/operation/EraseOperationTest.java @@ -738,6 +738,7 @@ public void testHistoryAfterVersionErase() throws IOException, FHIRParserExcepti Bundle historyBundle = historyResponse.readEntity(Bundle.class); List entries = historyBundle.getEntry(); assertEquals(entries.size(), 2); + // This really should be a 201 but we get a 200 instead due to https://github.com/IBM/FHIR/issues/3507#issuecomment-1081157116 assertEquals(entries.get(0).getResponse().getStatus().getValue(), "200"); // V2 assertEquals(entries.get(0).getRequest().getMethod().getValue(), "PUT"); // V2 assertEquals(entries.get(1).getResponse().getStatus().getValue(), "200"); // V1 @@ -784,7 +785,7 @@ public void testSearchAfterVersionErase() throws IOException, FHIRParserExceptio response = target.path("Condition/" + conditionId).request(FHIRMediaType.APPLICATION_FHIR_JSON).get(); assertResponse(response, Response.Status.OK.getStatusCode()); } - + // Perform a search showing that we can include Patient version 1 before we erase it WebTarget searchTarget = target.path("Condition").queryParam("_id", conditionId).queryParam("_include", "Condition:subject"); Response searchResponse = searchTarget.request(FHIRMediaType.APPLICATION_FHIR_JSON) 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 d61fc1938df..792721ecc2d 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 @@ -2317,8 +2317,8 @@ private void validateReferenceParams(List chainQueryParameters, /** * Creates a bundle that will hold the results of a history operation. * - * @param resources - * the list of resources to include in the bundle + * @param resourcesResults + * the list of resource results to include in the bundle, sorted with the more recent changes first * @param historyContext * the FHIRHistoryContext associated with the history operation * @param type @@ -2350,12 +2350,18 @@ private Bundle createHistoryBundle(List> reso // Determine the correct method to include in this history entry (POST, PUT, DELETE). HTTPVerb method; + String status; if (resourceResult.isDeleted() || resource == null) { method = HTTPVerb.DELETE; + status = "200"; } else if (resourceResult.getVersion() == 1) { + // it may have been a PUT in the create-on-update case, but we use POST here anyway method = HTTPVerb.POST; + status = "201"; } else { method = HTTPVerb.PUT; + // it may have been a 201 (Created) in the "undelete" case, but we use 200 here anyway + status = "200"; } // Create the 'request' entry, and set the request.url field. @@ -2368,19 +2374,21 @@ private Bundle createHistoryBundle(List> reso .url(Url.of(method == HTTPVerb.POST ? resourceType : resourcePath)) .build(); - Entry.Response response = - Entry.Response.builder() - .status(string("200")) + String fullUrl = getRequestBaseUri(type) + "/" + resourcePath; + + Entry.Response response = Entry.Response.builder() + .status(status) .etag(getEtagValue(resourceResult.getVersion())) .lastModified(com.ibm.fhir.model.type.Instant.of(resourceResult.getLastUpdated().atZone(UTC))) + .location(Uri.of(fullUrl + "/_history/" + resourceResult.getVersion())) .build(); - Entry entry = - Entry.builder().request(request) - .fullUrl(Uri.of(getRequestBaseUri(type) + "/" + resourcePath)) - .response(response) - .resource(resource) - .build(); + Entry entry = Entry.builder() + .request(request) + .fullUrl(Uri.of(fullUrl)) + .response(response) + .resource(resource) + .build(); bundleBuilder.entry(entry); } @@ -3116,19 +3124,19 @@ public Bundle doHistory(MultivaluedMap queryParameters, String r Entry.Response.Builder responseBuilder = Entry.Response.builder(); switch (changeRecord.getChangeType()) { case CREATE: - requestBuilder.method(HTTPVerb.POST); + requestBuilder.method(changeRecord.getVersionId() > 1 ? HTTPVerb.PUT : HTTPVerb.POST); requestBuilder.url(Url.of(changeRecord.getResourceTypeName())); - responseBuilder.status(com.ibm.fhir.model.type.String.of("201 Created")); + responseBuilder.status(com.ibm.fhir.model.type.String.of("201")); break; case UPDATE: requestBuilder.method(HTTPVerb.PUT); requestBuilder.url(Url.of(changeRecord.getResourceTypeName() + "/" + changeRecord.getLogicalId())); - responseBuilder.status(com.ibm.fhir.model.type.String.of("200 OK")); + responseBuilder.status(com.ibm.fhir.model.type.String.of("200")); break; case DELETE: requestBuilder.method(HTTPVerb.DELETE); requestBuilder.url(Url.of(changeRecord.getResourceTypeName() + "/" + changeRecord.getLogicalId())); - responseBuilder.status(com.ibm.fhir.model.type.String.of("200 OK")); + responseBuilder.status(com.ibm.fhir.model.type.String.of("200")); break; }