Update merge request diff files_count cache when removing diffs

The files_count cache column was created with the assumption that the
number of files never changes throughout the life of a diff. However,
we have discovered the DeleteDiffFilesWorker, which removes them (and
sets the state of the diff to :without_files) in some limited cases.

This MR updates the cache columns (both commits_count and files_count)
so they will be re-recorded on every update. Since updates are rare,
the additional cost of this should be very low.

app/models/merge_request_diff.rb

@@ -150,10 +150,10 @@ class MergeRequestDiff < ApplicationRecord
   # All diff information is collected from repository after object is created.
   # It allows you to override variables like head_commit_sha before getting diff.
   after_create :save_git_content, unless: :importing?
-  after_create :set_count_columns
   after_create_commit :set_as_latest_diff, unless: :importing?
 
   after_save :update_external_diff_store
+  after_save :set_count_columns
 
   def self.find_by_diff_refs(diff_refs)
     find_by(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha)

app/workers/delete_diff_files_worker.rb

@@ -12,11 +12,11 @@ class DeleteDiffFilesWorker # rubocop:disable Scalability/IdempotentWorker
     return if merge_request_diff.without_files?
 
     MergeRequestDiff.transaction do
-      merge_request_diff.clean!
-
       MergeRequestDiffFile
         .where(merge_request_diff_id: merge_request_diff.id)
         .delete_all
+
+      merge_request_diff.clean!
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord

changelogs/unreleased/240921-fix-merge-request-diff-files-count.yml

+---
+title: Fix incorrect merge request diff file count after deletion
+merge_request: 40384
+author:
+type: fixed

spec/workers/delete_diff_files_worker_spec.rb

@@ -19,6 +19,12 @@ RSpec.describe DeleteDiffFilesWorker do
         .from('collected').to('without_files')
     end
 
+    it 'resets the files_count of the diff' do
+      expect { described_class.new.perform(merge_request_diff.id) }
+        .to change { merge_request_diff.reload.files_count }
+        .from(20).to(0)
+    end
+
     it 'does nothing if diff was already marked as "without_files"' do
       merge_request_diff.clean!