1 files changed, 102 insertions, 20 deletions
diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb
index c0427639746..d2e7243ee05 100644
--- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb
+++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb
@@ -1,9 +1,9 @@
 require 'spec_helper'
 
-describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do
+describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :truncate do
   describe '#perform' do
-    set(:merge_request) { create(:merge_request) }
-    set(:merge_request_diff) { merge_request.merge_request_diff }
+    let(:merge_request) { create(:merge_request) }
+    let(:merge_request_diff) { merge_request.merge_request_diff }
     let(:updated_merge_request_diff) { MergeRequestDiff.find(merge_request_diff.id) }
 
     def diffs_to_hashes(diffs)
@@ -70,8 +70,8 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do
       before do
         merge_request.reload_diff(true)
 
-        convert_to_yaml(start_id, merge_request_diff.commits, merge_request_diff.diffs)
-        convert_to_yaml(stop_id, updated_merge_request_diff.commits, updated_merge_request_diff.diffs)
+        convert_to_yaml(start_id, merge_request_diff.commits, diffs_to_hashes(merge_request_diff.merge_request_diff_files))
+        convert_to_yaml(stop_id, updated_merge_request_diff.commits, diffs_to_hashes(updated_merge_request_diff.merge_request_diff_files))
 
         MergeRequestDiffCommit.delete_all
         MergeRequestDiffFile.delete_all
@@ -80,10 +80,32 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do
       context 'when BUFFER_ROWS is exceeded' do
         before do
           stub_const("#{described_class}::BUFFER_ROWS", 1)
+
+          allow(Gitlab::Database).to receive(:bulk_insert).and_call_original
+        end
+
+        it 'inserts commit rows in chunks of BUFFER_ROWS' do
+          # There are 29 commits in each diff, so we should have slices of 20 + 9 + 20 + 9.
+          stub_const("#{described_class}::BUFFER_ROWS", 20)
+
+          expect(Gitlab::Database).to receive(:bulk_insert)
+                                        .with('merge_request_diff_commits', anything)
+                                        .exactly(4)
+                                        .times
+                                        .and_call_original
+
+          subject.perform(start_id, stop_id)
         end
 
-        it 'updates and continues' do
-          expect(described_class::MergeRequestDiff).to receive(:transaction).twice
+        it 'inserts diff rows in chunks of DIFF_FILE_BUFFER_ROWS' do
+          # There are 20 files in each diff, so we should have slices of 20 + 20.
+          stub_const("#{described_class}::DIFF_FILE_BUFFER_ROWS", 20)
+
+          expect(Gitlab::Database).to receive(:bulk_insert)
+                                        .with('merge_request_diff_files', anything)
+                                        .exactly(2)
+                                        .times
+                                        .and_call_original
 
           subject.perform(start_id, stop_id)
         end
@@ -91,27 +113,87 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do
 
       context 'when BUFFER_ROWS is not exceeded' do
         it 'only updates once' do
-          expect(described_class::MergeRequestDiff).to receive(:transaction).once
+          expect(Gitlab::Database).to receive(:bulk_insert)
+                                        .with('merge_request_diff_commits', anything)
+                                        .once
+                                        .and_call_original
+
+          expect(Gitlab::Database).to receive(:bulk_insert)
+                                        .with('merge_request_diff_files', anything)
+                                        .once
+                                        .and_call_original
 
           subject.perform(start_id, stop_id)
         end
       end
-    end
 
-    context 'when the merge request diff update fails' do
-      before do
-        allow(described_class::MergeRequestDiff)
-          .to receive(:update_all).and_raise(ActiveRecord::Rollback)
-      end
+      context 'when some rows were already inserted due to a previous failure' do
+        before do
+          subject.perform(start_id, stop_id)
 
-      it 'does not add any diff commits' do
-        expect { subject.perform(merge_request_diff.id, merge_request_diff.id) }
-          .not_to change { MergeRequestDiffCommit.count }
+          convert_to_yaml(start_id, merge_request_diff.commits, diffs_to_hashes(merge_request_diff.merge_request_diff_files))
+          convert_to_yaml(stop_id, updated_merge_request_diff.commits, diffs_to_hashes(updated_merge_request_diff.merge_request_diff_files))
+        end
+
+        it 'does not raise' do
+          expect { subject.perform(start_id, stop_id) }.not_to raise_exception
+        end
+
+        it 'logs a message' do
+          expect(Rails.logger).to receive(:info)
+                                    .with(
+                                      a_string_matching(described_class.name).and(matching([start_id, stop_id].inspect))
+                                    )
+                                    .twice
+
+          subject.perform(start_id, stop_id)
+        end
+
+        it 'ends up with the correct rows' do
+          expect(updated_merge_request_diff.commits.count).to eq(29)
+          expect(updated_merge_request_diff.raw_diffs.count).to eq(20)
+        end
       end
 
-      it 'does not add any diff files' do
-        expect { subject.perform(merge_request_diff.id, merge_request_diff.id) }
-          .not_to change { MergeRequestDiffFile.count }
+      context 'when the merge request diff update fails' do
+        let(:exception) { ActiveRecord::RecordNotFound }
+
+        let(:perform_ignoring_exceptions) do
+          begin
+            subject.perform(start_id, stop_id)
+          rescue described_class::Error
+          end
+        end
+
+        before do
+          allow_any_instance_of(described_class::MergeRequestDiff::ActiveRecord_Relation)
+            .to receive(:update_all).and_raise(exception)
+        end
+
+        it 'raises an error' do
+          expect { subject.perform(start_id, stop_id) }
+            .to raise_exception(described_class::Error)
+        end
+
+        it 'logs the error' do
+          expect(Rails.logger).to receive(:info).with(
+            a_string_matching(described_class.name)
+              .and(matching([start_id, stop_id].inspect))
+              .and(matching(exception.name))
+          )
+
+          perform_ignoring_exceptions
+        end
+
+        it 'still adds diff commits' do
+          expect { perform_ignoring_exceptions }
+            .to change { MergeRequestDiffCommit.count }
+        end
+
+        it 'still adds diff files' do
+          expect { perform_ignoring_exceptions }
+            .to change { MergeRequestDiffFile.count }
+        end
       end
     end