Merge branch '36630-activerecord-statementinvalid-pg-notnullviolation-error-null-value-in-column-a_mode-violates-not-null-constraint' into 'master'

Resolve "ActiveRecord::StatementInvalid: PG::NotNullViolation: ERROR:  null value in column "a_mode" violates not-null constraint"

Closes #36630

See merge request !13663

2 files changed, 24 insertions, 1 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 310a69a4bd4..3fde1b09efb 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,12 +81,15 @@ module Gitlab
             relative_order: index
           )
 
-          # Compatibility with old diffs created with Psych.
           diff_hash.tap do |hash|
             diff_text = hash[:diff]
 
             hash[:too_large] = !!hash[:too_large]
 
+            hash[:a_mode] ||= guess_mode(hash[:new_file], hash[:diff])
+            hash[:b_mode] ||= guess_mode(hash[:deleted_file], hash[:diff])
+
+            # Compatibility with old diffs created with Psych.
             if diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only?
               hash[:binary] = true
               hash[:diff] = [diff_text].pack('m0')
@@ -97,6 +100,15 @@ module Gitlab
         [commit_rows, file_rows]
       end
 
+      # This doesn't have to be 100% accurate, because it's only used for
+      # display - it won't change file modes in the repository. Submodules are
+      # created as 600, regular files as 644.
+      def guess_mode(file_missing, diff)
+        return '0' if file_missing
+
+        diff.include?('Subproject commit') ? '160000' : '100644'
+      end
+
       # Unlike MergeRequestDiff#valid_raw_diff?, don't count Rugged objects as
       # valid, because we don't render them usefully anyway.
       def valid_raw_diffs?(diffs)
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 c5ec23b1ac6..22834a47111 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
@@ -134,6 +134,17 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do
       include_examples 'updated MR diff'
     end
 
+    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_diffs) { diffs_to_hashes(merge_request_diff.merge_request_diff_files) }
+
+      let(:diffs) do
+        expected_diffs.map { |diff| diff.except(:a_mode, :b_mode) }
+      end
+
+      include_examples 'updated MR diff'
+    end
+
     context 'when the merge request diffs have binary content' do
       let(:commits) { merge_request_diff.commits.map(&:to_hash) }
       let(:expected_diffs) { diffs }