diff options
Diffstat (limited to 'lib/gitlab/github_import')
19 files changed, 334 insertions, 94 deletions
diff --git a/lib/gitlab/github_import/bulk_importing.rb b/lib/gitlab/github_import/bulk_importing.rb index 28a39128ec9..0c91eff1d10 100644 --- a/lib/gitlab/github_import/bulk_importing.rb +++ b/lib/gitlab/github_import/bulk_importing.rb @@ -10,25 +10,38 @@ module Gitlab def initialize(project, client) @project = project @client = client + @validation_errors = [] end # Builds and returns an Array of objects to bulk insert into the - # database. + # database and array of validation errors if object is invalid. # # enum - An Enumerable that returns the objects to turn into database # rows. def build_database_rows(enum) + errors = [] rows = enum.each_with_object([]) do |(object, _), result| - result << build(object) unless already_imported?(object) + next if already_imported?(object) + + attrs = build_attributes(object) + build_record = model.new(attrs) + + if build_record.invalid? + log_error(object[:id], build_record.errors.full_messages) + errors << build_record.errors + next + end + + result << attrs end log_and_increment_counter(rows.size, :fetched) - rows + [rows, errors] end # Bulk inserts the given rows into the database. - def bulk_insert(model, rows, batch_size: 100) + def bulk_insert(rows, batch_size: 100) rows.each_slice(batch_size) do |slice| ApplicationRecord.legacy_bulk_insert(model.table_name, slice) # rubocop:disable Gitlab/BulkInsert @@ -40,6 +53,23 @@ module Gitlab raise NotImplementedError end + def bulk_insert_failures(validation_errors) + rows = validation_errors.map do |error| + correlation_id_value = Labkit::Correlation::CorrelationId.current_or_new_id + + { + source: self.class.name, + exception_class: 'ActiveRecord::RecordInvalid', + exception_message: error.full_messages.first.truncate(255), + correlation_id_value: correlation_id_value, + retry_count: nil, + created_at: Time.zone.now + } + end + + project.import_failures.insert_all(rows) + end + private def log_and_increment_counter(value, operation) @@ -57,6 +87,16 @@ module Gitlab value: value ) end + + def log_error(object_id, messages) + Gitlab::Import::Logger.error( + import_type: :github, + project_id: project.id, + importer: self.class.name, + message: messages, + github_identifier: object_id + ) + end end end end diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb index d6060141bce..065410693e5 100644 --- a/lib/gitlab/github_import/client.rb +++ b/lib/gitlab/github_import/client.rb @@ -15,6 +15,7 @@ module Gitlab # end class Client include ::Gitlab::Utils::StrongMemoize + include ::Gitlab::GithubImport::Clients::SearchRepos attr_reader :octokit @@ -182,19 +183,6 @@ module Gitlab end end - def search_repos_by_name(name, options = {}) - with_retry { octokit.search_repositories(search_query(str: name, type: :name), options).to_h } - end - - def search_query(str:, type:, include_collaborations: true, include_orgs: true) - query = "#{str} in:#{type} is:public,private user:#{octokit.user.to_h[:login]}" - - query = [query, collaborations_subquery].join(' ') if include_collaborations - query = [query, organizations_subquery].join(' ') if include_orgs - - query - end - # Returns `true` if we're still allowed to perform API calls. # Search API has rate limit of 30, use lowered threshold when search is used. def requests_remaining? diff --git a/lib/gitlab/github_import/clients/proxy.rb b/lib/gitlab/github_import/clients/proxy.rb new file mode 100644 index 00000000000..f6d1c8ed23c --- /dev/null +++ b/lib/gitlab/github_import/clients/proxy.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + module Clients + class Proxy + attr_reader :client + + def initialize(access_token, client_options) + @client = pick_client(access_token, client_options) + end + + def repos(search_text, pagination_options) + return { repos: filtered(client.repos, search_text) } if use_legacy? + + if use_graphql? + fetch_repos_via_graphql(search_text, pagination_options) + else + fetch_repos_via_rest(search_text, pagination_options) + end + end + + private + + def fetch_repos_via_rest(search_text, pagination_options) + { repos: client.search_repos_by_name(search_text, pagination_options)[:items] } + end + + def fetch_repos_via_graphql(search_text, pagination_options) + response = client.search_repos_by_name_graphql(search_text, pagination_options) + { + repos: response.dig(:data, :search, :nodes), + page_info: response.dig(:data, :search, :pageInfo) + } + end + + def pick_client(access_token, client_options) + return Gitlab::GithubImport::Client.new(access_token) unless use_legacy? + + Gitlab::LegacyGithubImport::Client.new(access_token, **client_options) + end + + def filtered(collection, search_text) + return collection if search_text.blank? + + collection.select { |item| item[:name].to_s.downcase.include?(search_text) } + end + + def use_legacy? + Feature.disabled?(:remove_legacy_github_client) + end + + def use_graphql? + Feature.enabled?(:github_client_fetch_repos_via_graphql) + end + end + end + end +end diff --git a/lib/gitlab/github_import/clients/search_repos.rb b/lib/gitlab/github_import/clients/search_repos.rb new file mode 100644 index 00000000000..bcd226087e7 --- /dev/null +++ b/lib/gitlab/github_import/clients/search_repos.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + module Clients + module SearchRepos + def search_repos_by_name_graphql(name, options = {}) + with_retry do + octokit.post( + '/graphql', + { query: graphql_search_repos_body(name, options) }.to_json + ).to_h + end + end + + def search_repos_by_name(name, options = {}) + with_retry do + octokit.search_repositories( + search_repos_query(str: name, type: :name), + options + ).to_h + end + end + + private + + def graphql_search_repos_body(name, options) + query = search_repos_query(str: name, type: :name) + query = "query: \"#{query}\"" + first = options[:first].present? ? ", first: #{options[:first]}" : '' + after = options[:after].present? ? ", after: \"#{options[:after]}\"" : '' + <<-TEXT + { + search(type: REPOSITORY, #{query}#{first}#{after}) { + nodes { + __typename + ... on Repository { + id: databaseId + name + full_name: nameWithOwner + owner { login } + } + } + pageInfo { + startCursor + endCursor + hasNextPage + hasPreviousPage + } + } + } + TEXT + end + + def search_repos_query(str:, type:, include_collaborations: true, include_orgs: true) + query = "#{str} in:#{type} is:public,private user:#{octokit.user.to_h[:login]}" + + query = [query, collaborations_subquery].join(' ') if include_collaborations + query = [query, organizations_subquery].join(' ') if include_orgs + + query + end + end + end + end +end diff --git a/lib/gitlab/github_import/importer/diff_note_importer.rb b/lib/gitlab/github_import/importer/diff_note_importer.rb index a9f8483d8c3..44ffcd7a1e4 100644 --- a/lib/gitlab/github_import/importer/diff_note_importer.rb +++ b/lib/gitlab/github_import/importer/diff_note_importer.rb @@ -18,7 +18,6 @@ module Gitlab def execute return if merge_request_id.blank? - note.project = project note.merge_request = merge_request build_author_attributes @@ -65,7 +64,7 @@ module Gitlab # To work around this we're using bulk_insert with a single row. This # allows us to efficiently insert data (even if it's just 1 row) # without having to use all sorts of hacks to disable callbacks. - ApplicationRecord.legacy_bulk_insert(LegacyDiffNote.table_name, [{ + attributes = { noteable_type: note.noteable_type, system: false, type: 'LegacyDiffNote', @@ -79,7 +78,12 @@ module Gitlab created_at: note.created_at, updated_at: note.updated_at, st_diff: note.diff_hash.to_yaml - }]) + } + + diff_note = LegacyDiffNote.new(attributes.merge(importing: true)) + diff_note.validate! + + ApplicationRecord.legacy_bulk_insert(LegacyDiffNote.table_name, [attributes]) end # rubocop:enabled Gitlab/BulkInsert diff --git a/lib/gitlab/github_import/importer/issue_importer.rb b/lib/gitlab/github_import/importer/issue_importer.rb index d964bae3dd2..b477468d327 100644 --- a/lib/gitlab/github_import/importer/issue_importer.rb +++ b/lib/gitlab/github_import/importer/issue_importer.rb @@ -60,6 +60,9 @@ module Gitlab work_item_type_id: issue.work_item_type_id } + issue = project.issues.new(attributes.merge(importing: true)) + issue.validate! + insert_and_return_id(attributes, project.issues) rescue ActiveRecord::InvalidForeignKey # It's possible the project has been deleted since scheduling this diff --git a/lib/gitlab/github_import/importer/label_links_importer.rb b/lib/gitlab/github_import/importer/label_links_importer.rb index 5e248c7cfc5..52c87dda347 100644 --- a/lib/gitlab/github_import/importer/label_links_importer.rb +++ b/lib/gitlab/github_import/importer/label_links_importer.rb @@ -22,7 +22,7 @@ module Gitlab def create_labels time = Time.zone.now - rows = [] + items = [] target_id = find_target_id issue.label_names.each do |label_name| @@ -31,16 +31,16 @@ module Gitlab # the project's labels. next unless (label_id = label_finder.id_for(label_name)) - rows << { + items << LabelLink.new( label_id: label_id, target_id: target_id, target_type: issue.issuable_type, created_at: time, updated_at: time - } + ) end - ApplicationRecord.legacy_bulk_insert(LabelLink.table_name, rows) # rubocop:disable Gitlab/BulkInsert + LabelLink.bulk_insert!(items) end def find_target_id diff --git a/lib/gitlab/github_import/importer/labels_importer.rb b/lib/gitlab/github_import/importer/labels_importer.rb index 9a011f17a18..d5d1cd28b7c 100644 --- a/lib/gitlab/github_import/importer/labels_importer.rb +++ b/lib/gitlab/github_import/importer/labels_importer.rb @@ -13,7 +13,10 @@ module Gitlab # rubocop: enable CodeReuse/ActiveRecord def execute - bulk_insert(Label, build_labels) + rows, validation_errors = build_labels + + bulk_insert(rows) + bulk_insert_failures(validation_errors) if validation_errors.any? build_labels_cache end @@ -29,7 +32,7 @@ module Gitlab LabelFinder.new(project).build_cache end - def build(label) + def build_attributes(label) time = Time.zone.now { @@ -49,6 +52,10 @@ module Gitlab def object_type :label end + + def model + Label + end end end end diff --git a/lib/gitlab/github_import/importer/lfs_objects_importer.rb b/lib/gitlab/github_import/importer/lfs_objects_importer.rb index 775afd5f53a..d064278e4a0 100644 --- a/lib/gitlab/github_import/importer/lfs_objects_importer.rb +++ b/lib/gitlab/github_import/importer/lfs_objects_importer.rb @@ -27,9 +27,9 @@ module Gitlab end def each_object_to_import - lfs_objects = Projects::LfsPointers::LfsObjectDownloadListService.new(project).execute + download_service = Projects::LfsPointers::LfsObjectDownloadListService.new(project) - lfs_objects.each do |object| + download_service.each_list_item do |object| Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched) yield object diff --git a/lib/gitlab/github_import/importer/milestones_importer.rb b/lib/gitlab/github_import/importer/milestones_importer.rb index 1a3a54d0053..560fbdc66e3 100644 --- a/lib/gitlab/github_import/importer/milestones_importer.rb +++ b/lib/gitlab/github_import/importer/milestones_importer.rb @@ -13,7 +13,10 @@ module Gitlab # rubocop: enable CodeReuse/ActiveRecord def execute - bulk_insert(Milestone, build_milestones) + rows, validation_errors = build_milestones + + bulk_insert(rows) + bulk_insert_failures(validation_errors) if validation_errors.any? build_milestones_cache end @@ -29,7 +32,7 @@ module Gitlab MilestoneFinder.new(project).build_cache end - def build(milestone) + def build_attributes(milestone) { iid: milestone[:number], title: milestone[:title], @@ -53,6 +56,10 @@ module Gitlab def object_type :milestone end + + def model + Milestone + end end end end diff --git a/lib/gitlab/github_import/importer/note_importer.rb b/lib/gitlab/github_import/importer/note_importer.rb index 69b7b2c2a38..04da015a33f 100644 --- a/lib/gitlab/github_import/importer/note_importer.rb +++ b/lib/gitlab/github_import/importer/note_importer.rb @@ -33,6 +33,9 @@ module Gitlab updated_at: note.updated_at } + note = Note.new(attributes.merge(importing: true)) + note.validate! + # We're using bulk_insert here so we can bypass any validations and # callbacks. Running these would result in a lot of unnecessary SQL # queries being executed when importing large projects. diff --git a/lib/gitlab/github_import/importer/pull_request_importer.rb b/lib/gitlab/github_import/importer/pull_request_importer.rb index 3c17ea1195e..5690a2cc997 100644 --- a/lib/gitlab/github_import/importer/pull_request_importer.rb +++ b/lib/gitlab/github_import/importer/pull_request_importer.rb @@ -61,6 +61,9 @@ module Gitlab updated_at: pull_request.updated_at } + mr = project.merge_requests.new(attributes.merge(importing: true)) + mr.validate! + create_merge_request_without_hooks(project, attributes, pull_request.iid) end @@ -93,7 +96,7 @@ module Gitlab return if project.repository.branch_exists?(source_branch) project.repository.add_branch(project.creator, source_branch, pull_request.source_branch_sha) - rescue Gitlab::Git::CommandError => e + rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommandError => e Gitlab::ErrorTracking.track_exception(e, source_branch: source_branch, project_id: merge_request.project.id, diff --git a/lib/gitlab/github_import/importer/pull_request_merged_by_importer.rb b/lib/gitlab/github_import/importer/pull_request_merged_by_importer.rb index 640914acf4d..f05aa26a449 100644 --- a/lib/gitlab/github_import/importer/pull_request_merged_by_importer.rb +++ b/lib/gitlab/github_import/importer/pull_request_merged_by_importer.rb @@ -4,42 +4,62 @@ module Gitlab module GithubImport module Importer class PullRequestMergedByImporter + # pull_request - An instance of + # `Gitlab::GithubImport::Representation::PullRequest` + # project - An instance of `Project` + # client - An instance of `Gitlab::GithubImport::Client` def initialize(pull_request, project, client) - @project = project @pull_request = pull_request + @project = project @client = client end def execute - merge_request = project.merge_requests.find_by_iid(pull_request.iid) - timestamp = Time.new.utc - merged_at = pull_request.merged_at user_finder = GithubImport::UserFinder.new(project, client) - gitlab_user_id = user_finder.user_id_for(pull_request.merged_by) + gitlab_user_id = begin + user_finder.user_id_for(pull_request.merged_by) + rescue ::Octokit::NotFound + nil + end + + metrics_upsert(gitlab_user_id) + + add_note! + end + + private + + attr_reader :project, :pull_request, :client + + def metrics_upsert(gitlab_user_id) MergeRequest::Metrics.upsert({ target_project_id: project.id, merge_request_id: merge_request.id, merged_by_id: gitlab_user_id, - merged_at: merged_at, + merged_at: pull_request.merged_at, created_at: timestamp, updated_at: timestamp }, unique_by: :merge_request_id) + end - unless gitlab_user_id - merge_request.notes.create!( - importing: true, - note: missing_author_note, - author_id: project.creator_id, - project: project, - created_at: merged_at - ) - end + def add_note! + merge_request.notes.create!( + importing: true, + note: missing_author_note, + author_id: project.creator_id, + project: project, + created_at: pull_request.merged_at + ) end - private + def merge_request + @merge_request ||= project.merge_requests.find_by_iid(pull_request.iid) + end - attr_reader :project, :pull_request, :client + def timestamp + @timestamp ||= Time.new.utc + end def missing_author_note s_("GitHubImporter|*Merged by: %{author} at %{timestamp}*") % { diff --git a/lib/gitlab/github_import/importer/pull_request_review_importer.rb b/lib/gitlab/github_import/importer/pull_request_review_importer.rb index b11af90aa6f..de66f310edf 100644 --- a/lib/gitlab/github_import/importer/pull_request_review_importer.rb +++ b/lib/gitlab/github_import/importer/pull_request_review_importer.rb @@ -4,6 +4,9 @@ module Gitlab module GithubImport module Importer class PullRequestReviewImporter + # review - An instance of `Gitlab::GithubImport::Representation::PullRequestReview` + # project - An instance of `Project` + # client - An instance of `Gitlab::GithubImport::Client` def initialize(review, project, client) @review = review @project = project @@ -13,7 +16,12 @@ module Gitlab def execute user_finder = GithubImport::UserFinder.new(project, client) - gitlab_user_id = user_finder.user_id_for(review.author) + + gitlab_user_id = begin + user_finder.user_id_for(review.author) + rescue ::Octokit::NotFound + nil + end if gitlab_user_id add_review_note!(gitlab_user_id) diff --git a/lib/gitlab/github_import/importer/releases_importer.rb b/lib/gitlab/github_import/importer/releases_importer.rb index fe6da30bbf8..62d579fda08 100644 --- a/lib/gitlab/github_import/importer/releases_importer.rb +++ b/lib/gitlab/github_import/importer/releases_importer.rb @@ -16,7 +16,10 @@ module Gitlab # to generate HTML version - you also need to regenerate it in # Gitlab::GithubImport::Importer::NoteAttachmentsImporter. def execute - bulk_insert(Release, build_releases) + rows, validation_errors = build_releases + + bulk_insert(rows) + bulk_insert_failures(validation_errors) if validation_errors.any? end def build_releases @@ -27,7 +30,7 @@ module Gitlab existing_tags.include?(release[:tag_name]) || release[:tag_name].nil? end - def build(release) + def build_attributes(release) existing_tags.add(release[:tag_name]) { @@ -66,6 +69,10 @@ module Gitlab def user_finder @user_finder ||= GithubImport::UserFinder.new(project, client) end + + def model + Release + end end end end diff --git a/lib/gitlab/github_import/markdown/attachment.rb b/lib/gitlab/github_import/markdown/attachment.rb index a5cf5ffa60e..1c814e34a39 100644 --- a/lib/gitlab/github_import/markdown/attachment.rb +++ b/lib/gitlab/github_import/markdown/attachment.rb @@ -28,6 +28,7 @@ module Gitlab def from_markdown_image(markdown_node) url = markdown_node.url + return unless url return unless github_url?(url, media: true) return unless whitelisted_type?(url, media: true) @@ -37,6 +38,7 @@ module Gitlab def from_markdown_link(markdown_node) url = markdown_node.url + return unless url return unless github_url?(url, docs: true) return unless whitelisted_type?(url, docs: true) @@ -46,7 +48,7 @@ module Gitlab def from_inline_html(markdown_node) img = Nokogiri::HTML.parse(markdown_node.string_content).xpath('//img')[0] - return unless img + return if img.nil? || img[:src].blank? return unless github_url?(img[:src], media: true) return unless whitelisted_type?(img[:src], media: true) diff --git a/lib/gitlab/github_import/page_counter.rb b/lib/gitlab/github_import/page_counter.rb index 3face4c794b..c238ccb8932 100644 --- a/lib/gitlab/github_import/page_counter.rb +++ b/lib/gitlab/github_import/page_counter.rb @@ -9,10 +9,10 @@ module Gitlab attr_reader :cache_key # The base cache key to use for storing the last page number. - CACHE_KEY = 'github-importer/page-counter/%{project}/%{collection}' + CACHE_KEY = '%{import_type}/page-counter/%{object}/%{collection}' - def initialize(project, collection) - @cache_key = CACHE_KEY % { project: project.id, collection: collection } + def initialize(object, collection, import_type = 'github-importer') + @cache_key = CACHE_KEY % { import_type: import_type, object: object.id, collection: collection } end # Sets the page number to the given value. diff --git a/lib/gitlab/github_import/representation/diff_note.rb b/lib/gitlab/github_import/representation/diff_note.rb index f3be90834c7..9259d0295d5 100644 --- a/lib/gitlab/github_import/representation/diff_note.rb +++ b/lib/gitlab/github_import/representation/diff_note.rb @@ -4,18 +4,15 @@ module Gitlab module GithubImport module Representation class DiffNote - include Gitlab::Utils::StrongMemoize include ToHash include ExposeAttribute - NOTEABLE_TYPE = 'MergeRequest' NOTEABLE_ID_REGEX = %r{/pull/(?<iid>\d+)}i.freeze - DISCUSSION_CACHE_KEY = 'github-importer/discussion-id-map/%{project_id}/%{noteable_id}/%{original_note_id}' expose_attribute :noteable_id, :commit_id, :file_path, :diff_hunk, :author, :created_at, :updated_at, :original_commit_id, :note_id, :end_line, :start_line, - :side, :in_reply_to_id + :side, :in_reply_to_id, :discussion_id # Builds a diff note from a GitHub API response. # @@ -45,7 +42,8 @@ module Gitlab end_line: note[:line], start_line: note[:start_line], side: note[:side], - in_reply_to_id: note[:in_reply_to_id] + in_reply_to_id: note[:in_reply_to_id], + discussion_id: DiffNotes::DiscussionId.new(note).find_or_generate } new(hash) @@ -59,7 +57,7 @@ module Gitlab new(hash) end - attr_accessor :merge_request, :project + attr_accessor :merge_request # attributes - A Hash containing the raw note details. The keys of this # Hash must be Symbols. @@ -74,7 +72,7 @@ module Gitlab end def noteable_type - NOTEABLE_TYPE + DiffNotes::DiscussionId::NOTEABLE_TYPE end def contains_suggestion? @@ -127,12 +125,6 @@ module Gitlab } end - def discussion_id - strong_memoize(:discussion_id) do - (in_reply_to_id.present? && current_discussion_id) || generate_discussion_id - end - end - private # Required by ExposeAttribute @@ -149,32 +141,6 @@ module Gitlab def addition? side == 'RIGHT' end - - def generate_discussion_id - Discussion.discussion_id( - Struct - .new(:noteable_id, :noteable_type) - .new(merge_request.id, NOTEABLE_TYPE) - ).tap do |discussion_id| - cache_discussion_id(discussion_id) - end - end - - def cache_discussion_id(discussion_id) - Gitlab::Cache::Import::Caching.write(discussion_id_cache_key(note_id), discussion_id) - end - - def current_discussion_id - Gitlab::Cache::Import::Caching.read(discussion_id_cache_key(in_reply_to_id)) - end - - def discussion_id_cache_key(id) - DISCUSSION_CACHE_KEY % { - project_id: project.id, - noteable_id: merge_request.id, - original_note_id: id - } - end end end end diff --git a/lib/gitlab/github_import/representation/diff_notes/discussion_id.rb b/lib/gitlab/github_import/representation/diff_notes/discussion_id.rb new file mode 100644 index 00000000000..38b560f21c0 --- /dev/null +++ b/lib/gitlab/github_import/representation/diff_notes/discussion_id.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + module Representation + module DiffNotes + class DiscussionId + NOTEABLE_TYPE = 'MergeRequest' + DISCUSSION_CACHE_REGEX = %r{/(?<repo>[^/]*)/pull/(?<iid>\d+)}i.freeze + DISCUSSION_CACHE_KEY = 'github-importer/discussion-id-map/%{project}/%{noteable_id}/%{original_note_id}' + + def initialize(note) + @note = note + @matches = note[:html_url].match(DISCUSSION_CACHE_REGEX) + end + + def find_or_generate + (note[:in_reply_to_id].present? && current_discussion_id) || generate_discussion_id + end + + private + + attr_reader :note, :matches + + def generate_discussion_id + discussion_id = Discussion.discussion_id( + Struct + .new(:noteable_id, :noteable_type) + .new(matches[:iid].to_i, NOTEABLE_TYPE) + ) + cache_discussion_id(discussion_id) + end + + def cache_discussion_id(discussion_id) + Gitlab::Cache::Import::Caching.write( + discussion_id_cache_key(note[:id]), discussion_id + ) + end + + def current_discussion_id + Gitlab::Cache::Import::Caching.read( + discussion_id_cache_key(note[:in_reply_to_id]) + ) + end + + def discussion_id_cache_key(id) + format(DISCUSSION_CACHE_KEY, + project: matches[:repo], + noteable_id: matches[:iid].to_i, + original_note_id: id + ) + end + end + end + end + end +end |