From 0fdb048098b9881e043bee95eb55d06fc17c5c2d Mon Sep 17 00:00:00 2001 From: Rodrigo Tomonari Date: Mon, 11 Dec 2023 12:20:18 -0300 Subject: [PATCH 1/2] Import more resources using events in GitHub Import This change updates GitHub Import to import merged by, review requests and reviews information using GitHub events via the timeline endpoint. By utilizing this approach, there will be a reduction in the number of requests required to migrate a project. --- .rubocop_todo/rspec/feature_category.yml | 1 - app/services/import/github_service.rb | 3 +- app/views/import/github/status.html.haml | 2 +- app/workers/all_queues.yml | 9 + .../github_import/advance_stage_worker.rb | 14 +- .../github_import/replay_events_worker.rb | 27 +++ .../stage/import_collaborators_worker.rb | 8 +- .../stage/import_issue_events_worker.rb | 17 +- .../github_import_extended_events.yml | 8 + lib/gitlab/cache/import/caching.rb | 42 +++++ lib/gitlab/github_import/events_cache.rb | 61 ++++++ .../importer/events/base_importer.rb | 7 +- .../importer/events/commented.rb | 27 +++ .../github_import/importer/events/merged.rb | 13 ++ .../github_import/importer/events/reviewed.rb | 26 +++ .../importer/issue_event_importer.rb | 9 + .../importer/pull_requests/review_importer.rb | 8 +- .../importer/replay_events_importer.rb | 60 ++++++ .../single_endpoint_issue_events_importer.rb | 41 ++++- .../representation/issue_event.rb | 9 +- .../representation/replay_event.rb | 31 ++++ lib/gitlab/github_import/settings.rb | 16 +- .../single_endpoint_notes_importing.rb | 3 + spec/lib/gitlab/cache/import/caching_spec.rb | 52 ++++++ .../gitlab/github_import/events_cache_spec.rb | 79 ++++++++ .../importer/events/commented_spec.rb | 69 +++++++ .../importer/events/merged_spec.rb | 27 +++ .../importer/events/reviewed_spec.rb | 85 +++++++++ .../importer/issue_event_importer_spec.rb | 12 ++ .../importer/issue_events_importer_spec.rb | 2 +- .../pull_requests/review_importer_spec.rb | 7 + .../importer/replay_events_importer_spec.rb | 108 +++++++++++ ...gle_endpoint_issue_events_importer_spec.rb | 173 +++++++++++++++++- .../representation/issue_event_spec.rb | 4 +- .../representation/replay_event_spec.rb | 24 +++ .../lib/gitlab/github_import/settings_spec.rb | 52 +++++- spec/services/import/github_service_spec.rb | 21 +++ spec/workers/every_sidekiq_worker_spec.rb | 1 + .../replay_events_worker_spec.rb | 28 +++ .../stage/import_issue_events_worker_spec.rb | 17 +- 40 files changed, 1164 insertions(+), 39 deletions(-) create mode 100644 app/workers/gitlab/github_import/replay_events_worker.rb create mode 100644 config/feature_flags/development/github_import_extended_events.yml create mode 100644 lib/gitlab/github_import/events_cache.rb create mode 100644 lib/gitlab/github_import/importer/events/commented.rb create mode 100644 lib/gitlab/github_import/importer/events/reviewed.rb create mode 100644 lib/gitlab/github_import/importer/replay_events_importer.rb create mode 100644 lib/gitlab/github_import/representation/replay_event.rb create mode 100644 spec/lib/gitlab/github_import/events_cache_spec.rb create mode 100644 spec/lib/gitlab/github_import/importer/events/commented_spec.rb create mode 100644 spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb create mode 100644 spec/lib/gitlab/github_import/importer/replay_events_importer_spec.rb create mode 100644 spec/lib/gitlab/github_import/representation/replay_event_spec.rb create mode 100644 spec/workers/gitlab/github_import/replay_events_worker_spec.rb diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index fac358d65435bd..15662272316016 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -2837,7 +2837,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/build_access_spec.rb' - 'spec/lib/gitlab/bullet_spec.rb' - 'spec/lib/gitlab/cache/helpers_spec.rb' - - 'spec/lib/gitlab/cache/import/caching_spec.rb' - 'spec/lib/gitlab/cache/metrics_spec.rb' - 'spec/lib/gitlab/cache/request_cache_spec.rb' - 'spec/lib/gitlab/cache_spec.rb' diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index a96bfd74cd0d68..e28c58794a954a 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -139,7 +139,8 @@ def store_import_settings(project) .new(project) .write( timeout_strategy: params[:timeout_strategy] || ProjectImportData::PESSIMISTIC_TIMEOUT, - optional_stages: params[:optional_stages] + optional_stages: params[:optional_stages], + extended_events: Feature.enabled?(:github_import_extended_events, current_user) ) end end diff --git a/app/views/import/github/status.html.haml b/app/views/import/github/status.html.haml index 97acafe24d05f0..0f56ae92557eab 100644 --- a/app/views/import/github/status.html.haml +++ b/app/views/import/github/status.html.haml @@ -12,4 +12,4 @@ cancel_path: cancel_import_github_path, details_path: details_import_github_path, status_import_github_group_path: status_import_github_group_path(format: :json), - optional_stages: Gitlab::GithubImport::Settings.stages_array + optional_stages: Gitlab::GithubImport::Settings.stages_array(current_user) diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index ec5156bb1d0de3..e4329ae2a8bcff 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1353,6 +1353,15 @@ :weight: 1 :idempotent: false :tags: [] +- :name: github_importer:github_import_replay_events + :worker_name: Gitlab::GithubImport::ReplayEventsWorker + :feature_category: :importers + :has_external_dependencies: true + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: github_importer:github_import_stage_finish_import :worker_name: Gitlab::GithubImport::Stage::FinishImportWorker :feature_category: :importers diff --git a/app/workers/gitlab/github_import/advance_stage_worker.rb b/app/workers/gitlab/github_import/advance_stage_worker.rb index 417b85985479e3..805252927ff82a 100644 --- a/app/workers/gitlab/github_import/advance_stage_worker.rb +++ b/app/workers/gitlab/github_import/advance_stage_worker.rb @@ -22,14 +22,20 @@ class AdvanceStageWorker # rubocop:disable Scalability/IdempotentWorker sidekiq_options dead: false # The known importer stages and their corresponding Sidekiq workers. + # + # Note: AdvanceStageWorker is not used for the repository, base_data, and pull_requests stages. + # They are included in the list for us to easily see all stage workers and the order in which they are executed. STAGES = { + repository: Stage::ImportRepositoryWorker, + base_data: Stage::ImportBaseDataWorker, + pull_requests: Stage::ImportPullRequestsWorker, collaborators: Stage::ImportCollaboratorsWorker, - pull_requests_merged_by: Stage::ImportPullRequestsMergedByWorker, - pull_request_review_requests: Stage::ImportPullRequestsReviewRequestsWorker, - pull_request_reviews: Stage::ImportPullRequestsReviewsWorker, + pull_requests_merged_by: Stage::ImportPullRequestsMergedByWorker, # Skipped on extended_events + pull_request_review_requests: Stage::ImportPullRequestsReviewRequestsWorker, # Skipped on extended_events + pull_request_reviews: Stage::ImportPullRequestsReviewsWorker, # Skipped on extended_events issues_and_diff_notes: Stage::ImportIssuesAndDiffNotesWorker, issue_events: Stage::ImportIssueEventsWorker, - notes: Stage::ImportNotesWorker, + notes: Stage::ImportNotesWorker, # Skipped on extended_events attachments: Stage::ImportAttachmentsWorker, protected_branches: Stage::ImportProtectedBranchesWorker, lfs_objects: Stage::ImportLfsObjectsWorker, diff --git a/app/workers/gitlab/github_import/replay_events_worker.rb b/app/workers/gitlab/github_import/replay_events_worker.rb new file mode 100644 index 00000000000000..680d5ec2d7d475 --- /dev/null +++ b/app/workers/gitlab/github_import/replay_events_worker.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + class ReplayEventsWorker + include ObjectImporter + + idempotent! + + def representation_class + Representation::ReplayEvent + end + + def importer_class + Importer::ReplayEventsImporter + end + + def object_type + :replay_event + end + + def increment_object_counter?(_object) + false + end + end + end +end diff --git a/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb b/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb index b5b1601e3ed01c..38e1fd52889c7b 100644 --- a/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb @@ -42,9 +42,15 @@ def skip_to_next_stage(project) def move_to_next_stage(project, waiters = {}) AdvanceStageWorker.perform_async( - project.id, waiters.deep_stringify_keys, 'pull_requests_merged_by' + project.id, waiters.deep_stringify_keys, next_stage(project) ) end + + def next_stage(project) + return 'issues_and_diff_notes' if import_settings(project).extended_events? + + 'pull_requests_merged_by' + end end end end diff --git a/app/workers/gitlab/github_import/stage/import_issue_events_worker.rb b/app/workers/gitlab/github_import/stage/import_issue_events_worker.rb index 27d14a1a108324..9618500604ab78 100644 --- a/app/workers/gitlab/github_import/stage/import_issue_events_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_issue_events_worker.rb @@ -15,7 +15,7 @@ class ImportIssueEventsWorker # rubocop:disable Scalability/IdempotentWorker # client - An instance of Gitlab::GithubImport::Client. # project - An instance of Project. def import(client, project) - return skip_to_next_stage(project) if import_settings(project).disabled?(:single_endpoint_issue_events_import) + return skip_to_next_stage(project) if skip_to_next_stage?(project) importer = ::Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter info(project.id, message: "starting importer", importer: importer.name) @@ -25,13 +25,26 @@ def import(client, project) private + def skip_to_next_stage?(project) + # This stage is mandatory when using extended_events + return false if import_settings(project).extended_events? + + import_settings(project).disabled?(:single_endpoint_issue_events_import) + end + def skip_to_next_stage(project) info(project.id, message: "skipping importer", importer: "IssueEventsImporter") move_to_next_stage(project) end def move_to_next_stage(project, waiters = {}) - AdvanceStageWorker.perform_async(project.id, waiters.deep_stringify_keys, 'notes') + AdvanceStageWorker.perform_async(project.id, waiters.deep_stringify_keys, next_stage(project)) + end + + def next_stage(project) + return "attachments" if import_settings(project).extended_events? + + "notes" end end end diff --git a/config/feature_flags/development/github_import_extended_events.yml b/config/feature_flags/development/github_import_extended_events.yml new file mode 100644 index 00000000000000..e4466bf958fc55 --- /dev/null +++ b/config/feature_flags/development/github_import_extended_events.yml @@ -0,0 +1,8 @@ +--- +name: github_import_extended_events +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139410 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/435089 +milestone: '16.8' +type: development +group: group::import and integrate +default_enabled: false diff --git a/lib/gitlab/cache/import/caching.rb b/lib/gitlab/cache/import/caching.rb index e81a90831f75a1..f3251d47e7a383 100644 --- a/lib/gitlab/cache/import/caching.rb +++ b/lib/gitlab/cache/import/caching.rb @@ -239,6 +239,48 @@ def self.values_from_hash(raw_key) end end + # Adds a value to a list. + # + # raw_key - The key of the list to add to. + # value - The field value to add to the list. + # timeout - The new timeout of the key. + # limit - The maximum number of members in the set. Older members will be trimmed to this limit. + def self.list_add(raw_key, value, timeout: TIMEOUT, limit: nil) + validate_redis_value!(value) + + key = cache_key_for(raw_key) + + with_redis do |redis| + redis.multi do |m| + m.rpush(key, value) + m.ltrim(key, -limit, -1) if limit + m.expire(key, timeout) + end + end + end + + # Returns the values of the given list. + # + # raw_key - The key of the list. + def self.values_from_list(raw_key) + key = cache_key_for(raw_key) + + with_redis do |redis| + redis.lrange(key, 0, -1) + end + end + + # Deletes a key + # + # raw_key - Key name + def self.del(raw_key) + key = cache_key_for(raw_key) + + with_redis do |redis| + redis.del(key) + end + end + def self.cache_key_for(raw_key) "#{Redis::Cache::CACHE_NAMESPACE}:#{raw_key}" end diff --git a/lib/gitlab/github_import/events_cache.rb b/lib/gitlab/github_import/events_cache.rb new file mode 100644 index 00000000000000..0986ccfaed1aac --- /dev/null +++ b/lib/gitlab/github_import/events_cache.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + class EventsCache + MAX_NUMBER_OF_EVENTS = 100 + MAX_EVENT_SIZE = 100.kilobytes + + def initialize(project) + @project = project + end + + # Add issue event as JSON to the cache + # + # @param record [ActiveRecord::Model] Model that responds to :iid + # @param event [GitLab::GitHubImport::Representation::IssueEvent] + def add(record, issue_event) + json = issue_event.to_hash.to_json + + if json.bytesize > MAX_EVENT_SIZE + Logger.warn( + message: 'Event too large to cache', + project_id: project.id, + github_identifiers: issue_event.github_identifiers + ) + + return + end + + Gitlab::Cache::Import::Caching.list_add(events_cache_key(record), json, limit: MAX_NUMBER_OF_EVENTS) + end + + # Reads issue events from cache + # + # @param record [ActiveRecord::Model] Model that responds to :iid + # @retun [Array] List of issue events + def events(record) + events = Gitlab::Cache::Import::Caching.values_from_list(events_cache_key(record)).map do |event| + Representation::IssueEvent.from_json_hash(Gitlab::Json.parse(event)) + end + + events.sort_by(&:created_at) + end + + # Deletes the cache + # + # @param record [ActiveRecord::Model] Model that responds to :iid + def delete(record) + Gitlab::Cache::Import::Caching.del(events_cache_key(record)) + end + + private + + attr_reader :project + + def events_cache_key(record) + "github-importer/events/#{project.id}/#{record.class.name}/#{record.iid}" + end + end + end +end diff --git a/lib/gitlab/github_import/importer/events/base_importer.rb b/lib/gitlab/github_import/importer/events/base_importer.rb index 8218acf2bfb8bf..1ebafec5afc0e9 100644 --- a/lib/gitlab/github_import/importer/events/base_importer.rb +++ b/lib/gitlab/github_import/importer/events/base_importer.rb @@ -10,6 +10,7 @@ class BaseImporter # client - An instance of `Gitlab::GithubImport::Client`. def initialize(project, client) @project = project + @client = client @user_finder = UserFinder.new(project, client) end @@ -20,7 +21,7 @@ def execute(issue_event) private - attr_reader :project, :user_finder + attr_reader :project, :user_finder, :client def author_id(issue_event, author_key: :actor) user_finder.author_id_for(issue_event, author_key: author_key).first @@ -42,6 +43,10 @@ def resource_event_belongs_to(issue_event) belongs_to_key = merge_request_event?(issue_event) ? :merge_request_id : :issue_id { belongs_to_key => issuable_db_id(issue_event) } end + + def import_settings + @import_settings ||= Gitlab::GithubImport::Settings.new(project) + end end end end diff --git a/lib/gitlab/github_import/importer/events/commented.rb b/lib/gitlab/github_import/importer/events/commented.rb new file mode 100644 index 00000000000000..c9ebc31fa0619d --- /dev/null +++ b/lib/gitlab/github_import/importer/events/commented.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + module Importer + module Events + class Commented < BaseImporter + def execute(issue_event) + return true unless import_settings.extended_events? + + note = Representation::Note.from_json_hash( + noteable_id: issue_event.issuable_id, + noteable_type: issue_event.issuable_type, + author: issue_event.actor&.to_hash, + note: issue_event.body, + created_at: issue_event.created_at, + updated_at: issue_event.updated_at, + note_id: issue_event.id + ) + + NoteImporter.new(note, project, client).execute + end + end + end + end + end +end diff --git a/lib/gitlab/github_import/importer/events/merged.rb b/lib/gitlab/github_import/importer/events/merged.rb index 6189fa8f429462..702ea7f1fd5390 100644 --- a/lib/gitlab/github_import/importer/events/merged.rb +++ b/lib/gitlab/github_import/importer/events/merged.rb @@ -6,6 +6,8 @@ module Importer module Events class Merged < BaseImporter def execute(issue_event) + create_note(issue_event) if import_settings.extended_events? + create_event(issue_event) create_state_event(issue_event) end @@ -37,6 +39,17 @@ def create_state_event(issue_event) ResourceStateEvent.create!(attrs) end + + def create_note(issue_event) + pull_request = Representation::PullRequest.from_json_hash({ + merged_by: issue_event.actor&.to_hash, + merged_at: issue_event.created_at, + iid: issue_event.issuable_id, + state: :closed + }) + + PullRequests::MergedByImporter.new(pull_request, project, client).execute + end end end end diff --git a/lib/gitlab/github_import/importer/events/reviewed.rb b/lib/gitlab/github_import/importer/events/reviewed.rb new file mode 100644 index 00000000000000..1c0e8a9e6e8288 --- /dev/null +++ b/lib/gitlab/github_import/importer/events/reviewed.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + module Importer + module Events + class Reviewed < BaseImporter + def execute(issue_event) + return true unless import_settings.extended_events? + + review = Representation::PullRequestReview.new( + merge_request_iid: issue_event.issuable_id, + author: issue_event.actor&.to_hash, + note: issue_event.body.to_s, + review_type: issue_event.state.upcase, # On timeline API, the state is in lower case + submitted_at: issue_event.submitted_at, + review_id: issue_event.id + ) + + PullRequests::ReviewImporter.new(review, project, client).execute({ add_reviewer: false }) + end + end + end + end + end +end diff --git a/lib/gitlab/github_import/importer/issue_event_importer.rb b/lib/gitlab/github_import/importer/issue_event_importer.rb index d20482eca6f849..2068f789fec6a6 100644 --- a/lib/gitlab/github_import/importer/issue_event_importer.rb +++ b/lib/gitlab/github_import/importer/issue_event_importer.rb @@ -22,6 +22,11 @@ class IssueEventImporter unlabeled ].freeze + EXTENDED_SUPPORTED_EVENTS = SUPPORTED_EVENTS + %w[ + commented + reviewed + ].freeze + # issue_event - An instance of `Gitlab::GithubImport::Representation::IssueEvent`. # project - An instance of `Project`. # client - An instance of `Gitlab::GithubImport::Client`. @@ -65,6 +70,10 @@ def event_importer_class(issue_event) Gitlab::GithubImport::Importer::Events::ChangedReviewer when 'merged' Gitlab::GithubImport::Importer::Events::Merged + when 'commented' + Gitlab::GithubImport::Importer::Events::Commented + when 'reviewed' + Gitlab::GithubImport::Importer::Events::Reviewed end end end diff --git a/lib/gitlab/github_import/importer/pull_requests/review_importer.rb b/lib/gitlab/github_import/importer/pull_requests/review_importer.rb index 6df130eb6e8af2..384880651ef936 100644 --- a/lib/gitlab/github_import/importer/pull_requests/review_importer.rb +++ b/lib/gitlab/github_import/importer/pull_requests/review_importer.rb @@ -14,10 +14,12 @@ def initialize(review, project, client) @review = review @project = project @client = client - @merge_request = project.merge_requests.find_by_id(review.merge_request_id) + @merge_request = project.merge_requests.find_by_iid(review.merge_request_iid) end - def execute + def execute(options = {}) + options = { add_reviewer: true }.merge(options) + user_finder = GithubImport::UserFinder.new(project, client) gitlab_user_id = user_finder.user_id_for(review.author) @@ -25,7 +27,7 @@ def execute if gitlab_user_id add_review_note!(gitlab_user_id) add_approval!(gitlab_user_id) - add_reviewer!(gitlab_user_id) + add_reviewer!(gitlab_user_id) if options[:add_reviewer] else add_complementary_review_note!(project.creator_id) end diff --git a/lib/gitlab/github_import/importer/replay_events_importer.rb b/lib/gitlab/github_import/importer/replay_events_importer.rb new file mode 100644 index 00000000000000..bcdaa4d3ce5b96 --- /dev/null +++ b/lib/gitlab/github_import/importer/replay_events_importer.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + module Importer + class ReplayEventsImporter + SUPPORTED_EVENTS = %w[review_request_removed review_requested].freeze + + # replay_event - An instance of `Gitlab::GithubImport::Representation::ReplayEvent`. + # project - An instance of `Project` + # client - An instance of `Gitlab::GithubImport::Client` + def initialize(replay_event, project, client) + @project = project + @client = client + @replay_event = replay_event + end + + def execute + association = case replay_event.issuable_type + when 'MergeRequest' + project.merge_requests.find_by_iid(replay_event.issuable_iid) + end + + return unless association + + events_cache = EventsCache.new(project) + + handle_review_requests(association, events_cache.events(association)) + + events_cache.delete(association) + end + + private + + attr_reader :project, :client, :replay_event + + def handle_review_requests(association, events) + reviewers = {} + + events.each do |event| + case event.event + when 'review_requested' + reviewers[event.requested_reviewer.login] = event.requested_reviewer.to_hash + when 'review_request_removed' + reviewers[event.requested_reviewer.login] = nil + end + end + + representation = Representation::PullRequests::ReviewRequests.from_json_hash( + merge_request_id: association.id, + merge_request_iid: association.iid, + users: reviewers.values.compact + ) + + Importer::PullRequests::ReviewRequestImporter.new(representation, project, client).execute + end + end + end + end +end diff --git a/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer.rb b/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer.rb index d7fa098a775738..20fff73ac3d7b3 100644 --- a/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer.rb +++ b/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer.rb @@ -30,7 +30,9 @@ def each_associated(parent_record, associated) compose_associated_id!(parent_record, associated) - return if already_imported?(associated) || importer_class::SUPPORTED_EVENTS.exclude?(associated[:event]) + return if already_imported?(associated) || supported_events.exclude?(associated[:event]) + + cache_event(parent_record, associated) Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched) @@ -98,6 +100,43 @@ def compose_associated_id!(issuable, event) event[:id] = "cross-reference##{issuable.iid}-in-#{event.dig(:source, :issue, :id)}" end + + def import_settings + @import_settings ||= Gitlab::GithubImport::Settings.new(project) + end + + def after_batch_processed(parent) + return unless import_settings.extended_events? + + events = events_cache.events(parent) + + return if events.empty? + + hash = Representation::ReplayEvent.new(issuable_type: parent.class.name.to_s, issuable_iid: parent.iid) + .to_hash.deep_stringify_keys + ReplayEventsWorker.perform_async(project.id, hash, job_waiter.key.to_s) + job_waiter.jobs_remaining = Gitlab::Cache::Import::Caching.increment(job_waiter_remaining_cache_key) + end + + def supported_events + return importer_class::EXTENDED_SUPPORTED_EVENTS if import_settings.extended_events? + + importer_class::SUPPORTED_EVENTS + end + + def cache_event(parent_record, associated) + return unless import_settings.extended_events? + + return if Importer::ReplayEventsImporter::SUPPORTED_EVENTS.exclude?(associated[:event]) + + representation = representation_class.from_api_response(associated) + + events_cache.add(parent_record, representation) + end + + def events_cache + @events_cache ||= EventsCache.new(project) + end end end end diff --git a/lib/gitlab/github_import/representation/issue_event.rb b/lib/gitlab/github_import/representation/issue_event.rb index 30608112f85aa6..fc3bc5a48ef7bd 100644 --- a/lib/gitlab/github_import/representation/issue_event.rb +++ b/lib/gitlab/github_import/representation/issue_event.rb @@ -8,7 +8,8 @@ class IssueEvent expose_attribute :id, :actor, :event, :commit_id, :label_title, :old_title, :new_title, :milestone_title, :issue, :source, :assignee, :review_requester, - :requested_reviewer, :created_at + :requested_reviewer, :created_at, :updated_at, :submitted_at, + :state, :body # attributes - A Hash containing the event details. The keys of this # Hash (and any nested hashes) must be symbols. @@ -51,7 +52,11 @@ def from_api_response(event, additional_data = {}) assignee: user_representation(event[:assignee]), requested_reviewer: user_representation(event[:requested_reviewer]), review_requester: user_representation(event[:review_requester]), - created_at: event[:created_at] + created_at: event[:created_at], + updated_at: event[:updated_at], + submitted_at: event[:submitted_at], + state: event[:state], + body: event[:body] ) end diff --git a/lib/gitlab/github_import/representation/replay_event.rb b/lib/gitlab/github_import/representation/replay_event.rb new file mode 100644 index 00000000000000..2d71c26abbbf4f --- /dev/null +++ b/lib/gitlab/github_import/representation/replay_event.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + module Representation + class ReplayEvent + include ToHash + include ExposeAttribute + + attr_reader :attributes + + expose_attribute :issuable_type, :issuable_iid + + def self.from_json_hash(raw_hash) + new Representation.symbolize_hash(raw_hash) + end + + def initialize(attributes) + @attributes = attributes + end + + def github_identifiers + { + issuable_type: issuable_type, + issuable_iid: issuable_iid + } + end + end + end + end +end diff --git a/lib/gitlab/github_import/settings.rb b/lib/gitlab/github_import/settings.rb index 3947ae3c63d0ea..da5833df3a1d13 100644 --- a/lib/gitlab/github_import/settings.rb +++ b/lib/gitlab/github_import/settings.rb @@ -38,8 +38,13 @@ class Settings } }.freeze - def self.stages_array - OPTIONAL_STAGES.map do |stage_name, data| + def self.stages_array(current_user) + deprecated_options = %i[single_endpoint_issue_events_import] + + OPTIONAL_STAGES.filter_map do |stage_name, data| + next if deprecated_options.include?(stage_name) && + Feature.enabled?(:github_import_extended_events, current_user) + { name: stage_name.to_s, label: s_(format("GitHubImport|%{text}", text: data[:label])), @@ -61,7 +66,8 @@ def write(user_settings) import_data = project.build_or_assign_import_data( data: { optional_stages: optional_stages, - timeout_strategy: user_settings[:timeout_strategy] + timeout_strategy: user_settings[:timeout_strategy], + extended_events: user_settings[:extended_events] }, credentials: project.import_data&.credentials ) @@ -77,6 +83,10 @@ def disabled?(stage_name) !enabled?(stage_name) end + def extended_events? + !!project.import_data&.data&.dig('extended_events') + end + private attr_reader :project diff --git a/lib/gitlab/github_import/single_endpoint_notes_importing.rb b/lib/gitlab/github_import/single_endpoint_notes_importing.rb index 3584288da57310..ed8dda8ebd2773 100644 --- a/lib/gitlab/github_import/single_endpoint_notes_importing.rb +++ b/lib/gitlab/github_import/single_endpoint_notes_importing.rb @@ -85,6 +85,7 @@ def process_batch(batch) yield parent_record, page end + after_batch_processed(parent_record) mark_parent_imported(parent_record) end end @@ -96,6 +97,8 @@ def mark_parent_imported(parent) ) end + def after_batch_processed(_parent); end + def already_imported_parents Gitlab::Cache::Import::Caching.values_from_set(parent_imported_cache_key) end diff --git a/spec/lib/gitlab/cache/import/caching_spec.rb b/spec/lib/gitlab/cache/import/caching_spec.rb index 8f1c552e0b744f..6cde51b668a9cc 100644 --- a/spec/lib/gitlab/cache/import/caching_spec.rb +++ b/spec/lib/gitlab/cache/import/caching_spec.rb @@ -224,4 +224,56 @@ subject { described_class.write_if_greater('foo', value) } end end + + describe '.list_add' do + it 'adds a value to a list' do + described_class.list_add('foo', 10) + described_class.list_add('foo', 20) + + key = described_class.cache_key_for('foo') + values = Gitlab::Redis::Cache.with { |r| r.lrange(key, 0, -1) } + + expect(values).to eq(%w[10 20]) + end + + context 'when a limit is provided' do + it 'limits the size of the list to the number of items defined by the limit' do + described_class.list_add('foo', 10, limit: 3) + described_class.list_add('foo', 20, limit: 3) + described_class.list_add('foo', 30, limit: 3) + described_class.list_add('foo', 40, limit: 3) + + key = described_class.cache_key_for('foo') + values = Gitlab::Redis::Cache.with { |r| r.lrange(key, 0, -1) } + + expect(values).to eq(%w[20 30 40]) + end + end + + it_behaves_like 'validated redis value' do + subject { described_class.list_add('foo', value) } + end + end + + describe '.values_from_list' do + it 'returns empty hash when the list is empty' do + expect(described_class.values_from_list('foo')).to eq([]) + end + + it 'returns the items stored in the list in order' do + described_class.list_add('foo', 10) + described_class.list_add('foo', 20) + described_class.list_add('foo', 10) + + expect(described_class.values_from_list('foo')).to eq(%w[10 20 10]) + end + end + + describe '.del' do + it 'deletes the key' do + described_class.write('foo', 'value') + + expect { described_class.del('foo') }.to change { described_class.read('foo') }.from('value').to(nil) + end + end end diff --git a/spec/lib/gitlab/github_import/events_cache_spec.rb b/spec/lib/gitlab/github_import/events_cache_spec.rb new file mode 100644 index 00000000000000..8637f2369773f2 --- /dev/null +++ b/spec/lib/gitlab/github_import/events_cache_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::EventsCache, :clean_gitlab_redis_cache, feature_category: :importers do + let(:project) { build_stubbed(:project, id: 1) } + let(:issue) { build_stubbed(:issue, iid: 2) } + + let(:event_cache) { described_class.new(project) } + + def build_event(event) + Gitlab::GithubImport::Representation::IssueEvent.from_json_hash(event) + end + + describe '#add' do + it 'adds event to cache' do + expect(Gitlab::Cache::Import::Caching).to receive(:list_add).with( + 'github-importer/events/1/Issue/2', + an_instance_of(String), + limit: described_class::MAX_NUMBER_OF_EVENTS + ) + + event_cache.add(issue, build_event({ event: 'closed' })) + end + + context 'when events is too large to cache' do + before do + stub_const("#{described_class}::MAX_EVENT_SIZE", 1.byte) + end + + it 'does not add event to cache' do + expect(Gitlab::Cache::Import::Caching).not_to receive(:list_add) + expect(Gitlab::GithubImport::Logger).to receive(:warn).with( + message: 'Event too large to cache', + project_id: project.id, + github_identifiers: { + event: 'closed', + id: '99', + issuable_iid: '2' + } + ) + + event_cache.add(issue, build_event({ event: 'closed', id: '99', issue: { number: '2' } })) + end + end + end + + describe '#events' do + it 'retrieves the list of events from the cache in the correct order' do + key = 'github-importer/events/1/Issue/2' + + Gitlab::Cache::Import::Caching.list_add(key, { event: 'merged', created_at: '2023-01-02T00:00:00Z' }.to_json) + Gitlab::Cache::Import::Caching.list_add(key, { event: 'closed', created_at: '2023-01-03T00:00:00Z' }.to_json) + Gitlab::Cache::Import::Caching.list_add(key, { event: 'commented', created_at: '2023-01-01T00:00:00Z' }.to_json) + + events = event_cache.events(issue).map(&:to_hash) + + expect(events).to match([ + a_hash_including(event: 'commented', created_at: '2023-01-01 00:00:00 UTC'), + a_hash_including(event: 'merged', created_at: '2023-01-02 00:00:00 UTC'), + a_hash_including(event: 'closed', created_at: '2023-01-03 00:00:00 UTC') + ]) + end + + context 'when no event was added' do + it 'returns an empty array' do + expect(event_cache.events(issue)).to eq([]) + end + end + end + + describe '#delete' do + it 'deletes the list' do + expect(Gitlab::Cache::Import::Caching).to receive(:del).with('github-importer/events/1/Issue/2') + + event_cache.delete(issue) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/events/commented_spec.rb b/spec/lib/gitlab/github_import/importer/events/commented_spec.rb new file mode 100644 index 00000000000000..bd3bea87688983 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/events/commented_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::Events::Commented, feature_category: :importers do + subject(:importer) { described_class.new(project, client) } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + + let(:client) { instance_double('Gitlab::GithubImport::Client') } + let(:issuable) { create(:issue, project: project) } + + let(:issue_event) do + Gitlab::GithubImport::Representation::IssueEvent.new( + id: 1196850910, + actor: { id: user.id, login: user.username }, + event: 'commented', + created_at: '2022-07-27T14:41:11Z', + updated_at: '2022-07-27T14:41:11Z', + body: 'This is my note', + issue: { number: issuable.iid, pull_request: issuable.is_a?(MergeRequest) } + ) + end + + let(:extended_events) { true } + + before do + allow_next_instance_of(Gitlab::GithubImport::IssuableFinder) do |finder| + allow(finder).to receive(:database_id).and_return(issuable.id) + end + allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder| + allow(finder).to receive(:find).with(user.id, user.username).and_return(user.id) + end + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end + end + + shared_examples 'new note' do + it 'creates a note' do + expect { importer.execute(issue_event) }.to change { Note.count }.by(1) + + expect(issuable.notes.last).to have_attributes( + note: 'This is my note', + author_id: user.id, + noteable_type: issuable.class.name.to_s + ) + end + + context 'when extended_events is disabled' do + let(:extended_events) { false } + + it 'does not create a note' do + expect { importer.execute(issue_event) }.not_to change { Note.count } + end + end + end + + context 'with Issue' do + it_behaves_like 'new note' + end + + context 'with MergeRequest' do + let(:issuable) { create(:merge_request, source_project: project, target_project: project) } + + it_behaves_like 'new note' + end +end diff --git a/spec/lib/gitlab/github_import/importer/events/merged_spec.rb b/spec/lib/gitlab/github_import/importer/events/merged_spec.rb index 4ea62557dd6859..30bc8aabe12526 100644 --- a/spec/lib/gitlab/github_import/importer/events/merged_spec.rb +++ b/spec/lib/gitlab/github_import/importer/events/merged_spec.rb @@ -11,6 +11,7 @@ let(:client) { instance_double('Gitlab::GithubImport::Client') } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:commit_id) { nil } + let(:extended_events) { false } let(:issue_event) do Gitlab::GithubImport::Representation::IssueEvent.from_json_hash( @@ -32,6 +33,9 @@ allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder| allow(finder).to receive(:find).with(user.id, user.username).and_return(user.id) end + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end end it 'creates expected event and state event' do @@ -71,4 +75,27 @@ expect(state_event.source_commit).to eq commit_id[0..40] end end + + describe 'extended events' do + context 'when using extended events' do + let(:extended_events) { true } + + it 'creates a merged by note' do + expect { importer.execute(issue_event) }.to change { Note.count }.by(1) + + last_note = merge_request.notes.last + expect(last_note.created_at).to eq(issue_event.created_at) + expect(last_note.author).to eq(project.owner) + expect(last_note.note).to eq("*Merged by: #{user.username} at #{issue_event.created_at}*") + end + end + + context 'when not using extended events' do + let(:extended_events) { false } + + it 'does not create a merged by note' do + expect { importer.execute(issue_event) }.not_to change { Note.count } + end + end + end end diff --git a/spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb b/spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb new file mode 100644 index 00000000000000..f60a9d65269592 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::Events::Reviewed, feature_category: :importers do + subject(:importer) { described_class.new(project, client) } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + + let(:client) { instance_double('Gitlab::GithubImport::Client') } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:extended_events) { true } + + let(:issue_event) do + Gitlab::GithubImport::Representation::IssueEvent.new( + id: 1196850910, + actor: { id: user.id, login: user.username }, + event: 'reviewed', + submitted_at: '2022-07-27T14:41:11Z', + body: 'This is my review', + state: state, + issue: { number: merge_request.iid, pull_request: true } + ) + end + + let(:state) { 'commented' } + + before do + allow_next_instance_of(Gitlab::GithubImport::IssuableFinder) do |finder| + allow(finder).to receive(:database_id).and_return(merge_request.id) + end + allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder| + allow(finder).to receive(:find).with(user.id, user.username).and_return(user.id) + end + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end + end + + it 'creates a review note', :aggregate_failures do + expect { importer.execute(issue_event) }.to change { Note.count }.by(1) + + last_note = merge_request.notes.last + expect(last_note.note).to include("This is my review") + expect(last_note.author).to eq(user) + expect(last_note.created_at).to eq(issue_event.submitted_at) + end + + it 'does not create a reviewer for the Merge Request', :aggregate_failures do + expect { importer.execute(issue_event) }.not_to change { MergeRequestReviewer.count } + end + + context 'when stage is approved' do + let(:state) { 'approved' } + + it 'creates an approval for the Merge Request', :aggregate_failures do + expect { importer.execute(issue_event) }.to change { Approval.count }.by(1).and change { Note.count }.by(2) + + expect(merge_request.approved_by_users.reload).to include(user) + expect(merge_request.approvals.last.created_at).to eq(issue_event.submitted_at) + + note = merge_request.notes.where(system: false).last + expect(note.note).to include("This is my review") + expect(note.author).to eq(user) + expect(note.created_at).to eq(issue_event.submitted_at) + + system_note = merge_request.notes.where(system: true).last + expect(system_note.note).to eq('approved this merge request') + expect(system_note.author).to eq(user) + expect(system_note.created_at).to eq(issue_event.submitted_at) + expect(system_note.system_note_metadata.action).to eq('approved') + end + end + + context 'when extended events is false' do + let(:extended_events) { false } + + it 'does nothing' do + expect { importer.execute(issue_event) } + .to not_change { Note.count } + .and not_change { Approval.count } + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/issue_event_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_event_importer_spec.rb index 2389489e86714e..ffe6c237506dfc 100644 --- a/spec/lib/gitlab/github_import/importer/issue_event_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/issue_event_importer_spec.rb @@ -115,6 +115,18 @@ it_behaves_like 'triggers specific event importer', Gitlab::GithubImport::Importer::Events::Merged end + context "when it's commented issue event" do + let(:event_name) { 'commented' } + + it_behaves_like 'triggers specific event importer', Gitlab::GithubImport::Importer::Events::Commented + end + + context "when it's reviewed issue event" do + let(:event_name) { 'reviewed' } + + it_behaves_like 'triggers specific event importer', Gitlab::GithubImport::Importer::Events::Reviewed + end + context "when it's unknown issue event" do let(:event_name) { 'fake' } diff --git a/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb index f7ee6fee6dc926..174cc72071703f 100644 --- a/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb @@ -13,7 +13,7 @@ struct = Struct.new( :id, :node_id, :url, :actor, :event, :commit_id, :commit_url, :label, :rename, :milestone, :source, :assignee, :assigner, :review_requester, :requested_reviewer, :issue, :created_at, :performed_via_github_app, - keyword_init: true + :body, :updated_at, :submitted_at, :state, keyword_init: true ) struct.new(id: rand(10), event: 'closed', created_at: '2022-04-26 18:30:53 UTC') end diff --git a/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb index 6846c99fb63faa..1651774b5cef65 100644 --- a/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb @@ -30,6 +30,12 @@ expect(merge_request.reviewers).to contain_exactly(author) end + context 'when add_reviewer option is false' do + it 'does not change Merge Request reviewers' do + expect { subject.execute(add_reviewer: false) }.not_to change { MergeRequestReviewer.count } + end + end + context 'when reviewer already exists' do before do create( @@ -309,6 +315,7 @@ def create_review(type:, **extra) extra.reverse_merge( author: { id: 999, login: 'author' }, merge_request_id: merge_request.id, + merge_request_iid: merge_request.iid, review_type: type, note: 'note', submitted_at: submitted_at.to_s diff --git a/spec/lib/gitlab/github_import/importer/replay_events_importer_spec.rb b/spec/lib/gitlab/github_import/importer/replay_events_importer_spec.rb new file mode 100644 index 00000000000000..a2233ffe8ccf71 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/replay_events_importer_spec.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::ReplayEventsImporter, feature_category: :importers do + let_it_be(:association) { create(:merged_merge_request) } + let_it_be(:project) { association.project } + let(:user1) { build(:user1) } + let(:user2) { build(:user2) } + let(:user3) { build(:user3) } + let(:client) { instance_double(Gitlab::GithubImport::Client) } + + let(:representation) do + Gitlab::GithubImport::Representation::ReplayEvent.new( + issuable_type: association.class.name.to_s, issuable_iid: association.iid + ) + end + + let(:importer) { described_class.new(representation, project, client) } + + describe '#execute' do + before do + events = [ + { + requested_reviewer: { id: 1, login: 'user1' }, + event: 'review_requested' + }, + { + requested_reviewer: { id: 1, login: 'user1' }, + event: 'review_request_removed' + }, + { + requested_reviewer: { id: 2, login: 'user2' }, + event: 'review_requested' + }, + { + requested_reviewer: { id: 2, login: 'user2' }, + event: 'review_request_removed' + }, + { + requested_reviewer: { id: 2, login: 'user2' }, + event: 'review_requested' + }, + { + requested_reviewer: { id: 3, login: 'user3' }, + event: 'review_requested' + } + ] + + representations = events.map { |e| Gitlab::GithubImport::Representation::IssueEvent.from_json_hash(e) } + + allow_next_instance_of(Gitlab::GithubImport::EventsCache) do |events_cache| + allow(events_cache).to receive(:events).with(association).and_return(representations) + end + end + + context 'when association is a MergeRequest' do + it 'imports reviewers' do + representation = instance_double(Gitlab::GithubImport::Representation::PullRequests::ReviewRequests) + + expect(Gitlab::GithubImport::Representation::PullRequests::ReviewRequests).to receive(:from_json_hash).with( + merge_request_id: association.id, + merge_request_iid: association.iid, + users: [ + { id: 2, login: 'user2' }, + { id: 3, login: 'user3' } + ] + ).and_return(representation) + + expect_next_instance_of( + Gitlab::GithubImport::Importer::PullRequests::ReviewRequestImporter, anything, project, client + ) do |review_impoter| + expect(review_impoter).to receive(:execute) + end + + importer.execute + end + end + + context 'when association is not found' do + let(:representation) do + Gitlab::GithubImport::Representation::ReplayEvent.new( + issuable_type: association.class.name.to_s, issuable_iid: -1 + ) + end + + it 'does not read events' do + expect(Gitlab::GithubImport::EventsCache).not_to receive(:new) + + importer.execute + end + end + + context 'when issueable type is not supported' do + let(:representation) do + Gitlab::GithubImport::Representation::ReplayEvent.new( + issuable_type: 'Issue', issuable_iid: association.iid + ) + end + + it 'does not read events' do + expect(Gitlab::GithubImport::EventsCache).not_to receive(:new) + + importer.execute + end + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb b/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb index 91f89f0779c0e1..ab51ecf7d61a1d 100644 --- a/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter, feature_category: :importers do - let(:client) { double } + let(:client) { Gitlab::GithubImport::Client.new('token') } - let_it_be(:project) { create(:project, :import_started, import_source: 'http://somegithub.com') } + let_it_be(:project) { create(:project, :import_started, import_source: 'foo/bar') } let!(:issuable) { create(:issue, project: project) } @@ -88,23 +88,32 @@ describe '#each_object_to_import', :clean_gitlab_redis_cache do let(:issue_event) do struct = Struct.new(:id, :event, :created_at, :issue, keyword_init: true) - struct.new(id: 1, event: 'closed', created_at: '2022-04-26 18:30:53 UTC') + struct.new(id: 1, event: event_name, created_at: '2022-04-26 18:30:53 UTC') end + let(:event_name) { 'closed' } + + let(:page_events) { [issue_event] } + let(:page) do instance_double( Gitlab::GithubImport::Client::Page, - number: 1, objects: [issue_event] + number: 1, objects: page_events ) end let(:page_counter) { instance_double(Gitlab::GithubImport::PageCounter) } + let(:extended_events) { true } + before do allow(Gitlab::Redis::SharedState).to receive(:with).and_return('OK') allow(client).to receive(:each_page).once.with(:issue_timeline, project.import_source, issuable.iid, { state: 'all', sort: 'created', direction: 'asc', page: 1 } ).and_yield(page) + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end end context 'with issues' do @@ -190,10 +199,7 @@ end context 'when event is not supported' do - let(:issue_event) do - struct = Struct.new(:id, :event, :created_at, :issue, keyword_init: true) - struct.new(id: 1, event: 'not_supported_event', created_at: '2022-04-26 18:30:53 UTC') - end + let(:event_name) { 'not_supported_event' } it "doesn't process this event" do counter = 0 @@ -201,5 +207,156 @@ expect(counter).to eq 0 end end + + describe 'save events' do + shared_examples 'saves event' do + it 'saves event' do + expect(Gitlab::GithubImport::Representation::IssueEvent).to receive(:from_api_response).with(issue_event.to_h) + .and_call_original + + expect_next_instance_of(Gitlab::GithubImport::EventsCache) do |events_cache| + expect(events_cache).to receive(:add).with( + issuable, + an_instance_of(Gitlab::GithubImport::Representation::IssueEvent) + ) + end + + subject.each_object_to_import { |event| event } + end + end + + context 'when event is review_requested' do + let(:event_name) { 'review_requested' } + + it_behaves_like 'saves event' + end + + context 'when event is review_request_removed' do + let(:event_name) { 'review_request_removed' } + + it_behaves_like 'saves event' + end + + context 'when event is closed' do + let(:event_name) { 'closed' } + + it 'does not save event' do + expect_next_instance_of(Gitlab::GithubImport::EventsCache) do |events_cache| + expect(events_cache).not_to receive(:add) + end + + subject.each_object_to_import { |event| event } + end + end + + context 'when extended_events is disabled' do + let(:event_name) { 'review_requested' } + let(:extended_events) { false } + + it 'does not save event' do + expect(Gitlab::GithubImport::EventsCache).not_to receive(:new) + + subject.each_object_to_import { |event| event } + end + end + end + + describe 'after batch processed' do + context 'when events should be replayed' do + let(:event_name) { 'review_requested' } + + it 'enqueues worker to replay events' do + allow(Gitlab::JobWaiter).to receive(:generate_key).and_return('job_waiter_key') + + expect(Gitlab::GithubImport::ReplayEventsWorker).to receive(:perform_async) + .with( + project.id, + { 'issuable_type' => issuable.class.name.to_s, 'issuable_iid' => issuable.iid }, + 'job_waiter_key' + ) + + subject.each_object_to_import { |event| event } + end + end + + context 'when events are not relevant' do + let(:event_name) { 'closed' } + + it 'does not replay events' do + expect(Gitlab::GithubImport::ReplayEventsWorker).not_to receive(:perform_async) + + subject.each_object_to_import { |event| event } + end + end + + context 'when extended_events is disabled' do + let(:extended_events) { false } + + it 'does not replay events' do + expect(Gitlab::GithubImport::ReplayEventsWorker).not_to receive(:perform_async) + + subject.each_object_to_import { |event| event } + end + end + end + end + + describe '#execute', :clean_gitlab_redis_cache do + let(:extended_events) { false } + + before do + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end + + stub_request(:get, 'https://api.github.com/rate_limit') + .to_return(status: 200, headers: { 'X-RateLimit-Limit' => 5000, 'X-RateLimit-Remaining' => 5000 }) + + events = [ + { + id: 1, + event: 'review_requested', + created_at: '2022-04-26 18:30:53 UTC', + issue: { + number: issuable.iid, + pull_request: true + } + } + ] + + endpoint = 'https://api.github.com/repos/foo/bar/issues/1/timeline' \ + '?direction=asc&page=1&per_page=100&sort=created&state=all' + + stub_request(:get, endpoint) + .to_return(status: 200, body: events.to_json, headers: { 'Content-Type' => 'application/json' }) + end + + context 'when extended_events is disabled' do + it 'enqueues importer worker' do + expect { subject.execute }.to change { Gitlab::GithubImport::ReplayEventsWorker.jobs.size }.by(0) + .and change { Gitlab::GithubImport::ImportIssueEventWorker.jobs.size }.by(1) + end + + it 'returns job waiter with the correct remaining jobs count' do + job_waiter = subject.execute + + expect(job_waiter.jobs_remaining).to eq(1) + end + end + + context 'when extended_events is enabled' do + let(:extended_events) { true } + + it 'enqueues importer worker and replay worker' do + expect { subject.execute }.to change { Gitlab::GithubImport::ReplayEventsWorker.jobs.size }.by(1) + .and change { Gitlab::GithubImport::ImportIssueEventWorker.jobs.size }.by(1) + end + + it 'returns job waiter with the correct remaining jobs count' do + job_waiter = subject.execute + + expect(job_waiter.jobs_remaining).to eq(2) + end + end end end diff --git a/spec/lib/gitlab/github_import/representation/issue_event_spec.rb b/spec/lib/gitlab/github_import/representation/issue_event_spec.rb index 6620dee0fd0203..de0509c3e5ee6b 100644 --- a/spec/lib/gitlab/github_import/representation/issue_event_spec.rb +++ b/spec/lib/gitlab/github_import/representation/issue_event_spec.rb @@ -168,8 +168,8 @@ describe '.from_api_response' do let(:response) do event_resource = Struct.new( - :id, :node_id, :url, :actor, :event, :commit_id, :commit_url, :label, :rename, :milestone, - :source, :assignee, :requested_reviewer, :review_requester, :issue, :created_at, + :id, :node_id, :url, :actor, :event, :commit_id, :commit_url, :label, :rename, :milestone, :state, :body, + :source, :assignee, :requested_reviewer, :review_requester, :issue, :created_at, :updated_at, :submitted_at, :performed_via_github_app, keyword_init: true ) diff --git a/spec/lib/gitlab/github_import/representation/replay_event_spec.rb b/spec/lib/gitlab/github_import/representation/replay_event_spec.rb new file mode 100644 index 00000000000000..1afefb76c6ac19 --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/replay_event_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::GithubImport::Representation::ReplayEvent, feature_category: :importers do + describe '.from_json_hash' do + it 'returns an instance of ReplayEvent' do + representation = described_class.from_json_hash(issuable_iid: 1, issuable_type: 'MergeRequest') + + expect(representation).to be_an_instance_of(described_class) + end + end + + describe '#github_identifiers' do + it 'returns a hash with needed identifiers' do + representation = described_class.new(issuable_type: 'MergeRequest', issuable_iid: 1) + + expect(representation.github_identifiers).to eq({ + issuable_type: 'MergeRequest', + issuable_iid: 1 + }) + end + end +end diff --git a/spec/lib/gitlab/github_import/settings_spec.rb b/spec/lib/gitlab/github_import/settings_spec.rb index ea1526ca25f5fb..d268f3a8650950 100644 --- a/spec/lib/gitlab/github_import/settings_spec.rb +++ b/spec/lib/gitlab/github_import/settings_spec.rb @@ -20,12 +20,6 @@ let(:expected_list) do stages = described_class::OPTIONAL_STAGES [ - { - name: 'single_endpoint_issue_events_import', - label: stages[:single_endpoint_issue_events_import][:label], - selected: false, - details: stages[:single_endpoint_issue_events_import][:details] - }, { name: 'single_endpoint_notes_import', label: stages[:single_endpoint_notes_import][:label], @@ -48,7 +42,31 @@ end it 'returns stages list as array' do - expect(described_class.stages_array).to match_array(expected_list) + expect(described_class.stages_array(project.owner)).to match_array(expected_list) + end + + context 'when `github_import_extended_events` feature flag is disabled' do + let(:expected_list_with_deprecated_options) do + stages = described_class::OPTIONAL_STAGES + + expected_list.concat( + [ + { + name: 'single_endpoint_issue_events_import', + label: stages[:single_endpoint_issue_events_import][:label], + selected: false, + details: stages[:single_endpoint_issue_events_import][:details] + } + ]) + end + + before do + stub_feature_flags(github_import_extended_events: false) + end + + it 'returns stages list as array' do + expect(described_class.stages_array(project.owner)).to match_array(expected_list_with_deprecated_options) + end end end @@ -99,4 +117,24 @@ expect(settings.disabled?(:collaborators_import)).to eq true end end + + describe '#extended_events?' do + it 'when extended_events is set to true' do + project.build_or_assign_import_data(data: { extended_events: true }) + + expect(settings.extended_events?).to eq(true) + end + + it 'when extended_events is set to false' do + project.build_or_assign_import_data(data: { extended_events: false }) + + expect(settings.extended_events?).to eq(false) + end + + it 'when extended_events is not present' do + project.build_or_assign_import_data(data: {}) + + expect(settings.extended_events?).to eq(false) + end + end end diff --git a/spec/services/import/github_service_spec.rb b/spec/services/import/github_service_spec.rb index fc649b61426d2b..06ce00260edcdd 100644 --- a/spec/services/import/github_service_spec.rb +++ b/spec/services/import/github_service_spec.rb @@ -31,6 +31,7 @@ allow(settings) .to receive(:write) .with( + extended_events: true, optional_stages: optional_stages, timeout_strategy: timeout_strategy ) @@ -92,6 +93,7 @@ expect(settings) .to have_received(:write) .with(optional_stages: nil, + extended_events: true, timeout_strategy: timeout_strategy ) expect_snowplow_event( @@ -117,6 +119,7 @@ .to have_received(:write) .with( optional_stages: nil, + extended_events: true, timeout_strategy: timeout_strategy ) expect_snowplow_event( @@ -149,6 +152,7 @@ .to have_received(:write) .with( optional_stages: nil, + extended_events: true, timeout_strategy: timeout_strategy ) expect_snowplow_event( @@ -185,6 +189,7 @@ .to have_received(:write) .with( optional_stages: optional_stages, + extended_events: true, timeout_strategy: timeout_strategy ) end @@ -200,6 +205,7 @@ .to have_received(:write) .with( optional_stages: optional_stages, + extended_events: true, timeout_strategy: timeout_strategy ) end @@ -213,10 +219,25 @@ .to have_received(:write) .with( optional_stages: optional_stages, + extended_events: true, timeout_strategy: timeout_strategy ) end end + + context 'when `github_import_extended_events`` feature flag is disabled' do + before do + stub_feature_flags(github_import_extended_events: false) + end + + it 'saves extend_events to import_data' do + expect(settings) + .to receive(:write) + .with(a_hash_including(extended_events: false)) + + subject.execute(access_params, :github) + end + end end context 'when import source is disabled' do diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index c60e8d37c2e6db..4f916f93b8b071 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -280,6 +280,7 @@ 'Gitlab::GithubImport::PullRequests::ImportMergedByWorker' => 5, 'Gitlab::GithubImport::ImportPullRequestWorker' => 5, 'Gitlab::GithubImport::RefreshImportJidWorker' => 5, + 'Gitlab::GithubImport::ReplayEventsWorker' => 5, 'Gitlab::GithubImport::Stage::FinishImportWorker' => 6, 'Gitlab::GithubImport::Stage::ImportBaseDataWorker' => 6, 'Gitlab::GithubImport::Stage::ImportIssuesAndDiffNotesWorker' => 6, diff --git a/spec/workers/gitlab/github_import/replay_events_worker_spec.rb b/spec/workers/gitlab/github_import/replay_events_worker_spec.rb new file mode 100644 index 00000000000000..270404c3ae9f3a --- /dev/null +++ b/spec/workers/gitlab/github_import/replay_events_worker_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::ReplayEventsWorker, feature_category: :importers do + let_it_be(:project) { create(:project, import_state: create(:import_state, :started)) } + let(:client) { instance_double(Gitlab::GithubImport::Client) } + + let(:worker) { described_class.new } + + describe '#import' do + it 'call replay events importer' do + hash = { + 'issuable_iid' => 1, + 'issuable_type' => 'Issue' + } + + expect_next_instance_of(Gitlab::GithubImport::Importer::ReplayEventsImporter, + an_instance_of(Gitlab::GithubImport::Representation::ReplayEvent), project, client) do |importer| + expect(importer).to receive(:execute) + end + + expect(Gitlab::GithubImport::ObjectCounter).not_to receive(:increment) + + worker.import(project, client, hash) + end + end +end diff --git a/spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb index bad3a5beb0e877..001c0eff7dc676 100644 --- a/spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb @@ -9,9 +9,12 @@ let!(:group) { create(:group, projects: [project]) } let(:settings) { ::Gitlab::GithubImport::Settings.new(project) } let(:stage_enabled) { true } + let(:extended_events) { false } before do - settings.write({ optional_stages: { single_endpoint_issue_events_import: stage_enabled } }) + settings.write({ + optional_stages: { single_endpoint_issue_events_import: stage_enabled }, extended_events: extended_events + }) end it_behaves_like Gitlab::GithubImport::StageMethods @@ -48,6 +51,18 @@ worker.import(client, project) end + + context 'when extended_events is enabled' do + let(:extended_events) { true } + + it 'does not skip the stage' do + expect_next_instance_of(Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter) do |importer| + expect(importer).to receive(:execute).and_return(Gitlab::JobWaiter.new) + end + + worker.import(client, project) + end + end end end end -- GitLab From 3e3e9a6ebaf831af692b8a4b8d04be053f9e2c42 Mon Sep 17 00:00:00 2001 From: Rodrigo Tomonari Date: Tue, 19 Dec 2023 16:16:22 -0300 Subject: [PATCH 2/2] Add missing spec --- .../workers/gitlab/github_import/replay_events_worker_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/workers/gitlab/github_import/replay_events_worker_spec.rb b/spec/workers/gitlab/github_import/replay_events_worker_spec.rb index 270404c3ae9f3a..99c9e838bbf1a1 100644 --- a/spec/workers/gitlab/github_import/replay_events_worker_spec.rb +++ b/spec/workers/gitlab/github_import/replay_events_worker_spec.rb @@ -25,4 +25,8 @@ worker.import(project, client, hash) end end + + describe '#object_type' do + it { expect(worker.object_type).to eq(:replay_event) } + end end -- GitLab