Merge branch '217811-update-historical-statistics-on-statistics-update' into 'master'

Update historical vulnerability statistics when statistics are updated

See merge request gitlab-org/gitlab!36970

ee/app/services/vulnerabilities/base_service.rb

@@ -17,6 +17,7 @@ module Vulnerabilities
 
       SystemNoteService.change_vulnerability_state(vulnerability, @user) if vulnerability.state_previously_changed?
       Vulnerabilities::Statistics::UpdateService.update_for(vulnerability)
+      Vulnerabilities::HistoricalStatistics::UpdateService.update_for(vulnerability.project)
       true
     end
 

ee/app/services/vulnerabilities/create_service.rb

@@ -21,6 +21,7 @@ module Vulnerabilities
 
         save_vulnerability(vulnerability, finding)
         Statistics::UpdateService.update_for(vulnerability)
+        HistoricalStatistics::UpdateService.update_for(@project)
       rescue ActiveRecord::RecordNotFound
         vulnerability.errors.add(:base, _('finding is not found or is already attached to a vulnerability'))
         raise ActiveRecord::Rollback

ee/app/services/vulnerabilities/historical_statistics/adjustment_service.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  module HistoricalStatistics
+    class AdjustmentService
+      TooManyProjectsError = Class.new(StandardError)
+
+      UPSERT_SQL = <<~SQL
+        INSERT INTO vulnerability_historical_statistics
+          (project_id, total, info, unknown, low, medium, high, critical, letter_grade, date, created_at, updated_at)
+          (%{stats_sql})
+        ON CONFLICT (project_id, date)
+        DO UPDATE SET
+          total = EXCLUDED.total,
+          info = EXCLUDED.info,
+          unknown = EXCLUDED.unknown,
+          low = EXCLUDED.low,
+          medium = EXCLUDED.medium,
+          high = EXCLUDED.high,
+          critical = EXCLUDED.critical,
+          letter_grade = EXCLUDED.letter_grade,
+          updated_at = EXCLUDED.updated_at
+      SQL
+
+      STATS_SQL = <<~SQL
+        SELECT
+          project_id,
+          total,
+          info,
+          unknown,
+          low,
+          medium,
+          high,
+          critical,
+          letter_grade,
+          updated_at AS date,
+          now() AS created_at,
+          now() AS updated_at
+        FROM vulnerability_statistics
+        WHERE project_id IN (%{project_ids})
+      SQL
+
+      MAX_PROJECTS = 1_000
+
+      def self.execute(project_ids)
+        new(project_ids).execute
+      end
+
+      def initialize(project_ids)
+        raise TooManyProjectsError, "Cannot adjust statistics for more than #{MAX_PROJECTS} projects" if project_ids.size > MAX_PROJECTS
+
+        @project_ids = project_ids.map { |id| Integer(id) }.join(', ')
+      end
+
+      def execute
+        ApplicationRecord.connection.execute(upsert_sql)
+      end
+
+      private
+
+      attr_reader :project_ids
+
+      def upsert_sql
+        UPSERT_SQL % { stats_sql: stats_sql }
+      end
+
+      def stats_sql
+        STATS_SQL % { project_ids: project_ids }
+      end
+    end
+  end
+end

ee/app/services/vulnerabilities/historical_statistics/update_service.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  module HistoricalStatistics
+    class UpdateService
+      VULNERABILITY_STATISTIC_ATTRIBUTES = %w(total critical high medium low unknown info letter_grade).freeze
+
+      def self.update_for(project)
+        new(project).execute
+      end
+
+      def initialize(project)
+        @project = project
+      end
+
+      # rubocop: disable CodeReuse/ActiveRecord
+      def execute
+        return if vulnerability_statistic.blank?
+
+        ::Vulnerabilities::HistoricalStatistic.safe_ensure_unique(retries: 1) do
+          historical_statistic = vulnerability_historical_statistics.find_or_initialize_by(date: vulnerability_statistic.updated_at)
+          historical_statistic.update(vulnerability_statistic.attributes.slice(*VULNERABILITY_STATISTIC_ATTRIBUTES))
+        end
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+
+      private
+
+      attr_reader :project
+
+      delegate :vulnerability_statistic, :vulnerability_historical_statistics, to: :project
+    end
+  end
+end

ee/app/services/vulnerabilities/update_service.rb

@@ -19,6 +19,7 @@ module Vulnerabilities
 
       vulnerability.update!(vulnerability_params)
       Statistics::UpdateService.update_for(vulnerability)
+      HistoricalStatistics::UpdateService.update_for(project)
 
       vulnerability
     end

ee/app/workers/vulnerabilities/statistics/adjustment_worker.rb

@@ -9,6 +9,7 @@ module Vulnerabilities
 
       def perform(project_ids)
         AdjustmentService.execute(project_ids)
+        HistoricalStatistics::AdjustmentService.execute(project_ids)
       end
     end
   end

ee/changelogs/unreleased/217811-update-historical-statistics-on-statistics-update.yml

+---
+title: Adjust historical vulnerability statistics after vulnerability update
+merge_request: 36970
+author:
+type: added

ee/spec/services/vulnerabilities/confirm_service_spec.rb

@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::ConfirmService do
     end
 
     it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
+    it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
 
     it 'confirms a vulnerability' do
       Timecop.freeze do

ee/spec/services/vulnerabilities/create_service_spec.rb

@@ -21,6 +21,7 @@ RSpec.describe Vulnerabilities::CreateService do
     end
 
     it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
+    it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
 
     it 'creates a vulnerability from finding and attaches it to the vulnerability' do
       expect { subject }.to change { project.vulnerabilities.count }.by(1)

ee/spec/services/vulnerabilities/dismiss_service_spec.rb

@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::DismissService do
     end
 
     it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
+    it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
 
     it 'dismisses a vulnerability and its associated findings' do
       Timecop.freeze do

ee/spec/services/vulnerabilities/historical_statistics/adjustment_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Vulnerabilities::HistoricalStatistics::AdjustmentService do
+  let_it_be(:project) { create(:project) }
+
+  describe '.execute' do
+    let(:project_ids) { [1, 2, 3] }
+    let(:mock_service_object) { instance_double(described_class, execute: true) }
+
+    subject(:execute_for_project_ids) { described_class.execute(project_ids) }
+
+    before do
+      allow(described_class).to receive(:new).and_return(mock_service_object)
+    end
+
+    it 'instantiates the service object for given project ids and calls `execute` on them', :aggregate_failures do
+      execute_for_project_ids
+
+      expect(described_class).to have_received(:new).with([1, 2, 3])
+      expect(mock_service_object).to have_received(:execute)
+    end
+  end
+
+  describe '#execute' do
+    let(:statistics) { project.vulnerability_historical_statistics.last.reload.as_json(except: [:id, :project_id, :created_at, :updated_at]) }
+    let(:project_ids) { [project.id] }
+
+    let(:expected_statistics) do
+      {
+        'total' => 2,
+        'critical' => 1,
+        'high' => 1,
+        'medium' => 0,
+        'low' => 0,
+        'info' => 0,
+        'unknown' => 0,
+        'letter_grade' => 'f',
+        'date' => Date.today.to_s
+      }
+    end
+
+    subject(:adjust_statistics) { described_class.new(project_ids).execute }
+
+    context 'when more than 1000 projects is provided' do
+      let(:project_ids) { (1..1001).to_a }
+
+      it 'raises error' do
+        expect { adjust_statistics }.to raise_error do |error|
+          expect(error.class).to eql(described_class::TooManyProjectsError)
+          expect(error.message).to eql('Cannot adjust statistics for more than 1000 projects')
+        end
+      end
+    end
+
+    context 'when there is no vulnerability_statistic record for project' do
+      it 'does not create a new record in database' do
+        expect { adjust_statistics }.not_to change { Vulnerabilities::Statistic.count }
+      end
+    end
+
+    context 'when there is vulnerability_statistic record for project' do
+      let!(:vulnerability_statistic) { create(:vulnerability_statistic, project: project, total: 2, critical: 1, high: 1) }
+
+      context 'when there is no vulnerability_historical_statistic record for project' do
+        it 'creates a new record' do
+          expect { adjust_statistics }.to change { Vulnerabilities::HistoricalStatistic.count }.by(1)
+        end
+
+        it 'sets the correct values for the record' do
+          adjust_statistics
+
+          expect(statistics).to eq(expected_statistics)
+        end
+      end
+
+      context 'when there is already a vulnerability_historical_statistic record for project' do
+        let!(:vulnerability_historical_statistic) { create(:vulnerability_historical_statistic, project: project) }
+
+        it 'does not create a new record in database' do
+          expect { adjust_statistics }.not_to change { Vulnerabilities::Statistic.count }
+        end
+
+        it 'sets the correct values for the record' do
+          adjust_statistics
+
+          expect(statistics).to eq(expected_statistics)
+        end
+      end
+    end
+  end
+end

ee/spec/services/vulnerabilities/historical_statistics/update_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Vulnerabilities::HistoricalStatistics::UpdateService do
+  let_it_be(:project) { create(:project) }
+
+  describe '.update_for' do
+    let(:mock_service_object) { instance_double(described_class, execute: true) }
+
+    subject(:update_stats) { described_class.update_for(project) }
+
+    before do
+      allow(described_class).to receive(:new).with(project).and_return(mock_service_object)
+    end
+
+    it 'instantiates an instance of service class and calls execute on it' do
+      update_stats
+
+      expect(mock_service_object).to have_received(:execute)
+    end
+  end
+
+  describe '#execute' do
+    subject(:update_stats) { described_class.new(project).execute }
+
+    context 'when the statistic is not empty' do
+      before do
+        create(:vulnerability_statistic, project: project, low: 2)
+      end
+
+      context 'when there is already a record in the database' do
+        it 'changes the existing historical statistic entity' do
+          historical_statistic = create(:vulnerability_historical_statistic, project: project, letter_grade: 'c')
+
+          expect { update_stats }.to change { historical_statistic.reload.letter_grade }.from('c').to('b')
+                                .and change { historical_statistic.reload.low }.to(2)
+        end
+      end
+
+      context 'when there is no existing record in the database' do
+        it 'creates a new record in the database' do
+          expect { update_stats }.to change { Vulnerabilities::HistoricalStatistic.count }.by(1)
+        end
+      end
+    end
+
+    context 'when the statistic is empty' do
+      it 'does not create any historical statistic entity' do
+        expect { update_stats }.not_to change { Vulnerabilities::Statistic.count }
+      end
+    end
+  end
+end

ee/spec/services/vulnerabilities/resolve_service_spec.rb

@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::ResolveService do
     end
 
     it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
+    it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
 
     it 'resolves a vulnerability' do
       Timecop.freeze do

ee/spec/services/vulnerabilities/update_service_spec.rb

@@ -23,6 +23,7 @@ RSpec.describe Vulnerabilities::UpdateService do
     end
 
     it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
+    it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
 
     context 'when neither severity nor confidence are overridden' do
       it 'updates the vulnerability from updated finding (title, severity and confidence only)', :aggregate_failures do

ee/spec/support/shared_examples/services/vulnerabilities/calls_vulnerabilities_historical_statistics_update_service_shared_example.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'calls Vulnerabilities::HistoricalStatistics::UpdateService' do
+  before do
+    allow(Vulnerabilities::HistoricalStatistics::UpdateService).to receive(:update_for)
+  end
+
+  it 'calls the service class' do
+    subject
+
+    expect(Vulnerabilities::HistoricalStatistics::UpdateService).to have_received(:update_for)
+  end
+end

ee/spec/workers/vulnerabilities/statistics/adjustment_worker_spec.rb

@@ -10,6 +10,7 @@ RSpec.describe Vulnerabilities::Statistics::AdjustmentWorker do
 
     before do
       allow(Vulnerabilities::Statistics::AdjustmentService).to receive(:execute)
+      allow(Vulnerabilities::HistoricalStatistics::AdjustmentService).to receive(:execute)
     end
 
     it 'calls `Vulnerabilities::Statistics::AdjustmentService` with given project_ids' do
@@ -17,5 +18,11 @@ RSpec.describe Vulnerabilities::Statistics::AdjustmentWorker do
 
       expect(Vulnerabilities::Statistics::AdjustmentService).to have_received(:execute).with(project_ids)
     end
+
+    it 'calls `Vulnerabilities::HistoricalStatistics::AdjustmentService` with given project_ids' do
+      worker.perform(project_ids)
+
+      expect(Vulnerabilities::HistoricalStatistics::AdjustmentService).to have_received(:execute).with(project_ids)
+    end
   end
 end