diff --git a/.gitignore b/.gitignore index 7502bcd..a703c1d 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ .ruby-gemset .ruby-version Gemfile.lock +gemfiles/*.lock InstalledFiles _yardoc coverage diff --git a/.travis.yml b/.travis.yml index 674f609..f4d5fa4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,10 @@ language: ruby +dist: bionic rvm: - - 2.4.9 - - 2.5.7 - - 2.6.5 - - 2.7.0 + - 2.4.10 + - 2.5.8 + - 2.6.6 + - 2.7.2 - 3.0.0 - jruby-9.2.14.0 gemfile: @@ -12,17 +13,15 @@ gemfile: - gemfiles/Gemfile.activesupport-6.x jobs: exclude: - - rvm: 2.4.9 + - rvm: 2.4.10 gemfile: gemfiles/Gemfile.activesupport-6.x - - rvm: 2.7.0 + - rvm: 2.7.2 gemfile: gemfiles/Gemfile.activesupport-4.x - rvm: 3.0.0 gemfile: gemfiles/Gemfile.activesupport-4.x include: - rvm: ruby-head gemfile: gemfiles/Gemfile.activesupport-edge - - rvm: jruby-head - gemfile: gemfiles/Gemfile.activesupport-edge allow_failures: - gemfile: gemfiles/Gemfile.activesupport-edge fast_finish: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 59aba22..6abdd47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ # Unreleased -[Compare master with v1.3.0](https://github.com/intrepidd/working_hours/compare/v1.3.0...master) +[Compare master with v1.3.1](https://github.com/intrepidd/working_hours/compare/v1.3.1...master) + +# v1.3.1 +* Improve computation accuracy in `advance_to_working_time` and `working_time_between` by using more exact (integer-based) time operations instead of floating point numbers - [#44](https://github.com/Intrepidd/working_hours/pull/44) +* Raise an exception when we detect an infinite loops in `advance_to_working_time` to improve resilience and make debugging easier - [#44](https://github.com/Intrepidd/working_hours/pull/44) +* Use a Rational number for the midnight value to avoid leaking sub-nanoseconds residue because of floating point accuracy - [#44](https://github.com/Intrepidd/working_hours/pull/44) # v1.3.0 * Improve supports for fractional seconds in input times by only rounding results at the end - [#42](https://github.com/Intrepidd/working_hours/issues/42) [#43](https://github.com/Intrepidd/working_hours/pull/43) diff --git a/gemfiles/Gemfile.activesupport-6.x b/gemfiles/Gemfile.activesupport-6.x index 0683088..1cb6b87 100644 --- a/gemfiles/Gemfile.activesupport-6.x +++ b/gemfiles/Gemfile.activesupport-6.x @@ -2,4 +2,4 @@ source 'https://rubygems.org' gemspec :path => '..' -gem 'activesupport', '~> 6.0' +gem 'activesupport', '~> 6.1' diff --git a/gemfiles/Gemfile.activesupport-edge b/gemfiles/Gemfile.activesupport-edge index cdbae92..3d3fe83 100644 --- a/gemfiles/Gemfile.activesupport-edge +++ b/gemfiles/Gemfile.activesupport-edge @@ -2,4 +2,4 @@ source 'https://rubygems.org' gemspec :path => '..' -gem 'activesupport', github: 'rails/rails' +gem 'activesupport', github: 'rails/rails', branch: 'main' diff --git a/lib/working_hours/computation.rb b/lib/working_hours/computation.rb index bc71e43..ba25b29 100644 --- a/lib/working_hours/computation.rb +++ b/lib/working_hours/computation.rb @@ -82,7 +82,7 @@ def advance_to_working_time time, config: nil time_in_day = time.seconds_since_midnight config[:working_hours][time.wday].each do |from, to| return time if time_in_day >= from and time_in_day < to - return time + (from - time_in_day) if from >= time_in_day + return time.beginning_of_day + from if from >= time_in_day end # if none is found, go to next day and loop time = (time + 1.day).beginning_of_day @@ -101,8 +101,7 @@ def advance_to_closing_time time, config: nil time_in_day = time.seconds_since_midnight time = time.beginning_of_day config[:working_hours][time.wday].each do |from, to| - return time + to if time_in_day >= from and time_in_day < to - return time + to if from >= time_in_day + return time + to if time_in_day < to end # if none is found, go to next day and loop time = time + 1.day @@ -183,20 +182,25 @@ def working_time_between from, to, config: nil to = in_config_zone(to, config: config) distance = 0 while from < to + from_was = from # look at working ranges time_in_day = from.seconds_since_midnight config[:working_hours][from.wday].each do |begins, ends| if time_in_day >= begins and time_in_day < ends - # take all we can - take = [ends - time_in_day, to - from].min - # advance time - from += take - # increase counter - distance += take + if (to - from) > (ends - time_in_day) + # take all the range and continue + distance += (ends - time_in_day) + from = from.beginning_of_day + ends + else + # take only what's needed and stop + distance += (to - from) + from = to + end end end # roll to next business period from = advance_to_working_time(from, config: config) + raise "Invalid loop detected in working_time_between (from=#{from.iso8601(12)}, to=#{to.iso8601(12)}, distance=#{distance}, config=#{config}), please open an issue ;)" unless from > from_was end distance.round # round up to supress miliseconds introduced by 24:00 hack end diff --git a/lib/working_hours/config.rb b/lib/working_hours/config.rb index 81dea66..9de17d0 100644 --- a/lib/working_hours/config.rb +++ b/lib/working_hours/config.rb @@ -7,6 +7,7 @@ class Config TIME_FORMAT = /\A([0-2][0-9])\:([0-5][0-9])(?:\:([0-5][0-9]))?\z/ DAYS_OF_WEEK = [:sun, :mon, :tue, :wed, :thu, :fri, :sat] + MIDNIGHT = Rational('86399.999999') class << self @@ -115,7 +116,7 @@ def compile_time time sec = time[TIME_FORMAT,3].to_i time = hour * 3600 + min * 60 + sec # Converts 24:00 to 23:59:59.999999 - return 86399.999999 if time == 86400 + return MIDNIGHT if time == 86400 time end diff --git a/lib/working_hours/version.rb b/lib/working_hours/version.rb index b8ad526..7335168 100644 --- a/lib/working_hours/version.rb +++ b/lib/working_hours/version.rb @@ -1,3 +1,3 @@ module WorkingHours - VERSION = "1.3.0" + VERSION = "1.3.1" end diff --git a/spec/working_hours/computation_spec.rb b/spec/working_hours/computation_spec.rb index 3928c80..d61d32f 100644 --- a/spec/working_hours/computation_spec.rb +++ b/spec/working_hours/computation_spec.rb @@ -173,6 +173,10 @@ WorkingHours::Config.time_zone = 'Tokyo' expect(advance_to_working_time(Time.new(2014, 4, 7, 0, 0, 0)).zone).to eq('JST') end + + it 'do not leak nanoseconds when advancing' do + expect(advance_to_working_time(Time.utc(2014, 4, 7, 5, 0, 0, 123456.789))).to eq(Time.utc(2014, 4, 7, 9, 0, 0, 0)) + end end describe '#advance_to_closing_time' do @@ -240,7 +244,7 @@ end let(:monday_morning) { Time.utc(2014, 4, 7, 0) } - let(:monday_closing) { Time.utc(2014, 4, 7) + 86399.999999 } + let(:monday_closing) { Time.utc(2014, 4, 7) + WorkingHours::Config::MIDNIGHT } let(:tuesday_closing) { Time.utc(2014, 4, 8, 17) } let(:sunday) { Time.utc(2014, 4, 6, 17) } @@ -255,6 +259,11 @@ it 'moves over midnight' do expect(advance_to_closing_time(sunday)).to eq(monday_closing) end + + it 'give precise computation with nothing other than miliseconds' do + pending "iso8601 is not precise enough on AS < 4" if ActiveSupport::VERSION::MAJOR <= 4 + expect(advance_to_closing_time(monday_morning).iso8601(25)).to eq("2014-04-07T23:59:59.9999990000000000000000000Z") + end end it 'works with any input timezone (converts to config)' do @@ -271,6 +280,10 @@ WorkingHours::Config.time_zone = 'Tokyo' expect(advance_to_closing_time(Time.new(2014, 4, 7, 0, 0, 0)).zone).to eq('JST') end + + it 'do not leak nanoseconds when advancing' do + expect(advance_to_closing_time(Time.utc(2014, 4, 7, 5, 0, 0, 123456.789))).to eq(Time.utc(2014, 4, 7, 17, 0, 0, 0)) + end end describe '#next_working_time' do @@ -544,6 +557,27 @@ )).to eq(7.hours) end + it 'uses precise computation to avoid useless loops' do + # +200 usec on each time, using floating point would cause + # precision issues and require several iterations + expect(self).to receive(:advance_to_working_time).twice.and_call_original + expect(working_time_between( + Time.utc(2014, 4, 7, 5, 0, 0, 200), + Time.utc(2014, 4, 7, 15, 0, 0, 200), + )).to eq(6.hours) + end + + it 'do not cause infinite loop if the time is not advancing properly' do + # simulate some computation/precision error + expect(self).to receive(:advance_to_working_time).twice do |time| + time.change(hour: 9) - 0.0001 + end + expect { working_time_between( + Time.utc(2014, 4, 7, 5, 0, 0), + Time.utc(2014, 4, 7, 15, 0, 0), + ) }.to raise_error(RuntimeError, /Invalid loop detected in working_time_between \(from=2014-04-07T08:59:59.999/) + end + # generates two times with +0ms, +250ms, +500ms, +750ms and +1s # then for each combination compare the result with a ruby diff context 'with precise miliseconds timings' do