From fc42f3dffa364a360c76ff2785813d74f42016c7 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 11 Apr 2017 22:33:54 -0300 Subject: Add basic client for the GitHub API --- lib/github/client.rb | 21 +++++++++++++++++++++ lib/github/collection.rb | 26 ++++++++++++++++++++++++++ lib/github/issues.rb | 20 ++++++++++++++++++++ lib/github/labels.rb | 20 ++++++++++++++++++++ lib/github/milestones.rb | 20 ++++++++++++++++++++ lib/github/pull_requests.rb | 20 ++++++++++++++++++++ lib/github/rate_limit.rb | 41 +++++++++++++++++++++++++++++++++++++++++ lib/github/releases.rb | 20 ++++++++++++++++++++ lib/github/repositories.rb | 17 +++++++++++++++++ lib/github/response.rb | 22 ++++++++++++++++++++++ lib/github/user.rb | 23 +++++++++++++++++++++++ 11 files changed, 250 insertions(+) create mode 100644 lib/github/client.rb create mode 100644 lib/github/collection.rb create mode 100644 lib/github/issues.rb create mode 100644 lib/github/labels.rb create mode 100644 lib/github/milestones.rb create mode 100644 lib/github/pull_requests.rb create mode 100644 lib/github/rate_limit.rb create mode 100644 lib/github/releases.rb create mode 100644 lib/github/repositories.rb create mode 100644 lib/github/response.rb create mode 100644 lib/github/user.rb (limited to 'lib') diff --git a/lib/github/client.rb b/lib/github/client.rb new file mode 100644 index 00000000000..2511523bf6a --- /dev/null +++ b/lib/github/client.rb @@ -0,0 +1,21 @@ +module Github + class Client + attr_reader :connection + + def initialize(token) + @connection = Faraday.new(url: 'https://api.github.com') do |faraday| + faraday.adapter :net_http_persistent + faraday.response :json, content_type: /\bjson$/ + faraday.authorization 'token', token + faraday.response :logger + end + end + + def get(url, query = {}) + rate_limit = RateLimit.new(connection) + sleep rate_limit.reset_in if rate_limit.exceed? + + Github::Response.new(connection.get(url, query)) + end + end +end diff --git a/lib/github/collection.rb b/lib/github/collection.rb new file mode 100644 index 00000000000..bbca12a1c84 --- /dev/null +++ b/lib/github/collection.rb @@ -0,0 +1,26 @@ +module Github + class Collection + def initialize(url) + @url = url + end + + def fetch(query = {}) + return [] if @url.blank? + + Enumerator.new do |yielder| + loop do + response = client.get(@url, query) + response.body.each { |item| yielder << item } + raise StopIteration unless response.rels.key?(:next) + @url = response.rels[:next] + end + end.lazy + end + + private + + def client + @client ||= Github::Client.new + end + end +end diff --git a/lib/github/issues.rb b/lib/github/issues.rb new file mode 100644 index 00000000000..27843e1cdd8 --- /dev/null +++ b/lib/github/issues.rb @@ -0,0 +1,20 @@ +module Github + class Issues + attr_reader :owner, :repo + + def initialize(owner, repo) + @owner = owner + @repo = repo + end + + def fetch + Collection.new(issues_url).fetch(state: :all, sort: :created, direction: :asc, per_page: 10) + end + + private + + def issues_url + "/repos/#{owner}/#{repo}/issues" + end + end +end diff --git a/lib/github/labels.rb b/lib/github/labels.rb new file mode 100644 index 00000000000..0ea023888b3 --- /dev/null +++ b/lib/github/labels.rb @@ -0,0 +1,20 @@ +module Github + class Labels + attr_reader :owner, :repo + + def initialize(owner, repo) + @owner = owner + @repo = repo + end + + def fetch + Collection.new(labels_url).fetch + end + + private + + def labels_url + "/repos/#{owner}/#{repo}/labels" + end + end +end diff --git a/lib/github/milestones.rb b/lib/github/milestones.rb new file mode 100644 index 00000000000..d50278105db --- /dev/null +++ b/lib/github/milestones.rb @@ -0,0 +1,20 @@ +module Github + class Milestones + attr_reader :owner, :repo + + def initialize(owner, repo) + @owner = owner + @repo = repo + end + + def fetch + Collection.new(milestones_url).fetch + end + + private + + def milestones_url + "/repos/#{owner}/#{repo}/milestones" + end + end +end diff --git a/lib/github/pull_requests.rb b/lib/github/pull_requests.rb new file mode 100644 index 00000000000..af04c90d454 --- /dev/null +++ b/lib/github/pull_requests.rb @@ -0,0 +1,20 @@ +module Github + class PullRequests + attr_reader :owner, :repo + + def initialize(owner, repo) + @owner = owner + @repo = repo + end + + def fetch + Collection.new(pull_requests_url).fetch(state: :all, sort: :created, direction: :asc) + end + + private + + def pull_requests_url + "/repos/#{owner}/#{repo}/pulls" + end + end +end diff --git a/lib/github/rate_limit.rb b/lib/github/rate_limit.rb new file mode 100644 index 00000000000..30b000f8a9a --- /dev/null +++ b/lib/github/rate_limit.rb @@ -0,0 +1,41 @@ +module Github + class RateLimit + SAFE_REMAINING_REQUESTS = 100.freeze + SAFE_RESET_TIME = 500.freeze + + attr_reader :connection + + def initialize(connection) + @connection = connection + end + + def exceed? + return false unless enabled? + + remaining <= SAFE_REMAINING_REQUESTS + end + + def remaining + @remaining ||= response.body.dig('rate', 'remaining').to_i + end + + def reset_in + @reset ||= response.body.dig('rate', 'reset').to_i + end + + private + + def rate_limit_url + '/rate_limit' + end + + def response + @response ||= connection.get(rate_limit_url) + end + + # GitHub Rate Limit API returns 404 when the rate limit is disabled + def enabled? + response.status != 404 + end + end +end diff --git a/lib/github/releases.rb b/lib/github/releases.rb new file mode 100644 index 00000000000..026ef4e6853 --- /dev/null +++ b/lib/github/releases.rb @@ -0,0 +1,20 @@ +module Github + class Releases + attr_reader :owner, :repo + + def initialize(owner, repo) + @owner = owner + @repo = repo + end + + def fetch + Collection.new(releases_url).fetch + end + + private + + def releases_url + "/repos/#{owner}/#{repo}/releases" + end + end +end diff --git a/lib/github/repositories.rb b/lib/github/repositories.rb new file mode 100644 index 00000000000..42342471102 --- /dev/null +++ b/lib/github/repositories.rb @@ -0,0 +1,17 @@ +module Github + class Repositories + def initialize(username) + @username = username + end + + def fetch + Collection.new(repos_url).fetch + end + + private + + def repos_url + '/user/repos' + end + end +end diff --git a/lib/github/response.rb b/lib/github/response.rb new file mode 100644 index 00000000000..c34b69aa4ea --- /dev/null +++ b/lib/github/response.rb @@ -0,0 +1,22 @@ +module Github + class Response + attr_reader :raw, :headers, :body, :status + + def initialize(response) + @raw = response + @headers = response.headers + @body = response.body + @status = response.status + end + + def rels + links = headers['Link'].to_s.split(', ').map do |link| + href, name = link.match(/<(.*?)>; rel="(\w+)"/).captures + + [name.to_sym, href] + end + + Hash[*links.flatten] + end + end +end diff --git a/lib/github/user.rb b/lib/github/user.rb new file mode 100644 index 00000000000..19fe6230820 --- /dev/null +++ b/lib/github/user.rb @@ -0,0 +1,23 @@ +module Github + class User + attr_reader :username + + def initialize(username) + @username = username + end + + def get + client.get(user_url).body + end + + private + + def client + @client ||= Github::Client.new + end + + def user_url + "/users/#{username}" + end + end +end -- cgit v1.2.1 From b43ecca9060b3d7ffd84d700caecf5f35fd403a9 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 11 Apr 2017 22:34:59 -0300 Subject: Add basic representations for the Github API results --- lib/github/representation/base.rb | 13 ++++ lib/github/representation/branch.rb | 52 +++++++++++++ lib/github/representation/label.rb | 17 +++++ lib/github/representation/milestone.rb | 37 +++++++++ lib/github/representation/pull_request.rb | 123 ++++++++++++++++++++++++++++++ lib/github/representation/repo.rb | 9 +++ lib/github/representation/user.rb | 19 +++++ 7 files changed, 270 insertions(+) create mode 100644 lib/github/representation/base.rb create mode 100644 lib/github/representation/branch.rb create mode 100644 lib/github/representation/label.rb create mode 100644 lib/github/representation/milestone.rb create mode 100644 lib/github/representation/pull_request.rb create mode 100644 lib/github/representation/repo.rb create mode 100644 lib/github/representation/user.rb (limited to 'lib') diff --git a/lib/github/representation/base.rb b/lib/github/representation/base.rb new file mode 100644 index 00000000000..6765bfb803b --- /dev/null +++ b/lib/github/representation/base.rb @@ -0,0 +1,13 @@ +module Github + module Representation + class Base + def initialize(raw) + @raw = raw + end + + private + + attr_reader :raw + end + end +end diff --git a/lib/github/representation/branch.rb b/lib/github/representation/branch.rb new file mode 100644 index 00000000000..7c65a948ede --- /dev/null +++ b/lib/github/representation/branch.rb @@ -0,0 +1,52 @@ +module Github + module Representation + class Branch < Representation::Base + attr_reader :repository + + def initialize(repository, raw) + @repository = repository + @raw = raw + end + + def user + raw.dig('user', 'login') || 'unknown' + end + + def repo + return @repo if defined?(@repo) + + @repo = Github::Representation::Repo.new(raw['repo']) if raw['repo'].present? + end + + def ref + raw['ref'] + end + + def sha + raw['sha'] + end + + def short_sha + Commit.truncate_sha(sha) + end + + def exists? + branch_exists? && commit_exists? + end + + def valid? + sha.present? && ref.present? + end + + private + + def branch_exists? + repository.branch_exists?(ref) + end + + def commit_exists? + repository.branch_names_contains(sha).include?(ref) + end + end + end +end diff --git a/lib/github/representation/label.rb b/lib/github/representation/label.rb new file mode 100644 index 00000000000..b3140ab76fc --- /dev/null +++ b/lib/github/representation/label.rb @@ -0,0 +1,17 @@ +module Github + module Representation + class Label < Representation::Base + def color + "##{raw['color']}" + end + + def title + raw['name'] + end + + def url + raw['url'] + end + end + end +end diff --git a/lib/github/representation/milestone.rb b/lib/github/representation/milestone.rb new file mode 100644 index 00000000000..5ea54eb178f --- /dev/null +++ b/lib/github/representation/milestone.rb @@ -0,0 +1,37 @@ +module Github + module Representation + class Milestone < Representation::Base + def iid + raw['number'] + end + + def title + raw['title'] + end + + def description + raw['description'] + end + + def due_date + raw['due_on'] + end + + def state + raw['state'] == 'closed' ? 'closed' : 'active' + end + + def url + raw['url'] + end + + def created_at + raw['created_at'] + end + + def updated_at + raw['updated_at'] + end + end + end +end diff --git a/lib/github/representation/pull_request.rb b/lib/github/representation/pull_request.rb new file mode 100644 index 00000000000..85f4e1bdac3 --- /dev/null +++ b/lib/github/representation/pull_request.rb @@ -0,0 +1,123 @@ +module Github + module Representation + class PullRequest < Representation::Base + attr_reader :project + + delegate :user, :repo, :ref, :sha, to: :source_branch, prefix: true + delegate :user, :exists?, :repo, :ref, :sha, :short_sha, to: :target_branch, prefix: true + + def initialize(project, raw) + @project = project + @raw = raw + end + + def iid + raw['number'] + end + + def title + raw['title'] + end + + def description + raw['body'] || '' + end + + def source_project + project + end + + def source_branch_exists? + !cross_project? && source_branch.exists? + end + + def source_branch_name + @source_branch_name ||= + if cross_project? || !source_branch_exists? + source_branch_name_prefixed + else + source_branch_ref + end + end + + def target_project + project + end + + def target_branch_name + @target_branch_name ||= target_branch_exists? ? target_branch_ref : target_branch_name_prefixed + end + + def milestone + return unless raw['milestone'].present? + + @milestone ||= Github::Representation::Milestone.new(raw['milestone']) + end + + def author + @author ||= Github::Representation::User.new(raw['user']) + end + + def assignee + return unless assigned? + + @assignee ||= Github::Representation::User.new(raw['assignee']) + end + + def state + return 'merged' if raw['state'] == 'closed' && raw['merged_at'].present? + return 'closed' if raw['state'] == 'closed' + + 'opened' + end + + def url + raw['url'] + end + + def created_at + raw['created_at'] + end + + def updated_at + raw['updated_at'] + end + + def assigned? + raw['assignee'].present? + end + + def opened? + state == 'opened' + end + + def valid? + source_branch.valid? && target_branch.valid? + end + + private + + def source_branch + @source_branch ||= Representation::Branch.new(project.repository, raw['head']) + end + + def source_branch_name_prefixed + "gh-#{target_branch_short_sha}/#{iid}/#{source_branch_user}/#{source_branch_ref}" + end + + def target_branch + @target_branch ||= Representation::Branch.new(project.repository, raw['base']) + end + + def target_branch_name_prefixed + "gl-#{target_branch_short_sha}/#{iid}/#{target_branch_user}/#{target_branch_ref}" + end + + def cross_project? + return true if source_branch_repo.nil? + + source_branch_repo.id != target_branch_repo.id + end + end + end +end diff --git a/lib/github/representation/repo.rb b/lib/github/representation/repo.rb new file mode 100644 index 00000000000..b9cae43450e --- /dev/null +++ b/lib/github/representation/repo.rb @@ -0,0 +1,9 @@ +module Github + module Representation + class Repo < Representation::Base + def id + raw['id'] + end + end + end +end diff --git a/lib/github/representation/user.rb b/lib/github/representation/user.rb new file mode 100644 index 00000000000..70a0ce000ae --- /dev/null +++ b/lib/github/representation/user.rb @@ -0,0 +1,19 @@ +module Github + module Representation + class User < Representation::Base + def id + raw['id'] + end + + def email + return @email if defined?(@email) + + @email = Github::User.new(username).get.fetch('email', nil) + end + + def username + raw['login'] + end + end + end +end -- cgit v1.2.1 From f76363e445153ea831fd508e366c0f31b24fa53e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 11 Apr 2017 22:36:33 -0300 Subject: Add basic importer for GitHub repositories --- lib/github/import.rb | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 lib/github/import.rb (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb new file mode 100644 index 00000000000..89f597551f9 --- /dev/null +++ b/lib/github/import.rb @@ -0,0 +1,166 @@ +module Github + class Import + class MergeRequest < ::MergeRequest + self.table_name = 'merge_requests' + + self.reset_callbacks :save + self.reset_callbacks :commit + self.reset_callbacks :update + self.reset_callbacks :validate + end + + attr_reader :project, :repository, :cached_user_ids, :errors + + def initialize(project) + @project = project + @repository = project.repository + @cached_user_ids = {} + @errors = [] + end + + def execute(owner, repo, token) + # Fetch repository + begin + project.create_repository + project.repository.add_remote('github', "https://#{token}@github.com/#{owner}/#{repo}.git") + project.repository.set_remote_as_mirror('github') + project.repository.fetch_remote('github', forced: true) + project.repository.remove_remote('github') + rescue Gitlab::Shell::Error => e + error(:project, "https://github.com/#{owner}/#{repo}.git", e.message) + end + + # Fetch labels + labels = Github::Labels.new(owner, repo).fetch + + labels.each do |raw| + begin + label = Github::Representation::Label.new(raw) + project.labels.create!(title: label.title, color: label.color) + rescue => e + error(:label, label.url, e.message) + end + end + + # Fetch milestones + milestones = Github::Milestones.new(owner, repo).fetch + + milestones.each do |raw| + begin + milestone = Github::Representation::Milestone.new(raw) + + project.milestones.create!( + iid: milestone.iid, + title: milestone.title, + description: milestone.description, + due_date: milestone.due_date, + state: milestone.state, + created_at: milestone.created_at, + updated_at: milestone.updated_at + ) + rescue => e + error(:milestone, milestone.url, e.message) + end + end + + # Fetch pull requests + pull_requests = Github::PullRequests.new(owner, repo).fetch + + pull_requests.each do |raw| + pull_request = Github::Representation::PullRequest.new(project, raw) + next unless pull_request.valid? + + begin + restore_source_branch(pull_request) unless pull_request.source_branch_exists? + restore_target_branch(pull_request) unless pull_request.target_branch_exists? + + merge_request = MergeRequest.find_or_initialize_by(iid: pull_request.iid, source_project_id: project.id) do |record| + record.iid = pull_request.iid + record.title = pull_request.title + record.description = pull_request.description + record.source_project = pull_request.source_project + record.source_branch = pull_request.source_branch_name + record.source_branch_sha = pull_request.source_branch_sha + record.target_project = pull_request.target_project + record.target_branch = pull_request.target_branch_name + record.target_branch_sha = pull_request.target_branch_sha + record.state = pull_request.state + record.milestone_id = milestone_id(pull_request.milestone) + record.author_id = user_id(pull_request.author, project.creator_id) + record.assignee_id = user_id(pull_request.assignee) + record.created_at = pull_request.created_at + record.updated_at = pull_request.updated_at + end + + merge_request.save(validate: false) + merge_request.merge_request_diffs.create + rescue => e + error(:pull_request, pull_request.url, "#{e.message}\n\n#{e.exception.backtrace.join('\n')}") + ensure + clean_up_restored_branches(pull_request) + end + end + + repository.expire_content_cache + + errors + end + + private + + def restore_source_branch(pull_request) + repository.create_branch(pull_request.source_branch_name, pull_request.source_branch_sha) + end + + def restore_target_branch(pull_request) + repository.create_branch(pull_request.target_branch_name, pull_request.target_branch_sha) + end + + def remove_branch(name) + repository.delete_branch(name) + rescue Rugged::ReferenceError + errors << { type: :branch, url: nil, error: "Could not clean up restored branch: #{name}" } + end + + def clean_up_restored_branches(pull_request) + return if pull_request.opened? + + remove_branch(pull_request.source_branch_name) unless pull_request.source_branch_exists? + remove_branch(pull_request.target_branch_name) unless pull_request.target_branch_exists? + end + + def milestone_id(milestone) + return unless milestone.present? + + project.milestones.select(:id).find_by(iid: milestone.iid)&.id + end + + def user_id(user, fallback_id = nil) + return unless user.present? + return cached_user_ids[user.id] if cached_user_ids.key?(user.id) + + cached_user_ids[user.id] = find_by_external_uid(user.id) || find_by_email(user.email) || fallback_id + end + + def find_by_email(email) + return nil unless email + + ::User.find_by_any_email(email)&.id + end + + def find_by_external_uid(id) + return nil unless id + + identities = ::Identity.arel_table + + ::User.select(:id) + .joins(:identities) + .where(identities[:provider].eq(:github).and(identities[:extern_uid].eq(id))) + .first&.id + end + + def error(type, url, message) + errors << { type: type, url: Gitlab::UrlSanitizer.sanitize(url), error: message } + end + end +end -- cgit v1.2.1 From 0a52ae8380d52bf697057d32a404c93f11c9f93d Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 12 Apr 2017 22:40:35 -0300 Subject: Remove unused GitHub endpoint wrappers --- lib/github/issues.rb | 20 -------------------- lib/github/labels.rb | 20 -------------------- lib/github/milestones.rb | 20 -------------------- lib/github/pull_requests.rb | 20 -------------------- lib/github/releases.rb | 20 -------------------- 5 files changed, 100 deletions(-) delete mode 100644 lib/github/issues.rb delete mode 100644 lib/github/labels.rb delete mode 100644 lib/github/milestones.rb delete mode 100644 lib/github/pull_requests.rb delete mode 100644 lib/github/releases.rb (limited to 'lib') diff --git a/lib/github/issues.rb b/lib/github/issues.rb deleted file mode 100644 index 27843e1cdd8..00000000000 --- a/lib/github/issues.rb +++ /dev/null @@ -1,20 +0,0 @@ -module Github - class Issues - attr_reader :owner, :repo - - def initialize(owner, repo) - @owner = owner - @repo = repo - end - - def fetch - Collection.new(issues_url).fetch(state: :all, sort: :created, direction: :asc, per_page: 10) - end - - private - - def issues_url - "/repos/#{owner}/#{repo}/issues" - end - end -end diff --git a/lib/github/labels.rb b/lib/github/labels.rb deleted file mode 100644 index 0ea023888b3..00000000000 --- a/lib/github/labels.rb +++ /dev/null @@ -1,20 +0,0 @@ -module Github - class Labels - attr_reader :owner, :repo - - def initialize(owner, repo) - @owner = owner - @repo = repo - end - - def fetch - Collection.new(labels_url).fetch - end - - private - - def labels_url - "/repos/#{owner}/#{repo}/labels" - end - end -end diff --git a/lib/github/milestones.rb b/lib/github/milestones.rb deleted file mode 100644 index d50278105db..00000000000 --- a/lib/github/milestones.rb +++ /dev/null @@ -1,20 +0,0 @@ -module Github - class Milestones - attr_reader :owner, :repo - - def initialize(owner, repo) - @owner = owner - @repo = repo - end - - def fetch - Collection.new(milestones_url).fetch - end - - private - - def milestones_url - "/repos/#{owner}/#{repo}/milestones" - end - end -end diff --git a/lib/github/pull_requests.rb b/lib/github/pull_requests.rb deleted file mode 100644 index af04c90d454..00000000000 --- a/lib/github/pull_requests.rb +++ /dev/null @@ -1,20 +0,0 @@ -module Github - class PullRequests - attr_reader :owner, :repo - - def initialize(owner, repo) - @owner = owner - @repo = repo - end - - def fetch - Collection.new(pull_requests_url).fetch(state: :all, sort: :created, direction: :asc) - end - - private - - def pull_requests_url - "/repos/#{owner}/#{repo}/pulls" - end - end -end diff --git a/lib/github/releases.rb b/lib/github/releases.rb deleted file mode 100644 index 026ef4e6853..00000000000 --- a/lib/github/releases.rb +++ /dev/null @@ -1,20 +0,0 @@ -module Github - class Releases - attr_reader :owner, :repo - - def initialize(owner, repo) - @owner = owner - @repo = repo - end - - def fetch - Collection.new(releases_url).fetch - end - - private - - def releases_url - "/repos/#{owner}/#{repo}/releases" - end - end -end -- cgit v1.2.1 From 0b1d1931fb00f15987f695d76efb5dac9d3992d7 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 12 Apr 2017 22:41:58 -0300 Subject: Add issue representation --- lib/github/representation/issue.rb | 57 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 lib/github/representation/issue.rb (limited to 'lib') diff --git a/lib/github/representation/issue.rb b/lib/github/representation/issue.rb new file mode 100644 index 00000000000..62c71cc191b --- /dev/null +++ b/lib/github/representation/issue.rb @@ -0,0 +1,57 @@ +module Github + module Representation + class Issue < Representation::Base + def iid + raw['number'] + end + + def title + raw['title'] + end + + def description + raw['body'] || '' + end + + def milestone + return unless raw['milestone'].present? + + @milestone ||= Github::Representation::Milestone.new(raw['milestone']) + end + + def author + @author ||= Github::Representation::User.new(raw['user']) + end + + def assignee + return unless assigned? + + @assignee ||= Github::Representation::User.new(raw['assignee']) + end + + def state + raw['state'] == 'closed' ? 'closed' : 'opened' + end + + def url + raw['url'] + end + + def created_at + raw['created_at'] + end + + def updated_at + raw['updated_at'] + end + + def assigned? + raw['assignee'].present? + end + + def pull_request? + raw['pull_request'].present? + end + end + end +end -- cgit v1.2.1 From 2c92cc52d74d41663e96a0a3eabf268db3f68947 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 12 Apr 2017 22:42:12 -0300 Subject: Add comment representation --- lib/github/representation/comment.rb | 58 ++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 lib/github/representation/comment.rb (limited to 'lib') diff --git a/lib/github/representation/comment.rb b/lib/github/representation/comment.rb new file mode 100644 index 00000000000..ac7832ce28b --- /dev/null +++ b/lib/github/representation/comment.rb @@ -0,0 +1,58 @@ +module Github + module Representation + class Comment < Representation::Base + def note + raw['body'] || '' + end + + def author + @author ||= Github::Representation::User.new(raw['user']) + end + + def commit_id + raw['commit_id'] + end + + def line_code + return unless on_diff? + + parsed_lines = Gitlab::Diff::Parser.new.parse(diff_hunk.lines) + generate_line_code(parsed_lines.to_a.last) + end + + def type + 'LegacyDiffNote' if on_diff? + end + + def url + raw['url'] + end + + def created_at + raw['created_at'] + end + + def updated_at + raw['updated_at'] + end + + private + + def generate_line_code(line) + Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos) + end + + def on_diff? + diff_hunk.present? + end + + def diff_hunk + raw_data.diff_hunk + end + + def file_path + raw_data.path + end + end + end +end -- cgit v1.2.1 From 8538066e00d8bda542f219fb03d104f8364760bd Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 12 Apr 2017 22:43:38 -0300 Subject: Refactoring collection wrapper --- lib/github/collection.rb | 12 ++++-------- lib/github/repositories.rb | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/github/collection.rb b/lib/github/collection.rb index bbca12a1c84..1b0c00928c5 100644 --- a/lib/github/collection.rb +++ b/lib/github/collection.rb @@ -1,18 +1,14 @@ module Github class Collection - def initialize(url) - @url = url - end - - def fetch(query = {}) - return [] if @url.blank? + def fetch(url, query = {}) + return [] if url.blank? Enumerator.new do |yielder| loop do - response = client.get(@url, query) + response = client.get(url, query) response.body.each { |item| yielder << item } raise StopIteration unless response.rels.key?(:next) - @url = response.rels[:next] + url = response.rels[:next] end end.lazy end diff --git a/lib/github/repositories.rb b/lib/github/repositories.rb index 42342471102..b6a03173673 100644 --- a/lib/github/repositories.rb +++ b/lib/github/repositories.rb @@ -5,7 +5,7 @@ module Github end def fetch - Collection.new(repos_url).fetch + Collection.new.fetch(repos_url) end private -- cgit v1.2.1 From 104144f373a58069680dd3639e89eb1fce9a3a9b Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 12 Apr 2017 22:44:55 -0300 Subject: Refactoring client to not parse response body automatically --- lib/github/client.rb | 7 +++---- lib/github/rate_limit.rb | 10 +++++++--- lib/github/response.rb | 11 +++++------ 3 files changed, 15 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/github/client.rb b/lib/github/client.rb index 2511523bf6a..07cf264e8d7 100644 --- a/lib/github/client.rb +++ b/lib/github/client.rb @@ -4,10 +4,8 @@ module Github def initialize(token) @connection = Faraday.new(url: 'https://api.github.com') do |faraday| - faraday.adapter :net_http_persistent - faraday.response :json, content_type: /\bjson$/ faraday.authorization 'token', token - faraday.response :logger + faraday.adapter :net_http end end @@ -15,7 +13,8 @@ module Github rate_limit = RateLimit.new(connection) sleep rate_limit.reset_in if rate_limit.exceed? - Github::Response.new(connection.get(url, query)) + response = connection.get(url, query) + Github::Response.new(response.headers, response.body, response.status) end end end diff --git a/lib/github/rate_limit.rb b/lib/github/rate_limit.rb index 30b000f8a9a..9dbdf2f4c68 100644 --- a/lib/github/rate_limit.rb +++ b/lib/github/rate_limit.rb @@ -16,11 +16,11 @@ module Github end def remaining - @remaining ||= response.body.dig('rate', 'remaining').to_i + @remaining ||= body.dig('rate', 'remaining').to_i end def reset_in - @reset ||= response.body.dig('rate', 'reset').to_i + @reset ||= body.dig('rate', 'reset').to_i end private @@ -30,7 +30,11 @@ module Github end def response - @response ||= connection.get(rate_limit_url) + connection.get(rate_limit_url) + end + + def body + @body ||= Oj.load(response.body, class_cache: false, mode: :compat) end # GitHub Rate Limit API returns 404 when the rate limit is disabled diff --git a/lib/github/response.rb b/lib/github/response.rb index c34b69aa4ea..2fa852c9fdc 100644 --- a/lib/github/response.rb +++ b/lib/github/response.rb @@ -1,12 +1,11 @@ module Github class Response - attr_reader :raw, :headers, :body, :status + attr_reader :headers, :body, :status - def initialize(response) - @raw = response - @headers = response.headers - @body = response.body - @status = response.status + def initialize(headers, body, status) + @headers = headers + @body = Oj.load(body, class_cache: false, mode: :compat) + @status = status end def rels -- cgit v1.2.1 From 4a3b895d913191102d0c69a4917a8af76a4f9b67 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 12 Apr 2017 22:46:31 -0300 Subject: Refactoring Github import to avoid memory leak --- lib/github/import.rb | 187 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 129 insertions(+), 58 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 89f597551f9..19d366b0444 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -9,6 +9,24 @@ module Github self.reset_callbacks :validate end + class Issue < ::Issue + self.table_name = 'issues' + + self.reset_callbacks :save + self.reset_callbacks :commit + self.reset_callbacks :update + self.reset_callbacks :validate + end + + class Note < ::Note + self.table_name = 'notes' + + self.reset_callbacks :save + self.reset_callbacks :commit + self.reset_callbacks :update + self.reset_callbacks :validate + end + attr_reader :project, :repository, :cached_user_ids, :errors def initialize(project) @@ -22,7 +40,7 @@ module Github # Fetch repository begin project.create_repository - project.repository.add_remote('github', "https://#{token}@github.com/#{owner}/#{repo}.git") + project.repository.add_remote('github', "https://881a01d03026458e51285a4c7038c9fe4daa5561@github.com/#{owner}/#{repo}.git") project.repository.set_remote_as_mirror('github') project.repository.fetch_remote('github', forced: true) project.repository.remove_remote('github') @@ -31,74 +49,127 @@ module Github end # Fetch labels - labels = Github::Labels.new(owner, repo).fetch - - labels.each do |raw| - begin - label = Github::Representation::Label.new(raw) - project.labels.create!(title: label.title, color: label.color) - rescue => e - error(:label, label.url, e.message) + url = "/repos/#{owner}/#{repo}/labels" + + loop do + response = Github::Client.new.get(url) + + response.body.each do |raw| + begin + label = Github::Representation::Label.new(raw) + next if project.labels.where(title: label.title).exists? + + project.labels.create!(title: label.title, color: label.color) + rescue => e + error(:label, label.url, e.message) + end end + + break unless url = response.rels[:next] end # Fetch milestones - milestones = Github::Milestones.new(owner, repo).fetch - - milestones.each do |raw| - begin - milestone = Github::Representation::Milestone.new(raw) - - project.milestones.create!( - iid: milestone.iid, - title: milestone.title, - description: milestone.description, - due_date: milestone.due_date, - state: milestone.state, - created_at: milestone.created_at, - updated_at: milestone.updated_at - ) - rescue => e - error(:milestone, milestone.url, e.message) + url = "/repos/#{owner}/#{repo}/milestones" + + loop do + response = Github::Client.new.get(url, state: :all) + + response.body.each do |raw| + begin + milestone = Github::Representation::Milestone.new(raw) + next if project.milestones.where(iid: milestone.iid).exists? + + project.milestones.create!( + iid: milestone.iid, + title: milestone.title, + description: milestone.description, + due_date: milestone.due_date, + state: milestone.state, + created_at: milestone.created_at, + updated_at: milestone.updated_at + ) + rescue => e + error(:milestone, milestone.url, e.message) + end end + + break unless url = response.rels[:next] end # Fetch pull requests - pull_requests = Github::PullRequests.new(owner, repo).fetch - - pull_requests.each do |raw| - pull_request = Github::Representation::PullRequest.new(project, raw) - next unless pull_request.valid? - - begin - restore_source_branch(pull_request) unless pull_request.source_branch_exists? - restore_target_branch(pull_request) unless pull_request.target_branch_exists? - - merge_request = MergeRequest.find_or_initialize_by(iid: pull_request.iid, source_project_id: project.id) do |record| - record.iid = pull_request.iid - record.title = pull_request.title - record.description = pull_request.description - record.source_project = pull_request.source_project - record.source_branch = pull_request.source_branch_name - record.source_branch_sha = pull_request.source_branch_sha - record.target_project = pull_request.target_project - record.target_branch = pull_request.target_branch_name - record.target_branch_sha = pull_request.target_branch_sha - record.state = pull_request.state - record.milestone_id = milestone_id(pull_request.milestone) - record.author_id = user_id(pull_request.author, project.creator_id) - record.assignee_id = user_id(pull_request.assignee) - record.created_at = pull_request.created_at - record.updated_at = pull_request.updated_at + url = "/repos/#{owner}/#{repo}/pulls" + + loop do + response = Github::Client.new.get(url, state: :all, sort: :created, direction: :asc) + + response.body.each do |raw| + pull_request = Github::Representation::PullRequest.new(project, raw) + merge_request = MergeRequest.find_or_initialize_by(iid: pull_request.iid, source_project_id: project.id) + next unless merge_request.new_record? && pull_request.valid? + + begin + restore_source_branch(pull_request) unless pull_request.source_branch_exists? + restore_target_branch(pull_request) unless pull_request.target_branch_exists? + + merge_request.iid = pull_request.iid + merge_request.title = pull_request.title + merge_request.description = pull_request.description + merge_request.source_project = pull_request.source_project + merge_request.source_branch = pull_request.source_branch_name + merge_request.source_branch_sha = pull_request.source_branch_sha + merge_request.target_project = pull_request.target_project + merge_request.target_branch = pull_request.target_branch_name + merge_request.target_branch_sha = pull_request.target_branch_sha + merge_request.state = pull_request.state + merge_request.milestone_id = milestone_id(pull_request.milestone) + merge_request.author_id = user_id(pull_request.author, project.creator_id) + merge_request.assignee_id = user_id(pull_request.assignee) + merge_request.created_at = pull_request.created_at + merge_request.updated_at = pull_request.updated_at + merge_request.save(validate: false) + + merge_request.merge_request_diffs.create + rescue => e + error(:pull_request, pull_request.url, e.message) + ensure + clean_up_restored_branches(pull_request) end + end + + break unless url = response.rels[:next] + end - merge_request.save(validate: false) - merge_request.merge_request_diffs.create - rescue => e - error(:pull_request, pull_request.url, "#{e.message}\n\n#{e.exception.backtrace.join('\n')}") - ensure - clean_up_restored_branches(pull_request) + # Fetch issues + url = "/repos/#{owner}/#{repo}/issues" + + loop do + response = Github::Client.new.get(url, state: :all, sort: :created, direction: :asc) + + response.body.each do |raw| + representation = Github::Representation::Issue.new(raw) + + next if representation.pull_request? + next if Issue.where(iid: representation.iid, project_id: project.id).exists? + + begin + issue = Issue.new + issue.iid = representation.iid + issue.project_id = project.id + issue.title = representation.title + issue.description = representation.description + issue.state = representation.state + issue.milestone_id = milestone_id(representation.milestone) + issue.author_id = user_id(representation.author, project.creator_id) + issue.assignee_id = user_id(representation.assignee) + issue.created_at = representation.created_at + issue.updated_at = representation.updated_at + issue.save(validate: false) + rescue => e + error(:issue, representation.url, e.message) + end end + + break unless url = response.rels[:next] end repository.expire_content_cache -- cgit v1.2.1 From 00912ed963a3495d59d8632d8d195515e9686129 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 13 Apr 2017 13:59:26 -0300 Subject: Refactoring Github response --- lib/github/client.rb | 5 ++--- lib/github/response.rb | 14 +++++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/github/client.rb b/lib/github/client.rb index 07cf264e8d7..1c24daba1ef 100644 --- a/lib/github/client.rb +++ b/lib/github/client.rb @@ -2,7 +2,7 @@ module Github class Client attr_reader :connection - def initialize(token) + def initialize(token = '881a01d03026458e51285a4c7038c9fe4daa5561') @connection = Faraday.new(url: 'https://api.github.com') do |faraday| faraday.authorization 'token', token faraday.adapter :net_http @@ -13,8 +13,7 @@ module Github rate_limit = RateLimit.new(connection) sleep rate_limit.reset_in if rate_limit.exceed? - response = connection.get(url, query) - Github::Response.new(response.headers, response.body, response.status) + Github::Response.new(connection.get(url, query)) end end end diff --git a/lib/github/response.rb b/lib/github/response.rb index 2fa852c9fdc..2fd07dd822e 100644 --- a/lib/github/response.rb +++ b/lib/github/response.rb @@ -1,11 +1,15 @@ module Github class Response - attr_reader :headers, :body, :status + attr_reader :raw, :headers, :status - def initialize(headers, body, status) - @headers = headers - @body = Oj.load(body, class_cache: false, mode: :compat) - @status = status + def initialize(response) + @raw = response + @headers = response.headers + @status = response.status + end + + def body + @body ||= Oj.load(raw.body, class_cache: false, mode: :compat) end def rels -- cgit v1.2.1 From eb95f0e5b2bc606eeffb2e214379082862d973d6 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 13 Apr 2017 16:24:55 -0300 Subject: Fix comment representation --- lib/github/representation/comment.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/github/representation/comment.rb b/lib/github/representation/comment.rb index ac7832ce28b..819e4107118 100644 --- a/lib/github/representation/comment.rb +++ b/lib/github/representation/comment.rb @@ -47,11 +47,11 @@ module Github end def diff_hunk - raw_data.diff_hunk + raw['diff_hunk'] end def file_path - raw_data.path + raw['path'] end end end -- cgit v1.2.1 From c26076664f557f697555ced7e97fff9ea8f88aab Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 13 Apr 2017 17:05:39 -0300 Subject: Import pull requests comments --- lib/github/import.rb | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 19d366b0444..c4b03da9bc1 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -126,9 +126,69 @@ module Github merge_request.assignee_id = user_id(pull_request.assignee) merge_request.created_at = pull_request.created_at merge_request.updated_at = pull_request.updated_at - merge_request.save(validate: false) + merge_request.save!(validate: false) merge_request.merge_request_diffs.create + + # Fetch review comments + review_comments_url = "/repos/#{owner}/#{repo}/pulls/#{pull_request.iid}/comments" + + loop do + review_comments = Github::Client.new.get(review_comments_url) + + ActiveRecord::Base.no_touching do + review_comments.body.each do |raw| + begin + comment = Github::Representation::Comment.new(raw) + + note = Note.new + note.project_id = project.id + note.noteable = merge_request + note.note = comment.note + note.commit_id = comment.commit_id + note.line_code = comment.line_code + note.author_id = user_id(comment.author, project.creator_id) + note.type = comment.type + note.created_at = comment.created_at + note.updated_at = comment.updated_at + note.save!(validate: false) + rescue => e + error(:review_comment, comment.url, e.message) + end + end + end + + break unless review_comments_url = review_comments.rels[:next] + end + + # Fetch comments + comments_url = "/repos/#{owner}/#{repo}/issues/#{pull_request.iid}/comments" + + loop do + comments = Github::Client.new.get(comments_url) + + ActiveRecord::Base.no_touching do + comments.body.each do |raw| + begin + comment = Github::Representation::Comment.new(raw) + + note = Note.new + note.project_id = project.id + note.noteable = merge_request + note.note = comment.note + note.author_id = user_id(comment.author, project.creator_id) + note.created_at = comment.created_at + note.updated_at = comment.updated_at + note.save!(validate: false) + rescue => e + error(:comment, comment.url, e.message) + end + end + end + + break unless comments_url = comments.rels[:next] + end + rescue => e error(:pull_request, pull_request.url, e.message) ensure -- cgit v1.2.1 From db3220092ade9cb604b24e56a2c21092d8708f9c Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 13 Apr 2017 17:31:29 -0300 Subject: Import issues comments --- lib/github/import.rb | 30 ++++++++++++++++++++++++++++++ lib/github/representation/issue.rb | 4 ++++ 2 files changed, 34 insertions(+) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index c4b03da9bc1..c1596e97b3a 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -224,6 +224,36 @@ module Github issue.created_at = representation.created_at issue.updated_at = representation.updated_at issue.save(validate: false) + + if issue.has_comments? + # Fetch comments + comments_url = "/repos/#{owner}/#{repo}/issues/#{issue.iid}/comments" + + loop do + comments = Github::Client.new.get(comments_url) + + ActiveRecord::Base.no_touching do + comments.body.each do |raw| + begin + comment = Github::Representation::Comment.new(raw) + + note = Note.new + note.project_id = project.id + note.noteable = issue + note.note = comment.note + note.author_id = user_id(comment.author, project.creator_id) + note.created_at = comment.created_at + note.updated_at = comment.updated_at + note.save!(validate: false) + rescue => e + error(:comment, comment.url, e.message) + end + end + end + + break unless comments_url = comments.rels[:next] + end + end rescue => e error(:issue, representation.url, e.message) end diff --git a/lib/github/representation/issue.rb b/lib/github/representation/issue.rb index 62c71cc191b..c73eefa0983 100644 --- a/lib/github/representation/issue.rb +++ b/lib/github/representation/issue.rb @@ -49,6 +49,10 @@ module Github raw['assignee'].present? end + def has_comments? + raw['comments'] > 0 + end + def pull_request? raw['pull_request'].present? end -- cgit v1.2.1 From 33c8f315b96a1460d5ff1885a728e0a033f82d7b Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 13 Apr 2017 18:21:03 -0300 Subject: Apply labels to issues/merge requests --- lib/github/import.rb | 108 ++++++++++++++++++++++--------------- lib/github/representation/issue.rb | 8 +++ 2 files changed, 74 insertions(+), 42 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index c1596e97b3a..538d6f77351 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -27,11 +27,12 @@ module Github self.reset_callbacks :validate end - attr_reader :project, :repository, :cached_user_ids, :errors + attr_reader :project, :repository, :cached_label_ids, :cached_user_ids, :errors def initialize(project) @project = project @repository = project.repository + @cached_label_ids = {} @cached_user_ids = {} @errors = [] end @@ -40,7 +41,7 @@ module Github # Fetch repository begin project.create_repository - project.repository.add_remote('github', "https://881a01d03026458e51285a4c7038c9fe4daa5561@github.com/#{owner}/#{repo}.git") + project.repository.add_remote('github', "https://{token}@github.com/#{owner}/#{repo}.git") project.repository.set_remote_as_mirror('github') project.repository.fetch_remote('github', forced: true) project.repository.remove_remote('github') @@ -57,6 +58,8 @@ module Github response.body.each do |raw| begin label = Github::Representation::Label.new(raw) + + # TODO: we should take group labels in account next if project.labels.where(title: label.title).exists? project.labels.create!(title: label.title, color: label.color) @@ -68,6 +71,12 @@ module Github break unless url = response.rels[:next] end + # Cache labels + # TODO: we should take group labels in account + project.labels.select(:id, :title).find_each do |label| + @cached_label_ids[label.title] = label.id + end + # Fetch milestones url = "/repos/#{owner}/#{repo}/milestones" @@ -208,50 +217,61 @@ module Github response.body.each do |raw| representation = Github::Representation::Issue.new(raw) - next if representation.pull_request? - next if Issue.where(iid: representation.iid, project_id: project.id).exists? - begin - issue = Issue.new - issue.iid = representation.iid - issue.project_id = project.id - issue.title = representation.title - issue.description = representation.description - issue.state = representation.state - issue.milestone_id = milestone_id(representation.milestone) - issue.author_id = user_id(representation.author, project.creator_id) - issue.assignee_id = user_id(representation.assignee) - issue.created_at = representation.created_at - issue.updated_at = representation.updated_at - issue.save(validate: false) - - if issue.has_comments? - # Fetch comments - comments_url = "/repos/#{owner}/#{repo}/issues/#{issue.iid}/comments" - - loop do - comments = Github::Client.new.get(comments_url) - - ActiveRecord::Base.no_touching do - comments.body.each do |raw| - begin - comment = Github::Representation::Comment.new(raw) - - note = Note.new - note.project_id = project.id - note.noteable = issue - note.note = comment.note - note.author_id = user_id(comment.author, project.creator_id) - note.created_at = comment.created_at - note.updated_at = comment.updated_at - note.save!(validate: false) - rescue => e - error(:comment, comment.url, e.message) + # Every pull request is an issue, but not every issue + # is a pull request. For this reason, "shared" actions + # for both features, like manipulating assignees, labels + # and milestones, are provided within the Issues API. + if representation.pull_request? + next unless representation.has_labels? + + merge_request = MergeRequest.find_by!(target_project_id: project.id, iid: representation.iid) + merge_request.update_attribute(:label_ids, label_ids(representation.labels)) + else + next if Issue.where(iid: representation.iid, project_id: project.id).exists? + + issue = Issue.new + issue.iid = representation.iid + issue.project_id = project.id + issue.title = representation.title + issue.description = representation.description + issue.state = representation.state + issue.label_ids = label_ids(representation.labels) + issue.milestone_id = milestone_id(representation.milestone) + issue.author_id = user_id(representation.author, project.creator_id) + issue.assignee_id = user_id(representation.assignee) + issue.created_at = representation.created_at + issue.updated_at = representation.updated_at + issue.save(validate: false) + + if representation.has_comments? + # Fetch comments + comments_url = "/repos/#{owner}/#{repo}/issues/#{issue.iid}/comments" + + loop do + comments = Github::Client.new.get(comments_url) + + ActiveRecord::Base.no_touching do + comments.body.each do |raw| + begin + comment = Github::Representation::Comment.new(raw) + + note = Note.new + note.project_id = project.id + note.noteable = issue + note.note = comment.note + note.author_id = user_id(comment.author, project.creator_id) + note.created_at = comment.created_at + note.updated_at = comment.updated_at + note.save!(validate: false) + rescue => e + error(:comment, comment.url, e.message) + end end end - end - break unless comments_url = comments.rels[:next] + break unless comments_url = comments.rels[:next] + end end end rescue => e @@ -290,6 +310,10 @@ module Github remove_branch(pull_request.target_branch_name) unless pull_request.target_branch_exists? end + def label_ids(issuable) + issuable.map { |attrs| cached_label_ids[attrs.fetch('name')] }.compact + end + def milestone_id(milestone) return unless milestone.present? diff --git a/lib/github/representation/issue.rb b/lib/github/representation/issue.rb index c73eefa0983..f155880b984 100644 --- a/lib/github/representation/issue.rb +++ b/lib/github/representation/issue.rb @@ -13,6 +13,10 @@ module Github raw['body'] || '' end + def labels + raw['labels'] + end + def milestone return unless raw['milestone'].present? @@ -53,6 +57,10 @@ module Github raw['comments'] > 0 end + def has_labels? + labels.count > 0 + end + def pull_request? raw['pull_request'].present? end -- cgit v1.2.1 From a32adb82dc5b27069835219a5189f497686b8b04 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 13 Apr 2017 19:00:10 -0300 Subject: Remove sensitive information --- lib/github/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/github/client.rb b/lib/github/client.rb index 1c24daba1ef..1450a8d3cc0 100644 --- a/lib/github/client.rb +++ b/lib/github/client.rb @@ -2,7 +2,7 @@ module Github class Client attr_reader :connection - def initialize(token = '881a01d03026458e51285a4c7038c9fe4daa5561') + def initialize(token) @connection = Faraday.new(url: 'https://api.github.com') do |faraday| faraday.authorization 'token', token faraday.adapter :net_http -- cgit v1.2.1 From f35573f12eb579b31b014fa99509c694021c33c7 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 19:14:40 -0300 Subject: Extract common attributes to Github::Representation::Base --- lib/github/representation/base.rb | 12 ++++++++++++ lib/github/representation/comment.rb | 12 ------------ lib/github/representation/issue.rb | 12 ------------ lib/github/representation/label.rb | 4 ---- lib/github/representation/milestone.rb | 12 ------------ lib/github/representation/pull_request.rb | 12 ------------ 6 files changed, 12 insertions(+), 52 deletions(-) (limited to 'lib') diff --git a/lib/github/representation/base.rb b/lib/github/representation/base.rb index 6765bfb803b..5ea294ed49c 100644 --- a/lib/github/representation/base.rb +++ b/lib/github/representation/base.rb @@ -5,6 +5,18 @@ module Github @raw = raw end + def url + raw['url'] + end + + def created_at + raw['created_at'] + end + + def updated_at + raw['updated_at'] + end + private attr_reader :raw diff --git a/lib/github/representation/comment.rb b/lib/github/representation/comment.rb index 819e4107118..02bcd9eaa0e 100644 --- a/lib/github/representation/comment.rb +++ b/lib/github/representation/comment.rb @@ -24,18 +24,6 @@ module Github 'LegacyDiffNote' if on_diff? end - def url - raw['url'] - end - - def created_at - raw['created_at'] - end - - def updated_at - raw['updated_at'] - end - private def generate_line_code(line) diff --git a/lib/github/representation/issue.rb b/lib/github/representation/issue.rb index f155880b984..3bb767a5daa 100644 --- a/lib/github/representation/issue.rb +++ b/lib/github/representation/issue.rb @@ -37,18 +37,6 @@ module Github raw['state'] == 'closed' ? 'closed' : 'opened' end - def url - raw['url'] - end - - def created_at - raw['created_at'] - end - - def updated_at - raw['updated_at'] - end - def assigned? raw['assignee'].present? end diff --git a/lib/github/representation/label.rb b/lib/github/representation/label.rb index b3140ab76fc..60aa51f9569 100644 --- a/lib/github/representation/label.rb +++ b/lib/github/representation/label.rb @@ -8,10 +8,6 @@ module Github def title raw['name'] end - - def url - raw['url'] - end end end end diff --git a/lib/github/representation/milestone.rb b/lib/github/representation/milestone.rb index 5ea54eb178f..917e6394ad4 100644 --- a/lib/github/representation/milestone.rb +++ b/lib/github/representation/milestone.rb @@ -20,18 +20,6 @@ module Github def state raw['state'] == 'closed' ? 'closed' : 'active' end - - def url - raw['url'] - end - - def created_at - raw['created_at'] - end - - def updated_at - raw['updated_at'] - end end end end diff --git a/lib/github/representation/pull_request.rb b/lib/github/representation/pull_request.rb index 85f4e1bdac3..b33561565bf 100644 --- a/lib/github/representation/pull_request.rb +++ b/lib/github/representation/pull_request.rb @@ -71,18 +71,6 @@ module Github 'opened' end - def url - raw['url'] - end - - def created_at - raw['created_at'] - end - - def updated_at - raw['updated_at'] - end - def assigned? raw['assignee'].present? end -- cgit v1.2.1 From 00e3d60c3d0ad1b6c981e3069d8b815d5119aa90 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 19:17:42 -0300 Subject: Extract Github::Representation::Issuable --- lib/github/representation/issuable.rb | 37 +++++++++++++++++++++++++++++++ lib/github/representation/issue.rb | 34 +--------------------------- lib/github/representation/pull_request.rb | 34 +--------------------------- 3 files changed, 39 insertions(+), 66 deletions(-) create mode 100644 lib/github/representation/issuable.rb (limited to 'lib') diff --git a/lib/github/representation/issuable.rb b/lib/github/representation/issuable.rb new file mode 100644 index 00000000000..a55976f9019 --- /dev/null +++ b/lib/github/representation/issuable.rb @@ -0,0 +1,37 @@ +module Github + module Representation + class Issuable < Representation::Base + def iid + raw['number'] + end + + def title + raw['title'] + end + + def description + raw['body'] || '' + end + + def milestone + return unless raw['milestone'].present? + + @milestone ||= Github::Representation::Milestone.new(raw['milestone']) + end + + def author + @author ||= Github::Representation::User.new(raw['user']) + end + + def assignee + return unless assigned? + + @assignee ||= Github::Representation::User.new(raw['assignee']) + end + + def assigned? + raw['assignee'].present? + end + end + end +end diff --git a/lib/github/representation/issue.rb b/lib/github/representation/issue.rb index 3bb767a5daa..df3540a6e6c 100644 --- a/lib/github/representation/issue.rb +++ b/lib/github/representation/issue.rb @@ -1,46 +1,14 @@ module Github module Representation - class Issue < Representation::Base - def iid - raw['number'] - end - - def title - raw['title'] - end - - def description - raw['body'] || '' - end - + class Issue < Representation::Issuable def labels raw['labels'] end - def milestone - return unless raw['milestone'].present? - - @milestone ||= Github::Representation::Milestone.new(raw['milestone']) - end - - def author - @author ||= Github::Representation::User.new(raw['user']) - end - - def assignee - return unless assigned? - - @assignee ||= Github::Representation::User.new(raw['assignee']) - end - def state raw['state'] == 'closed' ? 'closed' : 'opened' end - def assigned? - raw['assignee'].present? - end - def has_comments? raw['comments'] > 0 end diff --git a/lib/github/representation/pull_request.rb b/lib/github/representation/pull_request.rb index b33561565bf..0596b0425a2 100644 --- a/lib/github/representation/pull_request.rb +++ b/lib/github/representation/pull_request.rb @@ -1,6 +1,6 @@ module Github module Representation - class PullRequest < Representation::Base + class PullRequest < Representation::Issuable attr_reader :project delegate :user, :repo, :ref, :sha, to: :source_branch, prefix: true @@ -11,18 +11,6 @@ module Github @raw = raw end - def iid - raw['number'] - end - - def title - raw['title'] - end - - def description - raw['body'] || '' - end - def source_project project end @@ -48,22 +36,6 @@ module Github @target_branch_name ||= target_branch_exists? ? target_branch_ref : target_branch_name_prefixed end - def milestone - return unless raw['milestone'].present? - - @milestone ||= Github::Representation::Milestone.new(raw['milestone']) - end - - def author - @author ||= Github::Representation::User.new(raw['user']) - end - - def assignee - return unless assigned? - - @assignee ||= Github::Representation::User.new(raw['assignee']) - end - def state return 'merged' if raw['state'] == 'closed' && raw['merged_at'].present? return 'closed' if raw['state'] == 'closed' @@ -71,10 +43,6 @@ module Github 'opened' end - def assigned? - raw['assignee'].present? - end - def opened? state == 'opened' end -- cgit v1.2.1 From ac1634fac9ef2891ef98d499fe6391d315b98b30 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 19:41:17 -0300 Subject: Extract a method to import issues/pull requests comments --- lib/github/import.rb | 112 +++++++++++++++------------------------------------ 1 file changed, 33 insertions(+), 79 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 538d6f77351..333bfa0fe05 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -141,63 +141,11 @@ module Github # Fetch review comments review_comments_url = "/repos/#{owner}/#{repo}/pulls/#{pull_request.iid}/comments" - - loop do - review_comments = Github::Client.new.get(review_comments_url) - - ActiveRecord::Base.no_touching do - review_comments.body.each do |raw| - begin - comment = Github::Representation::Comment.new(raw) - - note = Note.new - note.project_id = project.id - note.noteable = merge_request - note.note = comment.note - note.commit_id = comment.commit_id - note.line_code = comment.line_code - note.author_id = user_id(comment.author, project.creator_id) - note.type = comment.type - note.created_at = comment.created_at - note.updated_at = comment.updated_at - note.save!(validate: false) - rescue => e - error(:review_comment, comment.url, e.message) - end - end - end - - break unless review_comments_url = review_comments.rels[:next] - end + fetch_comments(merge_request, :review_comment, review_comments_url) # Fetch comments comments_url = "/repos/#{owner}/#{repo}/issues/#{pull_request.iid}/comments" - - loop do - comments = Github::Client.new.get(comments_url) - - ActiveRecord::Base.no_touching do - comments.body.each do |raw| - begin - comment = Github::Representation::Comment.new(raw) - - note = Note.new - note.project_id = project.id - note.noteable = merge_request - note.note = comment.note - note.author_id = user_id(comment.author, project.creator_id) - note.created_at = comment.created_at - note.updated_at = comment.updated_at - note.save!(validate: false) - rescue => e - error(:comment, comment.url, e.message) - end - end - end - - break unless comments_url = comments.rels[:next] - end - + fetch_comments(merge_request, :comment, comments_url) rescue => e error(:pull_request, pull_request.url, e.message) ensure @@ -247,31 +195,7 @@ module Github if representation.has_comments? # Fetch comments comments_url = "/repos/#{owner}/#{repo}/issues/#{issue.iid}/comments" - - loop do - comments = Github::Client.new.get(comments_url) - - ActiveRecord::Base.no_touching do - comments.body.each do |raw| - begin - comment = Github::Representation::Comment.new(raw) - - note = Note.new - note.project_id = project.id - note.noteable = issue - note.note = comment.note - note.author_id = user_id(comment.author, project.creator_id) - note.created_at = comment.created_at - note.updated_at = comment.updated_at - note.save!(validate: false) - rescue => e - error(:comment, comment.url, e.message) - end - end - end - - break unless comments_url = comments.rels[:next] - end + fetch_comments(issue, :comment, comments_url) end end rescue => e @@ -289,6 +213,36 @@ module Github private + def fetch_comments(noteable, type, url) + loop do + comments = Github::Client.new.get(url) + + ActiveRecord::Base.no_touching do + comments.body.each do |raw| + begin + representation = Github::Representation::Comment.new(raw) + + note = Note.new + note.project_id = project.id + note.noteable = noteable + note.note = representation.note + note.commit_id = representation.commit_id + note.line_code = representation.line_code + note.author_id = user_id(representation.author, project.creator_id) + note.type = representation.type + note.created_at = representation.created_at + note.updated_at = representation.updated_at + note.save!(validate: false) + rescue => e + error(type, representation.url, e.message) + end + end + end + + break unless url = comments.rels[:next] + end + end + def restore_source_branch(pull_request) repository.create_branch(pull_request.source_branch_name, pull_request.source_branch_sha) end -- cgit v1.2.1 From 782aab1319bdcfbe1634d4b33444e8ce5b57d394 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 20:04:58 -0300 Subject: Pass a options hash to Github::Client --- lib/github/client.rb | 6 +++--- lib/github/collection.rb | 8 +++++++- lib/github/import.rb | 23 ++++++++++++----------- lib/github/representation/base.rb | 7 ++++--- lib/github/representation/comment.rb | 2 +- lib/github/representation/issuable.rb | 4 ++-- lib/github/representation/pull_request.rb | 5 +++-- lib/github/representation/user.rb | 2 +- lib/github/user.rb | 7 ++++--- 9 files changed, 37 insertions(+), 27 deletions(-) (limited to 'lib') diff --git a/lib/github/client.rb b/lib/github/client.rb index 1450a8d3cc0..95536cae57f 100644 --- a/lib/github/client.rb +++ b/lib/github/client.rb @@ -2,9 +2,9 @@ module Github class Client attr_reader :connection - def initialize(token) - @connection = Faraday.new(url: 'https://api.github.com') do |faraday| - faraday.authorization 'token', token + def initialize(options) + @connection = Faraday.new(url: options.fetch(:url)) do |faraday| + faraday.authorization 'token', options.fetch(:token) faraday.adapter :net_http end end diff --git a/lib/github/collection.rb b/lib/github/collection.rb index 1b0c00928c5..12d6703476b 100644 --- a/lib/github/collection.rb +++ b/lib/github/collection.rb @@ -1,5 +1,11 @@ module Github class Collection + attr_reader :options + + def initialize(options) + @options = options + end + def fetch(url, query = {}) return [] if url.blank? @@ -16,7 +22,7 @@ module Github private def client - @client ||= Github::Client.new + @client ||= Github::Client.new(options) end end end diff --git a/lib/github/import.rb b/lib/github/import.rb index 333bfa0fe05..17244d8aea1 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -27,17 +27,18 @@ module Github self.reset_callbacks :validate end - attr_reader :project, :repository, :cached_label_ids, :cached_user_ids, :errors + attr_reader :project, :repository, :options, :cached_label_ids, :cached_user_ids, :errors - def initialize(project) + def initialize(project, options) @project = project @repository = project.repository + @options = options @cached_label_ids = {} @cached_user_ids = {} @errors = [] end - def execute(owner, repo, token) + def execute(owner, repo) # Fetch repository begin project.create_repository @@ -53,7 +54,7 @@ module Github url = "/repos/#{owner}/#{repo}/labels" loop do - response = Github::Client.new.get(url) + response = Github::Client.new(options).get(url) response.body.each do |raw| begin @@ -81,7 +82,7 @@ module Github url = "/repos/#{owner}/#{repo}/milestones" loop do - response = Github::Client.new.get(url, state: :all) + response = Github::Client.new(options).get(url, state: :all) response.body.each do |raw| begin @@ -109,10 +110,10 @@ module Github url = "/repos/#{owner}/#{repo}/pulls" loop do - response = Github::Client.new.get(url, state: :all, sort: :created, direction: :asc) + response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc) response.body.each do |raw| - pull_request = Github::Representation::PullRequest.new(project, raw) + pull_request = Github::Representation::PullRequest.new(project, raw, options) merge_request = MergeRequest.find_or_initialize_by(iid: pull_request.iid, source_project_id: project.id) next unless merge_request.new_record? && pull_request.valid? @@ -160,10 +161,10 @@ module Github url = "/repos/#{owner}/#{repo}/issues" loop do - response = Github::Client.new.get(url, state: :all, sort: :created, direction: :asc) + response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc) response.body.each do |raw| - representation = Github::Representation::Issue.new(raw) + representation = Github::Representation::Issue.new(raw, options) begin # Every pull request is an issue, but not every issue @@ -215,12 +216,12 @@ module Github def fetch_comments(noteable, type, url) loop do - comments = Github::Client.new.get(url) + comments = Github::Client.new(options).get(url) ActiveRecord::Base.no_touching do comments.body.each do |raw| begin - representation = Github::Representation::Comment.new(raw) + representation = Github::Representation::Comment.new(raw, options) note = Note.new note.project_id = project.id diff --git a/lib/github/representation/base.rb b/lib/github/representation/base.rb index 5ea294ed49c..385e62ae99d 100644 --- a/lib/github/representation/base.rb +++ b/lib/github/representation/base.rb @@ -1,8 +1,9 @@ module Github module Representation class Base - def initialize(raw) - @raw = raw + def initialize(raw, options = {}) + @raw = raw + @options = options end def url @@ -19,7 +20,7 @@ module Github private - attr_reader :raw + attr_reader :raw, :options end end end diff --git a/lib/github/representation/comment.rb b/lib/github/representation/comment.rb index 02bcd9eaa0e..22cb98b0eff 100644 --- a/lib/github/representation/comment.rb +++ b/lib/github/representation/comment.rb @@ -6,7 +6,7 @@ module Github end def author - @author ||= Github::Representation::User.new(raw['user']) + @author ||= Github::Representation::User.new(raw['user'], options) end def commit_id diff --git a/lib/github/representation/issuable.rb b/lib/github/representation/issuable.rb index a55976f9019..9713b82615d 100644 --- a/lib/github/representation/issuable.rb +++ b/lib/github/representation/issuable.rb @@ -20,13 +20,13 @@ module Github end def author - @author ||= Github::Representation::User.new(raw['user']) + @author ||= Github::Representation::User.new(raw['user'], options) end def assignee return unless assigned? - @assignee ||= Github::Representation::User.new(raw['assignee']) + @assignee ||= Github::Representation::User.new(raw['assignee'], options) end def assigned? diff --git a/lib/github/representation/pull_request.rb b/lib/github/representation/pull_request.rb index 0596b0425a2..4119ca400c6 100644 --- a/lib/github/representation/pull_request.rb +++ b/lib/github/representation/pull_request.rb @@ -6,9 +6,10 @@ module Github delegate :user, :repo, :ref, :sha, to: :source_branch, prefix: true delegate :user, :exists?, :repo, :ref, :sha, :short_sha, to: :target_branch, prefix: true - def initialize(project, raw) + def initialize(project, raw, options) @project = project - @raw = raw + @raw = raw + @options = options end def source_project diff --git a/lib/github/representation/user.rb b/lib/github/representation/user.rb index 70a0ce000ae..79758555319 100644 --- a/lib/github/representation/user.rb +++ b/lib/github/representation/user.rb @@ -8,7 +8,7 @@ module Github def email return @email if defined?(@email) - @email = Github::User.new(username).get.fetch('email', nil) + @email = Github::User.new(username, options).get.fetch('email', nil) end def username diff --git a/lib/github/user.rb b/lib/github/user.rb index 19fe6230820..f88a29e590b 100644 --- a/lib/github/user.rb +++ b/lib/github/user.rb @@ -1,9 +1,10 @@ module Github class User - attr_reader :username + attr_reader :username, :options - def initialize(username) + def initialize(username, options) @username = username + @options = options end def get @@ -13,7 +14,7 @@ module Github private def client - @client ||= Github::Client.new + @client ||= Github::Client.new(options) end def user_url -- cgit v1.2.1 From 5691c9b0611645ac3a6f5e32d033d7bafb09d781 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 20:31:52 -0300 Subject: Does not remove the GitHub remote --- lib/github/import.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 17244d8aea1..17dd6dd0ccf 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -45,7 +45,6 @@ module Github project.repository.add_remote('github', "https://{token}@github.com/#{owner}/#{repo}.git") project.repository.set_remote_as_mirror('github') project.repository.fetch_remote('github', forced: true) - project.repository.remove_remote('github') rescue Gitlab::Shell::Error => e error(:project, "https://github.com/#{owner}/#{repo}.git", e.message) end -- cgit v1.2.1 From 181445307cbb4a8cb2904a3b969bc39568d133d5 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 20:39:04 -0300 Subject: Add a method to format issues/pull requests/comments body --- lib/github/import.rb | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 17dd6dd0ccf..0552320a474 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -27,7 +27,8 @@ module Github self.reset_callbacks :validate end - attr_reader :project, :repository, :options, :cached_label_ids, :cached_user_ids, :errors + attr_reader :project, :repository, :options, :cached_label_ids, + :cached_gitlab_users, :cached_user_ids, :errors def initialize(project, options) @project = project @@ -35,6 +36,7 @@ module Github @options = options @cached_label_ids = {} @cached_user_ids = {} + @cached_gitlab_users = {} @errors = [] end @@ -120,9 +122,10 @@ module Github restore_source_branch(pull_request) unless pull_request.source_branch_exists? restore_target_branch(pull_request) unless pull_request.target_branch_exists? + author_id = user_id(pull_request.author, project.creator_id) merge_request.iid = pull_request.iid merge_request.title = pull_request.title - merge_request.description = pull_request.description + merge_request.description = format_description(pull_request.description, pull_request.author) merge_request.source_project = pull_request.source_project merge_request.source_branch = pull_request.source_branch_name merge_request.source_branch_sha = pull_request.source_branch_sha @@ -131,7 +134,7 @@ module Github merge_request.target_branch_sha = pull_request.target_branch_sha merge_request.state = pull_request.state merge_request.milestone_id = milestone_id(pull_request.milestone) - merge_request.author_id = user_id(pull_request.author, project.creator_id) + merge_request.author_id = author_id merge_request.assignee_id = user_id(pull_request.assignee) merge_request.created_at = pull_request.created_at merge_request.updated_at = pull_request.updated_at @@ -178,19 +181,20 @@ module Github else next if Issue.where(iid: representation.iid, project_id: project.id).exists? + author_id = user_id(representation.author, project.creator_id) issue = Issue.new issue.iid = representation.iid issue.project_id = project.id issue.title = representation.title - issue.description = representation.description + issue.description = format_description(representation.description, representation.author) issue.state = representation.state issue.label_ids = label_ids(representation.labels) issue.milestone_id = milestone_id(representation.milestone) - issue.author_id = user_id(representation.author, project.creator_id) + issue.author_id = author_id issue.assignee_id = user_id(representation.assignee) issue.created_at = representation.created_at issue.updated_at = representation.updated_at - issue.save(validate: false) + issue.save!(validate: false) if representation.has_comments? # Fetch comments @@ -220,15 +224,16 @@ module Github ActiveRecord::Base.no_touching do comments.body.each do |raw| begin - representation = Github::Representation::Comment.new(raw, options) + representation = Github::Representation::Comment.new(raw, options) + author_id = user_id(representation.author, project.creator_id) note = Note.new note.project_id = project.id note.noteable = noteable - note.note = representation.note + note.note = format_description(representation.note, representation.author) note.commit_id = representation.commit_id note.line_code = representation.line_code - note.author_id = user_id(representation.author, project.creator_id) + note.author_id = author_id note.type = representation.type note.created_at = representation.created_at note.updated_at = representation.updated_at @@ -278,7 +283,10 @@ module Github return unless user.present? return cached_user_ids[user.id] if cached_user_ids.key?(user.id) - cached_user_ids[user.id] = find_by_external_uid(user.id) || find_by_email(user.email) || fallback_id + gitlab_user_id = find_by_external_uid(user.id) || find_by_email(user.email) + + cached_gitlab_users[user.id] = gitlab_user_id.present? + cached_user_ids[user.id] = gitlab_user_id || fallback_id end def find_by_email(email) @@ -298,6 +306,12 @@ module Github .first&.id end + def format_description(body, author) + return body if cached_gitlab_users[author.id] + + "*Created by: #{author.username}*\n\n#{body}" + end + def error(type, url, message) errors << { type: type, url: Gitlab::UrlSanitizer.sanitize(url), error: message } end -- cgit v1.2.1 From a7cb336e55a56797919dddeef3978997962127d3 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 20:45:49 -0300 Subject: Use while instead of loop/break --- lib/github/import.rb | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 0552320a474..e9632d3ee11 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -54,14 +54,12 @@ module Github # Fetch labels url = "/repos/#{owner}/#{repo}/labels" - loop do + while url response = Github::Client.new(options).get(url) response.body.each do |raw| begin label = Github::Representation::Label.new(raw) - - # TODO: we should take group labels in account next if project.labels.where(title: label.title).exists? project.labels.create!(title: label.title, color: label.color) @@ -70,7 +68,7 @@ module Github end end - break unless url = response.rels[:next] + url = response.rels[:next] end # Cache labels @@ -82,7 +80,7 @@ module Github # Fetch milestones url = "/repos/#{owner}/#{repo}/milestones" - loop do + while url response = Github::Client.new(options).get(url, state: :all) response.body.each do |raw| @@ -104,13 +102,13 @@ module Github end end - break unless url = response.rels[:next] + url = response.rels[:next] end # Fetch pull requests url = "/repos/#{owner}/#{repo}/pulls" - loop do + while url response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc) response.body.each do |raw| @@ -156,13 +154,13 @@ module Github end end - break unless url = response.rels[:next] + url = response.rels[:next] end # Fetch issues url = "/repos/#{owner}/#{repo}/issues" - loop do + while url response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc) response.body.each do |raw| @@ -207,7 +205,7 @@ module Github end end - break unless url = response.rels[:next] + url = response.rels[:next] end repository.expire_content_cache @@ -218,7 +216,7 @@ module Github private def fetch_comments(noteable, type, url) - loop do + while url comments = Github::Client.new(options).get(url) ActiveRecord::Base.no_touching do @@ -244,7 +242,7 @@ module Github end end - break unless url = comments.rels[:next] + url = comments.rels[:next] end end -- cgit v1.2.1 From 3aa89795568dc2fbc53f2fac8c4f8ac3499dadb0 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 21:03:46 -0300 Subject: Refactoring Github import --- lib/github/import.rb | 63 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 25 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index e9632d3ee11..12f4dbc9e43 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -27,12 +27,13 @@ module Github self.reset_callbacks :validate end - attr_reader :project, :repository, :options, :cached_label_ids, - :cached_gitlab_users, :cached_user_ids, :errors + attr_reader :project, :repository, :repo, :options, :errors, + :cached_label_ids, :cached_gitlab_users, :cached_user_ids def initialize(project, options) @project = project @repository = project.repository + @repo = project.import_source @options = options @cached_label_ids = {} @cached_user_ids = {} @@ -40,19 +41,32 @@ module Github @errors = [] end - def execute(owner, repo) - # Fetch repository + def execute + fetch_repository + fetch_labels + fetch_milestones + fetch_pull_requests + fetch_issues + expire_repository_cache + + errors + end + + private + + def fetch_repository begin project.create_repository - project.repository.add_remote('github', "https://{token}@github.com/#{owner}/#{repo}.git") + project.repository.add_remote('github', "https://{token}@github.com/#{repo}.git") project.repository.set_remote_as_mirror('github') project.repository.fetch_remote('github', forced: true) rescue Gitlab::Shell::Error => e - error(:project, "https://github.com/#{owner}/#{repo}.git", e.message) + error(:project, "https://github.com/#{repo}.git", e.message) end + end - # Fetch labels - url = "/repos/#{owner}/#{repo}/labels" + def fetch_labels + url = "/repos/#{repo}/labels" while url response = Github::Client.new(options).get(url) @@ -72,13 +86,13 @@ module Github end # Cache labels - # TODO: we should take group labels in account project.labels.select(:id, :title).find_each do |label| @cached_label_ids[label.title] = label.id end + end - # Fetch milestones - url = "/repos/#{owner}/#{repo}/milestones" + def fetch_milestones + url = "/repos/#{repo}/milestones" while url response = Github::Client.new(options).get(url, state: :all) @@ -104,9 +118,10 @@ module Github url = response.rels[:next] end + end - # Fetch pull requests - url = "/repos/#{owner}/#{repo}/pulls" + def fetch_pull_requests + url = "/repos/#{repo}/pulls" while url response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc) @@ -141,14 +156,13 @@ module Github merge_request.merge_request_diffs.create # Fetch review comments - review_comments_url = "/repos/#{owner}/#{repo}/pulls/#{pull_request.iid}/comments" + review_comments_url = "/repos/#{repo}/pulls/#{pull_request.iid}/comments" fetch_comments(merge_request, :review_comment, review_comments_url) # Fetch comments - comments_url = "/repos/#{owner}/#{repo}/issues/#{pull_request.iid}/comments" + comments_url = "/repos/#{repo}/issues/#{pull_request.iid}/comments" fetch_comments(merge_request, :comment, comments_url) rescue => e - error(:pull_request, pull_request.url, e.message) ensure clean_up_restored_branches(pull_request) end @@ -156,9 +170,10 @@ module Github url = response.rels[:next] end + end - # Fetch issues - url = "/repos/#{owner}/#{repo}/issues" + def fetch_issues + url = "/repos/#{repo}/issues" while url response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc) @@ -196,7 +211,7 @@ module Github if representation.has_comments? # Fetch comments - comments_url = "/repos/#{owner}/#{repo}/issues/#{issue.iid}/comments" + comments_url = "/repos/#{repo}/issues/#{issue.iid}/comments" fetch_comments(issue, :comment, comments_url) end end @@ -207,14 +222,8 @@ module Github url = response.rels[:next] end - - repository.expire_content_cache - - errors end - private - def fetch_comments(noteable, type, url) while url comments = Github::Client.new(options).get(url) @@ -310,6 +319,10 @@ module Github "*Created by: #{author.username}*\n\n#{body}" end + def expire_repository_cache + repository.expire_content_cache + end + def error(type, url, message) errors << { type: type, url: Gitlab::UrlSanitizer.sanitize(url), error: message } end -- cgit v1.2.1 From 3c0a713a379025c079f8a684943e95af087c9e86 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 21:18:11 -0300 Subject: Import Github releases --- lib/github/import.rb | 27 +++++++++++++++++++++++++++ lib/github/representation/release.rb | 17 +++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 lib/github/representation/release.rb (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 12f4dbc9e43..4ef0ac3e40c 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -255,6 +255,33 @@ module Github end end + def fetch_releases + url = "/repos/#{repo}/releases" + + while url + response = Github::Client.new(options).get(url) + + response.body.each do |raw| + representation = Github::Representation::Release.new(raw) + next unless representation.valid? + + release = ::Release.find_or_initialize_by(project_id: project.id, tag: representation.tag) + next unless relese.new_record? + + begin + release.description = representation.description + release.created_at = representation.created_at + release.updated_at = representation.updated_at + release.save!(validate: false) + rescue => e + error(:release, representation.url, e.message) + end + end + + url = response.rels[:next] + end + end + def restore_source_branch(pull_request) repository.create_branch(pull_request.source_branch_name, pull_request.source_branch_sha) end diff --git a/lib/github/representation/release.rb b/lib/github/representation/release.rb new file mode 100644 index 00000000000..e7e4b428c1a --- /dev/null +++ b/lib/github/representation/release.rb @@ -0,0 +1,17 @@ +module Github + module Representation + class Release < Representation::Base + def description + raw['body'] + end + + def tag + raw['tag_name'] + end + + def valid? + !raw['draft'] + end + end + end +end -- cgit v1.2.1 From bd9e5c5ddf1810da33c65090c16f4e44e087cbcf Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 22:01:14 -0300 Subject: Clone GitHub wiki --- lib/github/import.rb | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 4ef0ac3e40c..bf0df041f15 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -1,5 +1,6 @@ module Github class Import + class MergeRequest < ::MergeRequest self.table_name = 'merge_requests' @@ -47,6 +48,7 @@ module Github fetch_milestones fetch_pull_requests fetch_issues + fetch_wiki_repository expire_repository_cache errors @@ -57,7 +59,7 @@ module Github def fetch_repository begin project.create_repository - project.repository.add_remote('github', "https://{token}@github.com/#{repo}.git") + project.repository.add_remote('github', "https://{options.fetch(:token)}@github.com/#{repo}.git") project.repository.set_remote_as_mirror('github') project.repository.fetch_remote('github', forced: true) rescue Gitlab::Shell::Error => e @@ -65,6 +67,22 @@ module Github end end + def fetch_wiki_repository + wiki_url = "https://{options.fetch(:token)}@github.com/#{repo}.wiki.git" + wiki_path = "#{project.path_with_namespace}.wiki" + + unless project.wiki.repository_exists? + gitlab_shell.import_repository(project.repository_storage_path, wiki_path, wiki_url) + end + 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/ + errors(:wiki, wiki_url, e.message) + end + end + def fetch_labels url = "/repos/#{repo}/labels" @@ -163,6 +181,7 @@ module Github comments_url = "/repos/#{repo}/issues/#{pull_request.iid}/comments" fetch_comments(merge_request, :comment, comments_url) rescue => e + error(:pull_request, pull_request.url, e.message) ensure clean_up_restored_branches(pull_request) end @@ -209,8 +228,8 @@ module Github issue.updated_at = representation.updated_at issue.save!(validate: false) + # Fetch comments if representation.has_comments? - # Fetch comments comments_url = "/repos/#{repo}/issues/#{issue.iid}/comments" fetch_comments(issue, :comment, comments_url) end -- cgit v1.2.1 From 09a6d32817f5f4da8ff9f3331a7046670a10beed Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 22:01:37 -0300 Subject: Keep track of import errors --- lib/github/import.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index bf0df041f15..a1854f7f235 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -50,6 +50,7 @@ module Github fetch_issues fetch_wiki_repository expire_repository_cache + track_errors errors end @@ -369,6 +370,15 @@ module Github repository.expire_content_cache end + def track_errors + return unless errors.any? + + project.update_column(:import_error, { + message: 'The remote data could not be fully imported.', + errors: errors + }.to_json) + end + def error(type, url, message) errors << { type: type, url: Gitlab::UrlSanitizer.sanitize(url), error: message } end -- cgit v1.2.1 From e50606cd2d0c5a010d3c139cbad1eaf16701d8fd Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 19 Apr 2017 22:54:09 -0300 Subject: Refactor rake task to to import GitHub repositores --- lib/github/import.rb | 1 + lib/github/repositories.rb | 8 ++-- lib/tasks/import.rake | 111 +++++++++------------------------------------ 3 files changed, 28 insertions(+), 92 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index a1854f7f235..de987c263b6 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -1,5 +1,6 @@ module Github class Import + include Gitlab::ShellAdapter class MergeRequest < ::MergeRequest self.table_name = 'merge_requests' diff --git a/lib/github/repositories.rb b/lib/github/repositories.rb index b6a03173673..c1c9448f305 100644 --- a/lib/github/repositories.rb +++ b/lib/github/repositories.rb @@ -1,11 +1,13 @@ module Github class Repositories - def initialize(username) - @username = username + attr_reader :options + + def initialize(options) + @options = options end def fetch - Collection.new.fetch(repos_url) + Collection.new(options).fetch(repos_url) end private diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index a9dad6a1bf0..897b2f04ce1 100644 --- a/lib/tasks/import.rake +++ b/lib/tasks/import.rake @@ -1,67 +1,5 @@ require 'benchmark' require 'rainbow/ext/string' -require_relative '../gitlab/shell_adapter' -require_relative '../gitlab/github_import/importer' - -class NewImporter < ::Gitlab::GithubImport::Importer - def execute - # Same as ::Gitlab::GithubImport::Importer#execute, but showing some progress. - puts 'Importing repository...'.color(:aqua) - import_repository unless project.repository_exists? - - puts 'Importing labels...'.color(:aqua) - import_labels - - puts 'Importing milestones...'.color(:aqua) - import_milestones - - puts 'Importing pull requests...'.color(:aqua) - import_pull_requests - - puts 'Importing issues...'.color(:aqua) - import_issues - - puts 'Importing issue comments...'.color(:aqua) - import_comments(:issues) - - puts 'Importing pull request comments...'.color(:aqua) - import_comments(:pull_requests) - - puts 'Importing wiki...'.color(:aqua) - import_wiki - - # Gitea doesn't have a Release API yet - # See https://github.com/go-gitea/gitea/issues/330 - unless project.gitea_import? - import_releases - end - - handle_errors - - project.repository.after_import - project.import_finish - - true - end - - def import_repository - begin - raise 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url) - - project.create_repository - project.repository.add_remote(project.import_type, project.import_url) - project.repository.set_remote_as_mirror(project.import_type) - project.repository.fetch_remote(project.import_type, forced: true) - rescue => e - # Expire cache to prevent scenarios such as: - # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true - # 2. Retried import, repo is broken or not imported but +exists?+ still returns true - project.repository.expire_content_cache if project.repository_exists? - - raise "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}" - end - end -end class GithubImport def self.run!(*args) @@ -69,14 +7,14 @@ class GithubImport end def initialize(token, gitlab_username, project_path, extras) - @token = token + @options = { url: 'https://api.github.com', token: token } @project_path = project_path @current_user = User.find_by_username(gitlab_username) @github_repo = extras.empty? ? nil : extras.first end def run! - @repo = GithubRepos.new(@token, @current_user, @github_repo).choose_one! + @repo = GithubRepos.new(@options, @current_user, @github_repo).choose_one! raise 'No repo found!' unless @repo @@ -90,25 +28,24 @@ class GithubImport private def show_warning! - puts "This will import GH #{@repo.full_name.bright} into GL #{@project_path.bright} as #{@current_user.name}" + puts "This will import GH #{@repo['full_name'].bright} into GL #{@project_path.bright} as #{@current_user.name}" puts "Permission checks are ignored. Press any key to continue.".color(:red) STDIN.getch - puts 'Starting the import...'.color(:green) + puts 'Starting the import (this could take a while)'.color(:green) end def import! - import_url = @project.import_url.gsub(/\:\/\/(.*@)?/, "://#{@token}@") - @project.update(import_url: import_url) - @project.import_start timings = Benchmark.measure do - NewImporter.new(@project).execute + Github::Import.new(@project, @options).execute end puts "Import finished. Timings: #{timings}".color(:green) + + @project.import_finish end def new_project @@ -116,17 +53,17 @@ class GithubImport namespace_path, _sep, name = @project_path.rpartition('/') namespace = find_or_create_namespace(namespace_path) - Project.create!( - import_url: "https://#{@token}@github.com/#{@repo.full_name}.git", + Projects::CreateService.new( + @current_user, name: name, path: name, - description: @repo.description, - namespace: namespace, + description: @repo['description'], + namespace_id: namespace.id, visibility_level: visibility_level, import_type: 'github', - import_source: @repo.full_name, - creator: @current_user - ) + import_source: @repo['full_name'], + skip_wiki: @repo['has_wiki'] + ).execute end end @@ -159,13 +96,13 @@ class GithubImport end def visibility_level - @repo.private ? Gitlab::VisibilityLevel::PRIVATE : current_application_settings.default_project_visibility + @repo['private'] ? Gitlab::VisibilityLevel::PRIVATE : current_application_settings.default_project_visibility end end class GithubRepos - def initialize(token, current_user, github_repo) - @token = token + def initialize(options, current_user, github_repo) + @options = options @current_user = current_user @github_repo = github_repo end @@ -174,17 +111,17 @@ class GithubRepos return found_github_repo if @github_repo repos.each do |repo| - print "ID: #{repo[:id].to_s.bright} ".color(:green) - puts "- Name: #{repo[:full_name]}".color(:green) + print "ID: #{repo['id'].to_s.bright}".color(:green) + print "\tName: #{repo['full_name']}\n".color(:green) end print 'ID? '.bright - repos.find { |repo| repo[:id] == repo_id } + repos.find { |repo| repo['id'] == repo_id } end def found_github_repo - repos.find { |repo| repo[:full_name] == @github_repo } + repos.find { |repo| repo['full_name'] == @github_repo } end def repo_id @@ -192,11 +129,7 @@ class GithubRepos end def repos - @repos ||= client.repos - end - - def client - @client ||= Gitlab::GithubImport::Client.new(@token, {}) + Github::Repositories.new(@options).fetch end end -- cgit v1.2.1 From 2b7328c3171661ce8e4a5b3b840ff5f3df19d3e0 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 18:42:50 -0300 Subject: Rename find_by_email/find_by_external_uid methods --- lib/github/import.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index de987c263b6..cbda4be2576 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -338,19 +338,19 @@ module Github return unless user.present? return cached_user_ids[user.id] if cached_user_ids.key?(user.id) - gitlab_user_id = find_by_external_uid(user.id) || find_by_email(user.email) + gitlab_user_id = user_id_by_external_uid(user.id) || user_id_by_email(user.email) cached_gitlab_users[user.id] = gitlab_user_id.present? cached_user_ids[user.id] = gitlab_user_id || fallback_id end - def find_by_email(email) + def user_id_by_email(email) return nil unless email ::User.find_by_any_email(email)&.id end - def find_by_external_uid(id) + def user_id_by_external_uid(id) return nil unless id identities = ::Identity.arel_table -- cgit v1.2.1 From 9bdde5796a6e6ed56a29865b19a910bdea5d078e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 19:04:39 -0300 Subject: Add Github::Representation::Base#id --- lib/github/representation/base.rb | 4 ++++ lib/github/representation/repo.rb | 3 --- lib/github/representation/user.rb | 4 ---- 3 files changed, 4 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/github/representation/base.rb b/lib/github/representation/base.rb index 385e62ae99d..f26bdbdd546 100644 --- a/lib/github/representation/base.rb +++ b/lib/github/representation/base.rb @@ -6,6 +6,10 @@ module Github @options = options end + def id + raw['id'] + end + def url raw['url'] end diff --git a/lib/github/representation/repo.rb b/lib/github/representation/repo.rb index b9cae43450e..6938aa7db05 100644 --- a/lib/github/representation/repo.rb +++ b/lib/github/representation/repo.rb @@ -1,9 +1,6 @@ module Github module Representation class Repo < Representation::Base - def id - raw['id'] - end end end end diff --git a/lib/github/representation/user.rb b/lib/github/representation/user.rb index 79758555319..18591380e25 100644 --- a/lib/github/representation/user.rb +++ b/lib/github/representation/user.rb @@ -1,10 +1,6 @@ module Github module Representation class User < Representation::Base - def id - raw['id'] - end - def email return @email if defined?(@email) -- cgit v1.2.1 From c7935dcfaec18f4628101268b13b9509b6b412ba Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 19:26:07 -0300 Subject: Does not freeze integer values --- lib/github/rate_limit.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/github/rate_limit.rb b/lib/github/rate_limit.rb index 9dbdf2f4c68..38beb617173 100644 --- a/lib/github/rate_limit.rb +++ b/lib/github/rate_limit.rb @@ -1,7 +1,7 @@ module Github class RateLimit - SAFE_REMAINING_REQUESTS = 100.freeze - SAFE_RESET_TIME = 500.freeze + SAFE_REMAINING_REQUESTS = 100 + SAFE_RESET_TIME = 500 attr_reader :connection -- cgit v1.2.1 From f73a0280c96f76b1b19200990ab07adc732dbb92 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 19:28:08 -0300 Subject: Extract rate limit URL to a constant --- lib/github/rate_limit.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/github/rate_limit.rb b/lib/github/rate_limit.rb index 38beb617173..ab2b9c707d8 100644 --- a/lib/github/rate_limit.rb +++ b/lib/github/rate_limit.rb @@ -2,6 +2,7 @@ module Github class RateLimit SAFE_REMAINING_REQUESTS = 100 SAFE_RESET_TIME = 500 + RATE_LIMIT_URL = '/rate_limit'.freeze attr_reader :connection @@ -25,12 +26,8 @@ module Github private - def rate_limit_url - '/rate_limit' - end - def response - connection.get(rate_limit_url) + connection.get(RATE_LIMIT_URL) end def body -- cgit v1.2.1 From 275f00ee88cb9ef6ae3ae7879498a0ed4f448681 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 19:49:17 -0300 Subject: Refactoring Github::RateLimit --- lib/github/client.rb | 8 +++++--- lib/github/rate_limit.rb | 33 +++++++++------------------------ 2 files changed, 14 insertions(+), 27 deletions(-) (limited to 'lib') diff --git a/lib/github/client.rb b/lib/github/client.rb index 95536cae57f..bf1b2b0a523 100644 --- a/lib/github/client.rb +++ b/lib/github/client.rb @@ -1,17 +1,19 @@ module Github class Client - attr_reader :connection + attr_reader :connection, :rate_limit def initialize(options) @connection = Faraday.new(url: options.fetch(:url)) do |faraday| faraday.authorization 'token', options.fetch(:token) faraday.adapter :net_http end + + @rate_limit = RateLimit.new(connection) end def get(url, query = {}) - rate_limit = RateLimit.new(connection) - sleep rate_limit.reset_in if rate_limit.exceed? + exceed, reset_in = rate_limit.get + sleep reset_in if exceed Github::Response.new(connection.get(url, query)) end diff --git a/lib/github/rate_limit.rb b/lib/github/rate_limit.rb index ab2b9c707d8..884693d093c 100644 --- a/lib/github/rate_limit.rb +++ b/lib/github/rate_limit.rb @@ -10,33 +10,18 @@ module Github @connection = connection end - def exceed? - return false unless enabled? + def get + response = connection.get(RATE_LIMIT_URL) - remaining <= SAFE_REMAINING_REQUESTS - end - - def remaining - @remaining ||= body.dig('rate', 'remaining').to_i - end - - def reset_in - @reset ||= body.dig('rate', 'reset').to_i - end + # GitHub Rate Limit API returns 404 when the rate limit is disabled + return false unless response.status != 404 - private - - def response - connection.get(RATE_LIMIT_URL) - end - - def body - @body ||= Oj.load(response.body, class_cache: false, mode: :compat) - end + body = Oj.load(response.body, class_cache: false, mode: :compat) + remaining = body.dig('rate', 'remaining').to_i + reset_in = body.dig('rate', 'reset').to_i + exceed = remaining <= SAFE_REMAINING_REQUESTS - # GitHub Rate Limit API returns 404 when the rate limit is disabled - def enabled? - response.status != 404 + [exceed, reset_in] end end end -- cgit v1.2.1 From 1f498b73dabfd1bb1f2beb9cc6639db4e433ad28 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 20:00:11 -0300 Subject: Use only one cache hash with with a hash initializer by default --- lib/github/import.rb | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index cbda4be2576..8ffbb21bac0 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -29,17 +29,14 @@ module Github self.reset_callbacks :validate end - attr_reader :project, :repository, :repo, :options, :errors, - :cached_label_ids, :cached_gitlab_users, :cached_user_ids + attr_reader :project, :repository, :repo, :options, :errors, :cached def initialize(project, options) @project = project @repository = project.repository @repo = project.import_source @options = options - @cached_label_ids = {} - @cached_user_ids = {} - @cached_gitlab_users = {} + @cached = Hash.new { |hash, key| hash[key] = Hash.new } @errors = [] end @@ -107,7 +104,7 @@ module Github # Cache labels project.labels.select(:id, :title).find_each do |label| - @cached_label_ids[label.title] = label.id + cached[:label_ids][label.title] = label.id end end @@ -325,7 +322,7 @@ module Github end def label_ids(issuable) - issuable.map { |attrs| cached_label_ids[attrs.fetch('name')] }.compact + issuable.map { |attrs| cached[:label_ids][attrs.fetch('name')] }.compact end def milestone_id(milestone) @@ -336,12 +333,12 @@ module Github def user_id(user, fallback_id = nil) return unless user.present? - return cached_user_ids[user.id] if cached_user_ids.key?(user.id) + return cached[:user_ids][user.id] if cached[:user_ids].key?(user.id) gitlab_user_id = user_id_by_external_uid(user.id) || user_id_by_email(user.email) - cached_gitlab_users[user.id] = gitlab_user_id.present? - cached_user_ids[user.id] = gitlab_user_id || fallback_id + cached[:gitlab_user_ids][user.id] = gitlab_user_id.present? + cached[:user_ids][user.id] = gitlab_user_id || fallback_id end def user_id_by_email(email) @@ -362,7 +359,7 @@ module Github end def format_description(body, author) - return body if cached_gitlab_users[author.id] + return body if cached[:gitlab_user_ids][author.id] "*Created by: #{author.username}*\n\n#{body}" end -- cgit v1.2.1 From 5c72ba0ff1be82f31c292765e8d6a76277327979 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 20:28:06 -0300 Subject: Finish the import process if some error occurs when fetching the repo --- lib/github/error.rb | 6 ++++++ lib/github/import.rb | 11 ++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 lib/github/error.rb (limited to 'lib') diff --git a/lib/github/error.rb b/lib/github/error.rb new file mode 100644 index 00000000000..41cd4c67847 --- /dev/null +++ b/lib/github/error.rb @@ -0,0 +1,6 @@ +module Github + class Error < StandardError + end + + class RepositoryFetchError < Error; end +end diff --git a/lib/github/import.rb b/lib/github/import.rb index 8ffbb21bac0..56b0b7a3d4f 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -1,3 +1,4 @@ +require_relative 'error' module Github class Import include Gitlab::ShellAdapter @@ -48,9 +49,12 @@ module Github fetch_issues fetch_wiki_repository expire_repository_cache - track_errors - errors + true + rescue Github::RepositoryFetchError + false + ensure + keep_track_of_errors end private @@ -63,6 +67,7 @@ module Github project.repository.fetch_remote('github', forced: true) rescue Gitlab::Shell::Error => e error(:project, "https://github.com/#{repo}.git", e.message) + raise Github::RepositoryFetchError end end @@ -368,7 +373,7 @@ module Github repository.expire_content_cache end - def track_errors + def keep_track_of_errors return unless errors.any? project.update_column(:import_error, { -- cgit v1.2.1 From 22a33d8256101b0a5f348ae93623d4b189a96d47 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 20:45:51 -0300 Subject: Avoid unnecessary use of Arel to find users by external uid --- lib/github/import.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 56b0b7a3d4f..ba30ea0e2cd 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -355,11 +355,9 @@ module Github def user_id_by_external_uid(id) return nil unless id - identities = ::Identity.arel_table - ::User.select(:id) .joins(:identities) - .where(identities[:provider].eq(:github).and(identities[:extern_uid].eq(id))) + .merge(::Identity.where(provider: :github, extern_uid: id)) .first&.id end -- cgit v1.2.1 From 05255631ae2dad3b22796343453bb3288ca3dbff Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 21:03:42 -0300 Subject: Cache labels at the same time we fetch them from the GH API --- lib/github/import.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index ba30ea0e2cd..641147c40e4 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -95,10 +95,13 @@ module Github response.body.each do |raw| begin - label = Github::Representation::Label.new(raw) - next if project.labels.where(title: label.title).exists? + representation = Github::Representation::Label.new(raw) - project.labels.create!(title: label.title, color: label.color) + label = project.labels.find_or_create_by!(title: representation.title) do |label| + label.color = representation.color + end + + cached[:label_ids][label.title] = label.id rescue => e error(:label, label.url, e.message) end @@ -106,11 +109,6 @@ module Github url = response.rels[:next] end - - # Cache labels - project.labels.select(:id, :title).find_each do |label| - cached[:label_ids][label.title] = label.id - end end def fetch_milestones -- cgit v1.2.1 From 5d106f2597bb009f0e7b8d5ed02b6f2206237e7e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 21:13:51 -0300 Subject: Use the base initiliazer for representations --- lib/github/import.rb | 2 +- lib/github/representation/branch.rb | 9 ++++----- lib/github/representation/pull_request.rb | 14 ++++++-------- 3 files changed, 11 insertions(+), 14 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 641147c40e4..ef0ef9adc7a 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -147,7 +147,7 @@ module Github response = Github::Client.new(options).get(url, state: :all, sort: :created, direction: :asc) response.body.each do |raw| - pull_request = Github::Representation::PullRequest.new(project, raw, options) + pull_request = Github::Representation::PullRequest.new(raw, options.merge(project: project)) merge_request = MergeRequest.find_or_initialize_by(iid: pull_request.iid, source_project_id: project.id) next unless merge_request.new_record? && pull_request.valid? diff --git a/lib/github/representation/branch.rb b/lib/github/representation/branch.rb index 7c65a948ede..d1dac6944f0 100644 --- a/lib/github/representation/branch.rb +++ b/lib/github/representation/branch.rb @@ -3,11 +3,6 @@ module Github class Branch < Representation::Base attr_reader :repository - def initialize(repository, raw) - @repository = repository - @raw = raw - end - def user raw.dig('user', 'login') || 'unknown' end @@ -47,6 +42,10 @@ module Github def commit_exists? repository.branch_names_contains(sha).include?(ref) end + + def repository + @repository ||= options.fetch(:repository) + end end end end diff --git a/lib/github/representation/pull_request.rb b/lib/github/representation/pull_request.rb index 4119ca400c6..ac9c8283b4b 100644 --- a/lib/github/representation/pull_request.rb +++ b/lib/github/representation/pull_request.rb @@ -6,12 +6,6 @@ module Github delegate :user, :repo, :ref, :sha, to: :source_branch, prefix: true delegate :user, :exists?, :repo, :ref, :sha, :short_sha, to: :target_branch, prefix: true - def initialize(project, raw, options) - @project = project - @raw = raw - @options = options - end - def source_project project end @@ -54,8 +48,12 @@ module Github private + def project + @project ||= options.fetch(:project) + end + def source_branch - @source_branch ||= Representation::Branch.new(project.repository, raw['head']) + @source_branch ||= Representation::Branch.new(raw['head'], repository: project.repository) end def source_branch_name_prefixed @@ -63,7 +61,7 @@ module Github end def target_branch - @target_branch ||= Representation::Branch.new(project.repository, raw['base']) + @target_branch ||= Representation::Branch.new(raw['base'], repository: project.repository) end def target_branch_name_prefixed -- cgit v1.2.1 From 30794972f46de4d3af1c3f1843d46f1162660c3e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 21:21:58 -0300 Subject: Set timeout options to the Github::Client --- lib/github/client.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/github/client.rb b/lib/github/client.rb index bf1b2b0a523..e65d908d232 100644 --- a/lib/github/client.rb +++ b/lib/github/client.rb @@ -4,6 +4,8 @@ module Github def initialize(options) @connection = Faraday.new(url: options.fetch(:url)) do |faraday| + faraday.options.open_timeout = options.fetch(:timeout, 60) + faraday.options.timeout = options.fetch(:timeout, 60) faraday.authorization 'token', options.fetch(:token) faraday.adapter :net_http end -- cgit v1.2.1 From dd1157c80b90728a71b1f3a723e2cdeb335ab1b3 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 23:41:36 -0300 Subject: Use Class.new(SuperClass) to define an empty custom error class --- lib/github/error.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/github/error.rb b/lib/github/error.rb index 41cd4c67847..dfca3a18f9c 100644 --- a/lib/github/error.rb +++ b/lib/github/error.rb @@ -1,6 +1,4 @@ module Github - class Error < StandardError - end - - class RepositoryFetchError < Error; end + Error = Class.new(StandardError) + RepositoryFetchError = Class.new(Github::Error) end -- cgit v1.2.1 From aeb1684c524628f7279eadce5e461880e9865349 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 24 Apr 2017 23:41:46 -0300 Subject: Fix small typo --- lib/github/import.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index ef0ef9adc7a..fddd52af260 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -324,8 +324,8 @@ module Github remove_branch(pull_request.target_branch_name) unless pull_request.target_branch_exists? end - def label_ids(issuable) - issuable.map { |attrs| cached[:label_ids][attrs.fetch('name')] }.compact + def label_ids(labels) + labels.map { |attrs| cached[:label_ids][attrs.fetch('name')] }.compact end def milestone_id(milestone) -- cgit v1.2.1 From 7df974c433a7d675a2dcb2dab1063e61686476ec Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 25 Apr 2017 13:30:08 -0300 Subject: Add blank line before the raise method on Github::Collection --- lib/github/collection.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib') diff --git a/lib/github/collection.rb b/lib/github/collection.rb index 12d6703476b..014b2038c4b 100644 --- a/lib/github/collection.rb +++ b/lib/github/collection.rb @@ -13,6 +13,7 @@ module Github loop do response = client.get(url, query) response.body.each { |item| yielder << item } + raise StopIteration unless response.rels.key?(:next) url = response.rels[:next] end -- cgit v1.2.1 From 2f934ce22f18d2fa12a849014db541e133e1b3d1 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 25 Apr 2017 13:30:30 -0300 Subject: Remove the Github::Error base class --- lib/github/error.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/github/error.rb b/lib/github/error.rb index dfca3a18f9c..66d7afaa787 100644 --- a/lib/github/error.rb +++ b/lib/github/error.rb @@ -1,4 +1,3 @@ module Github - Error = Class.new(StandardError) - RepositoryFetchError = Class.new(Github::Error) + RepositoryFetchError = Class.new(StandardError) end -- cgit v1.2.1 From 000a723d844c0bccfb04cbb32c683cd4d7e7b07e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 25 Apr 2017 15:57:50 -0300 Subject: Fix small typo on GitHub::Import --- lib/github/import.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index fddd52af260..fefb713a29e 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -103,7 +103,7 @@ module Github cached[:label_ids][label.title] = label.id rescue => e - error(:label, label.url, e.message) + error(:label, representation.url, e.message) end end -- cgit v1.2.1 From 39ab842bc2ff1395ed1bf6768ff2a409a33bf3d3 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 25 Apr 2017 15:58:17 -0300 Subject: Create project repository only when it not exists --- lib/github/import.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index fefb713a29e..a0afd12f45f 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -61,7 +61,7 @@ module Github def fetch_repository begin - project.create_repository + project.create_repository unless project.repository.exists? project.repository.add_remote('github', "https://{options.fetch(:token)}@github.com/#{repo}.git") project.repository.set_remote_as_mirror('github') project.repository.fetch_remote('github', forced: true) -- cgit v1.2.1 From 44954c507ed99db892e9e4e40779bc6fc6f0a56f Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 25 Apr 2017 17:05:40 -0300 Subject: Fix import of notes on Pull Request diff --- lib/github/import.rb | 15 +++++++++++---- lib/github/representation/comment.rb | 4 ---- lib/github/response.rb | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index a0afd12f45f..96b69d78c55 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -30,6 +30,14 @@ module Github self.reset_callbacks :validate end + class LegacyDiffNote < ::LegacyDiffNote + self.table_name = 'notes' + + self.reset_callbacks :commit + self.reset_callbacks :update + self.reset_callbacks :validate + end + attr_reader :project, :repository, :repo, :options, :errors, :cached def initialize(project, options) @@ -177,7 +185,7 @@ module Github # Fetch review comments review_comments_url = "/repos/#{repo}/pulls/#{pull_request.iid}/comments" - fetch_comments(merge_request, :review_comment, review_comments_url) + fetch_comments(merge_request, :review_comment, review_comments_url, LegacyDiffNote) # Fetch comments comments_url = "/repos/#{repo}/issues/#{pull_request.iid}/comments" @@ -245,7 +253,7 @@ module Github end end - def fetch_comments(noteable, type, url) + def fetch_comments(noteable, type, url, klass = Note) while url comments = Github::Client.new(options).get(url) @@ -255,14 +263,13 @@ module Github representation = Github::Representation::Comment.new(raw, options) author_id = user_id(representation.author, project.creator_id) - note = Note.new + note = klass.new note.project_id = project.id note.noteable = noteable note.note = format_description(representation.note, representation.author) note.commit_id = representation.commit_id note.line_code = representation.line_code note.author_id = author_id - note.type = representation.type note.created_at = representation.created_at note.updated_at = representation.updated_at note.save!(validate: false) diff --git a/lib/github/representation/comment.rb b/lib/github/representation/comment.rb index 22cb98b0eff..1b5be91461b 100644 --- a/lib/github/representation/comment.rb +++ b/lib/github/representation/comment.rb @@ -20,10 +20,6 @@ module Github generate_line_code(parsed_lines.to_a.last) end - def type - 'LegacyDiffNote' if on_diff? - end - private def generate_line_code(line) diff --git a/lib/github/response.rb b/lib/github/response.rb index 2fd07dd822e..761c524b553 100644 --- a/lib/github/response.rb +++ b/lib/github/response.rb @@ -9,7 +9,7 @@ module Github end def body - @body ||= Oj.load(raw.body, class_cache: false, mode: :compat) + Oj.load(raw.body, class_cache: false, mode: :compat) end def rels -- cgit v1.2.1 From 70d15ae1d968070e82028dd48c15c363ac9f8fa9 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 25 Apr 2017 17:08:32 -0300 Subject: Fix Rubocop offenses --- lib/github/import.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index 96b69d78c55..f12f979ae02 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -160,8 +160,7 @@ module Github next unless merge_request.new_record? && pull_request.valid? begin - restore_source_branch(pull_request) unless pull_request.source_branch_exists? - restore_target_branch(pull_request) unless pull_request.target_branch_exists? + restore_branches(pull_request) author_id = user_id(pull_request.author, project.creator_id) merge_request.iid = pull_request.iid @@ -310,6 +309,11 @@ module Github end end + def restore_branches(pull_request) + restore_source_branch(pull_request) unless pull_request.source_branch_exists? + restore_target_branch(pull_request) unless pull_request.target_branch_exists? + end + def restore_source_branch(pull_request) repository.create_branch(pull_request.source_branch_name, pull_request.source_branch_sha) end -- cgit v1.2.1 From f184cb433b90e5b03d8a2ecf9916e7a1bfda5ee9 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 25 Apr 2017 23:19:30 -0300 Subject: Fix undefined attribute params from import task --- lib/tasks/import.rake | 1 - 1 file changed, 1 deletion(-) (limited to 'lib') diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index 897b2f04ce1..e22424c98d3 100644 --- a/lib/tasks/import.rake +++ b/lib/tasks/import.rake @@ -71,7 +71,6 @@ class GithubImport return @current_user.namespace if names == @current_user.namespace_path return @current_user.namespace unless @current_user.can_create_group? - names = params[:target_namespace].presence || names full_path_namespace = Namespace.find_by_full_path(names) return full_path_namespace if full_path_namespace -- cgit v1.2.1 From d082b78998d43349f6d0cbaeb90bb448aa1188c3 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 25 Apr 2017 23:47:49 -0300 Subject: Add basic progress output to GitHub import --- lib/github/import.rb | 11 ++++++++++- lib/tasks/import.rake | 4 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/github/import.rb b/lib/github/import.rb index f12f979ae02..3e9162ffb9d 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -38,24 +38,33 @@ module Github self.reset_callbacks :validate end - attr_reader :project, :repository, :repo, :options, :errors, :cached + attr_reader :project, :repository, :repo, :options, :errors, :cached, :verbose def initialize(project, options) @project = project @repository = project.repository @repo = project.import_source @options = options + @verbose = options.fetch(:verbose, false) @cached = Hash.new { |hash, key| hash[key] = Hash.new } @errors = [] end + # rubocop: disable Rails/Output def execute + puts 'Fetching repository...'.color(:aqua) if verbose fetch_repository + puts 'Fetching labels...'.color(:aqua) if verbose fetch_labels + puts 'Fetching milestones...'.color(:aqua) if verbose fetch_milestones + puts 'Fetching pull requests...'.color(:aqua) if verbose fetch_pull_requests + puts 'Fetching issues...'.color(:aqua) if verbose fetch_issues + puts 'Cloning wiki repository...'.color(:aqua) if verbose fetch_wiki_repository + puts 'Expiring repository cache...'.color(:aqua) if verbose expire_repository_cache true diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index e22424c98d3..bc76d7edc55 100644 --- a/lib/tasks/import.rake +++ b/lib/tasks/import.rake @@ -7,7 +7,7 @@ class GithubImport end def initialize(token, gitlab_username, project_path, extras) - @options = { url: 'https://api.github.com', token: token } + @options = { url: 'https://api.github.com', token: token, verbose: true } @project_path = project_path @current_user = User.find_by_username(gitlab_username) @github_repo = extras.empty? ? nil : extras.first @@ -28,7 +28,7 @@ class GithubImport private def show_warning! - puts "This will import GH #{@repo['full_name'].bright} into GL #{@project_path.bright} as #{@current_user.name}" + puts "This will import GitHub #{@repo['full_name'].bright} into GitLab #{@project_path.bright} as #{@current_user.name}" puts "Permission checks are ignored. Press any key to continue.".color(:red) STDIN.getch -- cgit v1.2.1