Bundle.entry.resource.id should not be stripped on POST from client #2297 (#2303)

This commit is contained in:
James Agnew 2021-01-19 11:12:42 -05:00 committed by GitHub
parent 240decc210
commit 874b16207f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 209 additions and 161 deletions

View File

@ -991,7 +991,7 @@ public abstract class BaseParser implements IParser {
protected boolean shouldEncodeResourceId(IBaseResource theResource, EncodeContext theEncodeContext) {
boolean retVal = true;
if (isOmitResourceId()) {
if (isOmitResourceId() && theEncodeContext.getPath().size() == 1) {
retVal = false;
} else {
if (myDontEncodeElements != null) {

View File

@ -57,6 +57,11 @@ public interface IParser {
*/
IIdType getEncodeForceResourceId();
/**
* When encoding, force this resource ID to be encoded as the resource ID
*/
IParser setEncodeForceResourceId(IIdType theForceResourceId);
/**
* Which encoding does this parser instance produce?
*/
@ -70,6 +75,25 @@ public interface IParser {
*/
List<Class<? extends IBaseResource>> getPreferTypes();
/**
* If set, when parsing resources the parser will try to use the given types when possible, in
* the order that they are provided (from highest to lowest priority). For example, if a custom
* type which declares to implement the Patient resource is passed in here, and the
* parser is parsing a Bundle containing a Patient resource, the parser will use the given
* custom type.
* <p>
* This feature is related to, but not the same as the
* {@link FhirContext#setDefaultTypeForProfile(String, Class)} feature.
* <code>setDefaultTypeForProfile</code> is used to specify a type to be used
* when a resource explicitly declares support for a given profile. This
* feature specifies a type to be used irrespective of the profile declaration
* in the metadata statement.
* </p>
*
* @param thePreferTypes The preferred types, or <code>null</code>
*/
void setPreferTypes(List<Class<? extends IBaseResource>> thePreferTypes);
/**
* Returns true if resource IDs should be omitted
*
@ -78,6 +102,22 @@ public interface IParser {
*/
boolean isOmitResourceId();
/**
* If set to <code>true</code> (default is <code>false</code>) the ID of any resources being encoded will not be
* included in the output. Note that this does not apply to contained resources, only to root resources. In other
* words, if this is set to <code>true</code>, contained resources will still have local IDs but the outer/containing
* ID will not have an ID.
* <p>
* If the resource being encoded is a Bundle or Parameters resource, this setting only applies to the
* outer resource being encoded, not any resources contained wihthin.
* </p>
*
* @param theOmitResourceId Should resource IDs be omitted
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
* @since 1.1
*/
IParser setOmitResourceId(boolean theOmitResourceId);
/**
* If set to <code>true<code> (which is the default), resource references containing a version
* will have the version removed when the resource is encoded. This is generally good behaviour because
@ -86,12 +126,30 @@ public interface IParser {
* links. In that case, this value should be set to <code>false</code>.
*
* @return Returns the parser instance's configuration setting for stripping versions from resource references when
* encoding. This method will retun <code>null</code> if no value is set, in which case
* the value from the {@link ParserOptions} will be used (default is <code>true</code>)
* encoding. This method will retun <code>null</code> if no value is set, in which case
* the value from the {@link ParserOptions} will be used (default is <code>true</code>)
* @see ParserOptions
*/
Boolean getStripVersionsFromReferences();
/**
* If set to <code>true<code> (which is the default), resource references containing a version
* will have the version removed when the resource is encoded. This is generally good behaviour because
* in most situations, references from one resource to another should be to the resource by ID, not
* by ID and version. In some cases though, it may be desirable to preserve the version in resource
* links. In that case, this value should be set to <code>false</code>.
* <p>
* This method provides the ability to globally disable reference encoding. If finer-grained
* control is needed, use {@link #setDontStripVersionsFromReferencesAtPaths(String...)}
* </p>
*
* @param theStripVersionsFromReferences Set this to <code>false<code> to prevent the parser from removing resource versions from references (or <code>null</code> to apply the default setting from the {@link ParserOptions}
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
* @see #setDontStripVersionsFromReferencesAtPaths(String...)
* @see ParserOptions
*/
IParser setStripVersionsFromReferences(Boolean theStripVersionsFromReferences);
/**
* Is the parser in "summary mode"? See {@link #setSummaryMode(boolean)} for information
*
@ -99,82 +157,75 @@ public interface IParser {
*/
boolean isSummaryMode();
/**
* If set to <code>true</code> (default is <code>false</code>) only elements marked by the FHIR specification as
* being "summary elements" will be included.
*
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
*/
IParser setSummaryMode(boolean theSummaryMode);
/**
* Parses a resource
*
* @param theResourceType
* The resource type to use. This can be used to explicitly specify a class which extends a built-in type
* (e.g. a custom type extending the default Patient class)
* @param theReader
* The reader to parse input from. Note that the Reader will not be closed by the parser upon completion.
* @param theResourceType The resource type to use. This can be used to explicitly specify a class which extends a built-in type
* (e.g. a custom type extending the default Patient class)
* @param theReader The reader to parse input from. Note that the Reader will not be closed by the parser upon completion.
* @return A parsed resource
* @throws DataFormatException
* If the resource can not be parsed because the data is not recognized or invalid for any reason
* @throws DataFormatException If the resource can not be parsed because the data is not recognized or invalid for any reason
*/
<T extends IBaseResource> T parseResource(Class<T> theResourceType, Reader theReader) throws DataFormatException;
/**
* Parses a resource
*
* @param theResourceType
* The resource type to use. This can be used to explicitly specify a class which extends a built-in type
* (e.g. a custom type extending the default Patient class)
* @param theInputStream
* The InputStream to parse input from, <b>with an implied charset of UTF-8</b>. Note that the InputStream will not be closed by the parser upon completion.
* @param theResourceType The resource type to use. This can be used to explicitly specify a class which extends a built-in type
* (e.g. a custom type extending the default Patient class)
* @param theInputStream The InputStream to parse input from, <b>with an implied charset of UTF-8</b>. Note that the InputStream will not be closed by the parser upon completion.
* @return A parsed resource
* @throws DataFormatException
* If the resource can not be parsed because the data is not recognized or invalid for any reason
* @throws DataFormatException If the resource can not be parsed because the data is not recognized or invalid for any reason
*/
<T extends IBaseResource> T parseResource(Class<T> theResourceType, InputStream theInputStream) throws DataFormatException;
/**
* Parses a resource
*
* @param theResourceType
* The resource type to use. This can be used to explicitly specify a class which extends a built-in type
* (e.g. a custom type extending the default Patient class)
* @param theString
* The string to parse
* @param theResourceType The resource type to use. This can be used to explicitly specify a class which extends a built-in type
* (e.g. a custom type extending the default Patient class)
* @param theString The string to parse
* @return A parsed resource
* @throws DataFormatException
* If the resource can not be parsed because the data is not recognized or invalid for any reason
* @throws DataFormatException If the resource can not be parsed because the data is not recognized or invalid for any reason
*/
<T extends IBaseResource> T parseResource(Class<T> theResourceType, String theString) throws DataFormatException;
/**
* Parses a resource
*
* @param theReader
* The reader to parse input from. Note that the Reader will not be closed by the parser upon completion.
* @param theReader The reader to parse input from. Note that the Reader will not be closed by the parser upon completion.
* @return A parsed resource. Note that the returned object will be an instance of {@link IResource} or
* {@link IAnyResource} depending on the specific FhirContext which created this parser.
* @throws DataFormatException
* If the resource can not be parsed because the data is not recognized or invalid for any reason
* {@link IAnyResource} depending on the specific FhirContext which created this parser.
* @throws DataFormatException If the resource can not be parsed because the data is not recognized or invalid for any reason
*/
IBaseResource parseResource(Reader theReader) throws ConfigurationException, DataFormatException;
/**
* Parses a resource
*
* @param theInputStream
* The InputStream to parse input from (charset is assumed to be UTF-8).
* Note that the stream will not be closed by the parser upon completion.
* @param theInputStream The InputStream to parse input from (charset is assumed to be UTF-8).
* Note that the stream will not be closed by the parser upon completion.
* @return A parsed resource. Note that the returned object will be an instance of {@link IResource} or
* {@link IAnyResource} depending on the specific FhirContext which created this parser.
* @throws DataFormatException
* If the resource can not be parsed because the data is not recognized or invalid for any reason
* {@link IAnyResource} depending on the specific FhirContext which created this parser.
* @throws DataFormatException If the resource can not be parsed because the data is not recognized or invalid for any reason
*/
IBaseResource parseResource(InputStream theInputStream) throws ConfigurationException, DataFormatException;
/**
* Parses a resource
*
* @param theMessageString
* The string to parse
* @param theMessageString The string to parse
* @return A parsed resource. Note that the returned object will be an instance of {@link IResource} or
* {@link IAnyResource} depending on the specific FhirContext which created this parser.
* @throws DataFormatException
* If the resource can not be parsed because the data is not recognized or invalid for any reason
* {@link IAnyResource} depending on the specific FhirContext which created this parser.
* @throws DataFormatException If the resource can not be parsed because the data is not recognized or invalid for any reason
*/
IBaseResource parseResource(String theMessageString) throws ConfigurationException, DataFormatException;
@ -195,8 +246,7 @@ public interface IParser {
* DSTU3+ mode.
* </p>
*
* @param theDontEncodeElements
* The elements to encode
* @param theDontEncodeElements The elements to encode
* @see #setEncodeElements(Set)
*/
IParser setDontEncodeElements(Set<String> theDontEncodeElements);
@ -213,8 +263,7 @@ public interface IParser {
* <li><b>*.(mandatory)</b> - This is a special case which causes any mandatory fields (min > 0) to be encoded</li>
* </ul>
*
* @param theEncodeElements
* The elements to encode
* @param theEncodeElements The elements to encode
* @see #setDontEncodeElements(Set)
*/
IParser setEncodeElements(Set<String> theEncodeElements);
@ -225,7 +274,7 @@ public interface IParser {
* resource (typically a Bundle), but will be applied to any sub-resources
* contained within it (i.e. search result resources in that bundle)
*/
void setEncodeElementsAppliesToChildResourcesOnly(boolean theEncodeElementsAppliesToChildResourcesOnly);
boolean isEncodeElementsAppliesToChildResourcesOnly();
/**
* If set to <code>true</code> (default is false), the values supplied
@ -233,60 +282,20 @@ public interface IParser {
* resource (typically a Bundle), but will be applied to any sub-resources
* contained within it (i.e. search result resources in that bundle)
*/
boolean isEncodeElementsAppliesToChildResourcesOnly();
/**
* When encoding, force this resource ID to be encoded as the resource ID
*/
IParser setEncodeForceResourceId(IIdType theForceResourceId);
/**
* If set to <code>true</code> (default is <code>false</code>) the ID of any resources being encoded will not be
* included in the output. Note that this does not apply to contained resources, only to root resources. In other
* words, if this is set to <code>true</code>, contained resources will still have local IDs but the outer/containing
* ID will not have an ID.
*
* @param theOmitResourceId
* Should resource IDs be omitted
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
* @since 1.1
*/
IParser setOmitResourceId(boolean theOmitResourceId);
void setEncodeElementsAppliesToChildResourcesOnly(boolean theEncodeElementsAppliesToChildResourcesOnly);
/**
* Registers an error handler which will be invoked when any parse errors are found
*
* @param theErrorHandler
* The error handler to set. Must not be null.
* @param theErrorHandler The error handler to set. Must not be null.
*/
IParser setParserErrorHandler(IParserErrorHandler theErrorHandler);
/**
* If set, when parsing resources the parser will try to use the given types when possible, in
* the order that they are provided (from highest to lowest priority). For example, if a custom
* type which declares to implement the Patient resource is passed in here, and the
* parser is parsing a Bundle containing a Patient resource, the parser will use the given
* custom type.
* <p>
* This feature is related to, but not the same as the
* {@link FhirContext#setDefaultTypeForProfile(String, Class)} feature.
* <code>setDefaultTypeForProfile</code> is used to specify a type to be used
* when a resource explicitly declares support for a given profile. This
* feature specifies a type to be used irrespective of the profile declaration
* in the metadata statement.
* </p>
*
* @param thePreferTypes
* The preferred types, or <code>null</code>
*/
void setPreferTypes(List<Class<? extends IBaseResource>> thePreferTypes);
/**
* Sets the "pretty print" flag, meaning that the parser will encode resources with human-readable spacing and
* newlines between elements instead of condensing output as much as possible.
*
* @param thePrettyPrint
* The flag
* @param thePrettyPrint The flag
* @return Returns an instance of <code>this</code> parser so that method calls can be chained together
*/
IParser setPrettyPrint(boolean thePrettyPrint);
@ -295,61 +304,41 @@ public interface IParser {
* Sets the server's base URL used by this parser. If a value is set, resource references will be turned into
* relative references if they are provided as absolute URLs but have a base matching the given base.
*
* @param theUrl
* The base URL, e.g. "http://example.com/base"
* @param theUrl The base URL, e.g. "http://example.com/base"
* @return Returns an instance of <code>this</code> parser so that method calls can be chained together
*/
IParser setServerBaseUrl(String theUrl);
/**
* If set to <code>true<code> (which is the default), resource references containing a version
* will have the version removed when the resource is encoded. This is generally good behaviour because
* in most situations, references from one resource to another should be to the resource by ID, not
* by ID and version. In some cases though, it may be desirable to preserve the version in resource
* links. In that case, this value should be set to <code>false</code>.
* <p>
* This method provides the ability to globally disable reference encoding. If finer-grained
* control is needed, use {@link #setDontStripVersionsFromReferencesAtPaths(String...)}
* </p>
*
* @param theStripVersionsFromReferences
* Set this to <code>false<code> to prevent the parser from removing resource versions from references (or <code>null</code> to apply the default setting from the {@link ParserOptions}
* @see #setDontStripVersionsFromReferencesAtPaths(String...)
* @see ParserOptions
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
*/
IParser setStripVersionsFromReferences(Boolean theStripVersionsFromReferences);
/**
* If set to <code>true</code> (which is the default), the Bundle.entry.fullUrl will override the Bundle.entry.resource's
* resource id if the fullUrl is defined. This behavior happens when parsing the source data into a Bundle object. Set this
* to <code>false</code> if this is not the desired behavior (e.g. the client code wishes to perform additional
* validation checks between the fullUrl and the resource id).
*
* @param theOverrideResourceIdWithBundleEntryFullUrl
* Set this to <code>false</code> to prevent the parser from overriding resource ids with the
* Bundle.entry.fullUrl (or <code>null</code> to apply the default setting from the {@link ParserOptions})
*
* @see ParserOptions
*
* @param theOverrideResourceIdWithBundleEntryFullUrl Set this to <code>false</code> to prevent the parser from overriding resource ids with the
* Bundle.entry.fullUrl (or <code>null</code> to apply the default setting from the {@link ParserOptions})
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
* @see ParserOptions
*/
IParser setOverrideResourceIdWithBundleEntryFullUrl(Boolean theOverrideResourceIdWithBundleEntryFullUrl);
/**
* If set to <code>true</code> (default is <code>false</code>) only elements marked by the FHIR specification as
* being "summary elements" will be included.
*
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
*/
IParser setSummaryMode(boolean theSummaryMode);
/**
* If set to <code>true</code> (default is <code>false</code>), narratives will not be included in the encoded
* values.
*/
IParser setSuppressNarratives(boolean theSuppressNarratives);
/**
* Returns the value supplied to {@link IParser#setDontStripVersionsFromReferencesAtPaths(String...)}
* or <code>null</code> if no value has been set for this parser (in which case the default from
* the {@link ParserOptions} will be used}
*
* @see #setDontStripVersionsFromReferencesAtPaths(String...)
* @see #setStripVersionsFromReferences(Boolean)
* @see ParserOptions
*/
Set<String> getDontStripVersionsFromReferencesAtPaths();
/**
* If supplied value(s), any resource references at the specified paths will have their
* resource versions encoded instead of being automatically stripped during the encoding
@ -360,15 +349,14 @@ public interface IParser {
* has been set to <code>true</code> (which is the default)
* </p>
*
* @param thePaths
* A collection of paths for which the resource versions will not be removed automatically
* when serializing, e.g. "Patient.managingOrganization" or "AuditEvent.object.reference". Note that
* only resource name and field names with dots separating is allowed here (no repetition
* indicators, FluentPath expressions, etc.). Set to <code>null</code> to use the value
* set in the {@link ParserOptions}
* @param thePaths A collection of paths for which the resource versions will not be removed automatically
* when serializing, e.g. "Patient.managingOrganization" or "AuditEvent.object.reference". Note that
* only resource name and field names with dots separating is allowed here (no repetition
* indicators, FluentPath expressions, etc.). Set to <code>null</code> to use the value
* set in the {@link ParserOptions}
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
* @see #setStripVersionsFromReferences(Boolean)
* @see ParserOptions
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
*/
IParser setDontStripVersionsFromReferencesAtPaths(String... thePaths);
@ -382,27 +370,15 @@ public interface IParser {
* has been set to <code>true</code> (which is the default)
* </p>
*
* @param thePaths
* A collection of paths for which the resource versions will not be removed automatically
* when serializing, e.g. "Patient.managingOrganization" or "AuditEvent.object.reference". Note that
* only resource name and field names with dots separating is allowed here (no repetition
* indicators, FluentPath expressions, etc.). Set to <code>null</code> to use the value
* set in the {@link ParserOptions}
* @param thePaths A collection of paths for which the resource versions will not be removed automatically
* when serializing, e.g. "Patient.managingOrganization" or "AuditEvent.object.reference". Note that
* only resource name and field names with dots separating is allowed here (no repetition
* indicators, FluentPath expressions, etc.). Set to <code>null</code> to use the value
* set in the {@link ParserOptions}
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
* @see #setStripVersionsFromReferences(Boolean)
* @see ParserOptions
* @return Returns a reference to <code>this</code> parser so that method calls can be chained together
*/
IParser setDontStripVersionsFromReferencesAtPaths(Collection<String> thePaths);
/**
* Returns the value supplied to {@link IParser#setDontStripVersionsFromReferencesAtPaths(String...)}
* or <code>null</code> if no value has been set for this parser (in which case the default from
* the {@link ParserOptions} will be used}
*
* @see #setDontStripVersionsFromReferencesAtPaths(String...)
* @see #setStripVersionsFromReferences(Boolean)
* @see ParserOptions
*/
Set<String> getDontStripVersionsFromReferencesAtPaths();
}

View File

@ -193,6 +193,14 @@ public class BundleBuilder {
return new CreateBuilder(request);
}
/**
* Adds an entry for a Collection bundle type
*/
public void addCollectionEntry(IBaseResource theResource) {
setType("collection");
addEntryAndReturnRequest(theResource);
}
/**
* Creates new entry and adds it to the bundle
*
@ -321,6 +329,16 @@ public class BundleBuilder {
return retVal;
}
/**
* Sets a value for <code>Bundle.type</code>. That this is a coded field so {@literal theType}
* must be an actual valid value for this field or a {@link ca.uhn.fhir.parser.DataFormatException}
* will be thrown.
*/
public void setType(String theType) {
setBundleField("type", theType);
}
public static class UpdateBuilder {
private final IPrimitiveType<?> myUrl;

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 2297
title: "When performing a FHIR create using the HAPI FHIR client, if the payload is a Bundle resource
the individual resources in the Bundle had their IDs removed by the client during payload serialization.
This has been corrected."

View File

@ -30,6 +30,7 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException;
import ca.uhn.fhir.util.BundleBuilder;
import ca.uhn.fhir.util.TestUtil;
import ca.uhn.fhir.util.UrlUtil;
import com.google.common.base.Charsets;
@ -48,6 +49,7 @@ import org.apache.http.message.BasicHeader;
import org.apache.http.message.BasicStatusLine;
import org.hamcrest.core.StringContains;
import org.hamcrest.core.StringEndsWith;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.r4.model.*;
import org.hl7.fhir.r4.model.Bundle.BundleEntryComponent;
@ -73,6 +75,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.either;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
@ -206,6 +209,51 @@ public class ClientR4Test {
}
}
/**
* See #2297
*/
@Test
public void testCreateBundlePreservesIds() throws Exception {
BundleBuilder bb = new BundleBuilder(ourCtx);
bb.setType("collection");
Patient patient = new Patient();
patient.setId("Patient/123");
patient.addIdentifier().setSystem("urn:foo").setValue("bar");
bb.addCollectionEntry(patient);
IBaseBundle inputBundle = bb.getBundle();
inputBundle.setId("ABC");
ArgumentCaptor<HttpUriRequest> capt = ArgumentCaptor.forClass(HttpUriRequest.class);
when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse);
when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 201, "OK"));
when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_TEXT + "; charset=UTF-8"));
when(myHttpResponse.getEntity().getContent()).thenReturn(new ReaderInputStream(new StringReader(""), StandardCharsets.UTF_8));
when(myHttpResponse.getAllHeaders()).thenReturn(toHeaderArray("Location", "http://example.com/fhir/Patient/100/_history/200"));
IGenericClient client = ourCtx.newRestfulGenericClient("http://foo");
client.setEncoding(EncodingEnum.JSON);
CapturingInterceptor interceptor = new CapturingInterceptor();
client.registerInterceptor(interceptor);
client.create().resource(inputBundle).execute();
assertEquals("http://foo/Bundle?_format=json", ((ApacheHttpRequest) interceptor.getLastRequest()).getApacheRequest().getURI().toASCIIString());
assertEquals(HttpPost.class, capt.getValue().getClass());
HttpPost post = (HttpPost) capt.getValue();
String requestBody = IOUtils.toString(post.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Request body: {}", requestBody);
assertThat(requestBody, StringContains.containsString("{\"resourceType\":\"Patient\""));
Bundle requestBundle = ourCtx.newJsonParser().parseResource(Bundle.class, requestBody);
assertEquals("123", requestBundle.getEntry().get(0).getResource().getIdElement().getIdPart());
assertThat(requestBody, containsString("\"id\":\"123\""));
assertThat(requestBody, not(containsString("\"id\":\"ABC\"")));
}
/**
* Some servers (older ones?) return the resourcde you created instead of an OperationOutcome. We just need to ignore
* it.