Conditional updates with optimistic locking is not working

[radupurdea]

Describe the bug
When performing a conditional update, using the If-Match with the etag, it seems to be ignoring the if-match header as the update is being performed regardless of incorrect version (etag).

FHIR Version?
R4

Data provider?
The default on an Azure Health Data FHIR Service install (the managed FHIR Service within Azure).

To Reproduce
Steps to reproduce the behavior:

1. Ensure there exists a resource that would match a search criteria.
   -- For example, an ImagingStudy resource with an identifier of

"identifier": [
        {
            "type": {
                "coding": [
                    {
                        "system": "http//terminology.hl7.org/NamingSystem/dui",
                        "code": "DUI"
                    }
                ]
            },
            "system": "urn:dicom:uid",
            "value": "urn:oid:the-dicom-study-id"
        }
    ]

3. Read the ImagingStudy using the search with the above criteria.
   -- For example: GET [baseFhirUrl]/ImagingStudy?identifier=urn%3Adicom%3Auid%7Curn%3Aoid%3Athe-dicom-study-id
   -- Make note of the Meta.Version value. Let's say the current version is 5.
4. Create the body (same as the GET response from the previous step), change a field to ensure version would be incremented on an update (say, change the status value). Update this resource using a PUT request with a search condition. Ensure there is an If-Match header with the previous version ETag. In this case, If-Match: W/"4".
   -- For example: PUT [baseFhirUrl]/ImagingStudy?identifier=urn%3Adicom%3Auid%7Curn%3Aoid%3Athe-dicom-study-id
   -- Header: If-Match: W/"4".

Expected behavior
The request should be rejected with 412 - Precondition failed.

Actual behavior
The request is accepted (with 200 - OK), the resource is updated and version is incremented to 6.

According to REST API capabilities in the FHIR service in Azure Health Data Services conditional updates with optimistic locking should be supported.

Mentioning that this sometimes works, seemingly randomly. The scenario we have does attempt to conditionally update (with optimistic locking) the same resource within a very short time frame (nano seconds). However, it is very easy to reproduce in a non-concurrent scenario as well, so above repro steps should work.

Perhaps related with Concurrent conditional creates results in duplicate data.

[EXPEkesheth]

We will investigate the issue and respond with next steps
AB548090626

[brendankowitz]

Thanks @radupurdea. The update scenario is different to the create scenario, the complexity with the FHIR spec on create is that you can technically have multiple unique search queries that might resolve the same resource (but a new ID needs to be assigned), update should resolve a single resource ID that we implement optimistic locking on.
With conditional update, after finding a (single) resource, it internally it uses that etag (version) of the resolved resource to process the update. The call current doesn't implement the If-Match while also processing the conditional match.
I'd be open to understand the scenario where you would know the version of a resource and not also know the resourceId? -- this would seem more efficient to simply use PUT with If-Match

[radupurdea]

@brendankowitz Thanks for getting back!

Yes, the scenario I have, I do have access to both the resourceId and Version. In fact, your suggestion was our workaround. The conditional update with an identifier query was initially supposed to create or update the resource based on the identifier. I was hoping to re-use this logic for subsequent updates (without then using the resource ID for updates) and, to ensure resource contention, provide the if-match header. To explain the context a bit more, this logic was handling EventGrid FHIR/DICOM resource events (which are highly transactional).

This left the conditional creation, as a separate logical branch, but this was not viable due to the open issue: #1382.

Eventually, I ended up splitting the logic to have the initial resource created at a different time, so only update events get handled.
(this, of course, is not ideal).

So really, if the conditional update is challenging, I would argue focusing towards the conditional create fix would be much more useful.