From b989445a6e86f4756c9c7ed628fbb2e1adbe4718 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 10 Sep 2022 00:09:59 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .rubocop_todo/gitlab/namespaced_class.yml | 1 - .../repositories/git_http_controller.rb | 2 +- app/services/issues/close_service.rb | 2 +- .../merge_requests/after_create_service.rb | 2 +- app/services/onboarding/progress_service.rb | 33 ++++++++ app/services/onboarding_progress_service.rb | 31 -------- app/services/post_receive_service.rb | 2 +- .../namespaces/onboarding_issue_created_worker.rb | 2 +- .../onboarding_pipeline_created_worker.rb | 2 +- .../namespaces/onboarding_progress_worker.rb | 2 +- .../namespaces/onboarding_user_added_worker.rb | 2 +- doc/administration/environment_variables.md | 1 + .../background_migration/batched_migration.rb | 4 +- lib/gitlab/redis/cache.rb | 2 +- .../background_migration/batched_migration_spec.rb | 44 +++++++++++ spec/lib/gitlab/redis/cache_spec.rb | 4 +- spec/services/onboarding/progress_service_spec.rb | 89 ++++++++++++++++++++++ spec/services/onboarding_progress_service_spec.rb | 89 ---------------------- spec/support/rspec_order_todo.yml | 1 - .../onboarding_progress_shared_examples.rb | 4 +- 20 files changed, 182 insertions(+), 137 deletions(-) create mode 100644 app/services/onboarding/progress_service.rb delete mode 100644 app/services/onboarding_progress_service.rb create mode 100644 spec/services/onboarding/progress_service_spec.rb delete mode 100644 spec/services/onboarding_progress_service_spec.rb diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index 79212670488..ae600f6af32 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -655,7 +655,6 @@ Gitlab/NamespacedClass: - 'app/services/metrics_service.rb' - 'app/services/note_summary.rb' - 'app/services/notification_service.rb' - - 'app/services/onboarding_progress_service.rb' - 'app/services/post_receive_service.rb' - 'app/services/preview_markdown_service.rb' - 'app/services/push_event_payload_service.rb' diff --git a/app/controllers/repositories/git_http_controller.rb b/app/controllers/repositories/git_http_controller.rb index c3c6a51239d..144ec4c0de9 100644 --- a/app/controllers/repositories/git_http_controller.rb +++ b/app/controllers/repositories/git_http_controller.rb @@ -83,7 +83,7 @@ module Repositories return if Gitlab::Database.read_only? return unless repo_type.project? - OnboardingProgressService.async(project.namespace_id).execute(action: :git_pull) + Onboarding::ProgressService.async(project.namespace_id).execute(action: :git_pull) return if Feature.enabled?(:disable_git_http_fetch_writes) diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 1799047cbdf..da888386e0a 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -40,7 +40,7 @@ module Issues if closed_via.is_a?(MergeRequest) store_first_mentioned_in_commit_at(issue, closed_via) - OnboardingProgressService.new(project.namespace).execute(action: :issue_auto_closed) + Onboarding::ProgressService.new(project.namespace).execute(action: :issue_auto_closed) end delete_milestone_closed_issue_counter_cache(issue.milestone) diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index 93a0d375b97..9d12eb80eb6 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -28,7 +28,7 @@ module MergeRequests merge_request.diffs(include_stats: false).write_cache merge_request.create_cross_references!(current_user) - OnboardingProgressService.new(merge_request.target_project.namespace).execute(action: :merge_request_created) + Onboarding::ProgressService.new(merge_request.target_project.namespace).execute(action: :merge_request_created) todo_service.new_merge_request(merge_request, current_user) merge_request.cache_merge_request_closes_issues!(current_user) diff --git a/app/services/onboarding/progress_service.rb b/app/services/onboarding/progress_service.rb new file mode 100644 index 00000000000..66f7f2bc33d --- /dev/null +++ b/app/services/onboarding/progress_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Onboarding + class ProgressService + class Async + attr_reader :namespace_id + + def initialize(namespace_id) + @namespace_id = namespace_id + end + + def execute(action:) + return unless Onboarding::Progress.not_completed?(namespace_id, action) + + Namespaces::OnboardingProgressWorker.perform_async(namespace_id, action) + end + end + + def self.async(namespace_id) + Async.new(namespace_id) + end + + def initialize(namespace) + @namespace = namespace&.root_ancestor + end + + def execute(action:) + return unless @namespace + + Onboarding::Progress.register(@namespace, action) + end + end +end diff --git a/app/services/onboarding_progress_service.rb b/app/services/onboarding_progress_service.rb deleted file mode 100644 index 7ba7fbc5b7f..00000000000 --- a/app/services/onboarding_progress_service.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -class OnboardingProgressService - class Async - attr_reader :namespace_id - - def initialize(namespace_id) - @namespace_id = namespace_id - end - - def execute(action:) - return unless Onboarding::Progress.not_completed?(namespace_id, action) - - Namespaces::OnboardingProgressWorker.perform_async(namespace_id, action) - end - end - - def self.async(namespace_id) - Async.new(namespace_id) - end - - def initialize(namespace) - @namespace = namespace&.root_ancestor - end - - def execute(action:) - return unless @namespace - - Onboarding::Progress.register(@namespace, action) - end -end diff --git a/app/services/post_receive_service.rb b/app/services/post_receive_service.rb index 15c978e6763..c376b4036f8 100644 --- a/app/services/post_receive_service.rb +++ b/app/services/post_receive_service.rb @@ -101,7 +101,7 @@ class PostReceiveService def record_onboarding_progress return unless project - OnboardingProgressService.new(project.namespace).execute(action: :git_write) + Onboarding::ProgressService.new(project.namespace).execute(action: :git_write) end end diff --git a/app/workers/namespaces/onboarding_issue_created_worker.rb b/app/workers/namespaces/onboarding_issue_created_worker.rb index aab5767e0f1..4f0cc71cd91 100644 --- a/app/workers/namespaces/onboarding_issue_created_worker.rb +++ b/app/workers/namespaces/onboarding_issue_created_worker.rb @@ -18,7 +18,7 @@ module Namespaces namespace = Namespace.find_by_id(namespace_id) return unless namespace - OnboardingProgressService.new(namespace).execute(action: :issue_created) + Onboarding::ProgressService.new(namespace).execute(action: :issue_created) end end end diff --git a/app/workers/namespaces/onboarding_pipeline_created_worker.rb b/app/workers/namespaces/onboarding_pipeline_created_worker.rb index 4172e286474..c3850880df0 100644 --- a/app/workers/namespaces/onboarding_pipeline_created_worker.rb +++ b/app/workers/namespaces/onboarding_pipeline_created_worker.rb @@ -18,7 +18,7 @@ module Namespaces namespace = Namespace.find_by_id(namespace_id) return unless namespace - OnboardingProgressService.new(namespace).execute(action: :pipeline_created) + Onboarding::ProgressService.new(namespace).execute(action: :pipeline_created) end end end diff --git a/app/workers/namespaces/onboarding_progress_worker.rb b/app/workers/namespaces/onboarding_progress_worker.rb index 77a31d85a9a..49629428459 100644 --- a/app/workers/namespaces/onboarding_progress_worker.rb +++ b/app/workers/namespaces/onboarding_progress_worker.rb @@ -19,7 +19,7 @@ module Namespaces namespace = Namespace.find_by_id(namespace_id) return unless namespace && action - OnboardingProgressService.new(namespace).execute(action: action.to_sym) + Onboarding::ProgressService.new(namespace).execute(action: action.to_sym) end end end diff --git a/app/workers/namespaces/onboarding_user_added_worker.rb b/app/workers/namespaces/onboarding_user_added_worker.rb index 4d17cf9a6e2..a1b349eedd3 100644 --- a/app/workers/namespaces/onboarding_user_added_worker.rb +++ b/app/workers/namespaces/onboarding_user_added_worker.rb @@ -15,7 +15,7 @@ module Namespaces def perform(namespace_id) namespace = Namespace.find(namespace_id) - OnboardingProgressService.new(namespace).execute(action: :user_added) + Onboarding::ProgressService.new(namespace).execute(action: :user_added) end end end diff --git a/doc/administration/environment_variables.md b/doc/administration/environment_variables.md index 3adb057daa4..e70758e2774 100644 --- a/doc/administration/environment_variables.md +++ b/doc/administration/environment_variables.md @@ -36,6 +36,7 @@ You can use the following environment variables to override certain values: | `GITLAB_SHARED_RUNNERS_REGISTRATION_TOKEN` | string | Sets the initial registration token used for runners. | | `RAILS_ENV` | string | The Rails environment; can be one of `production`, `development`, `staging`, or `test`. | | `UNSTRUCTURED_RAILS_LOG` | string | Enables the unstructured log in addition to JSON logs (defaults to `true`). | +| `GITLAB_RAILS_CACHE_DEFAULT_TTL_SECONDS` | integer | The default TTL used for entries stored in the Rails-cache. Default is `28800`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/95042) in 15.3. | ## Adding more variables diff --git a/lib/gitlab/database/background_migration/batched_migration.rb b/lib/gitlab/database/background_migration/batched_migration.rb index 6f95dfebe60..45f52765d0f 100644 --- a/lib/gitlab/database/background_migration/batched_migration.rb +++ b/lib/gitlab/database/background_migration/batched_migration.rb @@ -59,11 +59,11 @@ module Gitlab state :finalizing, value: 5 event :pause do - transition any => :paused + transition [:active, :paused] => :paused end event :execute do - transition any => :active + transition [:active, :paused, :failed] => :active end event :finish do diff --git a/lib/gitlab/redis/cache.rb b/lib/gitlab/redis/cache.rb index 4ab1024d528..043f14630d5 100644 --- a/lib/gitlab/redis/cache.rb +++ b/lib/gitlab/redis/cache.rb @@ -12,7 +12,7 @@ module Gitlab redis: pool, compress: Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_REDIS_CACHE_COMPRESSION', '1')), namespace: CACHE_NAMESPACE, - expires_in: ENV.fetch('GITLAB_RAILS_CACHE_DEFAULT_TTL_SECONDS', 2.weeks).to_i # Cache should not grow forever + expires_in: ENV.fetch('GITLAB_RAILS_CACHE_DEFAULT_TTL_SECONDS', 8.hours).to_i # Cache should not grow forever } end end diff --git a/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb b/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb index 87041db7769..3daed2508a2 100644 --- a/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_migration_spec.rb @@ -59,6 +59,50 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigration, type: :m end end + describe '#pause!' do + context 'when an invalid transition is applied' do + %i[finished failed finalizing].each do |state| + it 'raises an exception' do + batched_migration = create(:batched_background_migration, state) + + expect { batched_migration.pause! }.to raise_error(StateMachines::InvalidTransition, /Cannot transition status/) + end + end + end + + context 'when a valid transition is applied' do + %i[active paused].each do |state| + it 'moves to pause' do + batched_migration = create(:batched_background_migration, state) + + expect(batched_migration.pause!).to be_truthy + end + end + end + end + + describe '#execute!' do + context 'when an invalid transition is applied' do + %i[finished finalizing].each do |state| + it 'raises an exception' do + batched_migration = create(:batched_background_migration, state) + + expect { batched_migration.execute! }.to raise_error(StateMachines::InvalidTransition, /Cannot transition status/) + end + end + end + + context 'when a valid transition is applied' do + %i[active paused failed].each do |state| + it 'moves to active' do + batched_migration = create(:batched_background_migration, state) + + expect(batched_migration.execute!).to be_truthy + end + end + end + end + describe '.valid_status' do valid_status = [:paused, :active, :finished, :failed, :finalizing] diff --git a/spec/lib/gitlab/redis/cache_spec.rb b/spec/lib/gitlab/redis/cache_spec.rb index 1f0ebbe107f..82ff8a26199 100644 --- a/spec/lib/gitlab/redis/cache_spec.rb +++ b/spec/lib/gitlab/redis/cache_spec.rb @@ -17,8 +17,8 @@ RSpec.describe Gitlab::Redis::Cache do end describe '.active_support_config' do - it 'has a default ttl of 2 weeks' do - expect(described_class.active_support_config[:expires_in]).to eq(2.weeks) + it 'has a default ttl of 8 hours' do + expect(described_class.active_support_config[:expires_in]).to eq(8.hours) end it 'allows configuring the TTL through an env variable' do diff --git a/spec/services/onboarding/progress_service_spec.rb b/spec/services/onboarding/progress_service_spec.rb new file mode 100644 index 00000000000..e9b8ea2e859 --- /dev/null +++ b/spec/services/onboarding/progress_service_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Onboarding::ProgressService do + describe '.async' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:action) { :git_pull } + + subject(:execute_service) { described_class.async(namespace.id).execute(action: action) } + + context 'when not onboarded' do + it 'does not schedule a worker' do + expect(Namespaces::OnboardingProgressWorker).not_to receive(:perform_async) + + execute_service + end + end + + context 'when onboarded' do + before do + Onboarding::Progress.onboard(namespace) + end + + context 'when action is already completed' do + before do + Onboarding::Progress.register(namespace, action) + end + + it 'does not schedule a worker' do + expect(Namespaces::OnboardingProgressWorker).not_to receive(:perform_async) + + execute_service + end + end + + context 'when action is not yet completed' do + it 'schedules a worker' do + expect(Namespaces::OnboardingProgressWorker).to receive(:perform_async) + + execute_service + end + end + end + end + + describe '#execute' do + let(:namespace) { create(:namespace) } + let(:action) { :namespace_action } + + subject(:execute_service) { described_class.new(namespace).execute(action: :subscription_created) } + + context 'when the namespace is a root' do + before do + Onboarding::Progress.onboard(namespace) + end + + it 'registers a namespace onboarding progress action for the given namespace' do + execute_service + + expect(Onboarding::Progress.completed?(namespace, :subscription_created)).to eq(true) + end + end + + context 'when the namespace is not the root' do + let(:group) { create(:group, :nested) } + + before do + Onboarding::Progress.onboard(group) + end + + it 'does not register a namespace onboarding progress action' do + execute_service + + expect(Onboarding::Progress.completed?(group, :subscription_created)).to be(nil) + end + end + + context 'when no namespace is passed' do + let(:namespace) { nil } + + it 'does not register a namespace onboarding progress action' do + execute_service + + expect(Onboarding::Progress.completed?(namespace, :subscription_created)).to be(nil) + end + end + end +end diff --git a/spec/services/onboarding_progress_service_spec.rb b/spec/services/onboarding_progress_service_spec.rb deleted file mode 100644 index 6ff6b03d808..00000000000 --- a/spec/services/onboarding_progress_service_spec.rb +++ /dev/null @@ -1,89 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe OnboardingProgressService do - describe '.async' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:action) { :git_pull } - - subject(:execute_service) { described_class.async(namespace.id).execute(action: action) } - - context 'when not onboarded' do - it 'does not schedule a worker' do - expect(Namespaces::OnboardingProgressWorker).not_to receive(:perform_async) - - execute_service - end - end - - context 'when onboarded' do - before do - Onboarding::Progress.onboard(namespace) - end - - context 'when action is already completed' do - before do - Onboarding::Progress.register(namespace, action) - end - - it 'does not schedule a worker' do - expect(Namespaces::OnboardingProgressWorker).not_to receive(:perform_async) - - execute_service - end - end - - context 'when action is not yet completed' do - it 'schedules a worker' do - expect(Namespaces::OnboardingProgressWorker).to receive(:perform_async) - - execute_service - end - end - end - end - - describe '#execute' do - let(:namespace) { create(:namespace) } - let(:action) { :namespace_action } - - subject(:execute_service) { described_class.new(namespace).execute(action: :subscription_created) } - - context 'when the namespace is a root' do - before do - Onboarding::Progress.onboard(namespace) - end - - it 'registers a namespace onboarding progress action for the given namespace' do - execute_service - - expect(Onboarding::Progress.completed?(namespace, :subscription_created)).to eq(true) - end - end - - context 'when the namespace is not the root' do - let(:group) { create(:group, :nested) } - - before do - Onboarding::Progress.onboard(group) - end - - it 'does not register a namespace onboarding progress action' do - execute_service - - expect(Onboarding::Progress.completed?(group, :subscription_created)).to be(nil) - end - end - - context 'when no namespace is passed' do - let(:namespace) { nil } - - it 'does not register a namespace onboarding progress action' do - execute_service - - expect(Onboarding::Progress.completed?(namespace, :subscription_created)).to be(nil) - end - end - end -end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 9d54e746d63..208c02c028e 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -10195,7 +10195,6 @@ - './spec/services/notification_recipients/builder/new_note_spec.rb' - './spec/services/notification_recipients/build_service_spec.rb' - './spec/services/notification_service_spec.rb' -- './spec/services/onboarding_progress_service_spec.rb' - './spec/services/packages/cleanup/execute_policy_service_spec.rb' - './spec/services/packages/cleanup/update_policy_service_spec.rb' - './spec/services/packages/composer/composer_json_service_spec.rb' diff --git a/spec/support/shared_examples/services/onboarding_progress_shared_examples.rb b/spec/support/shared_examples/services/onboarding_progress_shared_examples.rb index 8c6c2271af3..07025dac689 100644 --- a/spec/support/shared_examples/services/onboarding_progress_shared_examples.rb +++ b/spec/support/shared_examples/services/onboarding_progress_shared_examples.rb @@ -4,7 +4,7 @@ RSpec.shared_examples 'records an onboarding progress action' do |action| include AfterNextHelpers it do - expect_next(OnboardingProgressService, namespace) + expect_next(Onboarding::ProgressService, namespace) .to receive(:execute).with(action: action).and_call_original subject @@ -13,7 +13,7 @@ end RSpec.shared_examples 'does not record an onboarding progress action' do it do - expect(OnboardingProgressService).not_to receive(:new) + expect(Onboarding::ProgressService).not_to receive(:new) subject end -- cgit v1.2.1