Delete non-latest merge request diff files upon diffs reload

2 files changed, 47 insertions, 19 deletions

diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index b4249d72fc8..48c01fc4d4e 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -47,6 +47,45 @@ describe MergeRequestDiff do
   end
 
   describe '#diffs' do
+    let(:merge_request) { create(:merge_request, :with_diffs) }
+    let!(:diff) { merge_request.merge_request_diff.reload }
+
+    context 'when it was not cleaned by the system' do
+      it 'returns persisted diffs' do
+        expect(diff).to receive(:load_diffs)
+
+        diff.diffs
+      end
+    end
+
+    context 'when diff was cleaned by the system' do
+      before do
+        diff.clean!
+      end
+
+      it 'returns diffs from repository if can compare with current diff refs' do
+        expect(diff).not_to receive(:load_diffs)
+
+        expect(Compare)
+          .to receive(:new)
+          .with(instance_of(Gitlab::Git::Compare), merge_request.target_project,
+                base_sha: diff.base_commit_sha, straight: false)
+          .and_call_original
+
+        diff.diffs
+      end
+
+      it 'returns persisted diffs if cannot compare with diff refs' do
+        expect(diff).to receive(:load_diffs)
+
+        diff.update!(head_commit_sha: 'invalid-sha')
+
+        diff.diffs
+      end
+    end
+  end
+
+  describe '#raw_diffs' do
     context 'when the :ignore_whitespace_change option is set' do
       it 'creates a new compare object instead of loading from the DB' do
         expect(diff_with_commits).not_to receive(:load_diffs)
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 7ae70c3afb4..2c75816654e 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1630,28 +1630,17 @@ describe MergeRequest do
   end
 
   describe "#reload_diff" do
-    let(:discussion) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject).to_discussion }
-    let(:commit) { subject.project.commit(sample_commit.id) }
-
-    it "does not change existing merge request diff" do
-      expect(subject.merge_request_diff).not_to receive(:save_git_content)
-      subject.reload_diff
-    end
+    it 'calls MergeRequests::ReloadDiffsService#execute with correct params' do
+      user = create(:user)
+      service = instance_double(MergeRequests::ReloadDiffsService, execute: nil)
 
-    it "creates new merge request diff" do
-      expect { subject.reload_diff }.to change { subject.merge_request_diffs.count }.by(1)
-    end
-
-    it "executes diff cache service" do
-      expect_any_instance_of(MergeRequests::MergeRequestDiffCacheService).to receive(:execute).with(subject, an_instance_of(MergeRequestDiff))
-
-      subject.reload_diff
-    end
+      expect(MergeRequests::ReloadDiffsService)
+        .to receive(:new).with(subject, user)
+        .and_return(service)
 
-    it "calls update_diff_discussion_positions" do
-      expect(subject).to receive(:update_diff_discussion_positions)
+      subject.reload_diff(user)
 
-      subject.reload_diff
+      expect(service).to have_received(:execute)
     end
 
     context 'when using the after_update hook to update' do