URL Encode Cross-Version Extension URLs #1549
Conversation
For paths that include [x], the brackets should be URL-encoded in the url elements and, as a result, also encoded in the corresponding element ids and slice names. See: https://chat.fhir.org/#narrow/channel/215610-shorthand/topic/Issue.20with.20polymorphic.20inter-version.20extensions/near/488766521
|
NOTE: Running regression does detect changes in some repos. For most, it is just the difference of Affected repos (if you want to see for yourself): |
There was a problem hiding this comment.
This looks good! I only have one minor change to request. It feels a little weird that we're breaking some of the FHIR spec around id, but since this is a temporary measure to maintain tool compatibility until the trailing [x]s get removed from extensions, I think it's fine.
| expect(pathParts[2]).toEqual({ | ||
| base: 'value[x]' | ||
| }); |
There was a problem hiding this comment.
I had forgotten that parseFSHPath doesn't include anything about previous slices for path parts that don't have any slices of their own. It's what we want, but I got scared for a minute.
There was a problem hiding this comment.
Yeah, me too. I originally rewrote this to include the previous slices even in this case -- and it broken a bunch of tests, so I reverted back to the original approach.
src/utils/PathUtils.ts
Outdated
| // Get the content from the outermost bracket pairs. (?![^\[]*\]) ensures we don't | ||
| // match nested closing brackets (thank you, claude.ai) | ||
| parsedPart.brackets = Array.from( | ||
| pathPart.slice(parsedPart.base.length).matchAll(/\[([^\[\]]|\[(?:[^\[\]]*)\])*\]/g) | ||
| ).map(match => match[0].slice(1, -1)); |
There was a problem hiding this comment.
It looks like claude had the right regular expression in the code, but not the comment. Typical claude!
Description: Cross-version extensions can represent choice elements, which means that
[x]might be part of the extension URL -- in which case, it should be URL-encoded (per specifications) to%5Bx%5D. This also means that the correspondingidshould also be URL-encoded to match the tail-end of the URL. Similarly, relative URLs in sub-extensions of complex extensions should also be URL-encoded and the slice names should match the URL-encoded relative URLs. See this Zulip discussion for reference: https://chat.fhir.org/#narrow/channel/215610-shorthand/topic/Issue.20with.20polymorphic.20inter-version.20extensions/near/488766521This PR does all those things. It follows the approach of being liberal in what it accepts and conservative in what it produces -- which is to say that authors can use unencoded
[x]in the URLs and slice names and SUSHI will correctly handle them, but SUSHI will always produce outputs with encoded%5Bx%5D. This PR also fixes theparseFSHPathfunction because the old implementation did not deal well with slice names containing[x]-- and, as it turns out, the old approach allowed for syntactically incorrect paths likecomponent[1}.codeorcomponent[systolic}.code.Testing Instructions: See the tests for different variations of examples. One thing you can try:
In the above case, the master branch will not recognize this cross-version extension, but this PR branch should.
You can also use the same example above, but replace the
containsrule with this one:In this case, the master branch will find the extension, but its output will not encode the URL. On this PR branch, it will find the extension but also output the URL in its encoded form.
Related Issue: #1544