From 58a401061d958881c747751f49e15a012035ab10 Mon Sep 17 00:00:00 2001 From: Arpit Gogia <12347103-arpitgogia@users.noreply.gitlab.com> Date: Wed, 15 Jan 2025 13:45:13 +0530 Subject: [PATCH 1/2] Update vulnerability_statistic.archived when project is archived Changelog: added EE: true --- ...ved_of_vulnerability_statistics_service.rb | 40 +++++++++ .../process_archived_events_worker.rb | 1 + ...f_vulnerability_statistics_service_spec.rb | 90 +++++++++++++++++++ .../process_archived_events_worker_spec.rb | 9 ++ 4 files changed, 140 insertions(+) create mode 100644 ee/app/services/vulnerabilities/update_archived_of_vulnerability_statistics_service.rb create mode 100644 ee/spec/services/vulnerabilities/update_archived_of_vulnerability_statistics_service_spec.rb diff --git a/ee/app/services/vulnerabilities/update_archived_of_vulnerability_statistics_service.rb b/ee/app/services/vulnerabilities/update_archived_of_vulnerability_statistics_service.rb new file mode 100644 index 00000000000000..b0f9043ba085b7 --- /dev/null +++ b/ee/app/services/vulnerabilities/update_archived_of_vulnerability_statistics_service.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Vulnerabilities + class UpdateArchivedOfVulnerabilityStatisticsService + include Gitlab::ExclusiveLeaseHelpers + + LEASE_TTL = 5.minutes + LEASE_TRY_AFTER = 3.seconds + + def self.execute(project_id) + new(project_id).execute + end + + def initialize(project_id) + @project_id = project_id + end + + def execute + return unless project + + in_lock(lease_key, ttl: LEASE_TTL, sleep_sec: LEASE_TRY_AFTER) { update_vulnerability_statistic } + end + + private + + attr_reader :project_id + + def lease_key + "update_vulnerability_statistic_archived:projects:#{project_id}" + end + + def project + @project ||= Project.find_by_id(project_id) + end + + def update_vulnerability_statistic + project.vulnerability_statistic.update(archived: project.archived) + end + end +end diff --git a/ee/app/workers/vulnerabilities/process_archived_events_worker.rb b/ee/app/workers/vulnerabilities/process_archived_events_worker.rb index 726fae7fb020bc..8dbf1eea6bd179 100644 --- a/ee/app/workers/vulnerabilities/process_archived_events_worker.rb +++ b/ee/app/workers/vulnerabilities/process_archived_events_worker.rb @@ -22,6 +22,7 @@ def handle_event(event) return unless project_setting Vulnerabilities::UpdateArchivedOfVulnerabilityReadsService.execute(project_setting.project_id) + Vulnerabilities::UpdateArchivedOfVulnerabilityStatisticsService.execute(project_setting.project_id) end end end diff --git a/ee/spec/services/vulnerabilities/update_archived_of_vulnerability_statistics_service_spec.rb b/ee/spec/services/vulnerabilities/update_archived_of_vulnerability_statistics_service_spec.rb new file mode 100644 index 00000000000000..b0f999cc211ead --- /dev/null +++ b/ee/spec/services/vulnerabilities/update_archived_of_vulnerability_statistics_service_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Vulnerabilities::UpdateArchivedOfVulnerabilityStatisticsService, feature_category: :vulnerability_management do + describe '.execute' do + it 'instantiates a new service object and calls execute' do + expect_next_instance_of(described_class, :project_id) do |instance| + expect(instance).to receive(:execute) + end + + described_class.execute(:project_id) + end + end + + describe '#execute' do + let(:service_object) { described_class.new(project_id) } + + subject(:update_archived) { service_object.execute } + + context 'when a project is not found for the given id' do + let(:project_id) { 0 } + + it 'does not raise an error' do + expect { update_archived }.not_to raise_error + end + end + + context 'when a project is found for the given id' do + let(:project_id) { project.id } + + let_it_be_with_reload(:project) { create(:project) } + let!(:vulnerability_statistic) { create(:vulnerability_statistic, project: project) } + + context 'when the project is archived' do + before do + project.update!(archived: true) + end + + it 'sets the vulnerability statistic record to also be archived' do + expect { update_archived } + .to change { vulnerability_statistic.reload.archived }.from(false).to(true) + end + end + + context 'when the project is unarchived' do + before do + vulnerability_statistic.update!(archived: true) + project.update!(archived: false) + end + + it 'sets the vulnerability statistic record to also not be archived' do + expect { update_archived } + .to change { vulnerability_statistic.reload.archived }.from(true).to(false) + end + end + + context 'when the project and vulnerability archived state mismatch' do + before do + vulnerability_statistic.update!(archived: true) + project.update!(archived: true) + end + + it 'sets the vulnerability statistic record to match the state of the project' do + expect { update_archived } + .not_to change { vulnerability_statistic.reload.archived } + end + end + + describe 'parallel execution' do + include ExclusiveLeaseHelpers + + context 'when the same lease key has already been taken by an already running job' do + let(:lease_key) { "update_vulnerability_statistic_archived:projects:#{project_id}" } + let(:lease_ttl) { 5.minutes } + + before do + stub_const("#{described_class}::LEASE_TRY_AFTER", 0.01) + stub_exclusive_lease_taken(lease_key, timeout: lease_ttl) + end + + it 'does not permit parallel execution of the logic' do + expect { update_archived }.to raise_error(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError) + .and not_change { vulnerability_statistic.reload.archived }.from(false) + end + end + end + end + end +end diff --git a/ee/spec/workers/vulnerabilities/process_archived_events_worker_spec.rb b/ee/spec/workers/vulnerabilities/process_archived_events_worker_spec.rb index 943192040a0a7e..7c43a7eef2b03a 100644 --- a/ee/spec/workers/vulnerabilities/process_archived_events_worker_spec.rb +++ b/ee/spec/workers/vulnerabilities/process_archived_events_worker_spec.rb @@ -6,6 +6,7 @@ let_it_be(:old_group) { create(:group) } let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :with_vulnerability, group: group) } + let_it_be(:vulnerability_statistic) { create(:vulnerability_statistic, project: project) } let_it_be(:project_without_vulnerabilities) { create(:project, group: group) } let(:event) do @@ -34,6 +35,14 @@ use_event end + + it 'enqueues a vulnerability statistics namespace id update job for the project id' do + expect(Vulnerabilities::UpdateArchivedOfVulnerabilityStatisticsService).to receive(:execute).with( + project.id + ) + + use_event + end end context 'when the associated project does not have vulnerabilities' do -- GitLab From 7ace2d2de3c806fff02973eb76730dfa3648c93d Mon Sep 17 00:00:00 2001 From: Arpit Gogia <12347103-arpitgogia@users.noreply.gitlab.com> Date: Thu, 16 Jan 2025 16:29:09 +0000 Subject: [PATCH 2/2] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Gregory Havenga <11164960-ghavenga@users.noreply.gitlab.com> --- .../workers/vulnerabilities/process_archived_events_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/workers/vulnerabilities/process_archived_events_worker.rb b/ee/app/workers/vulnerabilities/process_archived_events_worker.rb index 8dbf1eea6bd179..db72ae5f3f198e 100644 --- a/ee/app/workers/vulnerabilities/process_archived_events_worker.rb +++ b/ee/app/workers/vulnerabilities/process_archived_events_worker.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Vulnerabilities - # Ingest archived events to enqueue updating of vulnerability read denormalized column. + # Ingest archived events to enqueue updating of denormalized column. # Check for presence of vulnerabilities to avoid redundant job queueing. class ProcessArchivedEventsWorker -- GitLab