Fix RUBY-2145 bson 4.8.0 rounds Time during serialization to bson/ext…

…ended json (mongodb#190)

* Fix RUBY-2145 bson 4.8.0 rounds Time during serialization to bson/extended json

* fix for older ruby versions

* compatibility with ruby 2.3

* add jruby debugging tools

* test times that precede unix epoch

* work around jruby rounding times up

Co-authored-by: Oleg Pudeyev <p@users.noreply.github.com>

Gemfile

@@ -27,6 +27,9 @@ group :development, :test do
       gem 'byebug', platforms: :mri
     end
     gem 'pry', platforms: :jruby
+    gem 'pry-nav', platforms: :jruby
+    # https://github.com/jruby/jruby/wiki/UsingTheJRubyDebugger
+    gem 'ruby-debug', platforms: :jruby
   end
 end
 

docs/tutorials/bson-v4.txt

@@ -555,12 +555,46 @@ them.
      - ``{ "$regex" : "[abc]", "$options" : "i" }``
 
 
-Special Ruby Date Classes
--------------------------
+Time Instances
+--------------
 
-Ruby's ``Date`` and ``DateTime`` are able to be serialized, but when they are
-deserialized, they will always be returned as a ``Time`` since the BSON
-specification only has a ``Time`` type and knows nothing about Ruby.
+Times in Ruby can have nanosecond precision. Times in BSON (and MongoDB)
+can only have microsecond precision. When Ruby ``Time`` instances are
+serialized to BSON or Extended JSON, the times are floored to the nearest
+millisecond.
+
+.. note::
+
+  The time as always rounded down. If the time precedes the Unix epoch
+  (January 1, 1970 00:00:00 UTC), the absolute value of the time would
+  increase:
+
+  .. code-block:: ruby
+
+  time = Time.utc(1960, 1, 1, 0, 0, 0, 999_999)
+  time.to_f
+  # => -315619199.000001
+  time.floor(3).to_f
+  # => -315619199.001
+
+.. note::
+
+  JRuby as of version 9.2.11.0 `rounds pre-Unix epoch times up rather than
+  down <https://github.com/jruby/jruby/issues/6104>`_. bson-ruby works around
+  this and correctly floors the times when serializing on JRuby.
+
+Because of this flooring, applications are strongly recommended to perform
+all time calculations using integer math, as inexactness of floating point
+calculations may produce unexpected results.
+
+
+Date And DateTime Instances
+---------------------------
+
+BSON only supports storing the time as the number of seconds since the
+Unix epoch. Ruby's ``Date`` and ``DateTime`` instances can be serialized
+to BSON, but when the BSON is deserialized the times will be returned as
+``Time`` instances.
 
 
 Regular Expressions

lib/bson/ext_json.rb

@@ -256,7 +256,8 @@ module ExtJSON
             unless value.keys.sort == %w($numberLong)
               raise Error::ExtJSONParseError, "Invalid value for $date: #{value}"
             end
-            ::Time.at(value.values.first.to_i.to_f / 1000)
+            sec, msec = value.values.first.to_i.divmod(1000)
+            ::Time.at(sec, msec*1000)
           else
             raise Error::ExtJSONParseError, "Invalid value for $date: #{value}"
           end

lib/bson/time.rb

@@ -17,6 +17,20 @@ module BSON
   # Injects behaviour for encoding and decoding time values to
   # and from raw bytes as specified by the BSON spec.
   #
+  # @note
+  #   Ruby time can have nanosecond precision:
+  #   +Time.utc(2020, 1, 1, 0, 0, 0, 999_999_999/1000r)+
+  #   +Time#usec+ returns the number of microseconds in the time, and
+  #   if the time has nanosecond precision the sub-microsecond part is
+  #   truncated (the value is floored to the nearest millisecond).
+  #   MongoDB only supports millisecond precision; we truncate the
+  #   sub-millisecond part of microseconds (floor to the nearest millisecond).
+  #   Note that if a time is constructed from a floating point value,
+  #   the microsecond value may round to the starting floating point value
+  #   but due to flooring, the time after serialization may end up to
+  #   be different than the starting floating point value.
+  #   It is recommended that time calculations use integer math only.
+  #
   # @see http://bsonspec.org/#/specification
   #
   # @since 2.0.0
@@ -29,6 +43,8 @@ module Time
 
     # Get the time as encoded BSON.
     #
+    # @note The time is floored to the nearest millisecond.
+    #
     # @example Get the time as encoded BSON.
     #   Time.new(2012, 1, 1, 0, 0, 0).to_bson
     #
@@ -38,15 +54,15 @@ module Time
     #
     # @since 2.0.0
     def to_bson(buffer = ByteBuffer.new, validating_keys = Config.validating_keys?)
-      # A previous version of this method used the following implementation:
-      # buffer.put_int64((to_i * 1000) + (usec / 1000))
-      # Turns out, usec returned incorrect value - 999 for 1 millisecond.
-      buffer.put_int64((to_f * 1000).round)
+      value = _bson_to_i * 1000 + usec.divmod(1000).first
+      buffer.put_int64(value)
     end
 
     # Converts this object to a representation directly serializable to
     # Extended JSON (https://github.com/mongodb/specifications/blob/master/source/extended-json.rst).
     #
+    # @note The time is floored to the nearest millisecond.
+    #
     # @option options [ true | false ] :relaxed Whether to produce relaxed
     #   extended JSON representation.
     #
@@ -55,12 +71,30 @@ def as_extended_json(**options)
       utc_time = utc
       if options[:mode] == :relaxed && (1970..9999).include?(utc_time.year)
         if utc_time.usec != 0
+          if utc_time.respond_to?(:floor)
+            # Ruby 2.7+
+            utc_time = utc_time.floor(3)
+          else
+            utc_time -= utc_time.usec.divmod(1000).last.to_r / 1000000
+          end
           {'$date' => utc_time.strftime('%Y-%m-%dT%H:%M:%S.%LZ')}
         else
           {'$date' => utc_time.strftime('%Y-%m-%dT%H:%M:%SZ')}
         end
       else
-        {'$date' => {'$numberLong' => (utc_time.to_f * 1000).round.to_s}}
+        sec = utc_time._bson_to_i
+        msec = utc_time.usec.divmod(1000).first
+        {'$date' => {'$numberLong' => (sec * 1000 + msec).to_s}}
+      end
+    end
+
+    def _bson_to_i
+      # Workaround for JRuby's #to_i rounding negative timestamps up
+      # rather than down (https://github.com/jruby/jruby/issues/6104)
+      if BSON::Environment.jruby?
+        (self - usec.to_r/1000000).to_i
+      else
+        to_i
       end
     end
 

spec/bson/time_spec.rb

@@ -43,13 +43,216 @@
       end
     end
 
-    context "when the time is pre epoch" do
+    context "when the time precedes epoch" do
 
       let(:obj)  { Time.utc(1969, 1, 1, 0, 0, 0) }
       let(:bson) { [ (obj.to_f * 1000).to_i ].pack(BSON::Int64::PACK) }
 
       it_behaves_like "a serializable bson element"
       it_behaves_like "a deserializable bson element"
     end
+
+    context 'when value has sub-millisecond precision' do
+      let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999) }
+
+      let(:expected_round_tripped_obj) do
+        Time.utc(2012, 1, 1, 0, 0, 0, 999_000)
+      end
+
+      let(:round_tripped_obj) do
+        Time.from_bson(obj.to_bson)
+      end
+
+      it 'truncates to milliseconds when round-tripping' do
+        round_tripped_obj.should == expected_round_tripped_obj
+      end
+    end
+  end
+
+  describe '#as_extended_json' do
+
+    context 'canonical mode' do
+      context 'when value has sub-millisecond precision' do
+        let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999) }
+
+        let(:expected_serialization) do
+          {'$date' => {'$numberLong' => '1325376000999'}}
+        end
+
+        let(:serialization) do
+          obj.as_extended_json
+        end
+
+        shared_examples_for 'truncates to milliseconds when serializing' do
+          it 'truncates to milliseconds when serializing' do
+            serialization.should == expected_serialization
+          end
+        end
+
+        it_behaves_like 'truncates to milliseconds when serializing'
+
+        context 'when value has sub-microsecond precision' do
+          let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999_999/1000r) }
+
+          it_behaves_like 'truncates to milliseconds when serializing'
+        end
+
+        context "when the time precedes epoch" do
+          let(:obj)  { Time.utc(1960, 1, 1, 0, 0, 0, 999_999) }
+
+          let(:expected_serialization) do
+            {'$date' => {'$numberLong' => '-315619199001'}}
+          end
+
+          it_behaves_like 'truncates to milliseconds when serializing'
+        end
+      end
+    end
+
+    context 'relaxed mode' do
+      context 'when value has sub-millisecond precision' do
+        let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999) }
+
+        let(:expected_serialization) do
+          {'$date' => '2012-01-01T00:00:00.999Z'}
+        end
+
+        let(:serialization) do
+          obj.as_extended_json(mode: :relaxed)
+        end
+
+        shared_examples_for 'truncates to milliseconds when serializing' do
+          it 'truncates to milliseconds when serializing' do
+            serialization.should == expected_serialization
+          end
+        end
+
+        it_behaves_like 'truncates to milliseconds when serializing'
+
+        context 'when value has sub-microsecond precision' do
+          let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999_999/1000r) }
+
+          it_behaves_like 'truncates to milliseconds when serializing'
+        end
+
+        context "when the time precedes epoch" do
+          let(:obj)  { Time.utc(1960, 1, 1, 0, 0, 0, 999_999) }
+
+          let(:expected_serialization) do
+            {'$date' => {'$numberLong' => '-315619199001'}}
+          end
+
+          it_behaves_like 'truncates to milliseconds when serializing'
+        end
+      end
+    end
+  end
+
+  describe '#to_extended_json' do
+
+    context 'canonical mode' do
+      context 'when value has sub-millisecond precision' do
+        let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999) }
+
+        let(:expected_serialization) do
+          %q`{"$date":{"$numberLong":"1325376000999"}}`
+        end
+
+        let(:serialization) do
+          obj.to_extended_json
+        end
+
+        shared_examples_for 'truncates to milliseconds when serializing' do
+          it 'truncates to milliseconds when serializing' do
+            serialization.should == expected_serialization
+          end
+        end
+
+        it_behaves_like 'truncates to milliseconds when serializing'
+
+        context 'when value has sub-microsecond precision' do
+          let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999_999/1000r) }
+
+          it_behaves_like 'truncates to milliseconds when serializing'
+        end
+
+        context "when the time precedes epoch" do
+          let(:obj)  { Time.utc(1960, 1, 1, 0, 0, 0, 999_999) }
+
+          let(:expected_serialization) do
+            %q`{"$date":{"$numberLong":"-315619199001"}}`
+          end
+
+          it_behaves_like 'truncates to milliseconds when serializing'
+        end
+      end
+    end
+
+    context 'relaxed mode' do
+      context 'when value has sub-millisecond precision' do
+        let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999) }
+
+        let(:expected_serialization) do
+          %q`{"$date":"2012-01-01T00:00:00.999Z"}`
+        end
+
+        let(:serialization) do
+          obj.to_extended_json(mode: :relaxed)
+        end
+
+        shared_examples_for 'truncates to milliseconds when serializing' do
+          it 'truncates to milliseconds when serializing' do
+            serialization.should == expected_serialization
+          end
+        end
+
+        it_behaves_like 'truncates to milliseconds when serializing'
+
+        context 'when value has sub-microsecond precision' do
+          let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999_999/1000r) }
+
+          it_behaves_like 'truncates to milliseconds when serializing'
+        end
+      end
+    end
+  end
+
+  describe '#to_json' do
+
+    context 'when value has sub-millisecond precision' do
+      let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999) }
+
+      let(:expected_serialization) do
+        %q`"2012-01-01 00:00:00 UTC"`
+      end
+
+      let(:serialization) do
+        obj.to_json
+      end
+
+      shared_examples_for 'truncates to milliseconds when serializing' do
+        it 'truncates to milliseconds when serializing' do
+          serialization.should == expected_serialization
+        end
+      end
+
+      it_behaves_like 'truncates to milliseconds when serializing'
+
+      context 'when value has sub-microsecond precision' do
+        let(:obj)  { Time.utc(2012, 1, 1, 0, 0, 0, 999_999_999/1000r) }
+
+        it_behaves_like 'truncates to milliseconds when serializing'
+      end
+
+      context "when the time precedes epoch" do
+        let(:obj)  { Time.utc(1960, 1, 1, 0, 0, 0, 999_999) }
+
+        let(:expected_serialization) do
+          %q`"1960-01-01 00:00:00 UTC"`
+        end
+
+        it_behaves_like 'truncates to milliseconds when serializing'
+      end
+    end
   end
 end