Merge branch '66637-use-chronic-duration-in-thread-safe-way' into 'master'

Use `ChronicDuration` in a thread-safe way

See merge request gitlab-org/gitlab-ce!32817

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'

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)

changelogs/unreleased/66637-use-chronic-duration-in-thread-safe-way.yml

+---
+title: Use `ChronicDuration` in a thread-safe way
+merge_request: 32817
+author:
+type: fixed

config/initializers/chronic_duration.rb

 # frozen_string_literal: true
 
 ChronicDuration.raise_exceptions = true
-
-ChronicDuration.prepend Gitlab::Patch::ChronicDuration

lib/gitlab/patch/chronic_duration.rb

-# 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

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

spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb

 require 'fast_spec_helper'
-require 'chronic_duration'
+require 'gitlab_chronic_duration'
 require 'support/helpers/stub_feature_flags'
 require_dependency 'active_model'
 

spec/lib/gitlab/patch/chronic_duration_spec.rb

-# 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