Thanks for working on this patchset, I'm looking over 0004 and 0005 but came
across a thing I wanted to bring up one thing sooner than waiting for the
review. In parse_device_authz we have this:
{"user_code", JSON_TOKEN_STRING, {&authz->user_code}, REQUIRED},
{"verification_uri", JSON_TOKEN_STRING, {&authz->verification_uri}, REQUIRED},
/*
* The following fields are technically REQUIRED, but we don't use
* them anywhere yet:
*
* - expires_in
*/
{"interval", JSON_TOKEN_NUMBER, {&authz->interval_str}, OPTIONAL},
Together with a colleage we found the Azure provider use "verification_url"
rather than xxx_uri. Another discrepancy is that it uses a string for the
interval (ie: "interval":"5"). One can of course argue that Azure is wrong and
should feel bad, but I fear that virtually all (major) providers will have
differences like this, so we will have to deal with it in an extensible fashion
(compile time, not runtime configurable).
I was toying with making the name json_field name member an array, to allow
variations. That won't help with the fieldtype differences though, so another
train of thought was to have some form of REQUIRED_XOR where fields can tied
together. What do you think about something along these lines?
Another thing, shouldn't we really parse and interpret *all* REQUIRED fields
even if we don't use them to ensure that the JSON is wellformed? If the JSON
we get is malformed in any way it seems like the safe/conservative option to
error out.
--
Daniel Gustafsson
