diff options
24 files changed, 271 insertions, 52 deletions
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 99dd2130188..f1ee4d3f7a9 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -10,6 +10,7 @@ class MergeRequestDiff < ActiveRecord::Base VALID_CLASSES = [Hash, Rugged::Patch, Rugged::Diff::Delta].freeze belongs_to :merge_request + has_many :merge_request_diff_files, -> { order(:merge_request_diff_id, :relative_order) } serialize :st_commits # rubocop:disable Cop/ActiverecordSerialize serialize :st_diffs # rubocop:disable Cop/ActiverecordSerialize @@ -91,7 +92,7 @@ class MergeRequestDiff < ActiveRecord::Base head_commit_sha).diffs(options) else @raw_diffs ||= {} - @raw_diffs[options] ||= load_diffs(st_diffs, options) + @raw_diffs[options] ||= load_diffs(options) end end @@ -253,24 +254,44 @@ class MergeRequestDiff < ActiveRecord::Base update_columns_serialized(new_attributes) end - def dump_diffs(diffs) - if diffs.respond_to?(:map) - diffs.map(&:to_hash) + def create_merge_request_diff_files(diffs) + rows = diffs.map.with_index do |diff, index| + diff.to_hash.merge( + merge_request_diff_id: self.id, + relative_order: index + ) end + + Gitlab::Database.bulk_insert('merge_request_diff_files', rows) end - def load_diffs(raw, options) - if valid_raw_diff?(raw) - if paths = options[:paths] - raw = raw.select do |diff| - paths.include?(diff[:old_path]) || paths.include?(diff[:new_path]) - end - end + def load_diffs(options) + return Gitlab::Git::DiffCollection.new([]) unless diffs_from_database - Gitlab::Git::DiffCollection.new(raw, options) - else - Gitlab::Git::DiffCollection.new([]) + raw = diffs_from_database + + if paths = options[:paths] + raw = raw.select do |diff| + paths.include?(diff[:old_path]) || paths.include?(diff[:new_path]) + end end + + Gitlab::Git::DiffCollection.new(raw, options) + end + + def diffs_from_database + return @diffs_from_database if defined?(@diffs_from_database) + + @diffs_from_database = + if st_diffs.present? + if valid_raw_diff?(st_diffs) + st_diffs + end + elsif merge_request_diff_files.present? + merge_request_diff_files + .as_json(only: Gitlab::Git::Diff::SERIALIZE_KEYS) + .map(&:with_indifferent_access) + end end # Load diffs between branches related to current merge request diff from repo @@ -285,11 +306,10 @@ class MergeRequestDiff < ActiveRecord::Base new_attributes[:real_size] = diff_collection.real_size if diff_collection.any? - new_diffs = dump_diffs(diff_collection) new_attributes[:state] = :collected - end - new_attributes[:st_diffs] = new_diffs || [] + create_merge_request_diff_files(diff_collection) + end # Set our state to 'overflow' to make the #empty? and #collected? # methods (generated by StateMachine) return false. diff --git a/app/models/merge_request_diff_file.rb b/app/models/merge_request_diff_file.rb new file mode 100644 index 00000000000..598ebd4d829 --- /dev/null +++ b/app/models/merge_request_diff_file.rb @@ -0,0 +1,11 @@ +class MergeRequestDiffFile < ActiveRecord::Base + include Gitlab::EncodingHelper + + belongs_to :merge_request_diff + + def utf8_diff + return '' if diff.blank? + + encode_utf8(diff) if diff.respond_to?(:encoding) + end +end diff --git a/db/migrate/20170608171156_create_merge_request_diff_files.rb b/db/migrate/20170608171156_create_merge_request_diff_files.rb new file mode 100644 index 00000000000..bf0c0d29adc --- /dev/null +++ b/db/migrate/20170608171156_create_merge_request_diff_files.rb @@ -0,0 +1,22 @@ +class CreateMergeRequestDiffFiles < ActiveRecord::Migration + DOWNTIME = false + + disable_ddl_transaction! + + def change + create_table :merge_request_diff_files, id: false do |t| + t.belongs_to :merge_request_diff, null: false, foreign_key: { on_delete: :cascade } + t.integer :relative_order, null: false + t.boolean :new_file, null: false + t.boolean :renamed_file, null: false + t.boolean :deleted_file, null: false + t.boolean :too_large, null: false + t.string :a_mode, null: false + t.string :b_mode, null: false + t.text :new_path, null: false + t.text :old_path, null: false + t.text :diff, null: false + t.index [:merge_request_diff_id, :relative_order], name: 'index_merge_request_diff_files_on_mr_diff_id_and_order', unique: true + end + end +end diff --git a/db/migrate/20170614115405_merge_request_diff_file_limits_to_mysql.rb b/db/migrate/20170614115405_merge_request_diff_file_limits_to_mysql.rb new file mode 100644 index 00000000000..4c1cf08aa06 --- /dev/null +++ b/db/migrate/20170614115405_merge_request_diff_file_limits_to_mysql.rb @@ -0,0 +1 @@ +require_relative 'merge_request_diff_file_limits_to_mysql' diff --git a/db/migrate/merge_request_diff_file_limits_to_mysql.rb b/db/migrate/merge_request_diff_file_limits_to_mysql.rb new file mode 100644 index 00000000000..3958380e4b9 --- /dev/null +++ b/db/migrate/merge_request_diff_file_limits_to_mysql.rb @@ -0,0 +1,12 @@ +class MergeRequestDiffFileLimitsToMysql < ActiveRecord::Migration + DOWNTIME = false + + def up + return unless Gitlab::Database.mysql? + + change_column :merge_request_diff_files, :diff, :text, limit: 2147483647 + end + + def down + end +end diff --git a/db/schema.rb b/db/schema.rb index 803e36fba5a..f42827991aa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170609183112) do +ActiveRecord::Schema.define(version: 20170614115405) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -692,6 +692,22 @@ ActiveRecord::Schema.define(version: 20170609183112) do add_index "members", ["source_id", "source_type"], name: "index_members_on_source_id_and_source_type", using: :btree add_index "members", ["user_id"], name: "index_members_on_user_id", using: :btree + create_table "merge_request_diff_files", id: false, force: :cascade do |t| + t.integer "merge_request_diff_id", null: false + t.integer "relative_order", null: false + t.boolean "new_file", null: false + t.boolean "renamed_file", null: false + t.boolean "deleted_file", null: false + t.boolean "too_large", null: false + t.string "a_mode", null: false + t.string "b_mode", null: false + t.text "new_path", null: false + t.text "old_path", null: false + t.text "diff", null: false + end + + add_index "merge_request_diff_files", ["merge_request_diff_id", "relative_order"], name: "index_merge_request_diff_files_on_mr_diff_id_and_order", unique: true, using: :btree + create_table "merge_request_diffs", force: :cascade do |t| t.string "state" t.text "st_commits" @@ -1530,6 +1546,7 @@ ActiveRecord::Schema.define(version: 20170609183112) do add_foreign_key "labels", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "lists", "boards" add_foreign_key "lists", "labels" + add_foreign_key "merge_request_diff_files", "merge_request_diffs", on_delete: :cascade add_foreign_key "merge_request_metrics", "ci_pipelines", column: "pipeline_id", on_delete: :cascade add_foreign_key "merge_request_metrics", "merge_requests", on_delete: :cascade add_foreign_key "merge_requests_closing_issues", "issues", on_delete: :cascade diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md index 58d2fd76c61..35960ade3d4 100644 --- a/doc/user/project/settings/import_export.md +++ b/doc/user/project/settings/import_export.md @@ -27,14 +27,15 @@ with all their related data and be moved into a new GitLab instance. | GitLab version | Import/Export version | | -------- | -------- | -| 9.2.0 to current | 0.1.7 | -| 8.17.0 | 0.1.6 | -| 8.13.0 | 0.1.5 | -| 8.12.0 | 0.1.4 | -| 8.10.3 | 0.1.3 | -| 8.10.0 | 0.1.2 | -| 8.9.5 | 0.1.1 | -| 8.9.0 | 0.1.0 | +| 9.4.0 to current | 0.1.8 | +| 9.2.0 | 0.1.7 | +| 8.17.0 | 0.1.6 | +| 8.13.0 | 0.1.5 | +| 8.12.0 | 0.1.4 | +| 8.10.3 | 0.1.3 | +| 8.10.0 | 0.1.2 | +| 8.9.5 | 0.1.1 | +| 8.9.0 | 0.1.0 | > The table reflects what GitLab version we updated the Import/Export version at. > For instance, 8.10.3 and 8.11 will have the same Import/Export version (0.1.3) diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index d0bd1299671..0d5a7cf0694 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -83,6 +83,22 @@ module Gitlab end end + def self.bulk_insert(table, rows) + return if rows.empty? + + keys = rows.first.keys + columns = keys.map { |key| connection.quote_column_name(key) } + + tuples = rows.map do |row| + row.values_at(*keys).map { |value| connection.quote(value) } + end + + connection.execute <<-EOF.strip_heredoc + INSERT INTO #{table} (#{columns.join(', ')}) + VALUES #{tuples.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')} + EOF + end + # pool_size - The size of the DB pool. # host - An optional host name to use instead of the default one. def self.create_connection_pool(pool_size, host = nil) diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index 4b689f0e94f..f825568f194 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -16,11 +16,11 @@ module Gitlab alias_method :renamed_file?, :renamed_file attr_accessor :expanded + attr_writer :too_large alias_method :expanded?, :expanded - # We need this accessor because of `to_hash` and `init_from_hash` - attr_accessor :too_large + SERIALIZE_KEYS = %i(diff new_path old_path a_mode b_mode new_file renamed_file deleted_file too_large).freeze class << self # The maximum size of a diff to display. @@ -231,16 +231,10 @@ module Gitlab end end - def serialize_keys - @serialize_keys ||= %i(diff new_path old_path a_mode b_mode new_file renamed_file deleted_file too_large) - end - def to_hash hash = {} - keys = serialize_keys - - keys.each do |key| + SERIALIZE_KEYS.each do |key| hash[key] = send(key) end @@ -267,6 +261,9 @@ module Gitlab end end + # This is used by `to_hash` and `init_from_hash`. + alias_method :too_large, :too_large? + def too_large! @diff = '' @line_count = 0 @@ -315,7 +312,7 @@ module Gitlab def init_from_hash(hash) raw_diff = hash.symbolize_keys - serialize_keys.each do |key| + SERIALIZE_KEYS.each do |key| send(:"#{key}=", raw_diff[key.to_sym]) end end diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb index 27d5a9198b6..3470a09eaf0 100644 --- a/lib/gitlab/import_export.rb +++ b/lib/gitlab/import_export.rb @@ -3,7 +3,7 @@ module Gitlab extend self # For every version update, the version history in import_export.md has to be kept up to date. - VERSION = '0.1.7'.freeze + VERSION = '0.1.8'.freeze FILENAME_LIMIT = 50 def export_path(relative_path:) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index ff2b1d08c3c..72183e8aad4 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -26,7 +26,8 @@ project_tree: - notes: - :author - :events - - :merge_request_diff + - merge_request_diff: + - :merge_request_diff_files - :events - :timelogs - label_links: @@ -92,6 +93,8 @@ excluded_attributes: - :expired_at merge_request_diff: - :st_diffs + merge_request_diff_files: + - :diff issues: - :milestone_id merge_requests: @@ -113,6 +116,8 @@ methods: - :type merge_request_diff: - :utf8_st_diffs + merge_request_diff_files: + - :utf8_diff merge_requests: - :diff_head_sha project: diff --git a/lib/gitlab/import_export/json_hash_builder.rb b/lib/gitlab/import_export/json_hash_builder.rb index 48c09dafcb6..b48f63bcd7e 100644 --- a/lib/gitlab/import_export/json_hash_builder.rb +++ b/lib/gitlab/import_export/json_hash_builder.rb @@ -83,7 +83,9 @@ module Gitlab # +value+ existing model to be included in the hash # +json_config_hash+ the original hash containing the root model def add_model_value(current_key, value, json_config_hash) - @attributes_finder.parse(value) { |hash| value = { value => hash } } + @attributes_finder.parse(value) do |hash| + value = { value => hash } unless value.is_a?(Hash) + end add_to_array(current_key, json_config_hash, value) end diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 695852526cb..20580459046 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -71,6 +71,7 @@ module Gitlab @relation_hash['data'].deep_symbolize_keys! if @relation_name == :events && @relation_hash['data'] set_st_diff_commits if @relation_name == :merge_request_diff + set_diff if @relation_name == :merge_request_diff_files end def update_user_references @@ -202,6 +203,10 @@ module Gitlab HashUtil.deep_symbolize_array_with_date!(@relation_hash['st_commits']) end + def set_diff + @relation_hash['diff'] = @relation_hash.delete('utf8_diff') + end + def existing_or_new_object # Only find existing records to avoid mapping tables such as milestones # Otherwise always create the record, skipping the extra SELECT clause. diff --git a/lib/tasks/migrate/add_limits_mysql.rake b/lib/tasks/migrate/add_limits_mysql.rake index 761f275d42a..151f42a2222 100644 --- a/lib/tasks/migrate/add_limits_mysql.rake +++ b/lib/tasks/migrate/add_limits_mysql.rake @@ -1,9 +1,11 @@ require Rails.root.join('db/migrate/limits_to_mysql') require Rails.root.join('db/migrate/markdown_cache_limits_to_mysql') +require Rails.root.join('db/migrate/merge_request_diff_file_limits_to_mysql') desc "GitLab | Add limits to strings in mysql database" task add_limits_mysql: :environment do puts "Adding limits to schema.rb for mysql" LimitsToMysql.new.up MarkdownCacheLimitsToMysql.new.up + MergeRequestDiffFileLimitsToMysql.new.up end diff --git a/spec/features/projects/import_export/test_project_export.tar.gz b/spec/features/projects/import_export/test_project_export.tar.gz Binary files differindex 4efd5a26a82..e03e7b88174 100644 --- a/spec/features/projects/import_export/test_project_export.tar.gz +++ b/spec/features/projects/import_export/test_project_export.tar.gz 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 diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb new file mode 100644 index 00000000000..7276f5b5061 --- /dev/null +++ b/spec/models/merge_request_diff_file_spec.rb @@ -0,0 +1,11 @@ +require 'rails_helper' + +describe MergeRequestDiffFile, type: :model do + describe '#utf8_diff' do + it 'does not raise error when a hash value is in binary' do + subject.diff = "\x05\x00\x68\x65\x6c\x6c\x6f" + + expect { subject.utf8_diff }.not_to raise_error + end + end +end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 25f7062860b..4ad4abaa572 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -37,7 +37,7 @@ describe MergeRequestDiff, models: true do context 'when the raw diffs are empty' do before do - mr_diff.update_attributes(st_diffs: '') + MergeRequestDiffFile.delete_all(merge_request_diff_id: mr_diff.id) end it 'returns an empty DiffCollection' do @@ -48,6 +48,7 @@ describe MergeRequestDiff, models: true do context 'when the raw diffs have invalid content' do before do + MergeRequestDiffFile.delete_all(merge_request_diff_id: mr_diff.id) mr_diff.update_attributes(st_diffs: ["--broken-diff"]) end |