From 1e78b395bbc29cce9db1542df6b6c4489d47b23e Mon Sep 17 00:00:00 2001
From: Albert Salim <asalim@gitlab.com>
Date: Thu, 10 Feb 2022 11:51:26 +0000
Subject: [PATCH] Update namespace statistics when project is destroyed
When a project is destroyed, Projects::ProjectDeletedEvent is published.
A Namespaces::UpdateRootStatisticsWorker is subscribed to
Projects::ProjectDeletedEvent. The worker will
enqueue Namespaces::ScheduleAggregationWorker to
update the namespace's root statistics.
This change is behind a feature flag for publishing ProjectDeletedEvent.
Changelog: fixed
---
app/events/projects/project_deleted_event.rb | 16 +++++++++
app/services/projects/destroy_service.rb | 8 +++++
app/workers/all_queues.yml | 9 +++++
.../update_root_statistics_worker.rb | 17 ++++++++++
.../publish_project_deleted_event.yml | 8 +++++
config/sidekiq_queues.yml | 2 ++
lib/gitlab/event_store.rb | 1 +
.../projects/project_deleted_event_spec.rb | 34 +++++++++++++++++++
.../services/projects/destroy_service_spec.rb | 26 +++++++++++---
spec/support/matchers/event_store.rb | 12 +++++++
spec/tooling/quality/test_level_spec.rb | 4 +--
spec/workers/every_sidekiq_worker_spec.rb | 1 +
.../update_root_statistics_worker_spec.rb | 23 +++++++++++++
tooling/quality/test_level.rb | 1 +
14 files changed, 156 insertions(+), 6 deletions(-)
create mode 100644 app/events/projects/project_deleted_event.rb
create mode 100644 app/workers/namespaces/update_root_statistics_worker.rb
create mode 100644 config/feature_flags/development/publish_project_deleted_event.yml
create mode 100644 spec/events/projects/project_deleted_event_spec.rb
create mode 100644 spec/support/matchers/event_store.rb
create mode 100644 spec/workers/namespaces/update_root_statistics_worker_spec.rb
diff --git a/app/events/projects/project_deleted_event.rb b/app/events/projects/project_deleted_event.rb
new file mode 100644
index 000000000000..ac58c5c6755d
--- /dev/null
+++ b/app/events/projects/project_deleted_event.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module Projects
+ class ProjectDeletedEvent < ::Gitlab::EventStore::Event
+ def schema
+ {
+ 'type' => 'object',
+ 'properties' => {
+ 'project_id' => { 'type' => 'integer' },
+ 'namespace_id' => { 'type' => 'integer' }
+ },
+ 'required' => %w[project_id namespace_id]
+ }
+ end
+ end
+end
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 733a4b45cb2d..95af5a6863ff 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -37,6 +37,8 @@ def execute
system_hook_service.execute_hooks_for(project, :destroy)
log_info("Project \"#{project.full_path}\" was deleted")
+ publish_project_deleted_event_for(project) if Feature.enabled?(:publish_project_deleted_event, default_enabled: :yaml)
+
current_user.invalidate_personal_projects_count
true
@@ -260,6 +262,12 @@ def raise_error(message)
def flush_caches(project)
Projects::ForksCountService.new(project).delete_cache
end
+
+ def publish_project_deleted_event_for(project)
+ data = { project_id: project.id, namespace_id: project.namespace_id }
+ event = Projects::ProjectDeletedEvent.new(data: data)
+ Gitlab::EventStore.publish(event)
+ end
end
end
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 18646e5320b5..f39aee14a69a 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -2569,6 +2569,15 @@
:weight: 1
:idempotent: true
:tags: []
+- :name: namespaces_update_root_statistics
+ :worker_name: Namespaces::UpdateRootStatisticsWorker
+ :feature_category: :source_code_management
+ :has_external_dependencies:
+ :urgency: :low
+ :resource_boundary: :unknown
+ :weight: 1
+ :idempotent: true
+ :tags: []
- :name: new_issue
:worker_name: NewIssueWorker
:feature_category: :team_planning
diff --git a/app/workers/namespaces/update_root_statistics_worker.rb b/app/workers/namespaces/update_root_statistics_worker.rb
new file mode 100644
index 000000000000..9fdf8e2506b4
--- /dev/null
+++ b/app/workers/namespaces/update_root_statistics_worker.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module Namespaces
+ class UpdateRootStatisticsWorker
+ include Gitlab::EventStore::Subscriber
+
+ data_consistency :always
+
+ idempotent!
+
+ feature_category :source_code_management
+
+ def handle_event(event)
+ ScheduleAggregationWorker.perform_async(event.data[:namespace_id])
+ end
+ end
+end
diff --git a/config/feature_flags/development/publish_project_deleted_event.yml b/config/feature_flags/development/publish_project_deleted_event.yml
new file mode 100644
index 000000000000..1287ebe9f662
--- /dev/null
+++ b/config/feature_flags/development/publish_project_deleted_event.yml
@@ -0,0 +1,8 @@
+---
+name: publish_project_deleted_event
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78862
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351073
+milestone: '14.8'
+type: development
+group: group::pipeline insights
+default_enabled: false
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index d52659d80e76..9c5294b6b2b8 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -295,6 +295,8 @@
- 1
- - namespaces_sync_namespace_name
- 1
+- - namespaces_update_root_statistics
+ - 1
- - new_epic
- 2
- - new_issue
diff --git a/lib/gitlab/event_store.rb b/lib/gitlab/event_store.rb
index 7dbbcdbb1a78..e20ea1c73650 100644
--- a/lib/gitlab/event_store.rb
+++ b/lib/gitlab/event_store.rb
@@ -34,6 +34,7 @@ def self.configure!(store)
# Add subscriptions here:
store.subscribe ::MergeRequests::UpdateHeadPipelineWorker, to: ::Ci::PipelineCreatedEvent
+ store.subscribe ::Namespaces::UpdateRootStatisticsWorker, to: ::Projects::ProjectDeletedEvent
end
private_class_method :configure!
end
diff --git a/spec/events/projects/project_deleted_event_spec.rb b/spec/events/projects/project_deleted_event_spec.rb
new file mode 100644
index 000000000000..fd8cec7271b5
--- /dev/null
+++ b/spec/events/projects/project_deleted_event_spec.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::ProjectDeletedEvent do
+ where(:data, :valid) do
+ [
+ [{ project_id: 1, namespace_id: 2 }, true],
+ [{ project_id: 1 }, false],
+ [{ namespace_id: 1 }, false],
+ [{ project_id: 'foo', namespace_id: 2 }, false],
+ [{ project_id: 1, namespace_id: 'foo' }, false],
+ [{ project_id: [], namespace_id: 2 }, false],
+ [{ project_id: 1, namespace_id: [] }, false],
+ [{ project_id: {}, namespace_id: 2 }, false],
+ [{ project_id: 1, namespace_id: {} }, false],
+ ['foo', false],
+ [123, false],
+ [[], false]
+ ]
+ end
+
+ with_them do
+ it 'validates data' do
+ constructor = -> { described_class.new(data: data) }
+
+ if valid
+ expect { constructor.call }.not_to raise_error
+ else
+ expect { constructor.call }.to raise_error(Gitlab::EventStore::InvalidEvent)
+ end
+ end
+ end
+end
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index a6aa76c7e6ce..0beb5157ed6b 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -18,17 +18,35 @@
end
shared_examples 'deleting the project' do
- before do
- # Run sidekiq immediately to check that renamed repository will be removed
+ it 'deletes the project', :sidekiq_inline do
destroy_project(project, user, {})
- end
- it 'deletes the project', :sidekiq_inline do
expect(Project.unscoped.all).not_to include(project)
expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey
expect(project.gitlab_shell.repository_exists?(project.repository_storage, remove_path + '.git')).to be_falsey
end
+
+ it 'publishes a ProjectDeleted event with project id and namespace id' do
+ expected_data = { project_id: project.id, namespace_id: project.namespace_id }
+ expect(Gitlab::EventStore)
+ .to receive(:publish)
+ .with(event_type(Projects::ProjectDeletedEvent).containing(expected_data))
+
+ destroy_project(project, user, {})
+ end
+
+ context 'when feature flag publish_project_deleted_event is disabled' do
+ before do
+ stub_feature_flags(publish_project_deleted_event: false)
+ end
+
+ it 'does not publish an event' do
+ expect(Gitlab::EventStore).not_to receive(:publish)
+
+ destroy_project(project, user, {})
+ end
+ end
end
shared_examples 'deleting the project with pipeline and build' do
diff --git a/spec/support/matchers/event_store.rb b/spec/support/matchers/event_store.rb
new file mode 100644
index 000000000000..96a71ae3c22f
--- /dev/null
+++ b/spec/support/matchers/event_store.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+RSpec::Matchers.define :event_type do |event_class|
+ match do |actual|
+ actual.instance_of?(event_class) &&
+ actual.data == @expected_data
+ end
+
+ chain :containing do |expected_data|
+ @expected_data = expected_data
+ end
+end
diff --git a/spec/tooling/quality/test_level_spec.rb b/spec/tooling/quality/test_level_spec.rb
index 8a944a473d73..33d3a5b49b31 100644
--- a/spec/tooling/quality/test_level_spec.rb
+++ b/spec/tooling/quality/test_level_spec.rb
@@ -28,7 +28,7 @@
context 'when level is unit' do
it 'returns a pattern' do
expect(subject.pattern(:unit))
- .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb")
+ .to eq("spec/{bin,channels,config,db,dependencies,elastic,elastic_integration,experiments,events,factories,finders,frontend,graphql,haml_lint,helpers,initializers,javascripts,lib,metrics_server,models,policies,presenters,rack_servers,replicators,routing,rubocop,scripts,serializers,services,sidekiq,sidekiq_cluster,spam,support_specs,tasks,uploaders,validators,views,workers,tooling}{,/**/}*_spec.rb")
end
end
@@ -110,7 +110,7 @@
context 'when level is unit' do
it 'returns a regexp' do
expect(subject.regexp(:unit))
- .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)})
+ .to eq(%r{spec/(bin|channels|config|db|dependencies|elastic|elastic_integration|experiments|events|factories|finders|frontend|graphql|haml_lint|helpers|initializers|javascripts|lib|metrics_server|models|policies|presenters|rack_servers|replicators|routing|rubocop|scripts|serializers|services|sidekiq|sidekiq_cluster|spam|support_specs|tasks|uploaders|validators|views|workers|tooling)})
end
end
diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb
index a21cf3ed5f65..1cd5d23d8fc9 100644
--- a/spec/workers/every_sidekiq_worker_spec.rb
+++ b/spec/workers/every_sidekiq_worker_spec.rb
@@ -349,6 +349,7 @@
'Namespaces::OnboardingPipelineCreatedWorker' => 3,
'Namespaces::OnboardingProgressWorker' => 3,
'Namespaces::OnboardingUserAddedWorker' => 3,
+ 'Namespaces::RefreshRootStatisticsWorker' => 3,
'Namespaces::RootStatisticsWorker' => 3,
'Namespaces::ScheduleAggregationWorker' => 3,
'NetworkPolicyMetricsWorker' => 3,
diff --git a/spec/workers/namespaces/update_root_statistics_worker_spec.rb b/spec/workers/namespaces/update_root_statistics_worker_spec.rb
new file mode 100644
index 000000000000..a525904b7578
--- /dev/null
+++ b/spec/workers/namespaces/update_root_statistics_worker_spec.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Namespaces::UpdateRootStatisticsWorker do
+ let(:namespace_id) { 123 }
+
+ let(:event) do
+ Projects::ProjectDeletedEvent.new(data: { project_id: 1, namespace_id: namespace_id })
+ end
+
+ subject { consume_event(event) }
+
+ def consume_event(event)
+ described_class.new.perform(event.class.name, event.data)
+ end
+
+ it 'enqueues ScheduleAggregationWorker' do
+ expect(Namespaces::ScheduleAggregationWorker).to receive(:perform_async).with(namespace_id)
+
+ subject
+ end
+end
diff --git a/tooling/quality/test_level.rb b/tooling/quality/test_level.rb
index 50cbc69beb21..624564ecd059 100644
--- a/tooling/quality/test_level.rb
+++ b/tooling/quality/test_level.rb
@@ -24,6 +24,7 @@ class TestLevel
elastic
elastic_integration
experiments
+ events
factories
finders
frontend
--
GitLab