Commit ccfa4128 authored by Thiago Figueiró's avatar Thiago Figueiró

Remove feature flag

Flag: keep_historical_vulnerability_statistics_always_consistent

Changelog: removed
parent 8fc00aea
...@@ -16,7 +16,6 @@ module Vulnerabilities ...@@ -16,7 +16,6 @@ module Vulnerabilities
return false unless vulnerability.update(params) return false unless vulnerability.update(params)
Vulnerabilities::Statistics::UpdateService.update_for(vulnerability) Vulnerabilities::Statistics::UpdateService.update_for(vulnerability)
Vulnerabilities::HistoricalStatistics::UpdateService.update_for(vulnerability.project)
SystemNoteService.change_vulnerability_state(vulnerability, @user) if vulnerability.state_previously_changed? SystemNoteService.change_vulnerability_state(vulnerability, @user) if vulnerability.state_previously_changed?
true true
end end
......
...@@ -25,7 +25,6 @@ module Vulnerabilities ...@@ -25,7 +25,6 @@ module Vulnerabilities
if vulnerability.persisted? if vulnerability.persisted?
Statistics::UpdateService.update_for(vulnerability) Statistics::UpdateService.update_for(vulnerability)
HistoricalStatistics::UpdateService.update_for(@project)
end end
vulnerability vulnerability
......
# frozen_string_literal: true
module Vulnerabilities
module HistoricalStatistics
class UpdateService
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 = %{project_id}
SQL
def self.update_for(project)
new(project).execute
end
def initialize(project)
@project = project
end
def execute
return unless update_statistic?
ApplicationRecord.connection.execute(upsert_sql)
end
private
attr_reader :project
delegate :vulnerability_statistic, to: :project
def update_statistic?
keep_statistics_always_consistent? && vulnerability_statistic.present?
end
def keep_statistics_always_consistent?
Feature.enabled?(:keep_historical_vulnerability_statistics_always_consistent, project)
end
def upsert_sql
UPSERT_SQL % { stats_sql: stats_sql }
end
def stats_sql
STATS_SQL % { project_id: project.id }
end
end
end
end
...@@ -39,7 +39,6 @@ module Vulnerabilities ...@@ -39,7 +39,6 @@ module Vulnerabilities
finding.save! finding.save!
Statistics::UpdateService.update_for(vulnerability) Statistics::UpdateService.update_for(vulnerability)
HistoricalStatistics::UpdateService.update_for(@project)
ServiceResponse.success(payload: { vulnerability: vulnerability }) ServiceResponse.success(payload: { vulnerability: vulnerability })
end end
......
...@@ -35,7 +35,6 @@ module Vulnerabilities ...@@ -35,7 +35,6 @@ module Vulnerabilities
finding.save! finding.save!
Statistics::UpdateService.update_for(vulnerability) Statistics::UpdateService.update_for(vulnerability)
HistoricalStatistics::UpdateService.update_for(@project)
ServiceResponse.success(payload: { vulnerability: vulnerability }) ServiceResponse.success(payload: { vulnerability: vulnerability })
end end
......
...@@ -20,7 +20,6 @@ module Vulnerabilities ...@@ -20,7 +20,6 @@ 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
......
---
name: keep_historical_vulnerability_statistics_always_consistent
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68189
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338508
milestone: '14.2'
type: development
group: group::threat insights
default_enabled: false
...@@ -24,7 +24,6 @@ RSpec.describe Vulnerabilities::CreateService do ...@@ -24,7 +24,6 @@ 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)
...@@ -97,7 +96,6 @@ RSpec.describe Vulnerabilities::CreateService do ...@@ -97,7 +96,6 @@ RSpec.describe Vulnerabilities::CreateService do
it 'does not update vulnerability statistics' do it 'does not update vulnerability statistics' do
subject subject
expect(Vulnerabilities::HistoricalStatistics::UpdateService).not_to receive(:update_for)
expect(Vulnerabilities::Statistics::UpdateService).not_to receive(:update_for) expect(Vulnerabilities::Statistics::UpdateService).not_to receive(:update_for)
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 }
around do |example|
travel_to(Date.current) { example.run }
end
context 'when the `keep_historical_vulnerability_statistics_always_consistent` feature is enabled' do
context 'when the statistic is not empty' do
before do
create(:vulnerability_statistic, project: project, low: 2)
end
context 'when there exists 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 exists no 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
it 'changes the updated_at timestamp of 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.updated_at }
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
context 'when the `keep_historical_vulnerability_statistics_always_consistent` feature is disabled' do
before do
stub_feature_flags(keep_historical_vulnerability_statistics_always_consistent: false)
end
context 'when the statistic is not empty' do
before do
create(:vulnerability_statistic, project: project, low: 2)
end
context 'when there exists a record in the database' do
it 'does not change the existing historical statistic entity' do
historical_statistic = create(:vulnerability_historical_statistic, project: project, letter_grade: 'c')
expect { update_stats }.to not_change { historical_statistic.reload.letter_grade }.from('c')
.and not_change { historical_statistic.reload.low }.from(0)
end
end
context 'when there exists no record in the database' do
it 'does not create a new record in the database' do
expect { update_stats }.not_to change { Vulnerabilities::HistoricalStatistic.count }
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
end
...@@ -26,7 +26,6 @@ RSpec.describe Vulnerabilities::UpdateService do ...@@ -26,7 +26,6 @@ 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 finding name is longer than 255 characters' do context 'when finding name is longer than 255 characters' do
let(:finding_name) { 'a' * 256 } let(:finding_name) { 'a' * 256 }
......
# 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
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
RSpec.shared_examples 'calls vulnerability statistics utility services in order' do RSpec.shared_examples 'calls vulnerability statistics utility services in order' do
before do before do
allow(Vulnerabilities::Statistics::UpdateService).to receive(:update_for) allow(Vulnerabilities::Statistics::UpdateService).to receive(:update_for)
allow(Vulnerabilities::HistoricalStatistics::UpdateService).to receive(:update_for)
allow(SystemNoteService).to receive(:change_vulnerability_state) allow(SystemNoteService).to receive(:change_vulnerability_state)
end end
...@@ -11,7 +10,6 @@ RSpec.shared_examples 'calls vulnerability statistics utility services in order' ...@@ -11,7 +10,6 @@ RSpec.shared_examples 'calls vulnerability statistics utility services in order'
subject subject
expect(Vulnerabilities::Statistics::UpdateService).to have_received(:update_for).ordered expect(Vulnerabilities::Statistics::UpdateService).to have_received(:update_for).ordered
expect(Vulnerabilities::HistoricalStatistics::UpdateService).to have_received(:update_for).ordered
expect(SystemNoteService).to have_received(:change_vulnerability_state).ordered expect(SystemNoteService).to have_received(:change_vulnerability_state).ordered
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