Fix bug 2- Process Content-Location header in client read invocations

This commit is contained in:
James Agnew 2014-07-21 14:57:37 -04:00
parent 64481185c7
commit 063e2961ab
16 changed files with 114 additions and 32 deletions

View File

@ -31,6 +31,13 @@
Transaction client invocations with XML encoding were using the wrong content type ("application/xml+fhir" instead
of the correct "application/atom+xml"). Thanks to David Hay of Orion Health for surfacing this one!
</action>
<action type="add">
Bundle entries now support a link type of "search". Thanks to David Hay for the suggestion.
</action>
<action type="add" issue="2">
Read invocations in the client now process the "Content-Location" header and use it to
populate the ID of the returned resource.
</action>
<action type="fix" issue="3">
Fix issue where vread invocations on server incorrectly get routed to instance history method if one is
defined. Thanks to Neal Acharya from UHN for surfacing this one!

View File

@ -238,8 +238,9 @@ public class Bundle extends BaseBundle /* implements IElement */{
*
* @param theResource
* The resource to add
* @return Returns the newly created bundle entry that was added to the bundle
*/
public void addResource(IResource theResource, FhirContext theContext, String theServerBase) {
public BundleEntry addResource(IResource theResource, FhirContext theContext, String theServerBase) {
BundleEntry entry = addEntry();
entry.setResource(theResource);
@ -282,7 +283,9 @@ public class Bundle extends BaseBundle /* implements IElement */{
}
}
entry.getLinkSelf().setValue(b.toString());
String qualifiedId = b.toString();
entry.getLinkSelf().setValue(qualifiedId);
}
InstantDt published = ResourceMetadataKeyEnum.PUBLISHED.get(theResource);
@ -314,6 +317,7 @@ public class Bundle extends BaseBundle /* implements IElement */{
}
}
return entry;
}
public static Bundle withResources(ArrayList<IResource> theUploadBundle, FhirContext theContext, String theServerBase) {

View File

@ -42,6 +42,7 @@ public class BundleEntry extends BaseBundle {
private StringDt myDeletedByName;
private StringDt myDeletedComment;
private StringDt myLinkAlternate;
private StringDt myLinkSearch;
private StringDt myLinkSelf;
private InstantDt myPublished;
private IResource myResource;
@ -104,6 +105,13 @@ public class BundleEntry extends BaseBundle {
return myLinkAlternate;
}
public StringDt getLinkSearch() {
if (myLinkSearch == null) {
myLinkSearch = new StringDt();
}
return myLinkSearch;
}
public StringDt getLinkSelf() {
if (myLinkSelf == null) {
myLinkSelf = new StringDt();
@ -177,6 +185,10 @@ public class BundleEntry extends BaseBundle {
myLinkAlternate = theLinkAlternate;
}
public void setLinkSearch(StringDt theLinkSearch) {
myLinkSearch = theLinkSearch;
}
public void setLinkSelf(StringDt theLinkSelf) {
if (myLinkSelf == null) {
myLinkSelf = new StringDt();

View File

@ -398,6 +398,14 @@ public class IdDt extends BasePrimitive<String> {
}
retVal.append('/');
retVal.append(getIdPart());
if (hasVersionIdPart()) {
retVal.append('/');
retVal.append(Constants.PARAM_HISTORY);
retVal.append('/');
retVal.append(getVersionIdPart());
}
return retVal.toString();
}

View File

@ -189,6 +189,7 @@ public class JsonParser extends BaseParser implements IParser {
linkStarted = false;
linkStarted = writeAtomLink(eventWriter, "self", nextEntry.getLinkSelf(), linkStarted);
linkStarted = writeAtomLink(eventWriter, "alternate", nextEntry.getLinkAlternate(), linkStarted);
linkStarted = writeAtomLink(eventWriter, "search", nextEntry.getLinkSearch(), linkStarted);
if (linkStarted) {
eventWriter.writeEnd();
}

View File

@ -521,6 +521,8 @@ class ParserState<T> {
} else {
if ("self".equals(myRel)) {
myEntry.getLinkSelf().setValueAsString(myHref);
} else if ("search".equals(myRel)) {
myEntry.getLinkSearch().setValueAsString(myHref);
} else if ("alternate".equals(myRel)) {
myEntry.getLinkAlternate().setValueAsString(myHref);
}

View File

@ -198,6 +198,10 @@ public class XmlParser extends BaseParser implements IParser {
writeAtomLink(eventWriter, "alternate", nextEntry.getLinkAlternate());
}
if (!nextEntry.getLinkSearch().isEmpty()) {
writeAtomLink(eventWriter, "search", nextEntry.getLinkSearch());
}
IResource resource = nextEntry.getResource();
if (resource != null && !resource.isEmpty() && !deleted) {
eventWriter.writeStartElement("content");

View File

@ -741,6 +741,8 @@ public class GenericClient extends BaseClient implements IGenericClient {
retVal.setId(myId);
}
MethodUtil.parseClientRequestResourceHeaders(theHeaders, retVal);
return retVal;
}
}

View File

@ -60,4 +60,4 @@ public class CapturingInterceptor implements IClientInterceptor {
myLastResponse = theRequest;
}
}
}

View File

@ -25,7 +25,6 @@ import java.io.Reader;
import java.lang.reflect.Method;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@ -33,16 +32,11 @@ import java.util.Set;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.client.utils.DateUtils;
import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.api.Bundle;
import ca.uhn.fhir.model.api.IResource;
import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum;
import ca.uhn.fhir.model.api.annotation.ResourceDef;
import ca.uhn.fhir.model.primitive.InstantDt;
import ca.uhn.fhir.parser.IParser;
import ca.uhn.fhir.rest.client.exceptions.InvalidResponseException;
import ca.uhn.fhir.rest.server.Constants;
@ -56,7 +50,6 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Object> {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseResourceReturningMethodBinding.class);
protected static final Set<String> ALLOWED_PARAMS;
static {
HashSet<String> set = new HashSet<String>();
@ -146,18 +139,7 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
resource = parser.parseResource(theResponseReader);
}
List<String> lmHeaders = theHeaders.get(Constants.HEADER_LAST_MODIFIED_LOWERCASE);
if (lmHeaders != null && lmHeaders.size() > 0 && StringUtils.isNotBlank(lmHeaders.get(0))) {
String headerValue = lmHeaders.get(0);
Date headerDateValue;
try {
headerDateValue = DateUtils.parseDate(headerValue);
InstantDt lmValue = new InstantDt(headerDateValue);
resource.getResourceMetadata().put(ResourceMetadataKeyEnum.UPDATED, lmValue);
} catch (Exception e) {
ourLog.warn("Unable to parse date string '{}'. Error is: {}", headerValue, e.toString());
}
}
MethodUtil.parseClientRequestResourceHeaders(theHeaders, resource);
switch (getMethodReturnType()) {
case BUNDLE:
@ -223,7 +205,7 @@ abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding<Obje
} else if (result.size() > 1) {
throw new InternalErrorException("Method returned multiple resources");
}
RestfulServer.streamResponseAsResource(theServer, theResponse, result.getResources(0, 1).get(0), responseEncoding, prettyPrint, requestIsBrowser, narrativeMode, respondGzip);
RestfulServer.streamResponseAsResource(theServer, theResponse, result.getResources(0, 1).get(0), responseEncoding, prettyPrint, requestIsBrowser, narrativeMode, respondGzip, theRequest.getFhirServerBase());
break;
}
}

View File

@ -10,6 +10,7 @@ import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Map;
@ -19,6 +20,7 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.client.utils.DateUtils;
import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.context.FhirContext;
@ -34,6 +36,7 @@ import ca.uhn.fhir.model.api.TagList;
import ca.uhn.fhir.model.api.annotation.Description;
import ca.uhn.fhir.model.api.annotation.TagListParam;
import ca.uhn.fhir.model.dstu.resource.OperationOutcome;
import ca.uhn.fhir.model.primitive.InstantDt;
import ca.uhn.fhir.parser.IParser;
import ca.uhn.fhir.rest.annotation.Count;
import ca.uhn.fhir.rest.annotation.IdParam;
@ -77,6 +80,29 @@ import ca.uhn.fhir.util.ReflectionUtil;
public class MethodUtil {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(MethodUtil.class);
public static void parseClientRequestResourceHeaders(Map<String, List<String>> theHeaders, IResource resource) {
List<String> lmHeaders = theHeaders.get(Constants.HEADER_LAST_MODIFIED_LOWERCASE);
if (lmHeaders != null && lmHeaders.size() > 0 && StringUtils.isNotBlank(lmHeaders.get(0))) {
String headerValue = lmHeaders.get(0);
Date headerDateValue;
try {
headerDateValue = DateUtils.parseDate(headerValue);
InstantDt lmValue = new InstantDt(headerDateValue);
resource.getResourceMetadata().put(ResourceMetadataKeyEnum.UPDATED, lmValue);
} catch (Exception e) {
ourLog.warn("Unable to parse date string '{}'. Error is: {}", headerValue, e.toString());
}
}
List<String> clHeaders = theHeaders.get(Constants.HEADER_CONTENT_LOCATION_LC);
if (clHeaders != null && clHeaders.size() > 0 && StringUtils.isNotBlank(clHeaders.get(0))) {
String headerValue = clHeaders.get(0);
resource.getId().setValue(headerValue);
}
}
static void addTagsToPostOrPut(IResource resource, BaseHttpClientInvocation retVal) {
TagList list = (TagList) resource.getResourceMetadata().get(ResourceMetadataKeyEnum.TAG_LIST);
if (list != null) {

View File

@ -385,6 +385,7 @@ public class RestfulServer extends HttpServlet {
}
protected void handleRequest(SearchMethodBinding.RequestType theRequestType, HttpServletRequest theRequest, HttpServletResponse theResponse) throws ServletException, IOException {
String fhirServerBase = null;
try {
if (null != mySecurityManager) {
@ -418,7 +419,6 @@ public class RestfulServer extends HttpServlet {
requestPath = requestPath.substring(1);
}
String fhirServerBase;
fhirServerBase = myServerAddressStrategy.determineServerBase(theRequest);
if (fhirServerBase.endsWith("/")) {
@ -582,7 +582,7 @@ public class RestfulServer extends HttpServlet {
issue.getDetails().setValue(e.toString() + "\n\n" + ExceptionUtils.getStackTrace(e));
}
streamResponseAsResource(this, theResponse, oo, determineResponseEncoding(theRequest), true, false, NarrativeModeEnum.NORMAL, statusCode, false);
streamResponseAsResource(this, theResponse, oo, determineResponseEncoding(theRequest), true, false, NarrativeModeEnum.NORMAL, statusCode, false, fhirServerBase);
theResponse.setStatus(statusCode);
addHeadersToResponse(theResponse);
@ -1073,15 +1073,22 @@ public class RestfulServer extends HttpServlet {
}
public static void streamResponseAsResource(RestfulServer theServer, HttpServletResponse theHttpResponse, IResource theResource, EncodingEnum theResponseEncoding, boolean thePrettyPrint,
boolean theRequestIsBrowser, NarrativeModeEnum theNarrativeMode, boolean theRespondGzip) throws IOException {
boolean theRequestIsBrowser, NarrativeModeEnum theNarrativeMode, boolean theRespondGzip, String theServerBase) throws IOException {
int stausCode = 200;
streamResponseAsResource(theServer, theHttpResponse, theResource, theResponseEncoding, thePrettyPrint, theRequestIsBrowser, theNarrativeMode, stausCode, theRespondGzip);
streamResponseAsResource(theServer, theHttpResponse, theResource, theResponseEncoding, thePrettyPrint, theRequestIsBrowser, theNarrativeMode, stausCode, theRespondGzip, theServerBase);
}
private static void streamResponseAsResource(RestfulServer theServer, HttpServletResponse theHttpResponse, IResource theResource, EncodingEnum theResponseEncoding, boolean thePrettyPrint,
boolean theRequestIsBrowser, NarrativeModeEnum theNarrativeMode, int stausCode, boolean theRespondGzip) throws IOException {
boolean theRequestIsBrowser, NarrativeModeEnum theNarrativeMode, int stausCode, boolean theRespondGzip, String theServerBase) throws IOException {
theHttpResponse.setStatus(stausCode);
if (theResource.getId() != null && theResource.getId().hasIdPart() && isNotBlank(theServerBase)) {
String resName = theServer.getFhirContext().getResourceDefinition(theResource).getName();
String fullId = theResource.getId().withServerBase(theServerBase, resName);
theHttpResponse.addHeader(Constants.HEADER_CONTENT_LOCATION, fullId);
}
if (theResource instanceof Binary) {
Binary bin = (Binary) theResource;
if (isNotBlank(bin.getContentType())) {

View File

@ -372,6 +372,7 @@ public class XmlParserTest {
entry = b.addEntry();
entry.getId().setValue("2");
entry.setLinkAlternate(new StringDt("http://foo/bar"));
entry.setLinkSearch(new StringDt("http://foo/bar/search"));
entry.setResource(p2);
BundleEntry deletedEntry = b.addEntry();
@ -384,7 +385,7 @@ public class XmlParserTest {
List<String> strings = new ArrayList<String>();
strings.addAll(Arrays.asList("<published>", pub.getValueAsString(), "</published>"));
strings.addAll(Arrays.asList("<entry>", "<id>1</id>", "</entry>"));
strings.addAll(Arrays.asList("<entry>", "<id>2</id>", "<link rel=\"alternate\" href=\"http://foo/bar\"/>", "</entry>"));
strings.addAll(Arrays.asList("<entry>", "<id>2</id>", "<link rel=\"alternate\" href=\"http://foo/bar\"/>", "<link rel=\"search\" href=\"http://foo/bar/search\"/>","</entry>"));
strings.addAll(Arrays.asList("<at:deleted-entry", "ref=\"Patient/3", "/>"));
assertThat(bundleString, StringContainsInOrder.stringContainsInOrder(strings));
assertThat(bundleString, not(containsString("at:by")));
@ -787,6 +788,7 @@ public class XmlParserTest {
" <id>http://hl7.org/fhir/valueset/256a5231-a2bb-49bd-9fea-f349d428b70d</id>\n" +
" <link href=\"http://hl7.org/implement/standards/fhir/valueset/256a5231-a2bb-49bd-9fea-f349d428b70d\" rel=\"self\"/>\n" +
" <link href=\"http://hl7.org/foo\" rel=\"alternate\"/>\n" +
" <link href=\"http://hl7.org/foo/search\" rel=\"search\"/>\n" +
" <updated>2014-02-10T04:10:46.987-00:00</updated>\n" +
" <author>\n" +
" <name>HL7, Inc (FHIR Project)</name>\n" +
@ -842,6 +844,7 @@ public class XmlParserTest {
assertEquals("HL7, Inc (FHIR Project)", entry.getAuthorName().getValue());
assertEquals("http://hl7.org/fhir/valueset/256a5231-a2bb-49bd-9fea-f349d428b70d", entry.getId().getValue());
assertEquals("http://hl7.org/foo", entry.getLinkAlternate().getValue());
assertEquals("http://hl7.org/foo/search", entry.getLinkSearch().getValue());
assertEquals(1, entry.getCategories().size());
assertEquals("term", entry.getCategories().get(0).getTerm());
assertEquals("label", entry.getCategories().get(0).getLabel());

View File

@ -577,8 +577,10 @@ public class ClientTest {
ArgumentCaptor<HttpUriRequest> capt = ArgumentCaptor.forClass(HttpUriRequest.class);
when(httpClient.execute(capt.capture())).thenReturn(httpResponse);
when(httpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK"));
Header[] headers = new Header[1];
Header[] headers = new Header[2];
headers[0] = new BasicHeader(Constants.HEADER_LAST_MODIFIED, "Wed, 15 Nov 1995 04:58:08 GMT");
headers[1] = new BasicHeader(Constants.HEADER_CONTENT_LOCATION, "http://foo.com/Patient/123/_history/2333");
when(httpResponse.getAllHeaders()).thenReturn(headers);
when(httpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8"));
when(httpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(msg), Charset.forName("UTF-8")));
@ -590,6 +592,8 @@ public class ClientTest {
assertEquals("http://foo/Patient/111", capt.getValue().getURI().toString());
assertEquals("PRP1660", response.getIdentifier().get(0).getValue().getValue());
assertEquals("http://foo.com/Patient/123/_history/2333", response.getId().getValue());
InstantDt lm = (InstantDt) response.getResourceMetadata().get(ResourceMetadataKeyEnum.UPDATED);
lm.setTimeZoneZulu(true);

View File

@ -35,6 +35,7 @@ import ca.uhn.fhir.model.dstu.resource.Observation;
import ca.uhn.fhir.model.dstu.resource.Organization;
import ca.uhn.fhir.model.dstu.resource.Patient;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.model.primitive.InstantDt;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.client.exceptions.NonFhirResponseException;
import ca.uhn.fhir.rest.server.Constants;
@ -518,7 +519,11 @@ public class GenericClientTest {
when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK"));
when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8"));
when(myHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(msg), Charset.forName("UTF-8")));
Header[] headers = new Header[2];
headers[0] = new BasicHeader(Constants.HEADER_LAST_MODIFIED, "Wed, 15 Nov 1995 04:58:08 GMT");
headers[1] = new BasicHeader(Constants.HEADER_CONTENT_LOCATION, "http://foo.com/Patient/123/_history/2333");
when(myHttpResponse.getAllHeaders()).thenReturn(headers);
IGenericClient client = myCtx.newRestfulGenericClient("http://example.com/fhir");
//@formatter:off
@ -526,7 +531,12 @@ public class GenericClientTest {
//@formatter:on
assertThat(response.getNameFirstRep().getFamilyAsSingleString(), StringContains.containsString("Cardinal"));
assertEquals("Patient/1234", response.getId().getValue());
assertEquals("http://foo.com/Patient/123/_history/2333", response.getId().getValue());
InstantDt lm = (InstantDt) response.getResourceMetadata().get(ResourceMetadataKeyEnum.UPDATED);
lm.setTimeZoneZulu(true);
assertEquals("1995-11-15T04:58:08.000Z", lm.getValueAsString());
}

View File

@ -5,6 +5,7 @@ import static org.junit.Assert.*;
import java.util.concurrent.TimeUnit;
import org.apache.commons.io.IOUtils;
import org.apache.http.Header;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;
@ -49,6 +50,11 @@ public class ReadTest {
assertEquals("1", dt.getSystem().getValueAsString());
assertEquals(null, dt.getValue().getValueAsString());
Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC);
assertNotNull(cl);
assertEquals("http://localhost:" + ourPort + "/Patient/1/_history/1", cl.getValue());
}
}
@ -65,6 +71,10 @@ public class ReadTest {
IdentifierDt dt = ourCtx.newXmlParser().parseResource(Patient.class,responseContent).getIdentifierFirstRep();
assertEquals("1", dt.getSystem().getValueAsString());
assertEquals("2", dt.getValue().getValueAsString());
Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC);
assertNotNull(cl);
assertEquals("http://localhost:" + ourPort + "/Patient/1/_history/1", cl.getValue());
}
}