hapifhir / hapi-fhir

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

FhirPatch diff treats a simultaneous add/delete as an update #5513

Open TahaAttari opened 9 months ago

TahaAttari commented 9 months ago

https://github.com/hapifhir/hapi-fhir/blob/e15d0430d0aaac352f4e574773f84375129ef42e/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java#L508-L548

Array element comparison has the following assumptions:

  1. Order of elements never changes
  2. All insertions are appended to the end
  3. All deletions happen from the end
  4. There is no simultaneous insert + delete operation

Because of this a simultaneous insert + delete is treated as an update, and the same with reordering array elements

marcindz88 commented 2 months ago

The issue described is hard to solve i think, however we have a similar situation with a different case where not last item in an array was deleted (e.g. we have 3 CarePlan Activities and remove second) and hence it shows us 1 deletion (of last item) and 2 modifications of second to last item (as code and status differs).

I think that the diff operation should identify which element of an array has been deleted not by simply checking which list is larger, but instead compare array elements by equalsDeep, to identify, which one was deleted, exclude them from further checks and only then check modifications.