Skip to content

Commit

Permalink
Fix Time comparisons with _matches? to use millisecond precision. (#5094
Browse files Browse the repository at this point in the history
)

Co-authored-by: Jason Penny <[email protected]>
Co-authored-by: Dmitry Rybakov <[email protected]>
  • Loading branch information
3 people authored Nov 17, 2021
1 parent d13ba55 commit b7acf87
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 3 deletions.
13 changes: 13 additions & 0 deletions lib/mongoid/extensions/time_with_zone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ def mongoize
::ActiveSupport::TimeWithZone.mongoize(self)
end

# This code is copied from Time class extension in bson-ruby gem. It
# should be removed from here when added to bson-ruby.
# See https://jira.mongodb.org/browse/RUBY-2846.
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

module ClassMethods

# Convert the object from its mongo friendly ruby type to this type.
Expand Down
34 changes: 32 additions & 2 deletions lib/mongoid/matcher/eq_impl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,40 @@ module EqImpl
end
=end
else
value == condition ||
value.is_a?(Array) && value.include?(condition)
# When doing a comparison with Time objects, compare using millisecond precision
if value.kind_of?(Time) && condition.kind_of?(Time)
time_eq?(value, condition)
elsif value.is_a?(Array) && condition.kind_of?(Time)
value.map do |v|
if v.kind_of?(Time)
time_rounded_to_millis(v)
else
v
end
end.include?(time_rounded_to_millis(condition))
else
value == condition ||
value.is_a?(Array) && value.include?(condition)
end
end
end

# Per https://docs.mongodb.com/ruby-driver/current/tutorials/bson-v4/#time-instances,
# > Times in BSON (and MongoDB) can only have millisecond precision. When Ruby Time instances
# are serialized to BSON or Extended JSON, the times are floored to the nearest millisecond.
#
# > 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.
#
# As such, perform a similar operation to what the bson-ruby gem does
module_function def time_eq?(time_a, time_b)
time_rounded_to_millis(time_a) == time_rounded_to_millis(time_b)
end

module_function def time_rounded_to_millis(time)
return time._bson_to_i * 1000 + time.usec.divmod(1000).first
end
end
end
end
6 changes: 5 additions & 1 deletion lib/mongoid/matcher/eq_impl_with_regexp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ module EqImplWithRegexp
when ::BSON::Regexp::Raw
value =~ condition.compile
else
value == condition
if value.kind_of?(Time) && condition.kind_of?(Time)
EqImpl.time_eq?(value, condition)
else
value == condition
end
end
end
end
Expand Down
49 changes: 49 additions & 0 deletions spec/integration/matcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,55 @@
end
end

context 'when comparing Time objects' do
let(:time) {Time.utc(2021, 10, 25, 10, 30, 30, 581345)}
let(:document) do
ConsumptionPeriod.new(:started_at => time)
end

context 'comparing millisecond precision' do
let(:time_millis) {Time.utc(2021, 10, 25, 10, 30, 30, 581774)}

context 'with exact match' do
let(:query) do
{'started_at' => time_millis}
end

it_behaves_like 'is true'

context 'and query has different timezone' do
let(:time_millis) do
Time.utc(2021, 10, 25, 10, 30, 30, 581345).in_time_zone("Stockholm")
end

it_behaves_like 'is true'
end
end

context 'with $in' do
let(:query) do
{'started_at' => {:$in => [time_millis]}}
end

it_behaves_like 'is true'
end

context 'when matching an element in an array' do
let(:document) do
Mop.new(:array_field => [time])
end

context 'with equals match' do
let(:query) do
{'array_field' => time_millis}
end

it_behaves_like 'is true'
end
end
end
end

context 'when querying nested document' do
context 'embeds_one' do
let(:document) do
Expand Down

0 comments on commit b7acf87

Please sign in to comment.