diff --git a/CHANGELOG b/CHANGELOG index 56b7d2a485f..0192c593288 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -125,6 +125,7 @@ v 8.10.0 (unreleased) - Allow empty repositories on project import/export - Render only commit message title in builds (Katarzyna Kobierska Ula Budziszewska) - Allow bulk (un)subscription from issues in issue index + - Fix MR diff encoding issues exporting GitLab projects v 8.9.6 - Fix importing of events under notes for GitLab projects. !5154 diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index feaba925bad..3f520c8f3ff 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -1,6 +1,7 @@ class MergeRequestDiff < ActiveRecord::Base include Sortable include Importable + include EncodingHelper # Prevent store of diff if commits amount more then 500 COMMITS_SAFE_SIZE = 100 @@ -211,6 +212,14 @@ class MergeRequestDiff < ActiveRecord::Base branch_base_commit.try(:sha) end + def utf8_st_diffs + st_diffs.map do |diff| + diff.each do |k, v| + diff[k] = encode_utf8(v) if v.respond_to?(:encoding) + end + end + end + # # #save or #update_attributes providing changes on serialized attributes do a lot of # serialization and deserialization calls resulting in bad performance. diff --git a/lib/gitlab/import_export.rb b/lib/gitlab/import_export.rb index bab2ea73c4f..d6d14bd98a0 100644 --- a/lib/gitlab/import_export.rb +++ b/lib/gitlab/import_export.rb @@ -2,7 +2,7 @@ module Gitlab module ImportExport extend self - VERSION = '0.1.1' + VERSION = '0.1.2' 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 05f4ad527ac..15afe8174a4 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -53,7 +53,11 @@ included_attributes: excluded_attributes: snippets: - :expired_at + merge_request_diff: + - :st_diffs methods: statuses: - - :type \ No newline at end of file + - :type + merge_request_diff: + - :utf8_st_diffs \ No newline at end of file diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 6ba25a31641..e41c7e6bf4f 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -33,6 +33,7 @@ module Gitlab update_project_references reset_ci_tokens if @relation_name == 'Ci::Trigger' @relation_hash['data'].deep_symbolize_keys! if @relation_name == :events && @relation_hash['data'] + set_st_diffs if @relation_name == :merge_request_diff generate_imported_object end @@ -129,6 +130,10 @@ module Gitlab def parsed_relation_hash @relation_hash.reject { |k, _v| !relation_class.attribute_method?(k) } end + + def set_st_diffs + @relation_hash['st_diffs'] = @relation_hash.delete('utf8_st_diffs') + end end end end diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 4113d829c3c..b1a5d72c624 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -2765,7 +2765,7 @@ "committer_email": "dmitriy.zaporozhets@gmail.com" } ], - "st_diffs": [ + "utf8_st_diffs": [ { "diff": "Binary files a/.DS_Store and /dev/null differ\n", "new_path": ".DS_Store", @@ -3138,7 +3138,7 @@ "committer_email": "dmitriy.zaporozhets@gmail.com" } ], - "st_diffs": [ + "utf8_st_diffs": [ { "diff": "--- /dev/null\n+++ b/files/ruby/feature.rb\n@@ -0,0 +1,5 @@\n+class Feature\n+ def foo\n+ puts 'bar'\n+ end\n+end\n", "new_path": "files/ruby/feature.rb", @@ -3423,7 +3423,7 @@ "committer_email": "james@jameslopez.es" } ], - "st_diffs": [ + "utf8_st_diffs": [ { "diff": "--- /dev/null\n+++ b/test\n", "new_path": "test", @@ -3960,7 +3960,7 @@ "committer_email": "dmitriy.zaporozhets@gmail.com" } ], - "st_diffs": [ + "utf8_st_diffs": [ { "diff": "Binary files a/.DS_Store and /dev/null differ\n", "new_path": ".DS_Store", @@ -4597,7 +4597,7 @@ "committer_email": "marmis85@gmail.com" } ], - "st_diffs": [ + "utf8_st_diffs": [ { "diff": "--- a/CHANGELOG\n+++ b/CHANGELOG\n@@ -1,4 +1,6 @@\n-v 6.7.0\n+v6.8.0\n+\n+v6.7.0\n - Add support for Gemnasium as a Project Service (Olivier Gonzalez)\n - Add edit file button to MergeRequest diff\n - Public groups (Jason Hollingsworth)\n", "new_path": "CHANGELOG", @@ -5108,7 +5108,7 @@ "committer_email": "stanhu@packetzoom.com" } ], - "st_diffs": [ + "utf8_st_diffs": [ { "diff": "--- a/CHANGELOG\n+++ b/CHANGELOG\n@@ -1,4 +1,6 @@\n-v 6.7.0\n+v6.8.0\n+\n+v6.7.0\n - Add support for Gemnasium as a Project Service (Olivier Gonzalez)\n - Add edit file button to MergeRequest diff\n - Public groups (Jason Hollingsworth)\n", "new_path": "CHANGELOG", @@ -5434,7 +5434,7 @@ "id": 11, "state": "empty", "st_commits": null, - "st_diffs": [ + "utf8_st_diffs": [ ], "merge_request_id": 11, @@ -5961,7 +5961,7 @@ "committer_email": "dmitriy.zaporozhets@gmail.com" } ], - "st_diffs": [ + "utf8_st_diffs": [ { "diff": "Binary files a/.DS_Store and /dev/null differ\n", "new_path": ".DS_Store", @@ -6400,7 +6400,7 @@ "committer_email": "james@jameslopez.es" } ], - "st_diffs": [ + "utf8_st_diffs": [ { "diff": "--- /dev/null\n+++ b/test\n", "new_path": "test", 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 877be300262..6ae20c943b1 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do describe 'restore project tree' do + let(:user) { create(:user) } let(:namespace) { create(:namespace, owner: user) } let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: "", project_path: 'path') } @@ -53,6 +54,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do expect(event.note.noteable.project).not_to be_nil end end + + 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+ + + expect { restored_project_json }.to change(MergeRequestDiff.where.not(st_diffs: nil), :count).by(9) + end end end end 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 1424de9e60b..057ef6e76a0 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -102,12 +102,17 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do it 'has ci pipeline notes' do expect(saved_project_json['pipelines'].first['notes']).not_to be_empty end + + it 'does not complain about non UTF-8 characters in MR diffs' do + ActiveRecord::Base.connection.execute("UPDATE merge_request_diffs SET st_diffs = '---\n- :diff: !binary |-\n LS0tIC9kZXYvbnVsbAorKysgYi9pbWFnZXMvbnVjb3IucGRmCkBAIC0wLDAg\n KzEsMTY3OSBAQAorJVBERi0xLjUNJeLjz9MNCisxIDAgb2JqDTw8L01ldGFk\n YXR'") + + expect(project_tree_saver.save).to be true + end end end def setup_project issue = create(:issue, assignee: user) - merge_request = create(:merge_request) label = create(:label) snippet = create(:project_snippet) release = create(:release) @@ -115,12 +120,12 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do project = create(:project, :public, issues: [issue], - merge_requests: [merge_request], labels: [label], snippets: [snippet], releases: [release] ) + merge_request = create(:merge_request, source_project: project) commit_status = create(:commit_status, project: project) ci_pipeline = create(:ci_pipeline,