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..3322b2eab62 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 @@ -200,12 +200,15 @@ public Resource insert(Resource resource, List paramet * @param conn * @param parameterDao * @param ifNoneMatch 0 for conditional create-on-update behavior; otherwise null + * @param resourcePayloadKey + * @param outInteractionStatus + * @param outIfNoneMatchVersion * @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 +455,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 +524,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 ? "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-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..dc577016a2c 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. */ @@ -761,7 +762,7 @@ public void testBatchHistory() throws Exception { String method = "testBatchHistory"; WebTarget target = getWebTarget(); - // Perform a 'read' and a 'vread'. + // Get the history for patientB1 Bundle bundle = buildBundle(BundleType.BATCH); bundle = addRequestToBundle(null, bundle, HTTPVerb.GET, "Patient/" + patientB1.getId() + "/_history", null, null); @@ -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("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/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java b/fhir-server/src/main/java/com/ibm/fhir/server/util/FHIRRestHelper.java index 7ed2e0ea5f8..9096a3d1b4a 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 @@ -700,7 +700,7 @@ public FHIRRestOperationResponse doUpdateMeta(FHIRPersistenceEvent event, String event.setFhirResource(newResource); event.setPrevFhirResource(ior.getPrevResource()); - // Next, invoke the 'beforeUpdate' or 'beforeCreate' interceptor methods as appropriate. + // Next, invoke the 'beforeCreate', 'beforePatch', or 'beforeUpdate' interceptor methods as appropriate. boolean updateCreate = (ior.getPrevResource() == null); if (updateCreate) { getInterceptorMgr().fireBeforeCreateEvent(event); @@ -2335,12 +2335,17 @@ 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) { method = HTTPVerb.POST; + status = "201"; } else { method = HTTPVerb.PUT; + // TODO can we set this more intelligently (e.g. in the case of a DELETE followed by a PUT) + status = "200"; } // Create the 'request' entry, and set the request.url field. @@ -2353,14 +2358,17 @@ private Bundle createHistoryBundle(List> reso .url(Url.of(method == HTTPVerb.POST ? resourceType : resourcePath)) .build(); + String fullUrl = getRequestBaseUri(type) + "/" + resourcePath; + Entry.Response response = Entry.Response.builder() - .status(string("200")) + .status(status) .etag(getEtagValue(resourceResult.getVersion())) + .location(Uri.of(fullUrl + "/_history/" + resourceResult.getVersion())) .lastModified(com.ibm.fhir.model.type.Instant.of(resourceResult.getLastUpdated().atZone(UTC))) .build(); Entry entry = Entry.builder() - .fullUrl(Uri.of(getRequestBaseUri(type) + "/" + resourcePath)) + .fullUrl(Uri.of(fullUrl)) .request(request) .response(response) .resource(resource) @@ -3097,17 +3105,17 @@ public Bundle doHistory(MultivaluedMap queryParameters, String r case CREATE: 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")); + responseBuilder.status("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")); + responseBuilder.status("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")); + responseBuilder.status("200"); break; }