Handle null serialised commits in background migration39181-gitlab-backgroundmigration-deserializemergerequestdiffsandcommits-error-nomethoderror-undefined-method-map-for-nil-nilclass

This is already handled for diffs, but not commits.

2 files changed, 20 insertions, 2 deletions

diff --git a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb
index 8e5c95f2287..380802258f5 100644
--- a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb
+++ b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb
@@ -81,6 +81,7 @@ module Gitlab
       def single_diff_rows(merge_request_diff)
         sha_attribute = Gitlab::Database::ShaAttribute.new
         commits = YAML.load(merge_request_diff.st_commits) rescue []
+        commits ||= []
 
         commit_rows = commits.map.with_index do |commit, index|
           commit_hash = commit.to_hash.with_indifferent_access.except(:parent_ids)
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 d2e7243ee05..4d3fdbd9554 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
@@ -31,8 +31,8 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
       end
 
       it 'creates correct entries in the merge_request_diff_commits table' do
-        expect(updated_merge_request_diff.merge_request_diff_commits.count).to eq(commits.count)
-        expect(updated_merge_request_diff.commits.map(&:to_hash)).to eq(commits)
+        expect(updated_merge_request_diff.merge_request_diff_commits.count).to eq(expected_commits.count)
+        expect(updated_merge_request_diff.commits.map(&:to_hash)).to eq(expected_commits)
       end
 
       it 'creates correct entries in the merge_request_diff_files table' do
@@ -199,6 +199,16 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
 
     context 'when the merge request diff has valid commits and diffs' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
+      let(:expected_commits) { commits }
+      let(:diffs) { diffs_to_hashes(merge_request_diff.merge_request_diff_files) }
+      let(:expected_diffs) { diffs }
+
+      include_examples 'updated MR diff'
+    end
+
+    context 'when the merge request diff has diffs but no commits' do
+      let(:commits) { nil }
+      let(:expected_commits) { [] }
       let(:diffs) { diffs_to_hashes(merge_request_diff.merge_request_diff_files) }
       let(:expected_diffs) { diffs }
 
@@ -207,6 +217,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
 
     context 'when the merge request diffs do not have too_large set' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
+      let(:expected_commits) { commits }
       let(:expected_diffs) { diffs_to_hashes(merge_request_diff.merge_request_diff_files) }
 
       let(:diffs) do
@@ -218,6 +229,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
 
     context 'when the merge request diffs do not have a_mode and b_mode set' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
+      let(:expected_commits) { commits }
       let(:expected_diffs) { diffs_to_hashes(merge_request_diff.merge_request_diff_files) }
 
       let(:diffs) do
@@ -229,6 +241,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
 
     context 'when the merge request diffs have binary content' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
+      let(:expected_commits) { commits }
       let(:expected_diffs) { diffs }
 
       # The start of a PDF created by Illustrator
@@ -257,6 +270,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
 
     context 'when the merge request diff has commits, but no diffs' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
+      let(:expected_commits) { commits }
       let(:diffs) { [] }
       let(:expected_diffs) { diffs }
 
@@ -265,6 +279,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
 
     context 'when the merge request diffs have invalid content' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
+      let(:expected_commits) { commits }
       let(:diffs) { ['--broken-diff'] }
       let(:expected_diffs) { [] }
 
@@ -274,6 +289,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
     context 'when the merge request diffs are Rugged::Patch instances' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
       let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) }
+      let(:expected_commits) { commits }
       let(:diffs) { first_commit.rugged_diff_from_parent.patches }
       let(:expected_diffs) { [] }
 
@@ -283,6 +299,7 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :t
     context 'when the merge request diffs are Rugged::Diff::Delta instances' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
       let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) }
+      let(:expected_commits) { commits }
       let(:diffs) { first_commit.rugged_diff_from_parent.deltas }
       let(:expected_diffs) { [] }