Refactor verify after save

In preparation for mutable replicables, especially Git repos.

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

@@ -92,5 +92,14 @@ module Geo
     def checksummable?
       carrierwave_uploader.file_storage? && file_exists?
     end
+
+    # Return whether it's immutable
+    #
+    # @return [Boolean] whether the replicable is immutable
+    def immutable?
+      # Most blobs are supposed to be immutable.
+      # Override this in your specific Replicator class if needed.
+      true
+    end
   end
 end

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

@@ -189,7 +189,7 @@ module Geo
 
     # Schedules a verification job after a model record is created/updated
     def after_verifiable_update
-      verify_async if should_primary_verify?
+      verify_async if should_primary_verify_after_save?
     end
 
     def verify_async
@@ -242,10 +242,22 @@ module Geo
 
     private
 
-    def should_primary_verify?
-      self.class.verification_enabled? &&
-       primary_checksum.nil? && # Some models may populate this as part of creating the record
-       checksummable?
+    def should_primary_verify_after_save?
+      return false unless self.class.verification_enabled?
+
+      # Optimization: If the data is immutable, then there is no need to
+      # recalculate checksum when a record is created (some models calculate
+      # checksum as part of creation) or updated. Note that reverification
+      # should still run as usual.
+      return false if immutable? && primary_checksum.present?
+
+      checksummable?
+    end
+
+    # @abstract
+    # @return [Boolean] whether the replicable is supposed to be immutable
+    def immutable?
+      raise NotImplementedError, "#{self.class} does not implement #{__method__}"
     end
 
     # @abstract

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

@@ -341,14 +341,44 @@ RSpec.shared_examples 'a verifiable replicator' do
   end
 
   describe '#after_verifiable_update' do
-    it 'calls verify_async if needed' do
-      allow(described_class).to receive(:verification_enabled?).and_return(true)
-      allow(replicator).to receive(:primary_checksum).and_return(nil)
-      allow(replicator).to receive(:checksummable?).and_return(true)
+    using RSpec::Parameterized::TableSyntax
+
+    where(:verification_enabled, :immutable, :checksum, :checksummable, :expect_verify_async) do
+      true  | true  | nil      | true  | true
+      true  | true  | nil      | false | false
+      true  | true  | 'abc123' | true  | false
+      true  | true  | 'abc123' | false | false
+      true  | false | nil      | true  | true
+      true  | false | nil      | false | false
+      true  | false | 'abc123' | true  | true
+      true  | false | 'abc123' | false | false
+      false | true  | nil      | true  | false
+      false | true  | nil      | false | false
+      false | true  | 'abc123' | true  | false
+      false | true  | 'abc123' | false | false
+      false | false | nil      | true  | false
+      false | false | nil      | false | false
+      false | false | 'abc123' | true  | false
+      false | false | 'abc123' | false | false
+    end
+
+    with_them do
+      before do
+        allow(described_class).to receive(:verification_enabled?).and_return(verification_enabled)
+        allow(replicator).to receive(:immutable?).and_return(immutable)
+        allow(replicator).to receive(:primary_checksum).and_return(checksum)
+        allow(replicator).to receive(:checksummable?).and_return(checksummable)
+      end
 
-      expect(replicator).to receive(:verify_async)
+      it 'calls verify_async only if needed' do
+        if expect_verify_async
+          expect(replicator).to receive(:verify_async)
+        else
+          expect(replicator).not_to receive(:verify_async)
+        end
 
-      replicator.after_verifiable_update
+        replicator.after_verifiable_update
+      end
     end
   end