FirelyTeam / spark

Firely and Incendi's open source FHIR server
BSD 3-Clause "New" or "Revised" License
258 stars 166 forks source link

Transaction entries using ifMatches constraint are ignored resulting in potential data loss #714

Open JeremiahSanders opened 5 months ago

JeremiahSanders commented 5 months ago

Describe the bug

When a Bundle is submitted to IFhirService.TransactionAsync having PUT (update) entries bearing an ifMatch requirement in their RequestComponent, the updates are processed irrespective of the target resources' version.

As a result, this invisibly enables lost updates during transactions, which the ifMatch requirement seeks to avoid.

To Reproduce

Steps to reproduce the behavior:

Using Hl7.Fhir.R4 NuGet package...

  1. Create a new FHIR resource of any desired type in your Spark FHIR server.
  2. Retrieve its current version.
  3. Modify it.
  4. Create a transaction builder using the FhirClient's endpoint. (e.g., new TransactionBuilder(fhirClient.Endpoint, BundleType.Transaction)).
  5. Add the modified resource to the transaction builder using its .Update() method. You must include a versionId manually to ensure the ifMatch header is included. (e.g., transactionBuilder.Update(resource, "not-the-real-resource-version")). Note that the version specified must vary from the server's version in order to properly test this bug.
  6. Create a Bundle from the TransactionBuilder (transactionBuilder.ToBundle()). (You should be able to verify the entry header by examining the resulting bundle.Entry[].Request.IfMatch property—which should have a value containing a weak ETag equivalent of the non-matching version specified in step 5.)
  7. Send it to a Spark FHIR server (using fhirClient.TransactionAsync()).
  8. Receive a successful response with a modified resource. (This is the point which should be/contain a failure, due to the mismatch in version.)

Expected behavior

Rather than a success response with modified resource, I expect that the interaction which specifies an update with a non-matching ifMatch version specified will result in response following the FHIR batch processing rules or FHIR transaction processing rules, as applicable for the request's Bundle.

Screenshot Debugger view showing a Bundle received by a Spark FHIR server which bears a conditional update intended to be processed only if the resource's version is 1:

image

Spark version

Operating system + Database

Container service / Cloud infrastructure:

Additional context

As an additional test, I also modified a Spark server [HttpPost] Task<FhirResponse> Transaction(Bundle bundle) method to specifically iterate through all Bundle entries and change their embedded ifMatch value from a weak ETag to the unaffixed version id (using the same trimming RegEx mentioned in #713 ) prior to submitting the API request's Bundle to IFhirService.TransactionAsync(). (To verify that the weak ETag prefix and suffix weren't causing the issue.) That made no change, so I suspect (but have not verified) that the ifMatches requirement is being ignored, rather than misinterpreted, in transactions.