Allow external diffs to be used conditionally

Since external diffs are likely to be a bit slower than in-database
ones, add a mode that makes diffs external after they've been obsoleted
by events. This should strike a balance between performance and disk
space.

A background cron drives the majority of migrations, since diffs become
outdated through user actions.

1 files changed, 144 insertions, 3 deletions

diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 53f5307ea0b..0f00ea7e85e 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -51,7 +51,104 @@ describe MergeRequestDiff do
     end
   end
 
-  describe '#latest' do
+  describe '.ids_for_external_storage_migration' do
+    set(:merge_request) { create(:merge_request) }
+    set(:outdated) { merge_request.merge_request_diff }
+    set(:latest) { merge_request.create_merge_request_diff }
+
+    set(:closed_mr) { create(:merge_request, :closed_last_month) }
+    let(:closed) { closed_mr.merge_request_diff }
+
+    set(:merged_mr) { create(:merge_request, :merged_last_month) }
+    let(:merged) { merged_mr.merge_request_diff }
+
+    set(:recently_closed_mr) { create(:merge_request, :closed) }
+    let(:closed_recently) { recently_closed_mr.merge_request_diff }
+
+    set(:recently_merged_mr) { create(:merge_request, :merged) }
+    let(:merged_recently) { recently_merged_mr.merge_request_diff }
+
+    before do
+      merge_request.update!(latest_merge_request_diff: latest)
+    end
+
+    subject { described_class.ids_for_external_storage_migration(limit: 1000) }
+
+    context 'external diffs are disabled' do
+      before do
+        stub_external_diffs_setting(enabled: false)
+      end
+
+      it { is_expected.to be_empty }
+    end
+
+    context 'external diffs are misconfigured' do
+      before do
+        stub_external_diffs_setting(enabled: true, when: 'every second tuesday')
+      end
+
+      it { is_expected.to be_empty }
+    end
+
+    context 'external diffs are enabled unconditionally' do
+      before do
+        stub_external_diffs_setting(enabled: true)
+      end
+
+      it { is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id, closed_recently.id, merged_recently.id) }
+    end
+
+    context 'external diffs are enabled for outdated diffs' do
+      before do
+        stub_external_diffs_setting(enabled: true, when: 'outdated')
+      end
+
+      it 'returns records for outdated merge request versions' do
+        is_expected.to contain_exactly(outdated.id, closed.id, merged.id)
+      end
+    end
+
+    context 'with limit' do
+      it 'respects the limit' do
+        stub_external_diffs_setting(enabled: true)
+
+        expect(described_class.ids_for_external_storage_migration(limit: 3).count).to eq(3)
+      end
+    end
+  end
+
+  describe '#migrate_files_to_external_storage!' do
+    let(:diff) { create(:merge_request).merge_request_diff }
+
+    it 'converts from in-database to external storage' do
+      expect(diff).not_to be_stored_externally
+
+      stub_external_diffs_setting(enabled: true)
+      expect(diff).to receive(:save!)
+
+      diff.migrate_files_to_external_storage!
+
+      expect(diff).to be_stored_externally
+    end
+
+    it 'does nothing with an external diff' do
+      stub_external_diffs_setting(enabled: true)
+
+      expect(diff).to be_stored_externally
+      expect(diff).not_to receive(:save!)
+
+      diff.migrate_files_to_external_storage!
+    end
+
+    it 'does nothing if external diffs are disabled' do
+      expect(diff).not_to be_stored_externally
+      expect(diff).not_to receive(:save!)
+
+      diff.migrate_files_to_external_storage!
+    end
+  end
+
+  describe '#latest?' do
     let!(:mr) { create(:merge_request, :with_diffs) }
     let!(:first_diff) { mr.merge_request_diff }
     let!(:last_diff) { mr.create_merge_request_diff }
@@ -222,14 +319,58 @@ describe MergeRequestDiff do
     include_examples 'merge request diffs'
   end
 
-  describe 'external diffs configured' do
+  describe 'external diffs always enabled' do
     before do
-      stub_external_diffs_setting(enabled: true)
+      stub_external_diffs_setting(enabled: true, when: 'always')
     end
 
     include_examples 'merge request diffs'
   end
 
+  describe 'exernal diffs enabled for outdated diffs' do
+    before do
+      stub_external_diffs_setting(enabled: true, when: 'outdated')
+    end
+
+    include_examples 'merge request diffs'
+
+    it 'stores up-to-date diffs in the database' do
+      expect(diff).not_to be_stored_externally
+    end
+
+    it 'stores diffs for recently closed MRs in the database' do
+      mr = create(:merge_request, :closed)
+
+      expect(mr.merge_request_diff).not_to be_stored_externally
+    end
+
+    it 'stores diffs for recently merged MRs in the database' do
+      mr = create(:merge_request, :merged)
+
+      expect(mr.merge_request_diff).not_to be_stored_externally
+    end
+
+    it 'stores diffs for old MR versions in external storage' do
+      old_diff = diff
+      merge_request.create_merge_request_diff
+      old_diff.migrate_files_to_external_storage!
+
+      expect(old_diff).to be_stored_externally
+    end
+
+    it 'stores diffs for old closed MRs in external storage' do
+      mr = create(:merge_request, :closed_last_month)
+
+      expect(mr.merge_request_diff).to be_stored_externally
+    end
+
+    it 'stores diffs for old merged MRs in external storage' do
+      mr = create(:merge_request, :merged_last_month)
+
+      expect(mr.merge_request_diff).to be_stored_externally
+    end
+  end
+
   describe '#commit_shas' do
     it 'returns all commit SHAs using commits from the DB' do
       expect(diff_with_commits.commit_shas).not_to be_empty