Merge branch 'remove-keep_historical_vulnerability_statistics_always_consistent' into 'master'

Remove keep_historical_vulnerability_statistics_always_consistent feature flag

See merge request gitlab-org/gitlab!79707

ee/app/services/vulnerabilities/base_service.rb

@@ -16,7 +16,6 @@ module Vulnerabilities
       return false unless vulnerability.update(params)
 
       Vulnerabilities::Statistics::UpdateService.update_for(vulnerability)
-      Vulnerabilities::HistoricalStatistics::UpdateService.update_for(vulnerability.project)
       SystemNoteService.change_vulnerability_state(vulnerability, @user) if vulnerability.state_previously_changed?
       true
     end

ee/app/services/vulnerabilities/create_service.rb

@@ -25,7 +25,6 @@ module Vulnerabilities
 
       if vulnerability.persisted?
         Statistics::UpdateService.update_for(vulnerability)
-        HistoricalStatistics::UpdateService.update_for(@project)
       end
 
       vulnerability

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

-# 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

ee/app/services/vulnerabilities/manually_create_service.rb

@@ -39,7 +39,6 @@ module Vulnerabilities
         finding.save!
 
         Statistics::UpdateService.update_for(vulnerability)
-        HistoricalStatistics::UpdateService.update_for(@project)
 
         ServiceResponse.success(payload: { vulnerability: vulnerability })
       end

ee/app/services/vulnerabilities/starboard_vulnerability_create_service.rb

@@ -35,7 +35,6 @@ module Vulnerabilities
         finding.save!
 
         Statistics::UpdateService.update_for(vulnerability)
-        HistoricalStatistics::UpdateService.update_for(@project)
 
         ServiceResponse.success(payload: { vulnerability: vulnerability })
       end

ee/app/services/vulnerabilities/update_service.rb

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

ee/config/feature_flags/development/keep_historical_vulnerability_statistics_always_consistent.yml

----
-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

ee/spec/services/vulnerabilities/create_service_spec.rb

@@ -24,7 +24,6 @@ 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)
@@ -97,7 +96,6 @@ RSpec.describe Vulnerabilities::CreateService do
       it 'does not update vulnerability statistics' do
         subject
 
-        expect(Vulnerabilities::HistoricalStatistics::UpdateService).not_to receive(:update_for)
         expect(Vulnerabilities::Statistics::UpdateService).not_to receive(:update_for)
       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 }
-
-    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

ee/spec/services/vulnerabilities/update_service_spec.rb

@@ -26,7 +26,6 @@ RSpec.describe Vulnerabilities::UpdateService do
     end
 
     it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
-    it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
 
     context 'when finding name is longer than 255 characters' do
       let(:finding_name) { 'a' * 256 }

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/support/shared_examples/services/vulnerabilities/calls_vulnerability_statistics_utility_services_in_order.rb

@@ -3,7 +3,6 @@
 RSpec.shared_examples 'calls vulnerability statistics utility services in order' do
   before do
     allow(Vulnerabilities::Statistics::UpdateService).to receive(:update_for)
-    allow(Vulnerabilities::HistoricalStatistics::UpdateService).to receive(:update_for)
     allow(SystemNoteService).to receive(:change_vulnerability_state)
   end
 
@@ -11,7 +10,6 @@ RSpec.shared_examples 'calls vulnerability statistics utility services in order'
     subject
 
     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
   end
 end