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

Conditional updates with optimistic locking is not working #4647

Open
radupurdea opened this issue Sep 25, 2024 · 2 comments
Open

Conditional updates with optimistic locking is not working #4647

radupurdea opened this issue Sep 25, 2024 · 2 comments
Labels
Bug Bug bug bug. VSTS-Current We are working on this item in the current sprint

Comments

@radupurdea
Copy link

radupurdea commented Sep 25, 2024

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"
        }
    ]
  1. 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.
  2. 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.

@radupurdea radupurdea added the Bug Bug bug bug. label Sep 25, 2024
@EXPEkesheth
Copy link
Collaborator

We will investigate the issue and respond with next steps
AB548090626

@EXPEkesheth EXPEkesheth added the VSTS-Current We are working on this item in the current sprint label Sep 27, 2024
@brendankowitz
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug bug bug. VSTS-Current We are working on this item in the current sprint
Projects
None yet
Development

No branches or pull requests

3 participants