Stream: smart/scheduling-links
Topic: Validator
Laza Upatising (Apr 20 2021 at 04:43):
Hey all, I wrote a validator script in order to ensure that the published files conform to the spec (since I was having trouble parsing some of the files). https://github.com/lazau/scheduling-links-aggregator/tree/main/validator. Please try it and let me know if it's of any use.
Josh Mandel (Apr 20 2021 at 15:32):
Thanks Laza! BTW did you have a chance to try out the Inferno validator? (I can see the value too of having something lightweight and immediate!)
Robert Scanlon (Apr 21 2021 at 13:52):
This looks great! I'm going through our Inferno tests and comparing what you have here. I love how lightweight this is (Inferno started that way, but has become quite a bit bigger over time to support more and more FHIR-based use cases).
Robert Scanlon (Apr 21 2021 at 13:54):
One thing I noticed is that when invoking through the command line (for Mac bash at least), you need to escape the $ that is present on all of the URLs. That tripped me up for a minute. So you need to put in:
./validator all https://raw.githubusercontent.com/smart-on-fhir/smart-scheduling-links/master/examples/\$bulk-publish
I shouldn't be surprised by the well defined behavior of my own tools though ;-)
Josh Mandel (Apr 21 2021 at 14:36):
Yeah it took me a long time to develop the habit of single quoting everything when I pass these URLs into curl.
Rob Brackett (Jun 15 2021 at 00:17):
Question on this vs. the Inferno validator — I’ve got a system with some edge-case-y locations that have no associated phone number or URL, and Inferno says I should not have an empty array for telecom. But when I dropped the property, this validator requires it. I think Inferno is correct on that, but wanted to check and make sure.
Josh Mandel (Jun 15 2021 at 00:21):
If you don't have any telecoms, the thing to do in FHIR is to omit the element altogether rather than supplying an empty array. Let me see if I can cite chapter and verse on that...
Josh Mandel (Jun 15 2021 at 00:23):
http://build.fhir.org/json.html#xml ("arrays are never empty" -- but it's kinda buried ;-)
Rob Brackett (Jun 15 2021 at 01:03):
OK, so that's a bug in https://github.com/lazau/scheduling-links-aggregator/tree/main/validator, then.
Josh Mandel (Jun 15 2021 at 01:27):
I think the validator is implementing our constraint from https://github.com/smart-on-fhir/smart-scheduling-links/blob/master/specification.md#location-file that says telecom is required
Josh Mandel (Jun 15 2021 at 01:27):
In your case you can't satisfy the constraint
Robert Scanlon (Jun 15 2021 at 02:56):
Sounds like Inferno is failing you as expected -- but let me know if I'm misinterpreting this conclusion @Rob Brackett . For Inferno, we translated these requirements into FHIR's profiling language, which carries some rules that may not be spelled out in the scheduling links spec.
Robert Scanlon (Jun 15 2021 at 03:03):
You could probably get Inferno to pass you by providing a DataAbsentReason in this case. I don't think that this is of much practical value though -- I can't imagine many clients would be sophisticated enough to look out for that kind of thing for this use case.
Rob Brackett (Jun 15 2021 at 03:35):
I think the validator is implementing our constraint from https://github.com/smart-on-fhir/smart-scheduling-links/blob/master/specification.md#location-file that says telecom is required
Ah, geez, this is another one of those right-in-front-of-my-face moments — after seeing the “must not be present if empty” error from Inferno, I’d been looking at the fact that telecom has cardinality 0..* in FHIR proper (http://hl7.org/fhir/location.html) + the text that phone and URL should be set in SMART Scheduling Links, but totally overlooked the Y in the required column right next to it. :face_palm:
Rob Brackett (Jun 15 2021 at 03:45):
So between the fact that arrays should never be empty and the fact that telecom is required for SMART Scheduling Links, it might be helpful to note in the description column of the table of location properties that the telecom array SHALL have at least one contact point. I can make a PR for that if I’ve got the interpretation correct here.
Rob Brackett (Jun 15 2021 at 04:17):
Sounds like Inferno is failing you as expected
Yeah, it seems to be.
I think the end result here is that I was just getting twisted up in the subtlety that, because telecom is required and empty arrays aren’t allowed, the spec is essentially requiring at least one telecom contact point, but doesn’t say that directly. (If I had any complaint about Inferno here, it might be that I’d prefer a message about that (must have at least one entry) rather than the message that the property shouldn’t be present if it’s an empty array and a different message that it must be present once I’ve omitted it to satisfy the earlier error.)
Robert Scanlon (Jun 15 2021 at 13:13):
it might be helpful to note in the description column of the table of location properties that the telecom array SHALL have at least one contact point. I can make a PR for that if I’ve got the interpretation correct here.
this was our interpretation as well, and I don't see much harm reflecting this requirement (if valid) in the description because the extra 'should' language for url/phone obscures things a bit.
Robert Scanlon (Jun 15 2021 at 13:27):
In theory, in the 'telecom': [] case, Inferno should provide two errors, one like "Location.telecom: minimum required = 1, but only found 0", and one like "Location.telecom: Array cannot be empty - the property should not be present if it has no values". This isn't a big deal, but I'm curious if we only presented the second error in this case.
Robert Scanlon (Jun 15 2021 at 13:31):
Thanks for the feedback about the error messages, it's very helpful for us to know what people find confusing.
Josh Mandel (Jun 15 2021 at 13:44):
Thanks also for the feedback on the spec! Would be happy to take a PR clarifying this point.
Rob Brackett (Jun 15 2021 at 18:23):
:thumbs_up: Added a PR at https://github.com/smart-on-fhir/smart-scheduling-links/pull/47
@Laza Upatising I think that still leaves an issue on your validator that it allows telecom to be an empty array, when it should actually have at least one entry.
Laza Upatising (Jun 15 2021 at 18:54):
Fixed, thanks for the heads up!
There are a bunch of other arrays being validated without the non-empty requirement. Reading https://github.com/smart-on-fhir/smart-scheduling-links/pull/47#issue-670638023 implies that all arrays cannot be empty - I'll need to update the validator to enforce these rules on other arrays too (e.g. "Location.identifier", "Schedule.serviceType", "Schedule.serviceType.coding").
Actually, this brings up another question. If JSON arrays are never allowed to be empty, what's the requirements around the "extension" field in both Schedule and Slot files? Reading between the lines in the spec, it seems like the intent is to make these fields optional 'extensions', but the non-empty requirement means at least one extension must be provided?
Laza Upatising (Jun 15 2021 at 19:02):
Actually, replying to myself: it seems like if the field is not named we can bypass the non-empty requirement altogether. So only when a field is named is the non-empty requirement applied. Sorry for the confusion!
Josh Mandel (Jun 15 2021 at 19:07):
You figured this out in your follow-up, but just to confirm: when we say "non-empty" this just means "if you have no extensions, omit the extension property altogether instead of writing "extension": []. If all the extensions are optional on a given resource, omitting the extension property is fine.
Laza Upatising (Jun 15 2021 at 19:10):
Thank you for the clarification. I will add another validation rule around disallowing empty arrays ("extension": [] will yield a validation error), suggesting that implementers should just omit the field altogether.
Laza Upatising (Jun 16 2021 at 00:35):
Validator updated with new rules. Please @ me if you see unexpected results.
Rob Brackett (Jun 22 2021 at 19:07):
@Laza Upatising I only just noticed today that the validator requires Ruby 3+ (it crashes on bad dates in Ruby 2). Might be helpful to check RbConfig::CONFIG["MAJOR"] or something and warn users on Ruby 2 at the start of the script.
Rob Brackett (Jul 06 2021 at 18:42):
@Laza Upatising Also noticed the validator does not support the has-availability extension (from here: https://github.com/smart-on-fhir/smart-scheduling-links/blob/master/specification.md#describe-unknown-availability-capacity-or-slot-times). I get an error message like:
unrecognized field value 'http://fhir-registry.smarthealthit.org/StructureDefinition/has-availability'. Must be one of ["http://fhir-registry.smarthealthit.org/StructureDefinition/vaccine-product", "http://fhir-registry.smarthealthit.org/StructureDefinition/vaccine-dose"]
Rob Brackett (Jul 06 2021 at 18:57):
Looks like it’s also marking position as required for locations.
Laza Upatising (Jul 24 2021 at 00:56):
Thanks for the reports @Rob Brackett. I've fixed the 'has-availability' and position issues. I was unable to reproduce the Ruby 3 issue though. The only Date parsing code is in validator_lib.rb in the iso8601_validator method, which works in both Ruby 2 and 3. Let me know how I can reproduce it and I'll make the fix.
Rob Brackett (Jul 27 2021 at 23:15):
@Laza Upatising Took another look at this and the issue is < 2.7, not < 3.0! Specifically, in 2.6 and earlier, DateTime.iso8601 raises ArgumentError for an unparseable string, but you’re only catching Date::Error, which is what 2.7+ raises: https://github.com/lazau/scheduling-links-aggregator/blob/8072ab740f0ff057379dfae9785893c35f33193a/validator/validator_lib.rb#L851-L853
Rob Brackett (Jul 27 2021 at 23:16):
(I had been on 2.6.5, I think)
Last updated: Apr 12 2022 at 19:14 UTC
