diff --git a/app/models/project.rb b/app/models/project.rb
index 1855760e694208039f33241fc3ac8f0ee5246c7a..8cf093be4c3bb8ef539783e7597f11a9f4e6d3b8 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -471,8 +471,6 @@ class Project < ActiveRecord::Base
end
def reset_cache_and_import_attrs
- update(import_error: nil)
-
ProjectCacheWorker.perform_async(self.id)
self.import_data.destroy if self.import_data
diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb
index e6701078f71f86b972cb02d268127e5ab7a40f8e..d2ca8813ab9db2a955b037e3af95df7449f788bf 100644
--- a/app/workers/repository_import_worker.rb
+++ b/app/workers/repository_import_worker.rb
@@ -14,6 +14,8 @@ class RepositoryImportWorker
import_url: @project.import_url,
path: @project.path_with_namespace)
+ project.update_column(:import_error, nil)
+
result = Projects::ImportService.new(project, current_user).execute
if result[:status] == :error
diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb
index 9ddc8905bd6149f32bf58fbb896afc45787613c5..86b49a5021afadb358ed1aec6ed8a000fb041ab5 100644
--- a/lib/gitlab/github_import/importer.rb
+++ b/lib/gitlab/github_import/importer.rb
@@ -3,12 +3,13 @@ module Gitlab
class Importer
include Gitlab::ShellAdapter
- attr_reader :client, :project, :repo, :repo_url
+ attr_reader :client, :errors, :project, :repo, :repo_url
def initialize(project)
@project = project
@repo = project.import_source
@repo_url = project.import_url
+ @errors = []
if credentials
@client = Client.new(credentials[:user])
@@ -18,8 +19,14 @@ module Gitlab
end
def execute
- import_labels && import_milestones && import_issues &&
- import_pull_requests && import_wiki
+ import_labels
+ import_milestones
+ import_issues
+ import_pull_requests
+ import_wiki
+ handle_errors
+
+ true
end
private
@@ -28,22 +35,32 @@ module Gitlab
@credentials ||= project.import_data.credentials if project.import_data
end
+ def handle_errors
+ project.update_column(:import_error, errors.to_json) unless errors.empty?
+ end
+
def import_labels
labels = client.labels(repo, per_page: 100)
- labels.each { |raw| LabelFormatter.new(project, raw).create! }
- true
- rescue ActiveRecord::RecordInvalid => e
- raise Projects::ImportService::Error, e.message
+ labels.each do |raw|
+ begin
+ LabelFormatter.new(project, raw).create!
+ rescue => e
+ errors << { type: :label, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message }
+ end
+ end
end
def import_milestones
milestones = client.milestones(repo, state: :all, per_page: 100)
- milestones.each { |raw| MilestoneFormatter.new(project, raw).create! }
- true
- rescue ActiveRecord::RecordInvalid => e
- raise Projects::ImportService::Error, e.message
+ milestones.each do |raw|
+ begin
+ MilestoneFormatter.new(project, raw).create!
+ rescue => e
+ errors << { type: :milestone, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message }
+ end
+ end
end
def import_issues
@@ -53,15 +70,15 @@ module Gitlab
gh_issue = IssueFormatter.new(project, raw)
if gh_issue.valid?
- issue = gh_issue.create!
- apply_labels(issue)
- import_comments(issue) if gh_issue.has_comments?
+ begin
+ issue = gh_issue.create!
+ apply_labels(issue)
+ import_comments(issue) if gh_issue.has_comments?
+ rescue => e
+ errors << { type: :issue, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message }
+ end
end
end
-
- true
- rescue ActiveRecord::RecordInvalid => e
- raise Projects::ImportService::Error, e.message
end
def import_pull_requests
@@ -77,14 +94,12 @@ module Gitlab
apply_labels(merge_request)
import_comments(merge_request)
import_comments_on_diff(merge_request)
- rescue ActiveRecord::RecordInvalid => e
- raise Projects::ImportService::Error, e.message
+ rescue => e
+ errors << { type: :pull_request, url: Gitlab::UrlSanitizer.sanitize(pull_request.url), errors: e.message }
ensure
clean_up_restored_branches(pull_request)
end
end
-
- true
end
def restore_source_branch(pull_request)
@@ -98,7 +113,7 @@ module Gitlab
def remove_branch(name)
project.repository.delete_branch(name)
rescue Rugged::ReferenceError
- nil
+ errors << { type: :remove_branch, name: name }
end
def clean_up_restored_branches(pull_request)
@@ -112,9 +127,10 @@ module Gitlab
issue = client.issue(repo, issuable.iid)
if issue.labels.count > 0
- label_ids = issue.labels.map do |raw|
- Label.find_by(LabelFormatter.new(project, raw).attributes).try(:id)
- end
+ label_ids = issue.labels
+ .map { |raw| LabelFormatter.new(project, raw).attributes }
+ .map { |attrs| Label.find_by(attrs).try(:id) }
+ .compact
issuable.update_attribute(:label_ids, label_ids)
end
@@ -132,8 +148,12 @@ module Gitlab
def create_comments(issuable, comments)
comments.each do |raw|
- comment = CommentFormatter.new(project, raw)
- issuable.notes.create!(comment.attributes)
+ begin
+ comment = CommentFormatter.new(project, raw)
+ issuable.notes.create!(comment.attributes)
+ rescue => e
+ errors << { type: :comment, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message }
+ end
end
end
@@ -143,16 +163,12 @@ module Gitlab
gitlab_shell.import_repository(project.repository_storage_path, wiki.path_with_namespace, wiki.import_url)
project.update_attribute(:wiki_enabled, true)
end
-
- true
rescue Gitlab::Shell::Error => e
# GitHub error message when the wiki repo has not been created,
# this means that repo has wiki enabled, but have no pages. So,
# we can skip the import.
if e.message !~ /repository not exported/
- raise Projects::ImportService::Error, e.message
- else
- true
+ errors << { type: :wiki, errors: e.message }
end
end
end
diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb
index b84538a090a268a0e97af332dd2528579941ecb1..04aa3664f640388dfa0de539512496f60048f594 100644
--- a/lib/gitlab/github_import/pull_request_formatter.rb
+++ b/lib/gitlab/github_import/pull_request_formatter.rb
@@ -56,6 +56,10 @@ module Gitlab
end
end
+ def url
+ raw_data.url
+ end
+
private
def assigned?
diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2c8d86ef9b6bcafb0a15eb89a11c9df1c5255ce5
--- /dev/null
+++ b/spec/lib/gitlab/github_import/importer_spec.rb
@@ -0,0 +1,129 @@
+require 'spec_helper'
+
+describe Gitlab::GithubImport::Importer, lib: true do
+ describe '#execute' do
+ context 'when an error occurs' do
+ let(:project) { create(:project, import_url: 'https://github.com/octocat/Hello-World.git', wiki_enabled: false) }
+ let(:octocat) { double(id: 123456, login: 'octocat') }
+ let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') }
+ let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') }
+ let(:repository) { double(id: 1, fork: false) }
+ let(:source_sha) { create(:commit, project: project).id }
+ let(:source_branch) { double(ref: 'feature', repo: repository, sha: source_sha) }
+ let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id }
+ let(:target_branch) { double(ref: 'master', repo: repository, sha: target_sha) }
+
+ let(:label) do
+ double(
+ name: 'Bug',
+ color: 'ff0000',
+ url: 'https://api.github.com/repos/octocat/Hello-World/labels/bug'
+ )
+ end
+
+ let(:milestone) do
+ double(
+ number: 1347,
+ state: 'open',
+ title: '1.0',
+ description: 'Version 1.0',
+ due_on: nil,
+ created_at: created_at,
+ updated_at: updated_at,
+ closed_at: nil,
+ url: 'https://api.github.com/repos/octocat/Hello-World/milestones/1'
+ )
+ end
+
+ let(:issue1) do
+ double(
+ number: 1347,
+ milestone: nil,
+ state: 'open',
+ title: 'Found a bug',
+ body: "I'm having a problem with this.",
+ assignee: nil,
+ user: octocat,
+ comments: 0,
+ pull_request: nil,
+ created_at: created_at,
+ updated_at: updated_at,
+ closed_at: nil,
+ url: 'https://api.github.com/repos/octocat/Hello-World/issues/1347'
+ )
+ end
+
+ let(:issue2) do
+ double(
+ number: 1348,
+ milestone: nil,
+ state: 'open',
+ title: nil,
+ body: "I'm having a problem with this.",
+ assignee: nil,
+ user: octocat,
+ comments: 0,
+ pull_request: nil,
+ created_at: created_at,
+ updated_at: updated_at,
+ closed_at: nil,
+ url: 'https://api.github.com/repos/octocat/Hello-World/issues/1348'
+ )
+ end
+
+ let(:pull_request) do
+ double(
+ number: 1347,
+ milestone: nil,
+ state: 'open',
+ title: 'New feature',
+ body: 'Please pull these awesome changes',
+ head: source_branch,
+ base: target_branch,
+ assignee: nil,
+ user: octocat,
+ created_at: created_at,
+ updated_at: updated_at,
+ closed_at: nil,
+ merged_at: nil,
+ url: 'https://api.github.com/repos/octocat/Hello-World/pulls/1347'
+ )
+ end
+
+ before do
+ allow(project).to receive(:import_data).and_return(double.as_null_object)
+ allow_any_instance_of(Octokit::Client).to receive(:rate_limit!).and_raise(Octokit::NotFound)
+ allow_any_instance_of(Octokit::Client).to receive(:labels).and_return([label, label])
+ allow_any_instance_of(Octokit::Client).to receive(:milestones).and_return([milestone, milestone])
+ allow_any_instance_of(Octokit::Client).to receive(:issues).and_return([issue1, issue2])
+ allow_any_instance_of(Octokit::Client).to receive(:pull_requests).and_return([pull_request, pull_request])
+ allow_any_instance_of(Octokit::Client).to receive(:last_response).and_return(double(rels: { next: nil }))
+ allow_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_raise(Gitlab::Shell::Error)
+ end
+
+ it 'returns true' do
+ expect(described_class.new(project).execute).to eq true
+ end
+
+ it 'does not raise an error' do
+ expect { described_class.new(project).execute }.not_to raise_error
+ end
+
+ it 'stores error messages' do
+ errors = [
+ { type: :label, url: "https://api.github.com/repos/octocat/Hello-World/labels/bug", errors: "Validation failed: Title has already been taken" },
+ { type: :milestone, url: "https://api.github.com/repos/octocat/Hello-World/milestones/1", errors: "Validation failed: Title has already been taken" },
+ { type: :issue, url: "https://api.github.com/repos/octocat/Hello-World/issues/1347", errors: "Invalid Repository. Use user/repo format." },
+ { type: :issue, url: "https://api.github.com/repos/octocat/Hello-World/issues/1348", errors: "Validation failed: Title can't be blank, Title is too short (minimum is 0 characters)" },
+ { type: :pull_request, url: "https://api.github.com/repos/octocat/Hello-World/pulls/1347", errors: "Invalid Repository. Use user/repo format." },
+ { type: :pull_request, url: "https://api.github.com/repos/octocat/Hello-World/pulls/1347", errors: "Validation failed: Validate branches Cannot Create: This merge request already exists: [\"New feature\"]" },
+ { type: :wiki, errors: "Gitlab::Shell::Error" }
+ ]
+
+ described_class.new(project).execute
+
+ expect(project.import_error).to eq errors.to_json
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb
index aa28e360993434ba264a3362a65a4d5f5ac07e75..b667abf063d65e088307602081637dd8a7c13878 100644
--- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb
+++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb
@@ -27,7 +27,8 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
created_at: created_at,
updated_at: updated_at,
closed_at: nil,
- merged_at: nil
+ merged_at: nil,
+ url: 'https://api.github.com/repos/octocat/Hello-World/pulls/1347'
}
end
@@ -229,4 +230,12 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
end
end
end
+
+ describe '#url' do
+ let(:raw_data) { double(base_data) }
+
+ it 'return raw url' do
+ expect(pull_request.url).to eq 'https://api.github.com/repos/octocat/Hello-World/pulls/1347'
+ end
+ end
end