From 22d7ae80020e3d581d7bded2c2f3d5606a5e48ee Mon Sep 17 00:00:00 2001
From: James Lopez <james@gitlab.com>
Date: Tue, 4 Apr 2017 16:34:19 +0000
Subject: Fix issues importing forked projects

---
 app/models/merge_request_diff.rb                   | 18 ++++----
 changelogs/unreleased/fix-import-fork.yml          |  4 ++
 lib/gitlab/import_export/hash_util.rb              | 25 +++++++++++
 lib/gitlab/import_export/import_export.yml         |  2 +
 lib/gitlab/import_export/importer.rb               |  2 +-
 lib/gitlab/import_export/merge_request_parser.rb   | 41 ++++++++++++++++++
 lib/gitlab/import_export/project_tree_restorer.rb  |  2 +-
 lib/gitlab/import_export/relation_factory.rb       | 17 +++++---
 spec/lib/gitlab/import_export/fork_spec.rb         | 49 ++++++++++++++++++++++
 spec/lib/gitlab/import_export/hash_util_spec.rb    | 28 +++++++++++++
 .../import_export/merge_request_parser_spec.rb     | 31 ++++++++++++++
 .../import_export/project_tree_restorer_spec.rb    |  6 +++
 .../import_export/project_tree_saver_spec.rb       |  4 ++
 .../gitlab/import_export/relation_factory_spec.rb  |  2 +-
 spec/lib/gitlab/import_export/repo_bundler_spec.rb | 24 -----------
 spec/lib/gitlab/import_export/repo_saver_spec.rb   | 24 +++++++++++
 16 files changed, 239 insertions(+), 40 deletions(-)
 create mode 100644 changelogs/unreleased/fix-import-fork.yml
 create mode 100644 lib/gitlab/import_export/hash_util.rb
 create mode 100644 lib/gitlab/import_export/merge_request_parser.rb
 create mode 100644 spec/lib/gitlab/import_export/fork_spec.rb
 create mode 100644 spec/lib/gitlab/import_export/hash_util_spec.rb
 create mode 100644 spec/lib/gitlab/import_export/merge_request_parser_spec.rb
 delete mode 100644 spec/lib/gitlab/import_export/repo_bundler_spec.rb
 create mode 100644 spec/lib/gitlab/import_export/repo_saver_spec.rb

diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index baee00b8fcd..6ad56b842b2 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -177,6 +177,16 @@ class MergeRequestDiff < ActiveRecord::Base
     st_commits.count
   end
 
+  def utf8_st_diffs
+    return [] if st_diffs.blank?
+
+    st_diffs.map do |diff|
+      diff.each do |k, v|
+        diff[k] = encode_utf8(v) if v.respond_to?(:encoding)
+      end
+    end
+  end
+
   private
 
   # Old GitLab implementations may have generated diffs as ["--broken-diff"].
@@ -270,14 +280,6 @@ class MergeRequestDiff < ActiveRecord::Base
     project.merge_base_commit(head_commit_sha, start_commit_sha).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/changelogs/unreleased/fix-import-fork.yml b/changelogs/unreleased/fix-import-fork.yml
new file mode 100644
index 00000000000..ff8dd131995
--- /dev/null
+++ b/changelogs/unreleased/fix-import-fork.yml
@@ -0,0 +1,4 @@
+---
+title: Fix Import/Export MR diffs not showing and missing forked MRs
+merge_request:
+author:
diff --git a/lib/gitlab/import_export/hash_util.rb b/lib/gitlab/import_export/hash_util.rb
new file mode 100644
index 00000000000..d4adeeb3797
--- /dev/null
+++ b/lib/gitlab/import_export/hash_util.rb
@@ -0,0 +1,25 @@
+module Gitlab
+  module ImportExport
+    class HashUtil
+      def self.deep_symbolize_array!(array)
+        return if array.blank?
+
+        array.map! do |hash|
+          hash.deep_symbolize_keys!
+
+          yield(hash) if block_given?
+
+          hash
+        end
+      end
+
+      def self.deep_symbolize_array_with_date!(array)
+        self.deep_symbolize_array!(array) do |hash|
+          hash.select { |k, _v| k.to_s.end_with?('_date') }.each do |key, value|
+            hash[key] = Time.zone.parse(value)
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml
index ab74c8782f6..f69288f7d67 100644
--- a/lib/gitlab/import_export/import_export.yml
+++ b/lib/gitlab/import_export/import_export.yml
@@ -89,3 +89,5 @@ methods:
     - :type
   merge_request_diff:
     - :utf8_st_diffs
+  merge_requests:
+    - :diff_head_sha
diff --git a/lib/gitlab/import_export/importer.rb b/lib/gitlab/import_export/importer.rb
index 063ce74ecad..fbdd74788bc 100644
--- a/lib/gitlab/import_export/importer.rb
+++ b/lib/gitlab/import_export/importer.rb
@@ -9,7 +9,7 @@ module Gitlab
       end
 
       def execute
-        if import_file && check_version! && [project_tree, avatar_restorer, repo_restorer, wiki_restorer, uploads_restorer].all?(&:restore)
+        if import_file && check_version! && [repo_restorer, wiki_restorer, project_tree, avatar_restorer, uploads_restorer].all?(&:restore)
           project_tree.restored_project
         else
           raise Projects::ImportService::Error.new(@shared.errors.join(', '))
diff --git a/lib/gitlab/import_export/merge_request_parser.rb b/lib/gitlab/import_export/merge_request_parser.rb
new file mode 100644
index 00000000000..c20adc20bfd
--- /dev/null
+++ b/lib/gitlab/import_export/merge_request_parser.rb
@@ -0,0 +1,41 @@
+module Gitlab
+  module ImportExport
+    class MergeRequestParser
+      FORKED_PROJECT_ID = -1
+
+      def initialize(project, diff_head_sha, merge_request, relation_hash)
+        @project = project
+        @diff_head_sha = diff_head_sha
+        @merge_request = merge_request
+        @relation_hash = relation_hash
+      end
+
+      def parse!
+        if fork_merge_request? && @diff_head_sha
+          @merge_request.source_project_id = @relation_hash['project_id']
+
+          fetch_ref unless branch_exists?(@merge_request.source_branch)
+          create_target_branch unless branch_exists?(@merge_request.target_branch)
+        end
+
+        @merge_request
+      end
+
+      def create_target_branch
+        @project.repository.create_branch(@merge_request.target_branch, @merge_request.target_branch_sha)
+      end
+
+      def fetch_ref
+        @project.repository.fetch_ref(@project.repository.path, @diff_head_sha, @merge_request.source_branch)
+      end
+
+      def branch_exists?(branch_name)
+        @project.repository.branch_exists?(branch_name)
+      end
+
+      def fork_merge_request?
+        @relation_hash['source_project_id'] == FORKED_PROJECT_ID
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb
index cda6ddf0443..df21ff22216 100644
--- a/lib/gitlab/import_export/project_tree_restorer.rb
+++ b/lib/gitlab/import_export/project_tree_restorer.rb
@@ -119,7 +119,7 @@ module Gitlab
                                                        relation_hash: parsed_relation_hash(relation_hash),
                                                        members_mapper: members_mapper,
                                                        user: @user,
-                                                       project_id: restored_project.id)
+                                                       project: restored_project)
         end.compact
 
         relation_hash_list.is_a?(Array) ? relation_array : relation_array.first
diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb
index d44563333a5..fb43e7ccdbb 100644
--- a/lib/gitlab/import_export/relation_factory.rb
+++ b/lib/gitlab/import_export/relation_factory.rb
@@ -29,11 +29,12 @@ module Gitlab
         new(*args).create
       end
 
-      def initialize(relation_sym:, relation_hash:, members_mapper:, user:, project_id:)
+      def initialize(relation_sym:, relation_hash:, members_mapper:, user:, project:)
         @relation_name = OVERRIDES[relation_sym] || relation_sym
-        @relation_hash = relation_hash.except('noteable_id').merge('project_id' => project_id)
+        @relation_hash = relation_hash.except('noteable_id').merge('project_id' => project.id)
         @members_mapper = members_mapper
         @user = user
+        @project = project
         @imported_object_retries = 0
       end
 
@@ -66,7 +67,7 @@ module Gitlab
         remove_encrypted_attributes!
 
         @relation_hash['data'].deep_symbolize_keys! if @relation_name == :events && @relation_hash['data']
-        set_st_diffs if @relation_name == :merge_request_diff
+        set_st_diff_commits if @relation_name == :merge_request_diff
       end
 
       def update_user_references
@@ -105,6 +106,8 @@ module Gitlab
           imported_object do |object|
             object.commit_id = nil
           end
+        elsif @relation_name == :merge_requests
+          MergeRequestParser.new(@project, @relation_hash.delete('diff_head_sha'), imported_object, @relation_hash).parse!
         else
           imported_object
         end
@@ -115,7 +118,7 @@ module Gitlab
 
         # If source and target are the same, populate them with the new project ID.
         if @relation_hash['source_project_id']
-          @relation_hash['source_project_id'] = same_source_and_target? ? project_id : -1
+          @relation_hash['source_project_id'] = same_source_and_target? ? project_id : MergeRequestParser::FORKED_PROJECT_ID
         end
 
         # project_id may not be part of the export, but we always need to populate it if required.
@@ -166,6 +169,7 @@ module Gitlab
       def imported_object
         yield(existing_or_new_object) if block_given?
         existing_or_new_object.importing = true if existing_or_new_object.respond_to?(:importing)
+
         existing_or_new_object
       rescue ActiveRecord::RecordNotUnique
         # as the operation is not atomic, retry in the unlikely scenario an INSERT is
@@ -188,8 +192,11 @@ module Gitlab
                                                                                relation_class: relation_class)
       end
 
-      def set_st_diffs
+      def set_st_diff_commits
         @relation_hash['st_diffs'] = @relation_hash.delete('utf8_st_diffs')
+
+        HashUtil.deep_symbolize_array!(@relation_hash['st_diffs'])
+        HashUtil.deep_symbolize_array_with_date!(@relation_hash['st_commits'])
       end
 
       def existing_or_new_object
diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb
new file mode 100644
index 00000000000..c5ce06afd73
--- /dev/null
+++ b/spec/lib/gitlab/import_export/fork_spec.rb
@@ -0,0 +1,49 @@
+require 'spec_helper'
+
+describe 'forked project import', services: true do
+  let(:user) { create(:user) }
+  let!(:project_with_repo) { create(:project, :test_repo, name: 'test-repo-restorer', path: 'test-repo-restorer') }
+  let!(:project) { create(:empty_project) }
+  let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
+  let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.path_with_namespace) }
+  let(:forked_from_project) { create(:project) }
+  let(:fork_link) { create(:forked_project_link, forked_from_project: project_with_repo) }
+  let(:repo_saver) { Gitlab::ImportExport::RepoSaver.new(project: project_with_repo, shared: shared) }
+  let(:bundle_path) { File.join(shared.export_path, Gitlab::ImportExport.project_bundle_filename) }
+
+  let(:repo_restorer) do
+    Gitlab::ImportExport::RepoRestorer.new(path_to_bundle: bundle_path, shared: shared, project: project)
+  end
+
+  let!(:merge_request) do
+    create(:merge_request, source_project: fork_link.forked_to_project, target_project: project_with_repo)
+  end
+
+  let(:saver) do
+    Gitlab::ImportExport::ProjectTreeSaver.new(project: project_with_repo, current_user: user, shared: shared)
+  end
+
+  let(:restorer) do
+    Gitlab::ImportExport::ProjectTreeRestorer.new(user: user, shared: shared, project: project)
+  end
+
+  before do
+    allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+
+    saver.save
+    repo_saver.save
+
+    repo_restorer.restore
+    restorer.restore
+  end
+
+  after do
+    FileUtils.rm_rf(export_path)
+    FileUtils.rm_rf(project_with_repo.repository.path_to_repo)
+    FileUtils.rm_rf(project.repository.path_to_repo)
+  end
+
+  it 'can access the MR' do
+    expect(project.merge_requests.first.ensure_ref_fetched.first).to include('refs/merge-requests/1/head')
+  end
+end
diff --git a/spec/lib/gitlab/import_export/hash_util_spec.rb b/spec/lib/gitlab/import_export/hash_util_spec.rb
new file mode 100644
index 00000000000..1c3a0b23ece
--- /dev/null
+++ b/spec/lib/gitlab/import_export/hash_util_spec.rb
@@ -0,0 +1,28 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::HashUtil, lib: true do
+  let(:stringified_array) { [{ 'test' => 1 }] }
+  let(:stringified_array_with_date) { [{ 'test_date' => '2016-04-06 06:17:44 +0200' }] }
+
+  describe '.deep_symbolize_array!' do
+    it 'symbolizes keys' do
+      expect { described_class.deep_symbolize_array!(stringified_array) }.to change {
+        stringified_array.first.keys.first
+      }.from('test').to(:test)
+    end
+  end
+
+  describe '.deep_symbolize_array_with_date!' do
+    it 'symbolizes keys' do
+      expect { described_class.deep_symbolize_array_with_date!(stringified_array_with_date) }.to change {
+        stringified_array_with_date.first.keys.first
+      }.from('test_date').to(:test_date)
+    end
+
+    it 'transforms date strings into Time objects' do
+      expect { described_class.deep_symbolize_array_with_date!(stringified_array_with_date) }.to change {
+        stringified_array_with_date.first.values.first.class
+      }.from(String).to(ActiveSupport::TimeWithZone)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/import_export/merge_request_parser_spec.rb b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb
new file mode 100644
index 00000000000..349be4596b6
--- /dev/null
+++ b/spec/lib/gitlab/import_export/merge_request_parser_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::MergeRequestParser do
+  let(:user) { create(:user) }
+  let!(:project) { create(:project, :test_repo, name: 'test-repo-restorer', path: 'test-repo-restorer') }
+  let(:forked_from_project) { create(:project) }
+  let(:fork_link) { create(:forked_project_link, forked_from_project: project) }
+
+  let!(:merge_request) do
+    create(:merge_request, source_project: fork_link.forked_to_project, target_project: project)
+  end
+
+  let(:parsed_merge_request) do
+    described_class.new(project,
+                        merge_request.diff_head_sha,
+                        merge_request,
+                        merge_request.as_json).parse!
+  end
+
+  after do
+    FileUtils.rm_rf(project.repository.path_to_repo)
+  end
+
+  it 'has a source branch' do
+    expect(project.repository.branch_exists?(parsed_merge_request.source_branch)).to be true
+  end
+
+  it 'has a target branch' do
+    expect(project.repository.branch_exists?(parsed_merge_request.target_branch)).to be true
+  end
+end
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 c36f12dbd82..af9c25acb02 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -82,6 +82,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do
         expect(MergeRequestDiff.where.not(st_diffs: nil).count).to eq(9)
       end
 
+      it 'has the correct time for merge request st_commits' do
+        st_commits = MergeRequestDiff.where.not(st_commits: nil).first.st_commits
+
+        expect(st_commits.first[:committed_date]).to be_kind_of(Time)
+      end
+
       it 'has labels associated to label links, associated to issues' do
         expect(Label.first.label_links.first.target).not_to be_nil
       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 012c22ec5ad..d2d89e3b019 100644
--- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
@@ -79,6 +79,10 @@ describe Gitlab::ImportExport::ProjectTreeSaver, services: true do
         expect(saved_project_json['merge_requests'].first['merge_request_diff']).not_to be_empty
       end
 
+      it 'has merge requests diff st_diffs' do
+        expect(saved_project_json['merge_requests'].first['merge_request_diff']['utf8_st_diffs']).not_to be_nil
+      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/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb
index 57e412b0cef..fcc23a75ca1 100644
--- a/spec/lib/gitlab/import_export/relation_factory_spec.rb
+++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb
@@ -9,7 +9,7 @@ describe Gitlab::ImportExport::RelationFactory, lib: true do
                            relation_hash: relation_hash,
                            members_mapper: members_mapper,
                            user: user,
-                           project_id: project.id)
+                           project: project)
   end
 
   context 'hook object' do
diff --git a/spec/lib/gitlab/import_export/repo_bundler_spec.rb b/spec/lib/gitlab/import_export/repo_bundler_spec.rb
deleted file mode 100644
index a7f4e11271e..00000000000
--- a/spec/lib/gitlab/import_export/repo_bundler_spec.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::ImportExport::RepoSaver, services: true do
-  describe 'bundle a project Git repo' do
-    let(:user) { create(:user) }
-    let!(:project) { create(:empty_project, :public, name: 'searchable_project') }
-    let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
-    let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.path_with_namespace) }
-    let(:bundler) { described_class.new(project: project, shared: shared) }
-
-    before do
-      project.team << [user, :master]
-      allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
-    end
-
-    after do
-      FileUtils.rm_rf(export_path)
-    end
-
-    it 'bundles the repo successfully' do
-      expect(bundler.save).to be true
-    end
-  end
-end
diff --git a/spec/lib/gitlab/import_export/repo_saver_spec.rb b/spec/lib/gitlab/import_export/repo_saver_spec.rb
new file mode 100644
index 00000000000..a7f4e11271e
--- /dev/null
+++ b/spec/lib/gitlab/import_export/repo_saver_spec.rb
@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::RepoSaver, services: true do
+  describe 'bundle a project Git repo' do
+    let(:user) { create(:user) }
+    let!(:project) { create(:empty_project, :public, name: 'searchable_project') }
+    let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
+    let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.path_with_namespace) }
+    let(:bundler) { described_class.new(project: project, shared: shared) }
+
+    before do
+      project.team << [user, :master]
+      allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+    end
+
+    after do
+      FileUtils.rm_rf(export_path)
+    end
+
+    it 'bundles the repo successfully' do
+      expect(bundler.save).to be true
+    end
+  end
+end
-- 
cgit v1.2.1