diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index fac358d65435bdb3ef4f90cd74e46850bcd8b0cf..15662272316016750333ddf2ec53304efe7872c6 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 a96bfd74cd0d68fd79d717ca1971a76c5e1f7c26..e28c58794a954a35ce928e945da75fdc330fafd1 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 97acafe24d05f05b841b643a0616d7a4aaea0d87..0f56ae92557eab5a66fc424d67dd6a3fe48c5a68 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 ec5156bb1d0de3f88978d50ede0b769928f12d98..e4329ae2a8bcffe26e64bc81fcdbb35ab3bab7e5 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 417b85985479e366e7dbda17d2d5873bdead4caf..805252927ff82a2c3bb3a1e24d66fadae4d553a8 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 0000000000000000000000000000000000000000..680d5ec2d7d47598b63231bb8d32040e656bd7e2 --- /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 b5b1601e3ed01c1c9f6796aa44e5fade4f5d71cd..38e1fd52889c7b372e0f5ea0bc43550806cba5b2 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 27d14a1a10832461e4eaeb1584a1c6b1cf3430f2..9618500604ab78273ffb0d1f0a2211fa953dc1be 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 0000000000000000000000000000000000000000..e4466bf958fc550ff873ba2ceb5fa581b54cd545 --- /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 e81a90831f75a1a871da2586dae47d6b7d6f54ec..f3251d47e7a3836e77d5d3c5cd5a7b403a645fa6 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 0000000000000000000000000000000000000000..0986ccfaed1aac0d173772e2a23e82f2abb48caa --- /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 8218acf2bfb8bf6466992c89b990ea4183fde24e..1ebafec5afc0e9f465c00e650bae96683c81518f 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 0000000000000000000000000000000000000000..c9ebc31fa0619d565609307ddf08ad250ab59931 --- /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 6189fa8f429462cc087625d666a65d31b2fce3f1..702ea7f1fd53906e4724e3fc49c22d6a702a7712 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 0000000000000000000000000000000000000000..1c0e8a9e6e828878f0a6fe34f2dc9b97b5bf8bc0 --- /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 d20482eca6f8494397abb6944cff69306db3b673..2068f789fec6a667619dd2ca459339799b59e171 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 6df130eb6e8af24e8fa5641468c8b25b8a4027fe..384880651ef936a28651ba32373cd4b2848f1ec2 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 0000000000000000000000000000000000000000..bcdaa4d3ce5b96ffc752d767e82fae736a6147a2 --- /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 d7fa098a775738b3fd443d3c9c8404062d0c0383..20fff73ac3d7b3d309b029558f55418763bbd48f 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 30608112f85aa650c57921a59763bb76387fc6bd..fc3bc5a48ef7bd205fa2c82726ad912acca0cd78 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 0000000000000000000000000000000000000000..2d71c26abbbf4f0e4c66c683f233000413ab1458 --- /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 3947ae3c63d0ea318f9aa9b5148f00a35558839c..da5833df3a1d13463a544ca08cedac22a2bc46a8 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 3584288da573106b1a69a5acb6b74c4d1687d772..ed8dda8ebd2773beaa558af3a9ddcacfc0a50f50 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 8f1c552e0b744f974280b0df9fc858c0b4fb8285..6cde51b668a9cc6a823b146d0ae6f6346ce45397 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 0000000000000000000000000000000000000000..8637f2369773f2cbd0fa80b2cb2849c4e44209dc --- /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 0000000000000000000000000000000000000000..bd3bea8768898329269e976c36940d0e35088fa5 --- /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 4ea62557dd68591e9b60f09f5484c0d40056af95..30bc8aabe12526b1dfcc581d80694139fbcc8513 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 0000000000000000000000000000000000000000..f60a9d6526959252fd49b0e6d3bca355f1e6ac40 --- /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 2389489e86714eaa5880dcedcafbce482cd3aead..ffe6c237506dfc4b708d077cdea29585ba958c95 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 f7ee6fee6dc926858a53ff65dc0873ed47748972..174cc72071703f9fe75ea98cf7e08e19570c212d 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 6846c99fb63faab32c79290ab75d0600dd0adec3..1651774b5cef65f37019d39f2cc715e5f1a80793 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 0000000000000000000000000000000000000000..a2233ffe8ccf713c52043d2dd11935a708a13c10 --- /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 91f89f0779c0e11697bda9c439367a032e7b1835..ab51ecf7d61a1dd9bd427c81394b9b17023b9de2 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 6620dee0fd0203abc85ed13e1ec6994f0465e08d..de0509c3e5ee6b6edb1eabb91e95973303906cbc 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 0000000000000000000000000000000000000000..1afefb76c6ac19bfe6e10982250fa77749dd9d94 --- /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 ea1526ca25f5fb2ac3adc667b0f299ddb49ca23a..d268f3a865095050ead77b409cfac4903deb55b0 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 fc649b61426d2b6069311fe6a58f59fb869ab1fc..06ce00260edcddbb97396e1b6ee6772d259a9c96 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 c60e8d37c2e6db93ee574062d6d951e23892effa..4f916f93b8b0717ca5fa817e040191575006933f 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 0000000000000000000000000000000000000000..99c9e838bbf1a1112164541cd658ac10cf1395eb --- /dev/null +++ b/spec/workers/gitlab/github_import/replay_events_worker_spec.rb @@ -0,0 +1,32 @@ +# 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 + + describe '#object_type' do + it { expect(worker.object_type).to eq(:replay_event) } + 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 bad3a5beb0e87798cf08852bcede2953a5b3fd8c..001c0eff7dc676fd7d7ef2eaa1299aeb661abe9e 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