Improve error messages in JPA server

hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties

@@ -52,3 +52,5 @@ ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.failedToCreateWithClientAssignedId=C
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.invalidParameterChain=Invalid parameter chain: {0}
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.multipleParamsWithSameNameOneIsMissingTrue=This server does not know how to handle multiple "{0}" parameters where one has a value of :missing=true
 ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.unableToDeleteNotFound=Unable to find resource matching URL "{0}". Deletion failed.
+ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.successfulCreate=Successfully created resource "{0}" in {1}ms
+ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao.successfulUpdate=Successfully updated resource "{0}" in {1}ms

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java

@@ -293,6 +293,36 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 		return new HashSet<Long>(q.getResultList());
 	}
 
+	private boolean addPredicateMissingFalseIfPresent(CriteriaBuilder theBuilder, String theParamName, Root<? extends BaseResourceIndexedSearchParam> from, List<Predicate> codePredicates,
+			IQueryParameterType nextOr) {
+		boolean missingFalse = false;
+		if (nextOr.getMissing() != null) {
+			if (nextOr.getMissing().booleanValue() == true) {
+				throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "multipleParamsWithSameNameOneIsMissingTrue", theParamName));
+			}
+			Predicate singleCode = from.get("myId").isNotNull();
+			Predicate name = theBuilder.equal(from.get("myParamName"), theParamName);
+			codePredicates.add(theBuilder.and(name, singleCode));
+			missingFalse = true;
+		}
+		return missingFalse;
+	}
+
+	private boolean addPredicateMissingFalseIfPresentForResourceLink(CriteriaBuilder theBuilder, String theParamName, Root<? extends ResourceLink> from, List<Predicate> codePredicates,
+			IQueryParameterType nextOr) {
+		boolean missingFalse = false;
+		if (nextOr.getMissing() != null) {
+			if (nextOr.getMissing().booleanValue() == true) {
+				throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "multipleParamsWithSameNameOneIsMissingTrue", theParamName));
+			}
+			Predicate singleCode = from.get("mySourceResource").isNotNull();
+			Predicate name = createResourceLinkPathPredicate(theParamName, theBuilder, from);
+			codePredicates.add(theBuilder.and(name, singleCode));
+			missingFalse = true;
+		}
+		return missingFalse;
+	}
+
 	private Set<Long> addPredicateNumber(String theParamName, Set<Long> thePids, List<? extends IQueryParameterType> theList) {
 		if (theList == null || theList.isEmpty()) {
 			return thePids;
@@ -680,13 +710,6 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 		return new HashSet<Long>(q.getResultList());
 	}
 
-	private Predicate createResourceLinkPathPredicate(String theParamName, CriteriaBuilder builder, Root<? extends ResourceLink> from) {
-		RuntimeSearchParam param = getContext().getResourceDefinition(getResourceType()).getSearchParam(theParamName);
-		List<String> path = param.getPathsSplit();
-		Predicate type = from.get("mySourcePath").in(path);
-		return type;
-	}
-
 	private Set<Long> addPredicateString(String theParamName, Set<Long> thePids, List<? extends IQueryParameterType> theList) {
 		if (theList == null || theList.isEmpty()) {
 			return thePids;
@@ -727,36 +750,6 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 		return new HashSet<Long>(q.getResultList());
 	}
 
-	private boolean addPredicateMissingFalseIfPresent(CriteriaBuilder theBuilder, String theParamName, Root<? extends BaseResourceIndexedSearchParam> from, List<Predicate> codePredicates,
-			IQueryParameterType nextOr) {
-		boolean missingFalse = false;
-		if (nextOr.getMissing() != null) {
-			if (nextOr.getMissing().booleanValue() == true) {
-				throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "multipleParamsWithSameNameOneIsMissingTrue", theParamName));
-			}
-			Predicate singleCode = from.get("myId").isNotNull();
-			Predicate name = theBuilder.equal(from.get("myParamName"), theParamName);
-			codePredicates.add(theBuilder.and(name, singleCode));
-			missingFalse = true;
-		}
-		return missingFalse;
-	}
-
-	private boolean addPredicateMissingFalseIfPresentForResourceLink(CriteriaBuilder theBuilder, String theParamName, Root<? extends ResourceLink> from, List<Predicate> codePredicates,
-			IQueryParameterType nextOr) {
-		boolean missingFalse = false;
-		if (nextOr.getMissing() != null) {
-			if (nextOr.getMissing().booleanValue() == true) {
-				throw new InvalidRequestException(getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "multipleParamsWithSameNameOneIsMissingTrue", theParamName));
-			}
-			Predicate singleCode = from.get("mySourceResource").isNotNull();
-			Predicate name = createResourceLinkPathPredicate(theParamName, theBuilder, from);
-			codePredicates.add(theBuilder.and(name, singleCode));
-			missingFalse = true;
-		}
-		return missingFalse;
-	}
-
 	private Set<Long> addPredicateToken(String theParamName, Set<Long> thePids, List<? extends IQueryParameterType> theList) {
 		if (theList == null || theList.isEmpty()) {
 			return thePids;
@@ -871,12 +864,6 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 		return doCreate(theResource, theIfNoneExist, thePerformIndexing);
 	}
 
-	private IBaseOperationOutcome createErrorOperationOutcome(String theMessage) {
-		return createOperationOutcome("error", theMessage);
-	}
-	
-	protected abstract IBaseOperationOutcome createOperationOutcome(String theSeverity, String theMessage);
-
 	private Predicate createCompositeParamPart(CriteriaBuilder builder, Root<ResourceTable> from, RuntimeSearchParam left, IQueryParameterType leftValue) {
 		Predicate retVal = null;
 		switch (left.getParamType()) {
@@ -904,6 +891,16 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 		return retVal;
 	}
 
+	protected IBaseOperationOutcome createErrorOperationOutcome(String theMessage) {
+		return createOperationOutcome(IssueSeverityEnum.ERROR.getCode(), theMessage);
+	}
+	
+	protected IBaseOperationOutcome createInfoOperationOutcome(String theMessage) {
+		return createOperationOutcome(IssueSeverityEnum.INFORMATION.getCode(), theMessage);
+	}
+
+	protected abstract IBaseOperationOutcome createOperationOutcome(String theSeverity, String theMessage);
+
 	private Predicate createPredicateDate(CriteriaBuilder theBuilder, From<ResourceIndexedSearchParamDate, ResourceIndexedSearchParamDate> theFrom, IQueryParameterType theParam) {
 		Predicate p;
 		if (theParam instanceof DateParam) {
@@ -1046,6 +1043,13 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 		return singleCode;
 	}
 
+	private Predicate createResourceLinkPathPredicate(String theParamName, CriteriaBuilder builder, Root<? extends ResourceLink> from) {
+		RuntimeSearchParam param = getContext().getResourceDefinition(getResourceType()).getSearchParam(theParamName);
+		List<String> path = param.getPathsSplit();
+		Predicate type = from.get("mySourcePath").in(path);
+		return type;
+	}
+
 	private void createSort(CriteriaBuilder theBuilder, Root<ResourceTable> theFrom, SortSpec theSort, List<Order> theOrders) {
 		if (theSort == null || isBlank(theSort.getParamName())) {
 			return;
@@ -1186,18 +1190,12 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 		DaoMethodOutcome outcome = toMethodOutcome(entity, theResource).setCreated(true);
 
 		notifyWriteCompleted();
-		ourLog.info("Processed create on {} in {}ms", myResourceName, w.getMillisAndRestart());
+		
-		return outcome;
+		String msg = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "successfulCreate", outcome.getId(), w.getMillisAndRestart());
-	}
+		outcome.setOperationOutcome(createInfoOperationOutcome(msg));
 
-	/**
+		ourLog.info(msg);
-	 * May
+		return outcome;
-	 * 
-	 * @param theResource
-	 *           The resource that is about to be stored
-	 */
-	protected void preProcessResourceForStorage(T theResource) {
-		// nothing by default
 	}
 
 	@Override
@@ -1537,6 +1535,16 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 
 	}
 
+	/**
+	 * May be implemented by subclasses to validate resources prior to storage
+	 * 
+	 * @param theResource
+	 *           The resource that is about to be stored
+	 */
+	protected void preProcessResourceForStorage(T theResource) {
+		// nothing by default
+	}
+
 	@Override
 	public T read(IIdType theId) {
 		validateResourceTypeAndThrowIllegalArgumentException(theId);
@@ -2121,8 +2129,14 @@ public abstract class BaseHapiFhirResourceDao<T extends IResource> extends BaseH
 		ResourceTable savedEntity = updateEntity(theResource, entity, true, null, thePerformIndexing, true);
 
 		notifyWriteCompleted();
-		ourLog.info("Processed update on {} in {}ms", resourceId, w.getMillisAndRestart());
+		
-		return toMethodOutcome(savedEntity, theResource).setCreated(false);
+		DaoMethodOutcome outcome = toMethodOutcome(savedEntity, theResource).setCreated(false);
+
+		String msg = getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "successfulCreate", outcome.getId(), w.getMillisAndRestart());
+		outcome.setOperationOutcome(createInfoOperationOutcome(msg));
+
+		ourLog.info(msg);
+		return outcome;
 	}
 
 	private void validateGivenIdIsAppropriateToRetrieveResource(IIdType theId, BaseHasResource entity) {

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/ResourceProviderDstu2Test.java

@@ -294,12 +294,35 @@ public class ResourceProviderDstu2Test extends BaseJpaTest {
 			assertEquals(400, response.getStatusLine().getStatusCode());
 			String respString = IOUtils.toString(response.getEntity().getContent());
 			ourLog.info(respString);
+			assertThat(respString, containsString("<OperationOutcome xmlns=\"http://hl7.org/fhir\">"));
+			assertThat(respString, containsString("Can not create resource with ID[2], ID must not be supplied on a create (POST) operation"));
 		} finally {
 			response.getEntity().getContent().close();
 			response.close();
 		}
 	}
 
+	@Test
+	public void testCreateResourceReturnsOperationOutcomeByDefault() throws IOException {
+		String resource = "<Patient xmlns=\"http://hl7.org/fhir\"></Patient>";
+
+		HttpPost post = new HttpPost(ourServerBase + "/Patient");
+		post.setEntity(new StringEntity(resource, ContentType.create(Constants.CT_FHIR_XML, "UTF-8")));
+
+		CloseableHttpResponse response = ourHttpClient.execute(post);
+		try {
+			assertEquals(201, response.getStatusLine().getStatusCode());
+			String respString = IOUtils.toString(response.getEntity().getContent());
+			ourLog.info(response.toString());
+			ourLog.info(respString);
+			assertThat(respString, containsString("<OperationOutcome xmlns=\"http://hl7.org/fhir\">"));
+		} finally {
+			response.getEntity().getContent().close();
+			response.close();
+		}
+	}
+
+	
 	@Test
 	public void testDeepChaining() {
 		delete("Location", Location.SP_NAME, "testDeepChainingL1");

hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ExceptionTest.java

@@ -76,6 +76,22 @@ public class ExceptionTest {
 		}
 	}
 
+	@Test
+	public void testResourceNotFound() throws Exception {
+		ourExceptionType = ResourceNotFoundException.class;
+		ourGenerateOperationOutcome = false;
+		{
+			HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/123");
+			HttpResponse status = ourClient.execute(httpGet);
+			String responseContent = IOUtils.toString(status.getEntity().getContent());
+			IOUtils.closeQuietly(status.getEntity().getContent());
+			ourLog.info(responseContent);
+			assertEquals(404, status.getStatusLine().getStatusCode());
+			OperationOutcome oo = (OperationOutcome) servlet.getFhirContext().newXmlParser().parseResource(responseContent);
+			assertThat(oo.getIssueFirstRep().getDetails().getValue(), StringContains.containsString("Resource Patient/123 is not known"));
+		}
+	}
+
 	@Test
 	public void testInternalErrorFormatted() throws Exception {
 		{