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

MessagePack timestamp type #168

Merged
merged 18 commits into from
Jun 20, 2019
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

AllCops:
TargetRubyVersion: 2.3

# Offense count: 3
Lint/AmbiguousOperator:
Enabled: false
Expand Down
2 changes: 2 additions & 0 deletions lib/msgpack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
require "msgpack/factory"
require "msgpack/symbol"
require "msgpack/core_ext"
require "msgpack/timestamp"
require "msgpack/time"

module MessagePack
DefaultFactory = MessagePack::Factory.new
Expand Down
29 changes: 29 additions & 0 deletions lib/msgpack/time.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# frozen_string_literal: true

# MessagePack extention packer and unpacker for built-in Time class
module MessagePack
module Time
# 3-arg Time.at is available Ruby >= 2.5
TIME_AT_3_AVAILABLE = begin
!!::Time.at(0, 0, :nanosecond)
rescue ArgumentError
false
end
Copy link
Member Author

@gfx gfx Jun 18, 2019

Choose a reason for hiding this comment

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

FYI the above crazy indentations are made by rubocop.

Copy link
Member

@tagomoris tagomoris Jun 18, 2019

Choose a reason for hiding this comment

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

How about this code?

TIME_AT_3_AVAILABLE = !!::Time.at(0, 0, :nanosecond) rescue false

Copy link
Member Author

@gfx gfx Jun 18, 2019

Choose a reason for hiding this comment

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

rubocop fixes it to:

    TIME_AT_3_AVAILABLE = begin
                            !!::Time.at(0, 0, :nanosecond)
                          rescue StandardError
                            false
                          end

but yes, it's better than the current, so I've applied it f49a8ff

I wonder the rubocop behavior that it sometimes seems to change the code at random.


Unpacker = if TIME_AT_3_AVAILABLE
lambda do |payload|
tv = MessagePack::Timestamp.from_msgpack_ext(payload)
::Time.at(tv.sec, tv.nsec, :nanosecond)
end
else
lambda do |payload|
tv = MessagePack::Timestamp.from_msgpack_ext(payload)
::Time.at(tv.sec, tv.nsec / 1000.0)
end
end

Packer = lambda { |time|
MessagePack::Timestamp.to_msgpack_ext(time.tv_sec, time.tv_nsec)
}
end
end
61 changes: 61 additions & 0 deletions lib/msgpack/timestamp.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# frozen_string_literal: true

module MessagePack
Timestamp = Struct.new(:sec, :nsec)

class Timestamp # a.k.a. "TimeSpec"
Copy link
Member

Choose a reason for hiding this comment

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

That looks non-standard way to add methods to the class from Struct.new. Use block parameter for Struct.new to define methods on that class: https://docs.ruby-lang.org/en/2.6.0/Struct.html#method-c-new

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't find how to use the Struct.new BLOCK syntax with constants:

NG(1):

$ ruby -e 'C = Struct.new(:foo) do CONST = 42 end; pp(C::CONST)'
Traceback (most recent call last):
-e:1:in `<main>': uninitialized constant C::CONST (NameError)

NG(2):

$ ruby -e 'Struct.new("::C", :foo) do pp self; CONST = 42 end; pp(C::CONST)'
Traceback (most recent call last):
	1: from -e:1:in `<main>'
-e:1:in `new': identifier ::C needs to be constant (NameError)

NG(3; it's not what I want):

$ ruby -e 'Struct.new("C", :foo) do pp self; self::CONST = 42 end; pp(Struct::C::CONST)'
Struct::C
42

I'm sorry I'm not good at Ruby 😭

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean that there is no way to refer MessagePack::Timestamp::TYPE from other classes?
It's not impossible (for example, C = Struct.new(); C.const_set(:CONST, 42); p C::CONST), but I guess we should define normal modules/classes instead of using Struct.
Is there any reason to use Struct?

Copy link
Member Author

Choose a reason for hiding this comment

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

I use Struct because of its auto-generated #== method, which is useful only for testing.

But yes, I'll use a normal class; it's just easy.

Copy link
Member Author

Choose a reason for hiding this comment

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

done: 733ed14

# Because the byte-order of MessagePack is big-endian in,
# pack() and unpack() specifies ">".
# See https://docs.ruby-lang.org/en/trunk/Array.html#method-i-pack for details.

# The timestamp extension type defined in the MessagePack spec.
# See https://github.com/msgpack/msgpack/blob/master/spec.md#timestamp-extension-type for details.
TYPE = -1

TIMESTAMP32_MAX_SEC = (1 << 32) - 1
TIMESTAMP64_MAX_SEC = (1 << 34) - 1

def self.from_msgpack_ext(data)
case data.length
when 4
# timestamp32 (sec: uint32be)
sec, = data.unpack('L>')
new(sec, 0)
when 8
# timestamp64 (nsec: uint30be, sec: uint34be)
n, s = data.unpack('L>2')
sec = ((n & 0b11) << 32) | s
nsec = n >> 2
new(sec, nsec)
when 12
# timestam96 (nsec: uint32be, sec: int64be)
nsec, sec = data.unpack('L>q>')
new(sec, nsec)
else
raise "Invalid timestamp data size: #{data.length}"
end
end

def self.to_msgpack_ext(sec, nsec)
if sec >= 0 && nsec >= 0 && sec <= TIMESTAMP64_MAX_SEC
if nsec === 0 && sec <= TIMESTAMP32_MAX_SEC
# timestamp32 = (sec: uint32be)
[sec].pack('L>')
else
# timestamp64 (nsec: uint30be, sec: uint34be)
nsec30 = nsec << 2
sec_high2 = sec << 32 # high 2 bits (`x & 0b11` is redandunt)
sec_low32 = sec & 0xffffffff # low 32 bits
[nsec30 | sec_high2, sec_low32].pack('L>2')
end
else
# timestamp96 (nsec: uint32be, sec: int64be)
[nsec, sec].pack('L>q>')
end
end

def to_msgpack_ext
self.class.to_msgpack_ext(sec, nsec)
end
end
end
82 changes: 82 additions & 0 deletions spec/timestamp_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# frozen_string_literal: true

require 'spec_helper'

describe MessagePack::Timestamp do
describe 'register_type with Time' do
let(:factory) do
factory = MessagePack::Factory.new
factory.register_type(
MessagePack::Timestamp::TYPE,
Time,
packer: MessagePack::Time::Packer,
unpacker: MessagePack::Time::Unpacker
)
factory
end

let(:time) { Time.local(2019, 6, 17, 1, 2, 3, 123_456) }
it 'serializes and deserializes Time' do
packed = factory.pack(time)
unpacked = factory.unpack(packed)
expect(unpacked).to eq(time)
end
end

describe 'register_type with MessagePack::Timestamp' do
let(:factory) do
factory = MessagePack::Factory.new
factory.register_type(MessagePack::Timestamp::TYPE, MessagePack::Timestamp)
factory
end

let(:timestamp) { MessagePack::Timestamp.new(Time.now.tv_sec, 123_456_789) }
it 'serializes and deserializes MessagePack::Timestamp' do
packed = factory.pack(timestamp)
unpacked = factory.unpack(packed)
expect(unpacked).to eq(timestamp)
end
end

describe 'timestamp32' do
it 'handles [1, 0]' do
t = MessagePack::Timestamp.new(1, 0)

payload = t.to_msgpack_ext
unpacked = MessagePack::Timestamp.from_msgpack_ext(payload)

expect(unpacked).to eq(t)
end
end

describe 'timestamp64' do
it 'handles [1, 1]' do
t = MessagePack::Timestamp.new(1, 1)

payload = t.to_msgpack_ext
unpacked = MessagePack::Timestamp.from_msgpack_ext(payload)

expect(unpacked).to eq(t)
end
end

describe 'timestamp96' do
it 'handles [-1, 0]' do
t = MessagePack::Timestamp.new(-1, 0)

payload = t.to_msgpack_ext
unpacked = MessagePack::Timestamp.from_msgpack_ext(payload)

expect(unpacked).to eq(t)
end

it 'handles [-1, 999_999_999]' do
t = MessagePack::Timestamp.new(-1, 999_999_999)

payload = t.to_msgpack_ext
unpacked = MessagePack::Timestamp.from_msgpack_ext(payload)

expect(unpacked).to eq(t)
end
end
end