diff --git a/Gemfile b/Gemfile index 6d6ce7eddaccbe55ae281a3016f7e73fe5621558..f770046c2982970bcc43563ee623fa12631bdc82 100644 --- a/Gemfile +++ b/Gemfile @@ -324,7 +324,7 @@ gem 'fast_blank', '~> 1.0.1' # Parse time & duration gem 'gitlab-chronic', '~> 0.10.5' -gem 'gitlab_chronic_duration', '~> 0.10.6.2' +gem 'gitlab_chronic_duration', '~> 0.11' gem 'rack-proxy', '~> 0.7.6' diff --git a/Gemfile.checksum b/Gemfile.checksum index 9f4f03fe63b5698a772007d805a17ea6df3f94d7..cf898be05b74df880da9a0d02a8f8659137e60d5 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -217,7 +217,7 @@ {"name":"gitlab-markup","version":"1.9.0","platform":"ruby","checksum":"7eda045a08ec2d110084252fa13a8c9eac8bdac0e302035ca7db4b82bcbd7ed4"}, {"name":"gitlab-net-dns","version":"0.9.2","platform":"ruby","checksum":"f726d978479d43810819f12a45c0906d775a07e34df111bbe693fffbbef3059d"}, {"name":"gitlab-styles","version":"10.1.0","platform":"ruby","checksum":"f42745f5397d042fe24cf2d0eb56c995b37f9f43d8fb79b834d197a1cafdc84a"}, -{"name":"gitlab_chronic_duration","version":"0.10.6.2","platform":"ruby","checksum":"6dda4cfe7dca9b958f163ac8835c3d9cc70cf8df8cbb89bb2fbf9ba4375105fb"}, +{"name":"gitlab_chronic_duration","version":"0.11.0","platform":"ruby","checksum":"c2fd201724a9031ff0af23d07a30231cebefbf83c3e682daae452cda5f514ba6"}, {"name":"gitlab_omniauth-ldap","version":"2.2.0","platform":"ruby","checksum":"bb4d20acb3b123ed654a8f6a47d3fac673ece7ed0b6992edb92dca14bad2838c"}, {"name":"gitlab_quality-test_tooling","version":"0.9.3","platform":"ruby","checksum":"9751f3504b717499588bd0fa5517de9b6756e8b9548777ea0283b889694580f0"}, {"name":"globalid","version":"1.1.0","platform":"ruby","checksum":"b337e1746f0c8cb0a6c918234b03a1ddeb4966206ce288fbb57779f59b2d154f"}, diff --git a/Gemfile.lock b/Gemfile.lock index 01f62db25c41fefd06c1aa912b9be5eefbcb5f36..25437eb009c7038ece07ce39002bc28c1ad122b2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -676,7 +676,7 @@ GEM rubocop-performance (~> 1.15) rubocop-rails (~> 2.17) rubocop-rspec (~> 2.22) - gitlab_chronic_duration (0.10.6.2) + gitlab_chronic_duration (0.11.0) numerizer (~> 0.2) gitlab_omniauth-ldap (2.2.0) net-ldap (~> 0.16) @@ -1832,7 +1832,7 @@ DEPENDENCIES gitlab-sidekiq-fetcher! gitlab-styles (~> 10.1.0) gitlab-utils! - gitlab_chronic_duration (~> 0.10.6.2) + gitlab_chronic_duration (~> 0.11) gitlab_omniauth-ldap (~> 2.2.0) gitlab_quality-test_tooling (~> 0.9.3) gon (~> 6.4.0) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 107f3c5987a68c1847f645adf6bc24d64786442c..2b954e399ae514d3fd198397b3c3b3d746ff96af 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -414,7 +414,9 @@ def schedulable? end def options_scheduled_at - ChronicDuration.parse(options[:start_in])&.seconds&.from_now + ChronicDuration.parse( + options[:start_in], use_complete_matcher: Feature.enabled?(:update_chronic_duration) + )&.seconds&.from_now end def action? @@ -745,7 +747,9 @@ def artifacts_expire_in def artifacts_expire_in=(value) self.artifacts_expire_at = if value - ChronicDuration.parse(value)&.seconds&.from_now + ChronicDuration.parse( + value, use_complete_matcher: Feature.enabled?(:update_chronic_duration) + )&.seconds&.from_now end end diff --git a/app/models/concerns/chronic_duration_attribute.rb b/app/models/concerns/chronic_duration_attribute.rb index af4905115b1d1123acc35cbdac71ad3ac1948ea9..944ad252cc6500107d54b888460521728c03d79b 100644 --- a/app/models/concerns/chronic_duration_attribute.rb +++ b/app/models/concerns/chronic_duration_attribute.rb @@ -17,7 +17,14 @@ def chronic_duration_attr_writer(virtual_attribute, source_attribute, parameters chronic_duration_attributes[virtual_attribute] = value.presence || parameters[:default].presence.to_s begin - new_value = value.present? ? ChronicDuration.parse(value).to_i : parameters[:default].presence + new_value = if value.present? + ChronicDuration.parse( + value, use_complete_matcher: Feature.enabled?(:update_chronic_duration) + ).to_i + else + parameters[:default].presence + end + assign_attributes(source_attribute => new_value) rescue ChronicDuration::DurationParseError # ignore error as it will be caught by validation diff --git a/app/models/container_expiration_policy.rb b/app/models/container_expiration_policy.rb index aecb47f7a030a3f4687ec3b23b81cfc51fe742de..857b7eccf70ae764d90d9d4b90a0aa7e55af6750 100644 --- a/app/models/container_expiration_policy.rb +++ b/app/models/container_expiration_policy.rb @@ -80,7 +80,11 @@ def self.older_than_options end def set_next_run_at - self.next_run_at = Time.zone.now + ChronicDuration.parse(cadence).seconds + cadence_seconds = ChronicDuration.parse( + cadence, use_complete_matcher: Feature.enabled?(:update_chronic_duration) + ).seconds + + self.next_run_at = Time.zone.now + cadence_seconds end def disable! diff --git a/app/services/projects/container_repository/cleanup_tags_base_service.rb b/app/services/projects/container_repository/cleanup_tags_base_service.rb index 45557d0350297376a1594f0a73b547557b002e3f..185895698aff9ce4cf57b19b9b6834018e8bbb21 100644 --- a/app/services/projects/container_repository/cleanup_tags_base_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_base_service.rb @@ -100,7 +100,9 @@ def keep_n_as_integer def older_than_in_seconds strong_memoize(:older_than_in_seconds) do - ChronicDuration.parse(older_than).seconds + ChronicDuration.parse( + older_than, use_complete_matcher: Feature.enabled?(:update_chronic_duration) + ).seconds end end end diff --git a/app/validators/duration_validator.rb b/app/validators/duration_validator.rb index defd28d7d3b32e17f38271625f824ae5ea72584f..5d7eba0912267c162b6224ad08c59080ddef8ff3 100644 --- a/app/validators/duration_validator.rb +++ b/app/validators/duration_validator.rb @@ -12,7 +12,7 @@ # class DurationValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - ChronicDuration.parse(value) + ChronicDuration.parse(value, use_complete_matcher: Feature.enabled?(:update_chronic_duration)) rescue ChronicDuration::DurationParseError if options[:message] record.errors.add(:base, options[:message]) diff --git a/config/feature_flags/development/update_chronic_duration.yml b/config/feature_flags/development/update_chronic_duration.yml new file mode 100644 index 0000000000000000000000000000000000000000..4af6529961987e35fde82ffd7b2fa3db31d73274 --- /dev/null +++ b/config/feature_flags/development/update_chronic_duration.yml @@ -0,0 +1,8 @@ +--- +name: update_chronic_duration +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/130531 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/423696 +milestone: '16.4' +type: development +group: group::pipeline authoring +default_enabled: false diff --git a/lib/gitlab/ci/build/duration_parser.rb b/lib/gitlab/ci/build/duration_parser.rb index 9385dccd5f328928b33b5b35562685a54cb6e3c0..bc365fe4e9f8636472466be3ff87c10ed2c1a9e5 100644 --- a/lib/gitlab/ci/build/duration_parser.rb +++ b/lib/gitlab/ci/build/duration_parser.rb @@ -41,7 +41,7 @@ def safe_parse def parse return if never? - ChronicDuration.parse(value) + ChronicDuration.parse(value, use_complete_matcher: Feature.enabled?(:update_chronic_duration)) end def validation_cache diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index bc0c8016934e1a6afe214bf2a9ca3e3eba483bf7..bf38e2aedafc9156d92bbb123b4224867d8225b6 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -160,7 +160,7 @@ def value retry: retry_defined? ? retry_value : nil, parallel: has_parallel? ? parallel_value : nil, interruptible: interruptible_defined? ? interruptible_value : nil, - timeout: has_timeout? ? ChronicDuration.parse(timeout.to_s) : nil, + timeout: parsed_timeout, artifacts: artifacts_value, release: release_value, after_script: after_script_value, @@ -174,6 +174,12 @@ def value ).compact end + def parsed_timeout + return unless has_timeout? + + ChronicDuration.parse(timeout.to_s, use_complete_matcher: Feature.enabled?(:update_chronic_duration)) + end + def ignored? allow_failure_defined? ? static_allow_failure : manual_action? end diff --git a/lib/gitlab/config/entry/legacy_validation_helpers.rb b/lib/gitlab/config/entry/legacy_validation_helpers.rb index 415f6f77214c1a2b689b6d3c6a547feba148759a..9363c60e6db6715e972183088e04da9239ec6e38 100644 --- a/lib/gitlab/config/entry/legacy_validation_helpers.rb +++ b/lib/gitlab/config/entry/legacy_validation_helpers.rb @@ -12,7 +12,7 @@ def validate_duration(value, parser = nil) if parser && parser.respond_to?(:validate_duration) parser.validate_duration(value) else - ChronicDuration.parse(value) + ChronicDuration.parse(value, use_complete_matcher: Feature.enabled?(:update_chronic_duration)) end rescue ChronicDuration::DurationParseError false @@ -24,8 +24,12 @@ def validate_duration_limit(value, limit, parser = nil) if parser && parser.respond_to?(:validate_duration_limit) parser.validate_duration_limit(value, limit) else - ChronicDuration.parse(value).second.from_now < - ChronicDuration.parse(limit).second.from_now + ChronicDuration.parse( + value, use_complete_matcher: Feature.enabled?(:update_chronic_duration) + ).second.from_now < + ChronicDuration.parse( + limit, use_complete_matcher: Feature.enabled?(:update_chronic_duration) + ).second.from_now end rescue ChronicDuration::DurationParseError false diff --git a/lib/gitlab/time_tracking_formatter.rb b/lib/gitlab/time_tracking_formatter.rb index 7094db14c5d5e28574c0bb9001c1815a75a305ae..a8f85e204263a351402a9fac625d75606cf4390b 100644 --- a/lib/gitlab/time_tracking_formatter.rb +++ b/lib/gitlab/time_tracking_formatter.rb @@ -15,7 +15,10 @@ def parse(string, keep_zero: false) begin ChronicDuration.parse( string, - CUSTOM_DAY_AND_MONTH_LENGTH.merge(default_unit: 'hours', keep_zero: keep_zero)) + CUSTOM_DAY_AND_MONTH_LENGTH.merge( + default_unit: 'hours', keep_zero: keep_zero, + use_complete_matcher: Feature.enabled?(:update_chronic_duration) + )) rescue StandardError nil end diff --git a/spec/lib/gitlab/ci/build/duration_parser_spec.rb b/spec/lib/gitlab/ci/build/duration_parser_spec.rb index 7f5ff1eb0ee2f5cd68af4f40950fe099608ac255..bc905aa0a35221d2b803a1e688ac59f94dd521cf 100644 --- a/spec/lib/gitlab/ci/build/duration_parser_spec.rb +++ b/spec/lib/gitlab/ci/build/duration_parser_spec.rb @@ -25,8 +25,8 @@ it { is_expected.to be_truthy } it 'caches data' do - expect(ChronicDuration).to receive(:parse).with(value).once.and_call_original - expect(ChronicDuration).to receive(:parse).with(other_value).once.and_call_original + expect(ChronicDuration).to receive(:parse).with(value, use_complete_matcher: true).once.and_call_original + expect(ChronicDuration).to receive(:parse).with(other_value, use_complete_matcher: true).once.and_call_original 2.times do expect(described_class.validate_duration(value)).to eq(86400) @@ -41,7 +41,7 @@ it { is_expected.to be_falsy } it 'caches data' do - expect(ChronicDuration).to receive(:parse).with(value).once.and_call_original + expect(ChronicDuration).to receive(:parse).with(value, use_complete_matcher: true).once.and_call_original 2.times do expect(described_class.validate_duration(value)).to be_falsey diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 1a78d92987151bd602dbc7c5e1e0f46d22ff31e1..df58877f1205eabc9ad40f842fdce5ef66aeb14b 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -426,6 +426,18 @@ expect(entry.errors).to be_empty expect(entry.timeout).to eq('1m 1s') end + + context 'when update_chronic_duration is disabled' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'returns correct timeout' do + expect(entry).to be_valid + expect(entry.errors).to be_empty + expect(entry.timeout).to eq('1m 1s') + end + end end context 'when it is a release' do diff --git a/spec/lib/gitlab/time_tracking_formatter_spec.rb b/spec/lib/gitlab/time_tracking_formatter_spec.rb index aa755d64a7a24480d7d13a92bc2df8a4acab72b7..978f6dfd2d920b1c715c3085da716a7c2e1eab56 100644 --- a/spec/lib/gitlab/time_tracking_formatter_spec.rb +++ b/spec/lib/gitlab/time_tracking_formatter_spec.rb @@ -12,12 +12,28 @@ let(:duration_string) { '3h 20m' } it { expect(subject).to eq(12_000) } + + context 'when update_chronic_duration is false' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it { expect(subject).to eq(12_000) } + end end context 'negative durations' do let(:duration_string) { '-3h 20m' } it { expect(subject).to eq(-12_000) } + + context 'when update_chronic_duration is false' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it { expect(subject).to eq(-12_000) } + end end context 'durations with months' do @@ -26,6 +42,16 @@ it 'uses our custom conversions' do expect(subject).to eq(576_000) end + + context 'when update_chronic_duration is false' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'uses our custom conversions' do + expect(subject).to eq(576_000) + end + end end context 'when the duration is zero' do @@ -35,6 +61,16 @@ it 'returns nil' do expect(subject).to be_nil end + + context 'when update_chronic_duration is false' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'returns nil' do + expect(subject).to be_nil + end + end end context 'when keep_zero is true' do @@ -43,6 +79,16 @@ it 'returns zero' do expect(subject).to eq(0) end + + context 'when update_chronic_duration is false' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'returns zero' do + expect(subject).to eq(0) + end + end end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index b95dec7195ffaca56eb39f31c819eccaf235eb3e..15f31d98d980500a6e3a5a69ba323c66d0db1645 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -735,6 +735,18 @@ is_expected.to eq(1.day.since) end end + + context 'when update_chronic_duration is disabled' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'returns date after 1 day' do + freeze_time do + is_expected.to eq(1.day.since) + end + end + end end context 'when start_in is 1 week' do @@ -745,6 +757,18 @@ is_expected.to eq(1.week.since) end end + + context 'when update_chronic_duration is disabled' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'returns date after 1 week' do + freeze_time do + is_expected.to eq(1.week.since) + end + end + end end end @@ -1075,6 +1099,18 @@ is_expected.to be_nil end + + context 'when update_chronic_duration is disabled' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'assigns a valid duration' do + build.artifacts_expire_in = '7 days' + + is_expected.to be_within(10).of(7.days.to_i) + end + end end describe '#commit' do diff --git a/spec/models/concerns/chronic_duration_attribute_spec.rb b/spec/models/concerns/chronic_duration_attribute_spec.rb index 61b86455840bbbcbbc9cebe15d05a1094be09e47..fe4a3523909506a6cc0a2e559adf2d7dee6d0ed2 100644 --- a/spec/models/concerns/chronic_duration_attribute_spec.rb +++ b/spec/models/concerns/chronic_duration_attribute_spec.rb @@ -39,6 +39,16 @@ expect(subject.valid?).to be_truthy end + context 'when update_chronic_duration is disabled' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'parses chronic duration input' do + expect(subject.send(source_field)).to eq(600) + end + end + context 'when negative input is used' do before do subject.send("#{source_field}=", 3600) @@ -72,6 +82,16 @@ it 'passes validation' do expect(subject.valid?).to be_truthy end + + context 'when update_chronic_duration is disabled' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'writes default value' do + expect(subject.send(source_field)).to eq(default_value) + end + end end context 'when nil input is used' do @@ -90,6 +110,16 @@ it "doesn't raise exception" do expect { subject.send("#{virtual_field}=", nil) }.not_to raise_error end + + context 'when update_chronic_duration is disabled' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it 'writes default value' do + expect(subject.send(source_field)).to eq(default_value) + end + end end end diff --git a/spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb b/spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb index f9f8435c211ca08ed48d5b633787662e4ef57b45..d55c99b9eb147c2c047dbc3f5b9a3287fd035fcc 100644 --- a/spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb +++ b/spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb @@ -153,6 +153,17 @@ service_response_extra: service_response_extra, supports_caching: supports_caching, delete_expectations: delete_expectations + + context 'when update_chronic_duration is disabled' do + before do + stub_feature_flags(update_chronic_duration: false) + end + + it_behaves_like 'removing the expected tags', + service_response_extra: service_response_extra, + supports_caching: supports_caching, + delete_expectations: delete_expectations + end end RSpec.shared_examples 'when combining all parameters' do