Remarks on FhirPatch found while implementing it

FhirPatchComments.md

I have implemented FHIRPath Patch on top of the .NET FHIR API. I have used the tests provided as download in the specification as unittests. The "Quoted Names" refer to the names of those tests.

1. "Full Resource": would have expected valueXhtml for the new div instead of valueString. Now with valueString the actual types of the input div and the replacement div are different (xhtml vs string). But valueXhtml is not accepted, at least not by the .NET API.

2. "Add Anonymous type": structure with nested parts feels clumsy and was very hard to get parsed correctly. No other way?

3. What happens if sequential patches interact?

   1. For Delete:
      • array [0,1,2,3,4]
      • delete array[0]
      • delete array[3]
      • intuitive expectation: [1,2,4] (delete items from position 0 ('0') and 3 ('3) from the original list)
      • but if we follow the same logic as in "Reorder List #5", we would apply them sequentially, and end up with [0,1,2,3,4] -> remove item 0 -> [1,2,3,4] -> remove item 3 -> [1,2,3]
   2. For Insert:
      • array [0,1,2]
      • insert 4 at 0
      • insert 5 at 2
      • you might expect: [4,0,1,5,2] (with 5 after 1, what was originally spot nr. 2)
      • but if we apply them sequentially, you get [4,0,5,1,2]

4. "Add to list": 'accidentally' index was chosen as array.length, so it fits nicely at the end.

   • what if index > array.length? Still add it?
   • what if you insert multiple items at the end of the list, and you use the same index for each of them? Add all of them in order of appearance?
   • and if so, why not simply use 'add' as operation then?

5. The 'path' part is a FHIRPath expression. That expression may yield multiple locations in the resource.

   • e.g. on a Bundle you could delete the first tag from all the entries with operation 'delete' and path 'Bundle.entry.resource.meta.tag[0]'.
   • e.g. on Patient we could add a Given name to the patient and all its contacts with operation 'add' and path 'Patient.descendants().where($this is HumanName)'.
   • some of these cases could actually be useful, but we have to define whether it is allowed and what the behaviour is.

6. If a patch consists of multiple operations, what is the order of processing?

   • in the order they are in the Parameter.parameter array?
   • based on some kind of precedence (like in FHIR Transaction)?

   I think it should be in order of arrival, since one may build on top of another.

3i I would have expected 1,2,3 in the list, as it's a sequential set of changes to apply, not to the original list.

Agree with all you point 4 notes

Point 5 has me mixed, I think needs to resolve to a single node.
But the add is an oddity where you're selecting the parent node, and provide the name of the child to add

@brianpos: You're right on 3i - I corrected the example.

Added point 6.

Updated the spec in response to this. Comments:

1. yep has to be string - maybe can revisit this in R5
2. I don't like it either but not easy to resovle; maybe can revisit this in R5
3. made it explicit that patches are applied in order, and each patch is applied to the output from the last patch operation
4. index cannot be off the end: "must be equal or less than the number of elements in the list". Add is easier that insert at the end of the list
5. made it explicit that it's a single element for now. maybe can revisit in R5
6. see #3