Commit c0d620a6 authored by Alan (Maciej) Paruszewski's avatar Alan (Maciej) Paruszewski Committed by Igor Drozdov

Update historical vulnerability statistics when statistics are updated

This change adds updating vulnerability historical statistics when
statistics for the project are updated.
parent bff83d42
...@@ -17,6 +17,7 @@ module Vulnerabilities ...@@ -17,6 +17,7 @@ module Vulnerabilities
SystemNoteService.change_vulnerability_state(vulnerability, @user) if vulnerability.state_previously_changed? SystemNoteService.change_vulnerability_state(vulnerability, @user) if vulnerability.state_previously_changed?
Vulnerabilities::Statistics::UpdateService.update_for(vulnerability) Vulnerabilities::Statistics::UpdateService.update_for(vulnerability)
Vulnerabilities::HistoricalStatistics::UpdateService.update_for(vulnerability.project)
true true
end end
......
...@@ -21,6 +21,7 @@ module Vulnerabilities ...@@ -21,6 +21,7 @@ module Vulnerabilities
save_vulnerability(vulnerability, finding) save_vulnerability(vulnerability, finding)
Statistics::UpdateService.update_for(vulnerability) Statistics::UpdateService.update_for(vulnerability)
HistoricalStatistics::UpdateService.update_for(@project)
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
vulnerability.errors.add(:base, _('finding is not found or is already attached to a vulnerability')) vulnerability.errors.add(:base, _('finding is not found or is already attached to a vulnerability'))
raise ActiveRecord::Rollback raise ActiveRecord::Rollback
......
# 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
# 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
...@@ -19,6 +19,7 @@ module Vulnerabilities ...@@ -19,6 +19,7 @@ module Vulnerabilities
vulnerability.update!(vulnerability_params) vulnerability.update!(vulnerability_params)
Statistics::UpdateService.update_for(vulnerability) Statistics::UpdateService.update_for(vulnerability)
HistoricalStatistics::UpdateService.update_for(project)
vulnerability vulnerability
end end
......
...@@ -9,6 +9,7 @@ module Vulnerabilities ...@@ -9,6 +9,7 @@ module Vulnerabilities
def perform(project_ids) def perform(project_ids)
AdjustmentService.execute(project_ids) AdjustmentService.execute(project_ids)
HistoricalStatistics::AdjustmentService.execute(project_ids)
end end
end end
end end
......
---
title: Adjust historical vulnerability statistics after vulnerability update
merge_request: 36970
author:
type: added
...@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::ConfirmService do ...@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::ConfirmService do
end end
it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService' it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
it 'confirms a vulnerability' do it 'confirms a vulnerability' do
Timecop.freeze do Timecop.freeze do
......
...@@ -21,6 +21,7 @@ RSpec.describe Vulnerabilities::CreateService do ...@@ -21,6 +21,7 @@ RSpec.describe Vulnerabilities::CreateService do
end end
it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService' 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 it 'creates a vulnerability from finding and attaches it to the vulnerability' do
expect { subject }.to change { project.vulnerabilities.count }.by(1) expect { subject }.to change { project.vulnerabilities.count }.by(1)
......
...@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::DismissService do ...@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::DismissService do
end end
it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService' it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
it 'dismisses a vulnerability and its associated findings' do it 'dismisses a vulnerability and its associated findings' do
Timecop.freeze do Timecop.freeze do
......
# 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
# 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
...@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::ResolveService do ...@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::ResolveService do
end end
it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService' it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
it 'resolves a vulnerability' do it 'resolves a vulnerability' do
Timecop.freeze do Timecop.freeze do
......
...@@ -23,6 +23,7 @@ RSpec.describe Vulnerabilities::UpdateService do ...@@ -23,6 +23,7 @@ RSpec.describe Vulnerabilities::UpdateService do
end end
it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService' it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
context 'when neither severity nor confidence are overridden' do context 'when neither severity nor confidence are overridden' do
it 'updates the vulnerability from updated finding (title, severity and confidence only)', :aggregate_failures do it 'updates the vulnerability from updated finding (title, severity and confidence only)', :aggregate_failures do
......
# 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
...@@ -10,6 +10,7 @@ RSpec.describe Vulnerabilities::Statistics::AdjustmentWorker do ...@@ -10,6 +10,7 @@ RSpec.describe Vulnerabilities::Statistics::AdjustmentWorker do
before do before do
allow(Vulnerabilities::Statistics::AdjustmentService).to receive(:execute) allow(Vulnerabilities::Statistics::AdjustmentService).to receive(:execute)
allow(Vulnerabilities::HistoricalStatistics::AdjustmentService).to receive(:execute)
end end
it 'calls `Vulnerabilities::Statistics::AdjustmentService` with given project_ids' do it 'calls `Vulnerabilities::Statistics::AdjustmentService` with given project_ids' do
...@@ -17,5 +18,11 @@ RSpec.describe Vulnerabilities::Statistics::AdjustmentWorker do ...@@ -17,5 +18,11 @@ RSpec.describe Vulnerabilities::Statistics::AdjustmentWorker do
expect(Vulnerabilities::Statistics::AdjustmentService).to have_received(:execute).with(project_ids) expect(Vulnerabilities::Statistics::AdjustmentService).to have_received(:execute).with(project_ids)
end 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment