Merge branch 'merge-request-diffs-table' into 'master'

Add table for files in merge request diffs

See merge request !12047

7 files changed, 108 insertions, 12 deletions

diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb
index 26e5d73d333..428b6edb7d6 100644
--- a/spec/lib/gitlab/database_spec.rb
+++ b/spec/lib/gitlab/database_spec.rb
@@ -129,6 +129,55 @@ describe Gitlab::Database, lib: true do
     end
   end
 
+  describe '.bulk_insert' do
+    before do
+      allow(described_class).to receive(:connection).and_return(connection)
+      allow(connection).to receive(:quote_column_name, &:itself)
+      allow(connection).to receive(:quote, &:itself)
+      allow(connection).to receive(:execute)
+    end
+
+    let(:connection) { double(:connection) }
+
+    let(:rows) do
+      [
+        { a: 1, b: 2, c: 3 },
+        { c: 6, a: 4, b: 5 }
+      ]
+    end
+
+    it 'does nothing with empty rows' do
+      expect(connection).not_to receive(:execute)
+
+      described_class.bulk_insert('test', [])
+    end
+
+    it 'uses the ordering from the first row' do
+      expect(connection).to receive(:execute) do |sql|
+        expect(sql).to include('(1, 2, 3)')
+        expect(sql).to include('(4, 5, 6)')
+      end
+
+      described_class.bulk_insert('test', rows)
+    end
+
+    it 'quotes column names' do
+      expect(connection).to receive(:quote_column_name).with(:a)
+      expect(connection).to receive(:quote_column_name).with(:b)
+      expect(connection).to receive(:quote_column_name).with(:c)
+
+      described_class.bulk_insert('test', rows)
+    end
+
+    it 'quotes values' do
+      1.upto(6) do |i|
+        expect(connection).to receive(:quote).with(i)
+      end
+
+      described_class.bulk_insert('test', rows)
+    end
+  end
+
   describe '.create_connection_pool' do
     it 'creates a new connection pool with specific pool size' do
       pool = described_class.create_connection_pool(5)
diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb
index da213f617cc..78d741f0110 100644
--- a/spec/lib/gitlab/git/diff_spec.rb
+++ b/spec/lib/gitlab/git/diff_spec.rb
@@ -90,7 +90,7 @@ EOT
         let(:diff) { described_class.new(@rugged_diff) }
 
         it 'initializes the diff' do
-          expect(diff.to_hash).to eq(@raw_diff_hash.merge(too_large: nil))
+          expect(diff.to_hash).to eq(@raw_diff_hash)
         end
 
         it 'does not prune the diff' do
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 412eb33b35b..a5f09f1856e 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -88,6 +88,9 @@ merge_requests:
 - head_pipeline
 merge_request_diff:
 - merge_request
+- merge_request_diff_files
+merge_request_diff_files:
+- merge_request_diff
 pipelines:
 - project
 - user
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index e3599d6fe59..98c117b4cd8 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -2821,9 +2821,11 @@
             "committer_email": "dmitriy.zaporozhets@gmail.com"
           }
         ],
-        "utf8_st_diffs": [
+        "merge_request_diff_files": [
           {
-            "diff": "Binary files a/.DS_Store and /dev/null differ\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 0,
+            "utf8_diff": "Binary files a/.DS_Store and /dev/null differ\n",
             "new_path": ".DS_Store",
             "old_path": ".DS_Store",
             "a_mode": "100644",
@@ -2834,7 +2836,9 @@
             "too_large": false
           },
           {
-            "diff": "--- a/.gitignore\n+++ b/.gitignore\n@@ -17,3 +17,4 @@ rerun.txt\n pickle-email-*.html\n .project\n config/initializers/secret_token.rb\n+.DS_Store\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 1,
+            "utf8_diff": "--- a/.gitignore\n+++ b/.gitignore\n@@ -17,3 +17,4 @@ rerun.txt\n pickle-email-*.html\n .project\n config/initializers/secret_token.rb\n+.DS_Store\n",
             "new_path": ".gitignore",
             "old_path": ".gitignore",
             "a_mode": "100644",
@@ -2845,7 +2849,9 @@
             "too_large": false
           },
           {
-            "diff": "--- a/.gitmodules\n+++ b/.gitmodules\n@@ -1,3 +1,9 @@\n [submodule \"six\"]\n \tpath = six\n \turl = git://github.com/randx/six.git\n+[submodule \"gitlab-shell\"]\n+\tpath = gitlab-shell\n+\turl = https://github.com/gitlabhq/gitlab-shell.git\n+[submodule \"gitlab-grack\"]\n+\tpath = gitlab-grack\n+\turl = https://gitlab.com/gitlab-org/gitlab-grack.git\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 2,
+            "utf8_diff": "--- a/.gitmodules\n+++ b/.gitmodules\n@@ -1,3 +1,9 @@\n [submodule \"six\"]\n \tpath = six\n \turl = git://github.com/randx/six.git\n+[submodule \"gitlab-shell\"]\n+\tpath = gitlab-shell\n+\turl = https://github.com/gitlabhq/gitlab-shell.git\n+[submodule \"gitlab-grack\"]\n+\tpath = gitlab-grack\n+\turl = https://gitlab.com/gitlab-org/gitlab-grack.git\n",
             "new_path": ".gitmodules",
             "old_path": ".gitmodules",
             "a_mode": "100644",
@@ -2856,7 +2862,9 @@
             "too_large": false
           },
           {
-            "diff": "Binary files a/files/.DS_Store and /dev/null differ\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 3,
+            "utf8_diff": "Binary files a/files/.DS_Store and /dev/null differ\n",
             "new_path": "files/.DS_Store",
             "old_path": "files/.DS_Store",
             "a_mode": "100644",
@@ -2867,7 +2875,9 @@
             "too_large": false
           },
           {
-            "diff": "--- /dev/null\n+++ b/files/ruby/feature.rb\n@@ -0,0 +1,4 @@\n+# This file was changed in feature branch\n+# We put different code here to make merge conflict\n+class Conflict\n+end\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 4,
+            "utf8_diff": "--- /dev/null\n+++ b/files/ruby/feature.rb\n@@ -0,0 +1,4 @@\n+# This file was changed in feature branch\n+# We put different code here to make merge conflict\n+class Conflict\n+end\n",
             "new_path": "files/ruby/feature.rb",
             "old_path": "files/ruby/feature.rb",
             "a_mode": "0",
@@ -2878,7 +2888,9 @@
             "too_large": false
           },
           {
-            "diff": "--- a/files/ruby/popen.rb\n+++ b/files/ruby/popen.rb\n@@ -6,12 +6,18 @@ module Popen\n \n   def popen(cmd, path=nil)\n     unless cmd.is_a?(Array)\n-      raise \"System commands must be given as an array of strings\"\n+      raise RuntimeError, \"System commands must be given as an array of strings\"\n     end\n \n     path ||= Dir.pwd\n-    vars = { \"PWD\" =\u003e path }\n-    options = { chdir: path }\n+\n+    vars = {\n+      \"PWD\" =\u003e path\n+    }\n+\n+    options = {\n+      chdir: path\n+    }\n \n     unless File.directory?(path)\n       FileUtils.mkdir_p(path)\n@@ -19,6 +25,7 @@ module Popen\n \n     @cmd_output = \"\"\n     @cmd_status = 0\n+\n     Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|\n       @cmd_output \u003c\u003c stdout.read\n       @cmd_output \u003c\u003c stderr.read\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 5,
+            "utf8_diff": "--- a/files/ruby/popen.rb\n+++ b/files/ruby/popen.rb\n@@ -6,12 +6,18 @@ module Popen\n \n   def popen(cmd, path=nil)\n     unless cmd.is_a?(Array)\n-      raise \"System commands must be given as an array of strings\"\n+      raise RuntimeError, \"System commands must be given as an array of strings\"\n     end\n \n     path ||= Dir.pwd\n-    vars = { \"PWD\" =\u003e path }\n-    options = { chdir: path }\n+\n+    vars = {\n+      \"PWD\" =\u003e path\n+    }\n+\n+    options = {\n+      chdir: path\n+    }\n \n     unless File.directory?(path)\n       FileUtils.mkdir_p(path)\n@@ -19,6 +25,7 @@ module Popen\n \n     @cmd_output = \"\"\n     @cmd_status = 0\n+\n     Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|\n       @cmd_output \u003c\u003c stdout.read\n       @cmd_output \u003c\u003c stderr.read\n",
             "new_path": "files/ruby/popen.rb",
             "old_path": "files/ruby/popen.rb",
             "a_mode": "100644",
@@ -2889,7 +2901,9 @@
             "too_large": false
           },
           {
-            "diff": "--- a/files/ruby/regex.rb\n+++ b/files/ruby/regex.rb\n@@ -19,14 +19,12 @@ module Gitlab\n     end\n \n     def archive_formats_regex\n-      #|zip|tar|    tar.gz    |         tar.bz2         |\n-      /(zip|tar|tar\\.gz|tgz|gz|tar\\.bz2|tbz|tbz2|tb2|bz2)/\n+      /(zip|tar|7z|tar\\.gz|tgz|gz|tar\\.bz2|tbz|tbz2|tb2|bz2)/\n     end\n \n     def git_reference_regex\n       # Valid git ref regex, see:\n       # https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html\n-\n       %r{\n         (?!\n            (?# doesn't begins with)\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 6,
+            "utf8_diff": "--- a/files/ruby/regex.rb\n+++ b/files/ruby/regex.rb\n@@ -19,14 +19,12 @@ module Gitlab\n     end\n \n     def archive_formats_regex\n-      #|zip|tar|    tar.gz    |         tar.bz2         |\n-      /(zip|tar|tar\\.gz|tgz|gz|tar\\.bz2|tbz|tbz2|tb2|bz2)/\n+      /(zip|tar|7z|tar\\.gz|tgz|gz|tar\\.bz2|tbz|tbz2|tb2|bz2)/\n     end\n \n     def git_reference_regex\n       # Valid git ref regex, see:\n       # https://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html\n-\n       %r{\n         (?!\n            (?# doesn't begins with)\n",
             "new_path": "files/ruby/regex.rb",
             "old_path": "files/ruby/regex.rb",
             "a_mode": "100644",
@@ -2900,7 +2914,9 @@
             "too_large": false
           },
           {
-            "diff": "--- /dev/null\n+++ b/gitlab-grack\n@@ -0,0 +1 @@\n+Subproject commit 645f6c4c82fd3f5e06f67134450a570b795e55a6\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 7,
+            "utf8_diff": "--- /dev/null\n+++ b/gitlab-grack\n@@ -0,0 +1 @@\n+Subproject commit 645f6c4c82fd3f5e06f67134450a570b795e55a6\n",
             "new_path": "gitlab-grack",
             "old_path": "gitlab-grack",
             "a_mode": "0",
@@ -2911,7 +2927,9 @@
             "too_large": false
           },
           {
-            "diff": "--- /dev/null\n+++ b/gitlab-shell\n@@ -0,0 +1 @@\n+Subproject commit 79bceae69cb5750d6567b223597999bfa91cb3b9\n",
+            "merge_request_diff_id": 27,
+            "relative_order": 8,
+            "utf8_diff": "--- /dev/null\n+++ b/gitlab-shell\n@@ -0,0 +1 @@\n+Subproject commit 79bceae69cb5750d6567b223597999bfa91cb3b9\n",
             "new_path": "gitlab-shell",
             "old_path": "gitlab-shell",
             "a_mode": "0",
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 14338515892..c11b15a811b 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -86,8 +86,13 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do
 
       it 'has the correct data for merge request st_diffs' do
         # makes sure we are renaming the custom method +utf8_st_diffs+ into +st_diffs+
+        # one MergeRequestDiff uses the new format, where st_diffs is expected to be nil
 
-        expect(MergeRequestDiff.where.not(st_diffs: nil).count).to eq(9)
+        expect(MergeRequestDiff.where.not(st_diffs: nil).count).to eq(8)
+      end
+
+      it 'has the correct data for merge request diff files' do
+        expect(MergeRequestDiffFile.where.not(diff: nil).count).to eq(9)
       end
 
       it 'has the correct time for merge request st_commits' do
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 5aeb29b7fec..e52f79513f1 100644
--- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
@@ -83,6 +83,10 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do
         expect(saved_project_json['merge_requests'].first['merge_request_diff']['utf8_st_diffs']).not_to be_nil
       end
 
+      it 'has merge request diff files' do
+        expect(saved_project_json['merge_requests'].first['merge_request_diff']['merge_request_diff_files']).not_to be_empty
+      end
+
       it 'has merge requests comments' do
         expect(saved_project_json['merge_requests'].first['notes']).not_to be_empty
       end
@@ -145,6 +149,12 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do
         expect(project_tree_saver.save).to be true
       end
 
+      it 'does not complain about non UTF-8 characters in MR diff files' do
+        ActiveRecord::Base.connection.execute("UPDATE merge_request_diff_files SET diff = '---\n- :diff: !binary |-\n    LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n    KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n    YXR'")
+
+        expect(project_tree_saver.save).to be true
+      end
+
       context 'group members' do
         let(:user2) { create(:user, email: 'group@member.com') }
         let(:member_emails) do
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 50ff6ecc1e0..fadd3ad1330 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
+MergeRequestDiffFile:
+- merge_request_diff_id
+- relative_order
+- new_file
+- renamed_file
+- deleted_file
+- new_path
+- old_path
+- a_mode
+- b_mode
+- too_large
 Ci::Pipeline:
 - id
 - project_id