Use the verification state machine's scopes

Instead of deriving verification state from various checksum-related
fields, we can now query for records with a particular state.

doc/development/geo/framework.md

@@ -569,14 +569,16 @@ Metrics are gathered by `Geo::MetricsUpdateWorker`, persisted in
 1. Add the following to `spec/factories/widgets.rb`:
 
    ```ruby
-   trait(:checksummed) do
+   trait(:verification_succeeded) do
      with_file
      verification_checksum { 'abc' }
+     verification_state { Widget.verification_state_value(:verification_succeeded) }
    end
 
-   trait(:checksum_failure) do
+   trait(:verification_failed) do
      with_file
      verification_failure { 'Could not calculate the checksum' }
+     verification_state { Widget.verification_state_value(:verification_failed) }
    end
    ```
 

ee/app/models/concerns/geo/verifiable_replicator.rb

@@ -30,7 +30,7 @@ module Geo
         # Bonus: This causes the progress bar to be hidden.
         return unless verification_enabled?
 
-        model.available_replicables.checksummed.count
+        model.available_replicables.verification_succeeded.count
       end
 
       def checksum_failed_count
@@ -38,7 +38,7 @@ module Geo
         # Bonus: This causes the progress bar to be hidden.
         return unless verification_enabled?
 
-        model.available_replicables.checksum_failed.count
+        model.available_replicables.verification_failed.count
       end
     end
 

ee/app/models/ee/merge_request_diff.rb

@@ -24,8 +24,8 @@ module EE
       scope :has_external_diffs, -> { with_files.where(stored_externally: true) }
       scope :project_id_in, ->(ids) { where(merge_request_id: ::MergeRequest.where(target_project_id: ids)) }
 
-      scope :checksummed, -> { where(merge_request_diff_detail: ::MergeRequestDiffDetail.checksummed) }
-      scope :checksum_failed, -> { where(merge_request_diff_detail: ::MergeRequestDiffDetail.checksum_failed) }
+      scope :verification_succeeded, -> { where(merge_request_diff_detail: ::MergeRequestDiffDetail.verification_succeeded) }
+      scope :verification_failed, -> { where(merge_request_diff_detail: ::MergeRequestDiffDetail.verification_failed) }
       scope :available_replicables, -> { has_external_diffs }
     end
 

ee/app/models/merge_request_diff_detail.rb

@@ -5,6 +5,11 @@ class MergeRequestDiffDetail < ApplicationRecord
 
   belongs_to :merge_request_diff, inverse_of: :merge_request_diff_detail
 
-  scope :checksummed, -> { where.not(verification_checksum: nil) }
-  scope :checksum_failed, -> { where.not(verification_failure: nil) }
+  # Temporarily defining `verification_succeeded` and
+  # `verification_failed` for unverified models while verification is
+  # under development to avoid breaking GeoNodeStatusCheck code.
+  # TODO: Remove these after including `Gitlab::Geo::VerificationState` on
+  # all models. https://gitlab.com/gitlab-org/gitlab/-/issues/280768
+  scope :verification_succeeded, -> { none }
+  scope :verification_failed, -> { none }
 end

ee/lib/gitlab/geo/replicable_model.rb

@@ -11,8 +11,13 @@ module Gitlab
         after_create_commit -> { replicator.handle_after_create_commit if replicator.respond_to?(:handle_after_create_commit) }
         after_destroy -> { replicator.handle_after_destroy if replicator.respond_to?(:handle_after_destroy) }
 
-        scope :checksummed, -> { where.not(verification_checksum: nil) }
-        scope :checksum_failed, -> { where.not(verification_failure: nil) }
+        # Temporarily defining `verification_succeeded` and
+        # `verification_failed` for unverified models while verification is
+        # under development to avoid breaking GeoNodeStatusCheck code.
+        # TODO: Remove these after including `Gitlab::Geo::VerificationState` on
+        # all models. https://gitlab.com/gitlab-org/gitlab/-/issues/280768
+        scope :verification_succeeded, -> { none }
+        scope :verification_failed, -> { none }
         scope :available_replicables, -> { all }
       end
 

ee/lib/gitlab/geo/verification_state.rb

@@ -86,6 +86,12 @@ module Gitlab
         end
       end
 
+      class_methods do
+        def verification_state_value(state_string)
+          VERIFICATION_STATE_VALUES[state_string]
+        end
+      end
+
       # @param [String] message error information
       # @param [StandardError] error exception
       def set_verification_failure(message, error = nil)

ee/spec/factories/packages/package_file.rb

+# frozen_string_literal: true
+
+FactoryBot.modify do
+  factory :package_file do
+    trait(:verification_succeeded) do
+      verification_checksum { 'abc' }
+      verification_state { Packages::PackageFile.verification_state_value[:verification_succeeded] }
+    end
+
+    trait(:verification_failed) do
+      verification_failure { 'Could not calculate the checksum' }
+      verification_state { Packages::PackageFile.verification_state_value[:verification_failed] }
+    end
+  end
+end

ee/spec/models/geo_node_status_spec.rb

@@ -1206,15 +1206,17 @@ RSpec.describe GeoNodeStatus, :geo do
 
         context 'when verification is enabled' do
           before do
+            skip "#{replicator.model} does not include the VerificationState concern yet" unless replicator.model.respond_to?(:verification_state)
+
             stub_current_geo_node(primary)
             allow(replicator).to receive(:verification_enabled?).and_return(true)
           end
 
           context 'when there are replicables' do
             before do
-              create(model_factory, :checksummed)
-              create(model_factory, :checksummed)
-              create(model_factory, :checksum_failure)
+              create(model_factory, :verification_succeeded)
+              create(model_factory, :verification_succeeded)
+              create(model_factory, :verification_failed)
             end
 
             describe '#<replicable_name>_checksummed_count' do

ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb

@@ -39,6 +39,80 @@ RSpec.shared_examples 'a verifiable replicator' do
     end
   end
 
+  describe '.checksummed_count' do
+    context 'when verification is enabled' do
+      before do
+        allow(described_class).to receive(:verification_enabled?).and_return(true)
+      end
+
+      it 'returns the number of available replicables where verification succeeded' do
+        model_record.verification_started!
+        model_record.verification_succeeded_with_checksum!('some checksum', Time.current)
+
+        expect(described_class.checksummed_count).to eq(1)
+      end
+
+      it 'excludes other verification states' do
+        model_record.verification_started!
+
+        expect(described_class.checksummed_count).to eq(0)
+
+        model_record.verification_failed_with_message!('some error message')
+
+        expect(described_class.checksummed_count).to eq(0)
+
+        model_record.verification_pending!
+
+        expect(described_class.checksummed_count).to eq(0)
+      end
+    end
+
+    context 'when verification is disabled' do
+      it 'returns nil' do
+        allow(described_class).to receive(:verification_enabled?).and_return(false)
+
+        expect(described_class.checksummed_count).to be_nil
+      end
+    end
+  end
+
+  describe '.checksum_failed_count' do
+    context 'when verification is enabled' do
+      before do
+        allow(described_class).to receive(:verification_enabled?).and_return(true)
+      end
+
+      it 'returns the number of available replicables where verification failed' do
+        model_record.verification_started!
+        model_record.verification_failed_with_message!('some error message')
+
+        expect(described_class.checksum_failed_count).to eq(1)
+      end
+
+      it 'excludes other verification states' do
+        model_record.verification_started!
+
+        expect(described_class.checksum_failed_count).to eq(0)
+
+        model_record.verification_succeeded_with_checksum!('foo', Time.current)
+
+        expect(described_class.checksum_failed_count).to eq(0)
+
+        model_record.verification_pending!
+
+        expect(described_class.checksum_failed_count).to eq(0)
+      end
+    end
+
+    context 'when verification is disabled' do
+      it 'returns nil' do
+        allow(described_class).to receive(:verification_enabled?).and_return(false)
+
+        expect(described_class.checksum_failed_count).to be_nil
+      end
+    end
+  end
+
   describe '#after_verifiable_update' do
     it 'calls verify_async if needed' do
       expect(replicator).to receive(:verify_async)

spec/factories/packages/package_file.rb

@@ -152,14 +152,6 @@ FactoryBot.define do
       file_store { Packages::PackageFileUploader::Store::REMOTE }
     end
 
-    trait(:checksummed) do
-      verification_checksum { 'abc' }
-    end
-
-    trait(:checksum_failure) do
-      verification_failure { 'Could not calculate the checksum' }
-    end
-
     factory :package_file_with_file, traits: [:jar]
   end
 end