hapifhir / hapi-fhir

🔥 HAPI FHIR - Java API for HL7 FHIR Clients and Servers
http://hapifhir.io
Apache License 2.0
2.04k stars 1.33k forks source link

ifMatch request param for PATCH request inside a Bundle transaction fails with HAPI-0518 error. #3575

Open aditya-07 opened 2 years ago

aditya-07 commented 2 years ago

NOTE: Before filing a ticket, please see the following URL: https://github.com/hapifhir/hapi-fhir/wiki/Getting-Help

Describe the bug Including ifMatch with etag value for a PATCH request inside a Bundle transaction causes the transaction to fail. PUT on the other hand works fine.

FYI: Non Bundle Http Patch request against the Patient works fine though.

Failing PATCH request

{
    "resourceType": "Bundle",
    "type": "transaction",
    "entry": [{
        "fullUrl": "Patient/e642ec07-c4d1-4f3f-bae6-4cd9e7f9d0d4",
        "resource": {
            "resourceType": "Binary",
            "contentType": "application/json-patch+json",
            "data": "W3sib3AiOiJyZXBsYWNlIiwicGF0aCI6IlwvdGVsZWNvbVwvMFwvdmFsdWUiLCJ2YWx1ZSI6IjQwMTEyMzQ1NiJ9XQ=="
        },
        "request": {
            "method": "PATCH",
            "url": "Patient/e642ec07-c4d1-4f3f-bae6-4cd9e7f9d0d4",
            "ifMatch": "W/\"11\""
        }
    }]
}

Response

{
    "resourceType": "OperationOutcome",
    "text": {
        "status": "generated",
        "div": "<div xmlns=\"http://www.w3.org/1999/xhtml\"><h1>Operation Outcome</h1><table border=\"0\"><tr><td style=\"font-weight: bold;\">ERROR</td><td>[]</td><td><pre>HAPI-0518: Invalid match URL[Patient?W/&quot;11&quot;] - URL has no search parameters</pre></td>\n\t\t\t</tr>\n\t\t</table>\n\t</div>"
    },
    "issue": [
        {
            "severity": "error",
            "code": "processing",
            "diagnostics": "HAPI-0518: Invalid match URL[Patient?W/\"11\"] - URL has no search parameters"
        }
    ]
} 

Working PUT Request

{"resourceType":"Bundle","type":"transaction","entry":[{"fullUrl":"Patient/e642ec07-c4d1-4f3f-bae6-4cd9e7f9d0d4","resource":{
    "resourceType": "Patient",
    "id": "e642ec07-c4d1-4f3f-bae6-4cd9e7f9d0d4",
    "active": true,
    "name": [
        {
            "family": "Adams",
            "given": [
                "Adam"
            ]
        }
    ],
    "telecom": [
        {
            "system": "phone",
            "value": "401123456-01"
        }
    ],
    "gender": "male",
    "birthDate": "2020-02-14",
    "address": [
        {
            "city": "NAIROBI",
            "country": "Kenya"
        }
    ]
},"request":{"method":"PUT","url":"Patient/e642ec07-c4d1-4f3f-bae6-4cd9e7f9d0d4","ifMatch":"W/\"11\""}}]}

Response

{
    "resourceType": "Bundle",
    "id": "be5b83f0-a3d3-440e-a542-8b9e54e166a6",
    "type": "transaction-response",
    "link": [
        {
            "relation": "self",
            "url": "http://hapi.fhir.org/baseR4"
        }
    ],
    "entry": [
        {
            "response": {
                "status": "200 OK",
                "location": "Patient/e642ec07-c4d1-4f3f-bae6-4cd9e7f9d0d4/_history/12",
                "etag": "12",
                "lastModified": "2022-04-29T18:39:04.724+00:00"
            }
        }
    ]
}

To Reproduce Steps to reproduce the behavior:

  1. Use the requests shown above to do a POST request to the https://hapi.fhir.org/baseR4/ and see the response.

Expected behavior Transaction should be successful of if ifMatch has the latest value or fail gracefully for the right reason if the etag provided is not the latest one on the server just like the PUT operation . e.g. Like the PUT response above.

{
"entry": [
        {
            "response": {
                "status": "200 OK",
                "location": "Patient/e642ec07-c4d1-4f3f-bae6-4cd9e7f9d0d4/_history/12",
                "etag": "12",
                "lastModified": "2022-04-29T18:39:04.724+00:00"
            }
        }
    ]
}

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information): http://hapi.fhir.org/baseR4 HAPI FHIR Test/Demo Server R4 Endpoint HAPI FHIR Server 6.0.0-PRE8-SNAPSHOT/3f58e277e6/2022-03-23

Additional context Add any other context about the problem here.

aditya-07 commented 2 years ago

@jingtang10

jmorenomarti commented 1 year ago

I have exactly the same problem. Any update on this? Thank you!

jingtang10 commented 1 year ago

Seems like this is to do with the etag being escaped?

aditya-07 commented 1 year ago

@tadgh This seems to work fine now on the latest hapi server. Could you please confirm that it has been fixed and maybe the version where it got fixed?

I tried updating a Patient and it seems to work fine.

POST https://hapi.fhir.org/baseR4/
Content-Type: application/fhir+json; charset=utf-8
Content-Length: 404

{
    "resourceType": "Bundle",
    "type": "transaction",
    "entry": [
        {
            "fullUrl": "Patient/11370346",
            "resource": {
                "resourceType": "Binary",
                "contentType": "application/json-patch+json",
                "data": "W3sib3AiOiJhZGQiLCJwYXRoIjoiXC9hY3RpdmUiLCJ2YWx1ZSI6dHJ1ZX0seyJvcCI6InJlcGxhY2UiLCJwYXRoIjoiXC90ZWxlY29tXC8wXC92YWx1ZSIsInZhbHVlIjoiMTIzNDU2Nzg5MSJ9XQ=="
            },
            "request": {
                "method": "PATCH",
                "url": "Patient/11370346",
                "ifMatch": "W/\"3\""
            }
        }
    ]
}
--> END POST (404-byte body)

200 OK https://hapi.fhir.org/baseR4/ (357ms)
  Server: nginx/1.18.0 (Ubuntu)
  Date: Wed, 01 Nov 2023 09:16:58 GMT
  Content-Type: application/fhir+json;charset=utf-8
  Transfer-Encoding: chunked
  Connection: keep-alive
  X-Powered-By: HAPI FHIR 6.9.5-SNAPSHOT/7e3c42549d/2023-09-09 REST Server (FHIR Server; FHIR 4.0.1/R4)
  X-Request-ID: rsa60y5IdEEyqQPS
  Content-Location: https://hapi.fhir.org/baseR4/Bundle/5543c581-7333-42b2-97e7-b5ef79c05349
  Location: https://hapi.fhir.org/baseR4/Bundle/5543c581-7333-42b2-97e7-b5ef79c05349
{
    "resourceType": "Bundle",
    "id": "5543c581-7333-42b2-97e7-b5ef79c05349",
    "type": "transaction-response",
    "link": [
        {
            "relation": "self",
            "url": "https://hapi.fhir.org/baseR4"
        }
    ],
    "entry": [
        {
            "response": {
                "status": "200 OK",
                "location": "Patient/11370346/_history/4",
                "etag": "4",
                "lastModified": "2023-10-20T07:14:14.707+00:00",
                "outcome": {
                    "resourceType": "OperationOutcome",
                    "text": {
                        "status": "generated",
                        "div": "<div xmlns=\"http://www.w3.org/1999/xhtml\"><h1>Operation Outcome</h1><table border=\"0\"><tr><td style=\"font-weight: bold;\">INFORMATION</td><td>[]</td><td>Successfully patched resource &quot;Patient/11370346/_history/4&quot;.</td></tr></table></div>"
                    },
                    "issue": [
                        {
                            "severity": "information",
                            "code": "informational",
                            "details": {
                                "coding": [
                                    {
                                        "system": "https://hapifhir.io/fhir/CodeSystem/hapi-fhir-storage-response-code",
                                        "code": "SUCCESSFUL_PATCH",
                                        "display": "Patch succeeded."
                                    }
                                ]
                            },
                            "diagnostics": "Successfully patched resource \"Patient/11370346/_history/4\"."
                        }
                    ]
                }
            }
        }
    ]
}
<-- END HTTP (1253-byte body)
jingtang10 commented 3 months ago

as per aditya's last comment this should have been closed? @tadgh can you pls confirm?

tadgh commented 3 months ago

One moment, checking the code :)

tadgh commented 3 months ago

@aditya-07 @jingtang10 Just to be totally safe, I'm going to write a quick test. Beyond the original transaction in the OP, is there anything else I need to setup? Once I have a test I'll run a quick bisect and let you know where it got fixed.

tadgh commented 3 months ago

I have a test replicating success on master. see here: https://github.com/hapifhir/hapi-fhir/pull/6139 I will bisect from here, assuming the test I've written is sufficiently representative for you?

tadgh commented 3 months ago

I see that 6.0.0 is the first official version we know this does not work in, I'll start my bisect from there.

jingtang10 commented 3 months ago

thanks very much gary!

tadgh commented 3 months ago

Looks like its still broken on 6.2.0, working on 6.4.0! The bisect is painful because of how many architectural changes have gone in between 6_0_0 and _7_4_0. Standby for the exact fix...

tadgh commented 3 months ago

Fix was here: 10615eb64c3def3a9f33851e1f1114aa27b8c44c https://github.com/hapifhir/hapi-fhir/pull/4416

Here is the full git bisect:

❯ git bisect log   
git bisect start '--term-new=fixed' '--term-old=unfixed'
# status: waiting for both good and bad commits
# fixed: [428acff31bdb495148f5da2cd5228920daf60f74] 6.4.0 Mergeback (#4563)
git bisect fixed 428acff31bdb495148f5da2cd5228920daf60f74
# status: waiting for good commit(s), bad commit known
# unfixed: [43ed8ca0512c4cd921260b275a65e044a8964cd6] Issue 4052 addition of properties to the loinc terminology uploading process (#4100)
git bisect unfixed 43ed8ca0512c4cd921260b275a65e044a8964cd6
# unfixed: [c7d991c34554a132e54a8978559523346d7df13f] Disallow matching a resource to more than one golden record (#4344)
git bisect unfixed c7d991c34554a132e54a8978559523346d7df13f
# fixed: [7a31860376a623280d74f3695628c6e3826302b9] Improve query count accuracy (#4424)
git bisect fixed 7a31860376a623280d74f3695628c6e3826302b9
# unfixed: [155b579093d0948c8f58b35d7a96b61170db5a07] 4000 - Added some comments to the various loincupload.properties files to clarify that the Hierarchy filename was changed for v2.73. (#4382)
git bisect unfixed 155b579093d0948c8f58b35d7a96b61170db5a07
# unfixed: [6bd7f7a0e1150af3bc8ccc21e4ec823a6153afc7] PackageInstallerSvcImpl Says R4 is NOT Compatible with R4B (#4396)
git bisect unfixed 6bd7f7a0e1150af3bc8ccc21e4ec823a6153afc7
# unfixed: [02de3e49e39e3e9c7481e0e26df9d9e788971be4] Fix some issues with 'mvn test' on windows (#4223)
git bisect unfixed 02de3e49e39e3e9c7481e0e26df9d9e788971be4
# fixed: [65bf8d47ce31329b22162723008067472a342400] Prevent chunk from returning to in-progress unless it is errorred, in… (#4417)
git bisect fixed 65bf8d47ce31329b22162723008067472a342400
# fixed: [84d83f623044db0e051c15200b533342f00e8c7a] Restored a couple of the Unit Tests in ca.uhn.fhir.jpa.stresstest.StressTestR4Test.java (#4413)
git bisect fixed 84d83f623044db0e051c15200b533342f00e8c7a
# fixed: [10615eb64c3def3a9f33851e1f1114aa27b8c44c] Fix contention aware patch in transaction (#4416)
git bisect fixed 10615eb64c3def3a9f33851e1f1114aa27b8c44c
# first fixed commit: [10615eb64c3def3a9f33851e1f1114aa27b8c44c] Fix contention aware patch in transaction (#4416)