Skip to content
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

Add support for decoding timestamp from existing ULID #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cocoahero
Copy link

This is another attempt at @joshbeckman's original PR (#31) which adds support for decoding the timestamp portion of an existing ULID value.

The main difference is this version fixes the flakey spec by using to_r instead of to_f when comparing the two timestamp values. I used .to_r for the same reasons as already noted here: https://github.com/rafaelsales/ulid/blob/master/lib/ulid/generator.rb#L59-L69

Copy link

@dmke dmke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am in need of this being a part of the gem :)

@@ -103,4 +103,31 @@
end
end
end

describe 'decoding a timestamp' do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ULIDs are case-insensitive. This currently fails:

it 'decodes in a case-insensitve manner' do
  ulid = ULID.generate.downcase
  assert ULID.decode_time(ulid).is_a?(Time)
end

For completeness, I'd like to see a few known input/output pairs (but please don't feel obliged to include them):

it 'correctly parses known test vectors' do
  {
    "0000000000Y2GBSG3FEFT635J1" => Time.at(0),
    "013XRZP292318JWRM98F0YAPV9" => Time.at(1234567891234/1000r),
    "ZZZZZZZZZZ-this-is-ignored" => Time.at(1125899906842623/1000r),
  }.each do |vector, expected_time|
    assert_equal expected_time, ULID.decode_time(vector)
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants