Leverage state transitions

And add two convenience methods for setting a value at the same time as
calling a transition:

- verification_succeeded_with_checksum!
- verification_failed_with_message!

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

@@ -52,10 +52,9 @@ module Geo
 
     def verify
       checksum = model_record.calculate_checksum
-      model_record.update_verification_state!(checksum: checksum)
+      model_record.verification_succeeded_with_checksum!(checksum)
     rescue => e
-      log_error('Error calculating the checksum', e)
-      model_record.update_verification_state!(failure: e.message)
+      model_record.verification_failed_with_message!('Error calculating the checksum', e)
     end
 
     # Check if given checksum matches known one

ee/lib/gitlab/geo/verification_state.rb

@@ -13,6 +13,7 @@ module Gitlab
       extend ActiveSupport::Concern
       include ::ShaAttribute
       include Delay
+      include Gitlab::Geo::LogHelpers
 
       VERIFICATION_STATE_VALUES = {
         verification_pending: 0,
@@ -41,7 +42,9 @@ module Gitlab
         state_machine :verification_state, initial: :verification_pending do
           state :verification_pending, value: VERIFICATION_STATE_VALUES[:verification_pending]
           state :verification_started, value: VERIFICATION_STATE_VALUES[:verification_started]
-          state :verification_succeeded, value: VERIFICATION_STATE_VALUES[:verification_succeeded]
+          state :verification_succeeded, value: VERIFICATION_STATE_VALUES[:verification_succeeded] do
+            validates :verification_checksum, presence: true
+          end
           state :verification_failed, value: VERIFICATION_STATE_VALUES[:verification_failed] do
             validates :verification_failure, presence: true
           end
@@ -58,14 +61,18 @@ module Gitlab
           end
 
           before_transition any => :verification_failed do |instance, _|
+            instance.verification_checksum = nil
+            instance.verification_retry_count ||= 0
             instance.verification_retry_count += 1
             instance.verification_retry_at = instance.next_retry_time(instance.verification_retry_count)
+            instance.verified_at = Time.current
           end
 
           before_transition any => :verification_succeeded do |instance, _|
             instance.verification_retry_count = 0
             instance.verification_retry_at = nil
             instance.verification_failure = nil
+            instance.verified_at = Time.current
           end
 
           event :verification_started do
@@ -92,32 +99,27 @@ module Gitlab
         end
       end
 
-      # Update checksum on Geo primary database
+      # Convenience method to update checksum and transition to success state.
       #
       # @param [String] checksum value generated by the checksum routine
-      # @param [String] failure (optional) stacktrace from failed execution
-      def update_verification_state!(checksum: nil, failure: nil)
-        retry_at, retry_count = calculate_next_retry_attempt if failure.present?
-
-        update!(
-          verification_checksum: checksum,
-          verified_at: Time.current,
-          verification_failure: failure,
-          verification_retry_at: retry_at,
-          verification_retry_count: retry_count
-        )
-      end
+      def verification_succeeded_with_checksum!(checksum)
+        self.verification_checksum = checksum
 
-      def calculate_next_retry_attempt
-        retry_count = model_record.verification_retry_count.to_i + 1
-        [next_retry_time(retry_count), retry_count]
+        self.verification_succeeded!
       end
 
+      # Convenience method to update failure message and transition to failed
+      # state.
+      #
       # @param [String] message error information
       # @param [StandardError] error exception
-      def set_verification_failure(message, error = nil)
+      def verification_failed_with_message!(message, error = nil)
+        log_error('Error calculating the checksum', error)
+
         self.verification_failure = message
         self.verification_failure += ": #{error.message}" if error.respond_to?(:message)
+
+        self.verification_failed!
       end
     end
   end

ee/spec/lib/gitlab/geo/verification_state_spec.rb

@@ -19,26 +19,29 @@ RSpec.describe Gitlab::Geo::VerificationState do
   before do
     stub_dummy_replicator_class
     stub_dummy_model_class
-  end
-
-  subject { DummyModel.new }
 
-  describe '#update_verification_state!' do
-    before do
+    subject.verification_started
     subject.save!
   end
 
+  subject { DummyModel.new }
+
+  describe '#verification_succeeded_with_checksum!' do
     it 'saves the checksum' do
-      subject.update_verification_state!(checksum: 'abc123')
+      subject.verification_succeeded_with_checksum!('abc123')
 
       expect(subject.reload.verification_checksum).to eq('abc123')
       expect(subject.verified_at).not_to be_nil
     end
+  end
 
+  describe '#verification_failed_with_message!' do
     it 'saves the error message and increments retry counter' do
-      subject.update_verification_state!(failure: 'Failure to calculate checksum')
+      error = double('error', message: 'An error message')
+
+      subject.verification_failed_with_message!('Failure to calculate checksum', error)
 
-      expect(subject.reload.verification_failure).to eq 'Failure to calculate checksum'
+      expect(subject.reload.verification_failure).to eq 'Failure to calculate checksum: An error message'
       expect(subject.verification_retry_count).to be 1
     end
   end

ee/spec/support/helpers/ee/geo_helpers.rb

@@ -80,6 +80,7 @@ module EE
 
       DummyModel.class_eval do
         include ::Gitlab::Geo::ReplicableModel
+        include ::Gitlab::Geo::VerificationState
 
         with_replicator Geo::DummyReplicator
 
@@ -104,6 +105,12 @@ module EE
       ActiveRecord::Schema.define do
         create_table :dummy_models, force: true do |t|
           t.binary :verification_checksum
+          t.integer :verification_state
+          t.datetime_with_timezone :verification_started_at
+          t.datetime_with_timezone :verified_at
+          t.datetime_with_timezone :verification_retry_at
+          t.integer :verification_retry_count
+          t.text :verification_failure
         end
       end
     end

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

@@ -124,28 +124,42 @@ RSpec.shared_examples 'a verifiable replicator' do
 
   describe '#verify' do
     before do
+      model_record.verification_started
       model_record.save!
     end
 
-    context 'when a Geo primary' do
-      context 'when an error is raised during calculate_checksum!' do
+    context 'on a Geo primary' do
+      before do
+        stub_primary_node
+      end
+
+      context 'when verification was started' do
+        before do
+          model_record.verification_started!
+        end
+
+        context 'when the checksum succeeds' do
           it 'delegates checksum calculation and the state change to model_record' do
             expect(model_record).to receive(:calculate_checksum).and_return('abc123')
-          expect(model_record).to receive(:update_verification_state!).with(checksum: 'abc123')
+            expect(model_record).to receive(:verification_succeeded_with_checksum!).with('abc123')
 
             replicator.verify
           end
+        end
 
+        context 'when an error is raised during calculate_checksum' do
           it 'passes the error message' do
+            error = StandardError.new('Some exception')
             allow(model_record).to receive(:calculate_checksum) do
-            raise StandardError.new('Failure to calculate checksum')
+              raise error
             end
 
-          expect(model_record).to receive(:update_verification_state!).with(failure: 'Failure to calculate checksum')
+            expect(model_record).to receive(:verification_failed_with_message!).with('Error calculating the checksum', error)
 
             replicator.verify
           end
         end
       end
     end
+  end
 end

ee/spec/workers/geo/verification_worker_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe Geo::VerificationWorker, :geo do
     context 'when on a primary node' do
       before do
         stub_primary_node
-        package_file.update!(verification_checksum: nil)
+        package_file.verification_started!
       end
 
       it_behaves_like 'an idempotent worker' do