From 50cb6eca570a352a3b9799a66f77edad261763be Mon Sep 17 00:00:00 2001
From: Aleksei Lipniagov <alipniagov@gitlab.com>
Date: Wed, 11 Sep 2019 18:32:24 +0000
Subject: [PATCH] Use 'gitlab_chronic_duration' gem
Replace 'chronic_duration' to 'gitlab_chronic_duration', to make
relevant method calls thread-safe.
---
Gemfile | 2 +-
Gemfile.lock | 6 +--
...se-chronic-duration-in-thread-safe-way.yml | 5 ++
config/initializers/chronic_duration.rb | 2 -
lib/gitlab/patch/chronic_duration.rb | 35 --------------
lib/gitlab/time_tracking_formatter.rb | 47 ++++++++++---------
.../gitlab/ci/config/entry/rules/rule_spec.rb | 2 +-
.../lib/gitlab/patch/chronic_duration_spec.rb | 27 -----------
8 files changed, 34 insertions(+), 92 deletions(-)
create mode 100644 changelogs/unreleased/66637-use-chronic-duration-in-thread-safe-way.yml
delete mode 100644 lib/gitlab/patch/chronic_duration.rb
delete mode 100644 spec/lib/gitlab/patch/chronic_duration_spec.rb
diff --git a/Gemfile b/Gemfile
index 87e3e42c59a51..d79e97aabddfb 100644
--- a/Gemfile
+++ b/Gemfile
@@ -265,7 +265,7 @@ gem 'fast_blank'
# Parse time & duration
gem 'chronic', '~> 0.10.2'
-gem 'chronic_duration', '~> 0.10.6'
+gem 'gitlab_chronic_duration', '~> 0.10.6.1'
gem 'webpack-rails', '~> 0.9.10'
gem 'rack-proxy', '~> 0.6.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index fbdb78663e438..025542422d3b1 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -136,8 +136,6 @@ GEM
childprocess (0.9.0)
ffi (~> 1.0, >= 1.0.11)
chronic (0.10.2)
- chronic_duration (0.10.6)
- numerizer (~> 0.1.1)
chunky_png (1.3.5)
citrus (3.0.2)
claide (1.0.3)
@@ -356,6 +354,8 @@ GEM
rubocop-gitlab-security (~> 0.1.0)
rubocop-performance (~> 1.1.0)
rubocop-rspec (~> 1.19)
+ gitlab_chronic_duration (0.10.6.1)
+ numerizer (~> 0.1.1)
gitlab_omniauth-ldap (2.1.1)
net-ldap (~> 0.16)
omniauth (~> 1.3)
@@ -1089,7 +1089,6 @@ DEPENDENCIES
carrierwave (~> 1.3)
charlock_holmes (~> 0.7.5)
chronic (~> 0.10.2)
- chronic_duration (~> 0.10.6)
commonmarker (~> 0.17)
concurrent-ruby (~> 1.1)
connection_pool (~> 2.0)
@@ -1141,6 +1140,7 @@ DEPENDENCIES
gitlab-peek (~> 0.0.1)
gitlab-sidekiq-fetcher (= 0.5.2)
gitlab-styles (~> 2.7)
+ gitlab_chronic_duration (~> 0.10.6.1)
gitlab_omniauth-ldap (~> 2.1.1)
gon (~> 6.2)
google-api-client (~> 0.23)
diff --git a/changelogs/unreleased/66637-use-chronic-duration-in-thread-safe-way.yml b/changelogs/unreleased/66637-use-chronic-duration-in-thread-safe-way.yml
new file mode 100644
index 0000000000000..549f523076e3b
--- /dev/null
+++ b/changelogs/unreleased/66637-use-chronic-duration-in-thread-safe-way.yml
@@ -0,0 +1,5 @@
+---
+title: Use `ChronicDuration` in a thread-safe way
+merge_request: 32817
+author:
+type: fixed
diff --git a/config/initializers/chronic_duration.rb b/config/initializers/chronic_duration.rb
index aa43eef434c33..5528df6c66080 100644
--- a/config/initializers/chronic_duration.rb
+++ b/config/initializers/chronic_duration.rb
@@ -1,5 +1,3 @@
# frozen_string_literal: true
ChronicDuration.raise_exceptions = true
-
-ChronicDuration.prepend Gitlab::Patch::ChronicDuration
diff --git a/lib/gitlab/patch/chronic_duration.rb b/lib/gitlab/patch/chronic_duration.rb
deleted file mode 100644
index ab3cba3657f0f..0000000000000
--- a/lib/gitlab/patch/chronic_duration.rb
+++ /dev/null
@@ -1,35 +0,0 @@
-# frozen_string_literal: true
-
-# Fixes a bug where parsing months doesn't take into account
-# the ChronicDuration.days_per_week setting
-#
-# We can remove this when we do a refactor and push upstream in
-# https://gitlab.com/gitlab-org/gitlab-ce/issues/66637
-
-module Gitlab
- module Patch
- module ChronicDuration
- extend ActiveSupport::Concern
-
- class_methods do
- def duration_units_seconds_multiplier(unit)
- return 0 unless duration_units_list.include?(unit)
-
- case unit
- when 'months'
- 3600 * ::ChronicDuration.hours_per_day * ::ChronicDuration.days_per_month
- else
- super
- end
- end
-
- # ChronicDuration#output uses 1mo = 4w as the conversion so we do the same here.
- # We do need to add a special case for the default days_per_week value because
- # we want to retain existing behavior for the default case
- def days_per_month
- ::ChronicDuration.days_per_week == 7 ? 30 : ::ChronicDuration.days_per_week * 4
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/time_tracking_formatter.rb b/lib/gitlab/time_tracking_formatter.rb
index 302da91328a01..31883527135a8 100644
--- a/lib/gitlab/time_tracking_formatter.rb
+++ b/lib/gitlab/time_tracking_formatter.rb
@@ -4,37 +4,38 @@ module Gitlab
module TimeTrackingFormatter
extend self
- def parse(string)
- with_custom_config do
- string = string.sub(/\A-/, '')
+ # We may want to configure it through project settings in a future version.
+ CUSTOM_DAY_AND_WEEK_LENGTH = { hours_per_day: 8, days_per_month: 20 }.freeze
- seconds = ChronicDuration.parse(string, default_unit: 'hours') rescue nil
- seconds *= -1 if seconds && Regexp.last_match
- seconds
- end
+ def parse(string)
+ string = string.sub(/\A-/, '')
+
+ seconds =
+ begin
+ ChronicDuration.parse(
+ string,
+ CUSTOM_DAY_AND_WEEK_LENGTH.merge(default_unit: 'hours'))
+ rescue
+ nil
+ end
+
+ seconds *= -1 if seconds && Regexp.last_match
+ seconds
end
def output(seconds)
- with_custom_config do
- ChronicDuration.output(seconds, format: :short, limit_to_hours: limit_to_hours_setting, weeks: true) rescue nil
- end
+ ChronicDuration.output(
+ seconds,
+ CUSTOM_DAY_AND_WEEK_LENGTH.merge(
+ format: :short,
+ limit_to_hours: limit_to_hours_setting,
+ weeks: true))
+ rescue
+ nil
end
private
- def with_custom_config
- # We may want to configure it through project settings in a future version.
- ChronicDuration.hours_per_day = 8
- ChronicDuration.days_per_week = 5
-
- result = yield
-
- ChronicDuration.hours_per_day = 24
- ChronicDuration.days_per_week = 7
-
- result
- end
-
def limit_to_hours_setting
Gitlab::CurrentSettings.time_tracking_limit_to_hours
end
diff --git a/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb b/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb
index c25344ec1a447..18037a5612caf 100644
--- a/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb
@@ -1,5 +1,5 @@
require 'fast_spec_helper'
-require 'chronic_duration'
+require 'gitlab_chronic_duration'
require 'support/helpers/stub_feature_flags'
require_dependency 'active_model'
diff --git a/spec/lib/gitlab/patch/chronic_duration_spec.rb b/spec/lib/gitlab/patch/chronic_duration_spec.rb
deleted file mode 100644
index 541037ec1a204..0000000000000
--- a/spec/lib/gitlab/patch/chronic_duration_spec.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::Patch::ChronicDuration do
- subject { ChronicDuration.parse('1mo') }
-
- it 'uses default conversions' do
- expect(subject).to eq(2_592_000)
- end
-
- context 'with custom conversions' do
- before do
- ChronicDuration.hours_per_day = 8
- ChronicDuration.days_per_week = 5
- end
-
- after do
- ChronicDuration.hours_per_day = 24
- ChronicDuration.days_per_week = 7
- end
-
- it 'uses custom conversions' do
- expect(subject).to eq(576_000)
- end
- end
-end
--
GitLab