Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spark accepts a single contained with multiple resources #20

Closed
ewoutkramer opened this issue Dec 2, 2014 · 12 comments
Closed

Spark accepts a single contained with multiple resources #20

ewoutkramer opened this issue Dec 2, 2014 · 12 comments
Assignees
Labels
Projects

Comments

@ewoutkramer
Copy link
Member

You can send a Resource to Spark with just one tag, with multiple resources inside it. This is a bug, probably due to a bug in the .NET API.

@mharthoorn mharthoorn added this to the Spark 3.1 milestone Jul 21, 2015
@mharthoorn mharthoorn added the bug label Jul 21, 2015
@mbaltus
Copy link
Member

mbaltus commented Oct 19, 2015

If you put multiple resources within the tag, the server will create a resource, but only the first contained resource will be saved.

@mbaltus mbaltus modified the milestones: Spark 3.3 - DevDays2015, Spark 3.1 Oct 19, 2015
@CorinaCiocanea CorinaCiocanea self-assigned this Nov 27, 2015
@CorinaCiocanea
Copy link
Contributor

Could you also give me an example for this? It is not clear in what tag can one add multiple resources. I have tried it with a couple of tags and parsing throws an exception.

@ewoutkramer
Copy link
Member Author

It's been too long ago, I don't understand this bug-report myself anymore, but it seems Mirjam still does ;-)

@mbaltus
Copy link
Member

mbaltus commented Dec 15, 2015

The issue is with contained resources. Try posting:

<Patient xmlns="http://hl7.org/fhir">
    <contained>
        <Organization>
            <id value="myManagingOrganization"/>
            <name value="Just testing this"/>
        </Organization>
        <Organization>
            <id value="myManagingOrganization2"/>
            <name value="Just testing this 2"/>
        </Organization>
    </contained>
    <name>
        <use value="usual" />
    <given value="Jim" />
    </name>
    <managingOrganization>
        <reference value="#myManagingOrganization" />
    </managingOrganization>
</Patient>

The contained tag is invalid since it can only contain 1 resource, but spark accepts it and stores the first organization.

@CorinaCiocanea
Copy link
Contributor

  1. From what I understand you can have multiple contained resources in a contained tag (https://www.hl7.org/fhir/references.html). The json representation from the example seems to expect an arrray : contained: [ { "resourceType" : "Organization", "id" : "org1", .. whatever information is available ... } ]

or is this example wrong?
Is there some documentation where this is explicitly explained?

  1. Also from this constraint "A contained resource SHALL only be included in a resource if something in that resource (potentially another contained resource) has a reference to it" I understand that there can be multiple contained resources, but that could also be done using multiple contained tags. So what do you understand from this?
  2. At the same time the example from @mbaltus should still be invalid because one of the organization is not used anywhere in the resource. Should Spark check that?

@mbaltus
Copy link
Member

mbaltus commented Dec 15, 2015

Yes, it is possible to have multiple contained resources, but they should be in their own <contained> tag, just like if you have more than 1 Patient.name you get multiple <name> tags. So my post should have resulted in an error. Also, Spark should not have just thrown away the second resource in contained if my construction was valid, because I've tested with a reference to the second one (and no reference to the first) and it still got deleted.
So I still think my posted resource is invalid, and Spark should reject it.
I think if a contained resource was not referenced, Spark should also reject it and send back an OperationOutcome, since SHALL implies this is an error.

@CorinaCiocanea
Copy link
Contributor

Thank you. That clears things out.

@ewoutkramer
Copy link
Member Author

Yes, the fact that Json has an array notation ([]) and you cannot repeat properties in Json makes all the difference here...

@CorinaCiocanea
Copy link
Contributor

So, this seems indeed to be a FHIR .NET Api bug (at least the part about accepting more resources in a contained tag).
After debugging this, the problem appears to be in the XmlDomFhirReader.GetMembers() method. Here for parsing an XElement we just take the value = (XElement)elem.FirstNode; and ignore any other nodes. We should probably check here the Elements() collection and throw an exception if there are multiple elements in it. I will add a bug for this.

Related to the second problem that we don't check if the actual contained resource is used anywhere in the resource, this would imply a validation step after the resource is constructed. Should this happen in Spark? or it would be better to make sure all parsed resources are correct? I think this should be part of implementing the rest of the validation support in Spark (that is currently missing - see Spark.Service.Validate). Maybe after implementation this could also be exported as a separate library.
What do you tink?

@paulbarrynz
Copy link

paulbarrynz commented Jul 19, 2016

We are experiencing the same issue currently. So just to clarify, should we be saving our contained resources using the following format? Will this deserialise correctly?
<Patient xmlns="http://hl7.org/fhir"> <contained> <Organization> <id value="myManagingOrganization"/> <name value="Just testing this"/> </Organization> </contained> <contained> <Organization> <id value="myManagingOrganization2"/> <name value="Just testing this 2"/> </Organization> </contained> <name> <use value="usual" /> <given value="Jim" /> </name> <managingOrganization> <reference value="#myManagingOrganization" /> </managingOrganization> </Patient>

@cknaap
Copy link
Member

cknaap commented Jul 19, 2016

Yes, this should be handled correctly. If Spark doesn't please leave a new comment here.

@kennethmyhra kennethmyhra moved this from To do to In progress in R2019.2 Apr 23, 2019
@kennethmyhra kennethmyhra self-assigned this Apr 23, 2019
kennethmyhra added a commit that referenced this issue Apr 25, 2019
* Add PermissiveParsing setting to Web.config
* Refactor AddFhir extension method to accept a boolean PermissiveParsing parameter
* Refactor *Formatter classes to accept *Parser classes as constructor parameter
* fixes #152
* fixes #20
@kennethmyhra kennethmyhra moved this from In progress to Development done in R2019.2 Apr 29, 2019
@kennethmyhra
Copy link
Collaborator

Fixed by #152 and #153.

  • If PermissiveParsing is set to false it will fail and return an OperationOutcome.
  • If PermissiveParsing is set to true one of the resources will be removed from the contained section.

@kennethmyhra kennethmyhra moved this from Development done to Done in R2019.2 Apr 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
R2019.2
  
Done
Development

No branches or pull requests

7 participants