Missing T in expected to be valid timestamp 2008-09-15 15:53:00.601+00:00
Closed this issue · 9 comments
This commit removed the "T" from a timestamp the tests assert are valid: 599a3ee
This is a fine and confusing point so please check my reasoning carefully here, but I assert that we should not expect LRSs to accept that format, and likely should expect them to reject it (but that case is slightly weaker)
Consider the timestamp requirements for LRPs:
Timestamps:
A Timestamp shall be formatted according to ISO 8601.
A Timestamp shall be expressed using the format described in RFC 3339, which is a profile of ISO 8601.
A Timestamp shall be formatted to UTC
If used, An LRP should send a Timestamp with at least millisecond precision (3 decimal points beyond seconds).
and for LRSs
Timestamps:
An LRS shall convert Timestamps to UTC rather than rejecting Statements that send Timestamps not in UTC form
An LRS may truncated or round a Timestamp to a precision of at least 3 decimal digits for seconds.
First, note that the LRS requirements do not specify an expected format at all for "Timestamp", except that if it is not sent in UTC form, it shall convert it rather than rejecting it. If there is another place we specify in the LRS requirements what format should be used for Timestamp that I have missed, I'd love to be wrong on this point.
Given the LRS practically must expect some format for timestamps, it makes sense to look to the LRP requirements for guidance. There we see both ISO 8601 and RFC 3339 specified -- so the timestamp must be valid according to both of those specifications. Looking at those specifications, it appears that leaving out the "T" is valid RFC 3339, but not valid ISO 8601.
Therefore it seems reasonable for an LRS to reject timestamps without the "T", and wrong to test that an LRS accepts such a timestamp. In fact, it seems better that we test that an LRS does reject such a timestamp, though that's not directly supported by the LRS requirements. Still -- I think we have to lean on the LRP requirements here because if we limit timestamp testing to what is supported by LRS requirements, I'm not finding the requirement that allows us to expect the LRS to enforce any particular format at all.
Logged a related issue against the spec as well: https://gitlab.com/IEEE-SA/xapi/9274.1.1/xapi-base-standard-documentation/-/issues/10
I would like to second this opinion and would suggest that if any other characters besides "T" are allowed in that position (for example, a blank space), that the xAPI specification should explicitly enumerate what those characters are.
The note in RFC 3339 states:
NOTE: ISO 8601 defines date and time separated by "T".
Applications using this syntax may choose, for the sake of
readability, to specify a full-date and full-time separated by
(say) a space character.
My read of that note is that specific implementations may choose to use a space (or other character) in that position but that would then become a requirement for that context. It is not suggesting that a consumer of a RFC 3339 date-time formatted timestamp must accept any character in that position.
This seems to be supported by the following comment by one of the RFC authors on the following discussion regarding this same issue:
An LRS should not have to support any character in that position that an LRP may decide to use, but perhaps it would be reasonable to support both "T" and a blank space (if that were made explicit in the specification).
Hmmm, maybe? The only mention of the timestamp's actual format is in the LRP section as you mentioned:
A Timestamp shall be expressed using the format described in RFC 3339, which is a profile of ISO 8601.
As @integralla mentioned, the RFC document does clearly state that spaces are allowed, but that separation character is enforced as T by the ISO 8601 format overall. The test here doesn't forbid anyone from also allowing alternative characters (including the T), it just checks that they also allow the commonly used space format for RFC.
There are a few tests in the 2.0 branch that do actually send statements as ISO 8601 and expect them to be accepted, namely the UTC conversion line already mentioned:
Since the ISO 8601 format is valid RFC 3339, what would the most sensible outcome be here?
@vbhayden note that ISO 8601 is specified on its own, without reference to RFC 3339 as part of the requirement
A Timestamp shall be formatted according to ISO 8601.
So while timestamp without the T is valid RFC 3339, it's not valid according to ISO 8601, and therefore not valid for an LRP to use
An LRS should not have to support any character in that position that an LRP may decide to use, but perhaps it would be reasonable to support both "T" and a blank space (if that were made explicit in the specification).
I agree, if it were made explicit in the specification it wouldn't be unreasonable to support a space as well (or not to, I really would just like it to be easier to interpret than it is). Note I've also logged an issue against the specification here, though that's focused on the fact these requirements are only listed on the LRP side not the LRS side: https://gitlab.com/IEEE-SA/xapi/9274.1.1/xapi-base-standard-documentation/-/issues/10
Maybe I am just not understanding their original point of mentioning RFC 3339 at all.
If it has to conform to both A and B, but A's formatting is a superset of B, then it only needs to conform to B -- ISO 8601 in this case -- so why even mention RFC 3339?
It doesn't make sense to include as a requirement for the LRS atm then, especially since it's not part of the LRS portion of the spec as-written. We are already considering removing the future version prohibition from the 1.0.3 section, so maybe this could be included in that batch of updates if everyone's in agreeance on your interpretation.
If it has to conform to both A and B, but A's formatting is a superset of B, then it only needs to conform to B -- ISO 8601 in this case -- so why even mention RFC 3339?
Ah, A's formatting is not quite a superset of B. I found this visualization helpful (includes things that aren't full timestamps, but still helpful): https://ijmacd.github.io/rfc3339-iso8601/
I believe our original intent was to specify RFC 3339 specifically, and was surprised to see ISO 8601 listed as a stand alone requirement -- but it is there. This wording appears to me to be confusing but not ambiguous , the format without the "T" is not both valid RFC 3339 and valid ISO 8601, so it is not valid (for the LRP). Of course there is no specific format listed for the LRS at all, and that is actual ambiguity. I suggest the only reasonable way to resolve that ambiguity is to apply the LRP requirements, except where explicitly contradicted by the LRS requirements (the LRS being required to convert non-UTC even though the LRP is required to use UTC)
It doesn't make sense to include as a requirement for the LRS atm then, especially since it's not part of the LRS portion of the spec as-written.
I'm not sure exactly what you mean here -- I'm not suggesting we add a requirement but to fix the expected response for a particular test case.
We are already considering removing the future version prohibition from the 1.0.3 section, so maybe this could be included in that batch of updates if everyone's in agreeance on your interpretation.
Certainly, I don't think we need to rush to change it vs including it in a batch of updates. People need time to weigh in anyway.
For the character in question though (whether or not to only check that the LRS accepts the format with T), then wouldn't it be though? With T works for both, with only works for RFC 3339 -- saying that the LRP must conform to both as-written would restrict it to only allowing the T one.
For the requirement portion, I'm agreeing with you guys that the LRS portion of the spec should include guidance on how the timestamp should be formatted and which formats are valid to parse + accept. Without it, sending an RFC 3339 timestamp and expecting a successful response code is opinionated behavior, as the IEEE LRS document never mentions it at all etc.
Oh sure. For the character in question, we can just look at ISO 8601 that requires the T, because (again for that character) RFC 3339 is a superset of ISO 8601