From d5fbcf8e82af95d93a4a0997476646974eee675d Mon Sep 17 00:00:00 2001 From: James Date: Tue, 7 Feb 2017 07:44:57 -0500 Subject: [PATCH] Return an OperationOutcome in the response for a delete in JPA --- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 12 +- .../dstu3/ResourceProviderDstu3Test.java | 116 ++++++++++++------ src/changes/changes.xml | 4 + 3 files changed, 90 insertions(+), 42 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 9f504f5d11b..446534ad694 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -226,8 +226,16 @@ public abstract class BaseHapiFhirResourceDao extends B validateDeleteConflictsEmptyOrThrowException(deleteConflicts); + IBaseOperationOutcome oo = OperationOutcomeUtil.newInstance(getContext()); + String message = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "successfulDeletes", 1, w.getMillis()); + String severity = "information"; + String code = "informational"; + OperationOutcomeUtil.addIssue(getContext(), oo, severity, message, null, code); + ourLog.info("Processed delete on {} in {}ms", theId.getValue(), w.getMillisAndRestart()); - return toMethodOutcome(savedEntity, null); + DaoMethodOutcome retVal = toMethodOutcome(savedEntity, null); + retVal.setOperationOutcome(oo); + return retVal; } @Override @@ -287,7 +295,7 @@ public abstract class BaseHapiFhirResourceDao extends B OperationOutcomeUtil.addIssue(getContext(), oo, severity, message, null, code); } else { oo = OperationOutcomeUtil.newInstance(getContext()); - String message = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "successfulDeletes", theUrl, deletedResources.size(), w.getMillis()); + String message = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "successfulDeletes", deletedResources.size(), w.getMillis()); String severity = "information"; String code = "informational"; OperationOutcomeUtil.addIssue(getContext(), oo, severity, message, null, code); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 341adb09a2d..8a6eebe4060 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -149,7 +149,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { @After public void after() throws Exception { super.after(); - + myDaoConfig.setAllowMultipleDelete(new DaoConfig().isAllowMultipleDelete()); myDaoConfig.setAllowExternalReferences(new DaoConfig().isAllowExternalReferences()); } @@ -160,15 +160,14 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { myDaoConfig.setAllowMultipleDelete(true); } - - + private void checkParamMissing(String paramName) throws IOException, ClientProtocolException { HttpGet get = new HttpGet(ourServerBase + "/Observation?" + paramName + ":missing=false"); CloseableHttpResponse resp = ourHttpClient.execute(get); IOUtils.closeQuietly(resp.getEntity().getContent()); assertEquals(200, resp.getStatusLine().getStatusCode()); } - + private ArrayList genResourcesOfType(Bundle theRes, Class theClass) { ArrayList retVal = new ArrayList(); for (BundleEntryComponent next : theRes.getEntry()) { @@ -197,7 +196,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { List exts = basic.getExtensionsByUrl("http://localhost:1080/hapi-fhir-jpaserver-example/baseDstu2/StructureDefinition/DateID"); assertEquals(1, exts.size()); } - + private List searchAndReturnUnqualifiedIdValues(String uri) throws IOException, ClientProtocolException { List ids; HttpGet get = new HttpGet(uri); @@ -339,9 +338,9 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { Binary binary = new Binary(); binary.setContent(arr); binary.setContentType("dansk"); - + IIdType resource = ourClient.create().resource(binary).execute().getId(); - + Binary fromDB = ourClient.read().resource(Binary.class).withId(resource.toVersionless()).execute(); assertEquals("1", fromDB.getIdElement().getVersionIdPart()); @@ -355,7 +354,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } finally { IOUtils.closeQuietly(resp); } - + fromDB = ourClient.read().resource(Binary.class).withId(resource.toVersionless()).execute(); assertEquals("2", fromDB.getIdElement().getVersionIdPart()); @@ -370,12 +369,12 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } finally { IOUtils.closeQuietly(resp); } - + fromDB = ourClient.read().resource(Binary.class).withId(resource.toVersionless()).execute(); assertEquals("3", fromDB.getIdElement().getVersionIdPart()); // Now an update with the wrong ID in the body - + arr[0] = 4; binary.setId(""); encoded = myFhirCtx.newJsonParser().encodeResourceToString(binary); @@ -387,7 +386,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } finally { IOUtils.closeQuietly(resp); } - + fromDB = ourClient.read().resource(Binary.class).withId(resource.toVersionless()).execute(); assertEquals("3", fromDB.getIdElement().getVersionIdPart()); @@ -726,7 +725,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { * Test for #345 */ @Test - public void testDeleteNormal() throws IOException { + public void testDeleteNormal() { Patient p = new Patient(); p.addName().setFamily("FAM"); IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); @@ -743,6 +742,17 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } } + @Test + public void testDeleteReturnsOperationOutcome() { + Patient p = new Patient(); + p.addName().setFamily("FAM"); + IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); + + IBaseOperationOutcome resp = ourClient.delete().resourceById(id).execute(); + OperationOutcome oo = (OperationOutcome) resp; + assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Successfully deleted 1 resource(s) in ")); + } + @Test public void testDeleteResourceConditional1() throws IOException { String methodName = "testDeleteResourceConditional1"; @@ -768,6 +778,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { response = ourHttpClient.execute(delete); try { assertEquals(200, response.getStatusLine().getStatusCode()); + String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(resp); + OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, resp); + assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Successfully deleted 1 resource(s) in ")); } finally { response.close(); } @@ -777,6 +791,24 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { try { ourLog.info(response.toString()); assertEquals(Constants.STATUS_HTTP_410_GONE, response.getStatusLine().getStatusCode()); + String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(resp); + OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, resp); + assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Resource was deleted at")); + } finally { + response.close(); + } + + // Delete should now have no matches + + delete = new HttpDelete(ourServerBase + "/Patient?name=" + methodName); + response = ourHttpClient.execute(delete); + try { + assertEquals(200, response.getStatusLine().getStatusCode()); + String resp = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info(resp); + OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, resp); + assertThat(oo.getIssueFirstRep().getDiagnostics(), startsWith("Unable to find resource matching URL \"Patient?name=testDeleteResourceConditional1\". Deletion failed.")); } finally { response.close(); } @@ -1781,7 +1813,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info("Response: {}", respString); assertEquals(400, response.getStatusLine().getStatusCode()); OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, respString); - assertEquals("Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"AAA\" does not match URL ID of \"" + id.getIdPart() + "\"", oo.getIssue().get(0).getDiagnostics()); + assertEquals( + "Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"AAA\" does not match URL ID of \"" + + id.getIdPart() + "\"", + oo.getIssue().get(0).getDiagnostics()); } finally { response.close(); } @@ -1809,12 +1844,11 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { @Test public void testIncludeWithExternalReferences() { myDaoConfig.setAllowExternalReferences(true); - + Patient p = new Patient(); p.getManagingOrganization().setReference("http://example.com/Organization/123"); ourClient.create().resource(p).execute(); - - + Bundle b = ourClient.search().forResource("Patient").include(Patient.INCLUDE_ORGANIZATION).returnBundle(Bundle.class).execute(); assertEquals(1, b.getEntry().size()); } @@ -1834,7 +1868,6 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } } - @Test public void testMetadataSuperParamsAreIncluded() throws IOException { StructureDefinition p = new StructureDefinition(); @@ -1843,12 +1876,12 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { IIdType id = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); Bundle resp = ourClient - .search() - .forResource(StructureDefinition.class) - .where(StructureDefinition.URL.matches().value("http://example.com/foo")) - .returnBundle(Bundle.class) - .execute(); - + .search() + .forResource(StructureDefinition.class) + .where(StructureDefinition.URL.matches().value("http://example.com/foo")) + .returnBundle(Bundle.class) + .execute(); + assertEquals(1, resp.getTotal()); } @@ -2101,9 +2134,10 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { String input = IOUtils.toString(getClass().getResourceAsStream("/two_questionnaires.json"), StandardCharsets.UTF_8); String respString = ourClient.transaction().withBundle(input).prettyPrint().execute(); ourLog.info(respString); - - ourHttpClient.execute(new HttpGet("http://localhost:" + ourPort + "/QuestionnaireResponse?patient=QR3295&questionnaire=profile&_sort:desc=authored&_count=5&_include=QuestionnaireResponse:questionnaire&_include=QuestionnaireResponse:subject")); -// Bundle bundle = + + ourHttpClient.execute(new HttpGet("http://localhost:" + ourPort + + "/QuestionnaireResponse?patient=QR3295&questionnaire=profile&_sort:desc=authored&_count=5&_include=QuestionnaireResponse:questionnaire&_include=QuestionnaireResponse:subject")); + // Bundle bundle = } @Test @@ -2284,14 +2318,14 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { } Bundle found = ourClient - .search() - .forResource(Patient.class) - .where(BaseResource.RES_ID.exactly().systemAndValues(null, id1.getIdPart(), id2.getIdPart())) - .returnBundle(Bundle.class) - .execute(); + .search() + .forResource(Patient.class) + .where(BaseResource.RES_ID.exactly().systemAndValues(null, id1.getIdPart(), id2.getIdPart())) + .returnBundle(Bundle.class) + .execute(); assertThat(toUnqualifiedVersionlessIds(found), empty()); - + found = ourClient .search() .forResource(Patient.class) @@ -2319,14 +2353,14 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { .execute(); assertThat(toUnqualifiedVersionlessIds(found), containsInAnyOrder(id1)); - + found = ourClient - .search() - .forResource(Patient.class) - .where(BaseResource.RES_ID.exactly().codes(Arrays.asList(id1.getIdPart(), id2.getIdPart(), "FOOOOO"))) - .and(BaseResource.RES_ID.exactly().code(id1.getIdPart())) - .returnBundle(Bundle.class) - .execute(); + .search() + .forResource(Patient.class) + .where(BaseResource.RES_ID.exactly().codes(Arrays.asList(id1.getIdPart(), id2.getIdPart(), "FOOOOO"))) + .and(BaseResource.RES_ID.exactly().code(id1.getIdPart())) + .returnBundle(Bundle.class) + .execute(); assertThat(toUnqualifiedVersionlessIds(found), containsInAnyOrder(id1)); @@ -3445,7 +3479,9 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info(responseString); assertEquals(400, response.getStatusLine().getStatusCode()); OperationOutcome oo = myFhirCtx.newXmlParser().parseResource(OperationOutcome.class, responseString); - assertEquals("Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"333\" does not match URL ID of \"A2\"", oo.getIssue().get(0).getDiagnostics()); + assertEquals( + "Can not update resource, resource body must contain an ID element which matches the request URL for update (PUT) operation - Resource body ID of \"333\" does not match URL ID of \"A2\"", + oo.getIssue().get(0).getDiagnostics()); } finally { response.close(); } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 65791e5e565..722289db81a 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -127,6 +127,10 @@ Client revincludes did not include the :recurse modifier. Thanks to Jenny Meinsma for pointing this out on Zulip! + + JPA server did not return an OperationOutcome in the response for + a normal delete operation. +