feat: add skipNormalization flag to duration struct and implement NewISODurationFromString function - #222
feat: add skipNormalization flag to duration struct and implement NewISODurationFromString function#222ShocOne wants to merge 1 commit into
Conversation
…ISODurationFromString function - Introduced a skipNormalization boolean in the duration struct to control normalization behavior. - Added NewISODurationFromString function to parse ISO 8601 duration strings while preserving their original format. - Updated tests to verify the correct parsing and preservation of various ISO duration formats.
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
I think the issue here also outlines a normalization issue. So I created #223 and #224 to help sort this out.
Interestingly enough, the dotnet implementation does not support weeks at all, and nobody has complained about this to date. But anyway, it does not normalize days to years for example.
This is because the underlying API used is in fact implementing the W3C standard, which is a subset of the ISO 8601 docs.
So overall, I think it'd be fair for a parse -> serialize scenario to be symmetric and not lead to any normalization. Only the components based instances should lead to a normalization as a way to ensure we don't end up serializing invalid values.
Ideally, instead of normalizing, we should probably do validation in the constructors to give feedback as early as possible. But the error return pattern does not go well with constructors not typically returning errors.
|
|
||
| // NewISODurationFromString parses an ISO 8601 duration string and preserves the original | ||
| // format on serialization, bypassing normalization. Use this when an API requires an exact | ||
| // duration format such as P90D rather than its normalized equivalent P12W6D. |
There was a problem hiding this comment.
| // duration format such as P90D rather than its normalized equivalent P12W6D. | |
| // duration format such as P7D rather than its normalized equivalent P1W. |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| d.skipNormalization = true |
There was a problem hiding this comment.
arguably, should we simply flip the flag in the parse method instead of introducing a new one?
Introduced a skipNormalization boolean in the duration struct to control normalization behavior.
Added NewISODurationFromString function to parse ISO 8601 duration strings while preserving their original format.
Updated tests to verify the correct parsing and preservation of various ISO duration formats.
Fixes Microsoft Graph SDK ISO 8601 Duration Normalization #213
The ISODuration.String() method unconditionally calls normalize() during serialization, which
converts day-based durations into week equivalents (e.g. P90D → P12W6D). This breaks APIs that
require exact ISO 8601 duration formats such as the Microsoft Graph /agreements endpoint.
flag, so the original format is preserved on serialization
Examples
// Before: normalization was unconditional
d := serialization.NewDuration(0, 0, 90, 0, 0, 0, 0)
d.String() // "P12W6D" — breaks Graph /agreements API
// After: use NewISODurationFromString to preserve format
d, err := serialization.NewISODurationFromString("P90D")
d.String() // "P90D" — correct
Test plan