Add table for merge request commits

This is an ID-less table with just three columns: an association to the merge
request diff the commit belongs to, the relative order of the commit within the
merge request diff, and the commit SHA itself.

Previously we stored much more information about the commits, so that we could
display them even when they were deleted from the repo. Since 8.0, we ensure
that those commits are kept around for as long as the target repo itself is, so
we don't need to duplicate that data in the database.

9 files changed, 70 insertions, 38 deletions

diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index a5f09f1856e..562f0c2991c 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -88,7 +88,10 @@ merge_requests:
 - head_pipeline
 merge_request_diff:
 - merge_request
+- merge_request_diff_commits
 - merge_request_diff_files
+merge_request_diff_commits:
+- merge_request_diff
 merge_request_diff_files:
 - merge_request_diff
 pipelines:
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 98c117b4cd8..469a014e4d2 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -2741,13 +2741,12 @@
       "merge_request_diff": {
         "id": 27,
         "state": "collected",
-        "st_commits": [
+        "merge_request_diff_commits": [
           {
-            "id": "bb5206fee213d983da88c47f9cf4cc6caf9c66dc",
+            "merge_request_diff_id": 27,
+            "relative_order": 0,
+            "sha": "bb5206fee213d983da88c47f9cf4cc6caf9c66dc",
             "message": "Feature conflcit added\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
-            "parent_ids": [
-              "5937ac0a7beb003549fc5fd26fc247adbce4a52e"
-            ],
             "authored_date": "2014-08-06T08:35:52.000+02:00",
             "author_name": "Dmitriy Zaporozhets",
             "author_email": "dmitriy.zaporozhets@gmail.com",
@@ -2756,11 +2755,10 @@
             "committer_email": "dmitriy.zaporozhets@gmail.com"
           },
           {
-            "id": "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
+            "merge_request_diff_id": 27,
+            "relative_order": 1,
+            "sha": "5937ac0a7beb003549fc5fd26fc247adbce4a52e",
             "message": "Add submodule from gitlab.com\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
-            "parent_ids": [
-              "570e7b2abdd848b95f2f578043fc23bd6f6fd24d"
-            ],
             "authored_date": "2014-02-27T10:01:38.000+01:00",
             "author_name": "Dmitriy Zaporozhets",
             "author_email": "dmitriy.zaporozhets@gmail.com",
@@ -2769,11 +2767,10 @@
             "committer_email": "dmitriy.zaporozhets@gmail.com"
           },
           {
-            "id": "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
+            "merge_request_diff_id": 27,
+            "relative_order": 2,
+            "sha": "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
             "message": "Change some files\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
-            "parent_ids": [
-              "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"
-            ],
             "authored_date": "2014-02-27T09:57:31.000+01:00",
             "author_name": "Dmitriy Zaporozhets",
             "author_email": "dmitriy.zaporozhets@gmail.com",
@@ -2782,11 +2779,10 @@
             "committer_email": "dmitriy.zaporozhets@gmail.com"
           },
           {
-            "id": "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9",
+            "merge_request_diff_id": 27,
+            "relative_order": 3,
+            "sha": "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9",
             "message": "More submodules\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
-            "parent_ids": [
-              "d14d6c0abdd253381df51a723d58691b2ee1ab08"
-            ],
             "authored_date": "2014-02-27T09:54:21.000+01:00",
             "author_name": "Dmitriy Zaporozhets",
             "author_email": "dmitriy.zaporozhets@gmail.com",
@@ -2795,11 +2791,10 @@
             "committer_email": "dmitriy.zaporozhets@gmail.com"
           },
           {
-            "id": "d14d6c0abdd253381df51a723d58691b2ee1ab08",
+            "merge_request_diff_id": 27,
+            "relative_order": 4,
+            "sha": "d14d6c0abdd253381df51a723d58691b2ee1ab08",
             "message": "Remove ds_store files\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
-            "parent_ids": [
-              "c1acaa58bbcbc3eafe538cb8274ba387047b69f8"
-            ],
             "authored_date": "2014-02-27T09:49:50.000+01:00",
             "author_name": "Dmitriy Zaporozhets",
             "author_email": "dmitriy.zaporozhets@gmail.com",
@@ -2808,11 +2803,10 @@
             "committer_email": "dmitriy.zaporozhets@gmail.com"
           },
           {
-            "id": "c1acaa58bbcbc3eafe538cb8274ba387047b69f8",
+            "merge_request_diff_id": 27,
+            "relative_order": 5,
+            "sha": "c1acaa58bbcbc3eafe538cb8274ba387047b69f8",
             "message": "Ignore DS files\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
-            "parent_ids": [
-              "ae73cb07c9eeaf35924a10f713b364d32b2dd34f"
-            ],
             "authored_date": "2014-02-27T09:48:32.000+01:00",
             "author_name": "Dmitriy Zaporozhets",
             "author_email": "dmitriy.zaporozhets@gmail.com",
diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
index c11b15a811b..d50d238ddcd 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -95,6 +95,11 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do
         expect(MergeRequestDiffFile.where.not(diff: nil).count).to eq(9)
       end
 
+      it 'has the correct data for merge request diff commits in serialised and table formats' do
+        expect(MergeRequestDiff.where.not(st_commits: nil).count).to eq(7)
+        expect(MergeRequestDiffCommit.count).to eq(6)
+      end
+
       it 'has the correct time for merge request st_commits' do
         st_commits = MergeRequestDiff.where.not(st_commits: nil).first.st_commits
 
diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
index e52f79513f1..22a65e24f26 100644
--- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
@@ -87,6 +87,10 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do
         expect(saved_project_json['merge_requests'].first['merge_request_diff']['merge_request_diff_files']).not_to be_empty
       end
 
+      it 'has merge request diff commits' do
+        expect(saved_project_json['merge_requests'].first['merge_request_diff']['merge_request_diff_commits']).not_to be_empty
+      end
+
       it 'has merge requests comments' do
         expect(saved_project_json['merge_requests'].first['notes']).not_to be_empty
       end
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 697ddf52af9..b4a7e956686 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -172,6 +172,17 @@ MergeRequestDiff:
 - real_size
 - head_commit_sha
 - start_commit_sha
+MergeRequestDiffCommit:
+- merge_request_diff_id
+- relative_order
+- sha
+- authored_date
+- committed_date
+- author_name
+- author_email
+- committer_name
+- committer_email
+- message
 MergeRequestDiffFile:
 - merge_request_diff_id
 - relative_order
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 2b10791ad6d..431fcda165d 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -863,7 +863,7 @@ describe Ci::Build, :models do
         pipeline2 = create(:ci_pipeline, project: project)
         @build2 = create(:ci_build, pipeline: pipeline2)
 
-        allow(@merge_request).to receive(:commits_sha)
+        allow(@merge_request).to receive(:commit_shas)
           .and_return([pipeline.sha, pipeline2.sha])
         allow(MergeRequest).to receive_message_chain(:includes, :where, :reorder).and_return([@merge_request])
       end
diff --git a/spec/models/merge_request_diff_commit_spec.rb b/spec/models/merge_request_diff_commit_spec.rb
new file mode 100644
index 00000000000..dbfd1526518
--- /dev/null
+++ b/spec/models/merge_request_diff_commit_spec.rb
@@ -0,0 +1,15 @@
+require 'rails_helper'
+
+describe MergeRequestDiffCommit, type: :model do
+  let(:merge_request) { create(:merge_request) }
+  subject { merge_request.commits.first }
+
+  describe '#to_hash' do
+    it 'returns the same results as Commit#to_hash, except for parent_ids' do
+      commit_from_repo = merge_request.project.repository.commit(subject.sha)
+      commit_from_repo_hash = commit_from_repo.to_hash.merge(parent_ids: [])
+
+      expect(subject.to_hash).to eq(commit_from_repo_hash)
+    end
+  end
+end
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 4ad4abaa572..edc2f4bb9f0 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -98,7 +98,7 @@ describe MergeRequestDiff, models: true do
     end
 
     it 'saves empty state' do
-      allow_any_instance_of(MergeRequestDiff).to receive(:commits)
+      allow_any_instance_of(MergeRequestDiff).to receive_message_chain(:compare, :commits)
         .and_return([])
 
       mr_diff = create(:merge_request).merge_request_diff
@@ -107,14 +107,14 @@ describe MergeRequestDiff, models: true do
     end
   end
 
-  describe '#commits_sha' do
+  describe '#commit_shas' do
     it 'returns all commits SHA using serialized commits' do
       subject.st_commits = [
         { id: 'sha1' },
         { id: 'sha2' }
       ]
 
-      expect(subject.commits_sha).to eq(%w(sha1 sha2))
+      expect(subject.commit_shas).to eq(%w(sha1 sha2))
     end
   end
 
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index d91f1f1a11c..ea405fabff0 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -720,14 +720,14 @@ describe MergeRequest, models: true do
     subject { create :merge_request, :simple }
   end
 
-  describe '#commits_sha' do
+  describe '#commit_shas' do
     before do
-      allow(subject.merge_request_diff).to receive(:commits_sha)
+      allow(subject.merge_request_diff).to receive(:commit_shas)
         .and_return(['sha1'])
     end
 
     it 'delegates to merge request diff' do
-      expect(subject.commits_sha).to eq ['sha1']
+      expect(subject.commit_shas).to eq ['sha1']
     end
   end
 
@@ -752,7 +752,7 @@ describe MergeRequest, models: true do
   describe '#all_pipelines' do
     shared_examples 'returning pipelines with proper ordering' do
       let!(:all_pipelines) do
-        subject.all_commits_sha.map do |sha|
+        subject.all_commit_shas.map do |sha|
           create(:ci_empty_pipeline,
                  project: subject.source_project,
                  sha: sha,
@@ -794,16 +794,16 @@ describe MergeRequest, models: true do
     end
   end
 
-  describe '#all_commits_sha' do
+  describe '#all_commit_shas' do
     context 'when merge request is persisted' do
-      let(:all_commits_sha) do
+      let(:all_commit_shas) do
         subject.merge_request_diffs.flat_map(&:commits).map(&:sha).uniq
       end
 
       shared_examples 'returning all SHA' do
         it 'returns all SHA from all merge_request_diffs' do
           expect(subject.merge_request_diffs.size).to eq(2)
-          expect(subject.all_commits_sha).to eq(all_commits_sha)
+          expect(subject.all_commit_shas).to eq(all_commit_shas)
         end
       end
 
@@ -834,7 +834,7 @@ describe MergeRequest, models: true do
         end
 
         it 'returns commits from compare commits temporary data' do
-          expect(subject.all_commits_sha).to eq [commit, commit]
+          expect(subject.all_commit_shas).to eq [commit, commit]
         end
       end
 
@@ -842,7 +842,7 @@ describe MergeRequest, models: true do
         subject { build(:merge_request) }
 
         it 'returns array with diff head sha element only' do
-          expect(subject.all_commits_sha).to eq [subject.diff_head_sha]
+          expect(subject.all_commit_shas).to eq [subject.diff_head_sha]
         end
       end
     end