-
-
Notifications
You must be signed in to change notification settings - Fork 591
Support milliseconds in cacheDuration parsing #586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,45 @@ | ||
| require File.expand_path(File.join(File.dirname(__FILE__), "test_helper")) | ||
|
|
||
| class UtilsTest < Minitest::Test | ||
| describe ".parse_duration" do | ||
| DURATIONS_FROM_EPOCH = { | ||
| # Basic formats | ||
| "P1Y1M1D" => "1971-02-02T00:00:00.000Z", | ||
| "PT1H1M1S" => "1970-01-01T01:01:01.000Z", | ||
| "P1W" => "1970-01-08T00:00:00.000Z", | ||
| "P1Y1M1DT1H1M1S" => "1971-02-02T01:01:01.000Z", | ||
|
|
||
| # Negative duration | ||
| "-P1Y1M1DT1H1M1S" => "1968-11-29T22:58:59.000Z", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are negative values supported for each unit, e.g.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not certain that negative durations are part of the spec, as it mentions them only once, to say:
From conversations in the moment.js repo (1, 2) it appears that this is an area where implementations vary, some supporting prefix signs, some supporting infix. For the purposes of this PR, I'm just trying to cover the existing code, which supports a prefix sign alone.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some info about the xsd:duration type: http://www.datypic.com/sc/xsd/t-xsd_duration.html The numbers may be any unsigned integer, with the exception of the number of seconds, which may be an unsigned decimal number. So P15.5Y seems invalid
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, cool, thanks for the extra info! As a result of the earlier conversations I've permitted decimal hours and minutes; would you like me to back that change out, since the xsd:duration spec is tighter than the ISO one?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes please, and sorry for providing late this info.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No worries, glad to have it resolved! I've updated the branch and referred to the xsd:duration docs in the commit message. |
||
|
|
||
| # Nominal wraparounds | ||
| "P13M" => "1971-02-01T00:00:00.000Z", | ||
| "P31D" => "1970-02-01T00:00:00.000Z", | ||
|
|
||
| # Decimal seconds | ||
| "PT0.5S" => "1970-01-01T00:00:00.500Z", | ||
| "PT0,5S" => "1970-01-01T00:00:00.500Z" | ||
| } | ||
|
|
||
| def result(duration, reference = 0) | ||
| Time.at( | ||
| OneLogin::RubySaml::Utils.parse_duration(duration, reference) | ||
| ).utc.iso8601(3) | ||
| end | ||
|
|
||
| DURATIONS_FROM_EPOCH.each do |duration, expected| | ||
| it "parses #{duration} to return #{expected} from the given timestamp" do | ||
| assert_equal expected, result(duration) | ||
| end | ||
| end | ||
|
|
||
| it "returns the last calendar day of the next month when advancing from a longer month to a shorter one" do | ||
| initial_timestamp = Time.iso8601("1970-01-31T00:00:00.000Z").to_i | ||
|
|
||
| assert_equal "1970-02-28T00:00:00.000Z", result("P1M", initial_timestamp) | ||
| end | ||
| end | ||
|
|
||
| describe ".format_cert" do | ||
| let(:formatted_certificate) {read_certificate("formatted_certificate")} | ||
| let(:formatted_chained_certificate) {read_certificate("formatted_chained_certificate")} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this x?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the "ignore whitespace" regexp flag. It's what allows me to split the regexp over multiple lines, and also insert comments. You can see it here documented for 2.6, but it's supported back to 1.8.x:
https://ruby-doc.org/core-2.6.4/Regexp.html#class-Regexp-label-Options