Extract GitHub branch formatter

app/models/repository.rb

@@ -195,6 +195,10 @@ class Repository
     cache.fetch(:branch_names) { branches.map(&:name) }
   end
 
+  def branch_exists?(branch_name)
+    branch_names.include?(branch_name)
+  end
+
   def tag_names
     cache.fetch(:tag_names) { raw_repository.tag_names }
   end

lib/gitlab/github_import/branch_formatter.rb

+module Gitlab
+  module GithubImport
+    class BranchFormatter < BaseFormatter
+      delegate :repo, :sha, :ref, to: :raw_data
+
+      def exists?
+        project.repository.branch_exists?(ref)
+      end
+
+      def name
+        @name ||= exists? ? ref : "#{ref}-#{short_id}"
+      end
+
+      def valid?
+        repo.present?
+      end
+
+      private
+
+      def short_id
+        sha.to_s[0..7]
+      end
+    end
+  end
+end

lib/gitlab/github_import/importer.rb

@@ -6,8 +6,8 @@ module Gitlab
       attr_reader :client, :project, :repo, :repo_url
 
       def initialize(project)
-        @project = project
-        @repo = project.import_source
+        @project  = project
+        @repo     = project.import_source
         @repo_url = project.import_url
 
         if credentials
@@ -30,7 +30,7 @@ module Gitlab
       end
 
       def import_labels
-        client.labels(project.import_source).each do |raw_data|
+        client.labels(repo).each do |raw_data|
           Label.create!(LabelFormatter.new(project, raw_data).attributes)
         end
 
@@ -40,7 +40,7 @@ module Gitlab
       end
 
       def import_milestones
-        client.list_milestones(project.import_source, state: :all).each do |raw_data|
+        client.list_milestones(repo, state: :all).each do |raw_data|
           Milestone.create!(MilestoneFormatter.new(project, raw_data).attributes)
         end
 
@@ -50,9 +50,7 @@ module Gitlab
       end
 
       def import_issues
-        client.list_issues(project.import_source, state: :all,
-                                                  sort: :created,
-                                                  direction: :asc).each do |raw_data|
+        client.list_issues(repo, state: :all, sort: :created, direction: :asc).each do |raw_data|
           gh_issue = IssueFormatter.new(project, raw_data)
 
           if gh_issue.valid?
@@ -75,8 +73,8 @@ module Gitlab
                               .map { |raw| PullRequestFormatter.new(project, raw) }
                               .select(&:valid?)
 
-        source_branches_removed = pull_requests.reject(&:source_branch_exists?).map { |pr| [pr.source_branch, pr.source_sha] }
-        target_branches_removed = pull_requests.reject(&:target_branch_exists?).map { |pr| [pr.target_branch, pr.target_sha] }
+        source_branches_removed = pull_requests.reject(&:source_branch_exists?).map { |pr| [pr.source_branch_name, pr.source_branch_sha] }
+        target_branches_removed = pull_requests.reject(&:target_branch_exists?).map { |pr| [pr.target_branch_name, pr.target_branch_sha] }
         branches_removed = source_branches_removed | target_branches_removed
 
         create_refs(branches_removed)
@@ -101,19 +99,19 @@ module Gitlab
       end
 
       def create_refs(branches)
-        branches.each do |branch|
-          client.create_ref(repo, "refs/heads/#{branch.first}", branch.last)
+        branches.each do |name, sha|
+          client.create_ref(repo, "refs/heads/#{name}", sha)
         end
       end
 
       def delete_refs(branches)
-        branches.each do |branch|
-          client.delete_ref(repo, "heads/#{branch.first}")
+        branches.each do |name, _|
+          client.delete_ref(repo, "heads/#{name}")
         end
       end
 
       def apply_labels(number, issuable)
-        issue = client.issue(project.import_source, number)
+        issue = client.issue(repo, number)
 
         if issue.labels.count > 0
           label_ids = issue.labels.map do |raw|
@@ -125,12 +123,12 @@ module Gitlab
       end
 
       def import_comments(issue_number, noteable)
-        comments = client.issue_comments(project.import_source, issue_number)
+        comments = client.issue_comments(repo, issue_number)
         create_comments(comments, noteable)
       end
 
       def import_comments_on_diff(pull_request_number, merge_request)
-        comments = client.pull_request_comments(project.import_source, pull_request_number)
+        comments = client.pull_request_comments(repo, pull_request_number)
         create_comments(comments, merge_request)
       end
 

lib/gitlab/github_import/pull_request_formatter.rb

 module Gitlab
   module GithubImport
     class PullRequestFormatter < BaseFormatter
+      delegate :exists?, :name, :project, :repo, :sha, to: :source_branch, prefix: true
+      delegate :exists?, :name, :project, :repo, :sha, to: :target_branch, prefix: true
+
       def attributes
         {
           iid: number,
           title: raw_data.title,
           description: description,
-          source_project: source_project,
-          source_branch: source_branch,
-          head_source_sha: source_sha,
-          target_project: target_project,
-          target_branch: target_branch,
-          base_target_sha: target_sha,
+          source_project: source_branch_project,
+          source_branch: source_branch_name,
+          head_source_sha: source_branch_sha,
+          target_project: target_branch_project,
+          target_branch: target_branch_name,
+          base_target_sha: target_branch_sha,
           state: state,
           milestone: milestone,
           author_id: author_id,
@@ -29,40 +32,12 @@ module Gitlab
         !cross_project?
       end
 
-      def source_branch_exists?
-        source_project.repository.branch_exists?(source_ref)
-      end
-
       def source_branch
-        @source_branch ||= if source_branch_exists?
-                             source_ref
-                           else
-                             "#{source_ref}-#{short_id(source_sha)}"
-                           end
-      end
-
-      def short_id(sha, length = 7)
-        sha.to_s[0..length]
-      end
-
-      def source_sha
-        raw_data.head.sha
-      end
-
-      def target_branch_exists?
-        target_project.repository.branch_exists?(target_ref)
+        @source_branch ||= BranchFormatter.new(project, raw_data.head)
       end
 
       def target_branch
-        @target_branch ||= if target_branch_exists?
-                             target_ref
-                           else
-                             "#{target_ref}-#{short_id(target_sha)}"
-                           end
-      end
-
-      def target_sha
-        raw_data.base.sha
+        @target_branch ||= BranchFormatter.new(project, raw_data.base)
       end
 
       private
@@ -90,7 +65,8 @@ module Gitlab
       end
 
       def cross_project?
-        source_repo.present? && target_repo.present? && source_repo.id != target_repo.id
+        source_branch_repo.present? && target_branch_repo.present? &&
+          source_branch_repo.id != target_branch_repo.id
       end
 
       def description
@@ -103,30 +79,6 @@ module Gitlab
         end
       end
 
-      def source_project
-        project
-      end
-
-      def source_repo
-        raw_data.head.repo
-      end
-
-      def source_ref
-        raw_data.head.ref
-      end
-
-      def target_project
-        project
-      end
-
-      def target_repo
-        raw_data.base.repo
-      end
-
-      def target_ref
-        raw_data.base.ref
-      end
-
       def state
         @state ||= case true
                    when raw_data.state == 'closed' && raw_data.merged_at.present?

spec/lib/gitlab/github_import/branch_formatter_spec.rb

+require 'spec_helper'
+
+describe Gitlab::GithubImport::BranchFormatter, lib: true do
+  let(:project) { create(:project) }
+  let(:repo) { double }
+  let(:raw) do
+    {
+      ref: 'feature',
+      repo: repo,
+      sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
+    }
+  end
+
+  describe '#exists?' do
+    it 'returns true when branch exists' do
+      branch = described_class.new(project, double(raw))
+
+      expect(branch.exists?).to eq true
+    end
+
+    it 'returns false when branch does not exist' do
+      branch = described_class.new(project, double(raw.merge(ref: 'removed-branch')))
+
+      expect(branch.exists?).to eq false
+    end
+  end
+
+  describe '#name' do
+    it 'returns raw ref when branch exists' do
+      branch = described_class.new(project, double(raw))
+
+      expect(branch.name).to eq 'feature'
+    end
+
+    it 'returns formatted ref when branch does not exist' do
+      branch = described_class.new(project, double(raw.merge(ref: 'removed-branch')))
+
+      expect(branch.name).to eq 'removed-branch-2e5d3239'
+    end
+  end
+
+  describe '#repo' do
+    it 'returns raw repo' do
+      branch = described_class.new(project, double(raw))
+
+      expect(branch.repo).to eq repo
+    end
+  end
+
+  describe '#sha' do
+    it 'returns raw sha' do
+      branch = described_class.new(project, double(raw))
+
+      expect(branch.sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
+    end
+  end
+end

spec/lib/gitlab/github_import/pull_request_formatter_spec.rb

@@ -163,94 +163,6 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
     end
   end
 
-  describe '#source_branch' do
-    context 'when source branch exists' do
-      let(:raw_data) { double(base_data) }
-
-      it 'returns head ref' do
-        expect(pull_request.source_branch).to eq 'feature'
-      end
-    end
-
-    context 'when source branch does not exist' do
-      let(:raw_data) { double(base_data.merge(head: double(ref: 'removed-branch', sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'))) }
-
-      it 'returns head ref' do
-        expect(pull_request.source_branch).to eq 'removed-branch-2e5d3239'
-      end
-    end
-  end
-
-  describe '#source_sha' do
-    let(:raw_data) { double(base_data) }
-
-    it 'returns head sha' do
-      expect(pull_request.source_sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
-    end
-  end
-
-  describe '#source_branch_exists?' do
-    context 'when source branch exists' do
-      let(:raw_data) { double(base_data) }
-
-      it 'returns true' do
-        expect(pull_request.source_branch_exists?).to eq true
-      end
-    end
-
-    context 'when source branch does not exist' do
-      let(:raw_data) { double(base_data.merge(head: double(ref: 'removed-branch').as_null_object)) }
-
-      it 'returns false' do
-        expect(pull_request.source_branch_exists?).to eq false
-      end
-    end
-  end
-
-  describe '#target_branch' do
-    context 'when target branch exists' do
-      let(:raw_data) { double(base_data) }
-
-      it 'returns base ref' do
-        expect(pull_request.target_branch).to eq 'master'
-      end
-    end
-
-    context 'when target branch does not exist' do
-      let(:raw_data) { double(base_data.merge(base: double(ref: 'removed-branch', sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7'))) }
-
-      it 'returns head ref' do
-        expect(pull_request.target_branch).to eq 'removed-branch-8ffb3c15'
-      end
-    end
-  end
-
-  describe '#target_sha' do
-    let(:raw_data) { double(base_data) }
-
-    it 'returns base sha' do
-      expect(pull_request.target_sha).to eq '8ffb3c15a5475e59ae909384297fede4badcb4c7'
-    end
-  end
-
-  describe '#target_branch_exists?' do
-    context 'when target branch exists' do
-      let(:raw_data) { double(base_data) }
-
-      it 'returns true' do
-        expect(pull_request.target_branch_exists?).to eq true
-      end
-    end
-
-    context 'when target branch does not exist' do
-      let(:raw_data) { double(base_data.merge(base: double(ref: 'removed-branch').as_null_object)) }
-
-      it 'returns false' do
-        expect(pull_request.target_branch_exists?).to eq false
-      end
-    end
-  end
-
   describe '#valid?' do
     context 'when source, and target repos are not a fork' do
       let(:raw_data) { double(base_data) }