diff --git a/.rubocop_todo/lint/empty_block.yml b/.rubocop_todo/lint/empty_block.yml index bc538062538fcc4ee258a730d79018653a1767ca..cfe9e1b6f8e5c87f74c7567609409b60bab7f961 100644 --- a/.rubocop_todo/lint/empty_block.yml +++ b/.rubocop_todo/lint/empty_block.yml @@ -120,8 +120,6 @@ Lint/EmptyBlock: - 'spec/lib/gitlab/github_import/importer/protected_branches_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb' - 'spec/lib/gitlab/gpg_spec.rb' - 'spec/lib/gitlab/graphql/negatable_arguments_spec.rb' - 'spec/lib/gitlab/http_io_spec.rb' diff --git a/.rubocop_todo/lint/unused_method_argument.yml b/.rubocop_todo/lint/unused_method_argument.yml index 67f62f691ef8283f24be952a2a8b017c6751b1a3..6275557462f4684322e1a90c9182cd6bbbe64250 100644 --- a/.rubocop_todo/lint/unused_method_argument.yml +++ b/.rubocop_todo/lint/unused_method_argument.yml @@ -442,7 +442,6 @@ Lint/UnusedMethodArgument: - 'lib/gitlab/gitaly_client/commit_service.rb' - 'lib/gitlab/gitaly_client/operation_service.rb' - 'lib/gitlab/github_gists_import/representation/gist.rb' - - 'lib/gitlab/github_import/importer/pull_requests/review_requests_importer.rb' - 'lib/gitlab/github_import/representation/diff_note.rb' - 'lib/gitlab/github_import/representation/issue_event.rb' - 'lib/gitlab/github_import/representation/lfs_object.rb' diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index aeed39b8464c822e2cbf68cfb13b5d08c7fc451e..c5ade40fc91682cf4bf4fca5066132501ca6fd18 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -3409,8 +3409,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/github_import/importer/repository_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb' - 'spec/lib/gitlab/github_import/label_finder_spec.rb' - 'spec/lib/gitlab/github_import/logger_spec.rb' - 'spec/lib/gitlab/github_import/markdown_text_spec.rb' diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml index f7a02f23d61af6a87e3c01d3e3e93d76c359847b..514871a5db49919ec928bb4617d6c0409e08608b 100644 --- a/.rubocop_todo/rspec/named_subject.yml +++ b/.rubocop_todo/rspec/named_subject.yml @@ -2174,14 +2174,10 @@ RSpec/NamedSubject: - 'spec/lib/gitlab/github_gists_import/importer/gist_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/protected_branches_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/pull_requests/all_merged_by_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/pull_requests/merged_by_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/pull_requests/reviews_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb' - 'spec/lib/gitlab/github_import/representation/note_text_spec.rb' - 'spec/lib/gitlab/gl_repository/repo_type_spec.rb' - 'spec/lib/gitlab/grape_logging/loggers/cloudflare_logger_spec.rb' diff --git a/.rubocop_todo/rspec/verified_doubles.yml b/.rubocop_todo/rspec/verified_doubles.yml index 7639f7eb031516490d8c72e6ee1838737bce45b3..86cb0113beeabe857abc6d470e5c34a46d7ec63f 100644 --- a/.rubocop_todo/rspec/verified_doubles.yml +++ b/.rubocop_todo/rspec/verified_doubles.yml @@ -526,8 +526,6 @@ RSpec/VerifiedDoubles: - 'spec/lib/gitlab/github_import/importer/releases_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/repository_importer_spec.rb' - 'spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb' - - 'spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb' - 'spec/lib/gitlab/github_import/issuable_finder_spec.rb' - 'spec/lib/gitlab/github_import/markdown_text_spec.rb' - 'spec/lib/gitlab/github_import/milestone_finder_spec.rb' @@ -970,9 +968,6 @@ RSpec/VerifiedDoubles: - 'spec/workers/gitlab/github_import/stage/finish_import_worker_spec.rb' - 'spec/workers/gitlab/github_import/stage/import_base_data_worker_spec.rb' - 'spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb' - - 'spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb' - - 'spec/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker_spec.rb' - - 'spec/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker_spec.rb' - 'spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb' - 'spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb' - 'spec/workers/gitlab_performance_bar_stats_worker_spec.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index 18d0dbf561775f230f97b281772905ed0228bf2c..d8f4e088c3ada58730561bdd915e76965cc3eab1 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -2427,13 +2427,10 @@ Style/InlineDisableAnnotation: - 'lib/gitlab/github_import/importer/labels_importer.rb' - 'lib/gitlab/github_import/importer/milestones_importer.rb' - 'lib/gitlab/github_import/importer/note_importer.rb' - - 'lib/gitlab/github_import/importer/pull_requests/review_requests_importer.rb' - 'lib/gitlab/github_import/importer/releases_importer.rb' - 'lib/gitlab/github_import/importer/repository_importer.rb' - 'lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer.rb' - 'lib/gitlab/github_import/importer/single_endpoint_issue_events_importer.rb' - - 'lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer.rb' - - 'lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer.rb' - 'lib/gitlab/github_import/label_finder.rb' - 'lib/gitlab/github_import/markdown_text.rb' - 'lib/gitlab/github_import/milestone_finder.rb' diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index 6d30ab50a15f3e7e083b7f75325950345a869fb0..8c613435e85be233129528278fddbd416135d791 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -173,8 +173,7 @@ def store_import_settings(project) .new(project) .write( timeout_strategy: params[:timeout_strategy] || ProjectImportData::PESSIMISTIC_TIMEOUT, - optional_stages: params[:optional_stages], - extended_events: Feature.enabled?(:github_import_extended_events, current_user) + optional_stages: params[:optional_stages] ) end end diff --git a/app/workers/gitlab/github_import/advance_stage_worker.rb b/app/workers/gitlab/github_import/advance_stage_worker.rb index 82b3303e28cf81eee18f55868e9b8cda6b45b9d4..ca47de2bebb5f03ab42a0c330699ac5c79015721 100644 --- a/app/workers/gitlab/github_import/advance_stage_worker.rb +++ b/app/workers/gitlab/github_import/advance_stage_worker.rb @@ -25,12 +25,8 @@ class AdvanceStageWorker # rubocop:disable Scalability/IdempotentWorker base_data: Stage::ImportBaseDataWorker, pull_requests: Stage::ImportPullRequestsWorker, collaborators: Stage::ImportCollaboratorsWorker, - 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, # Skipped on extended_events attachments: Stage::ImportAttachmentsWorker, protected_branches: Stage::ImportProtectedBranchesWorker, lfs_objects: Stage::ImportLfsObjectsWorker, diff --git a/app/workers/gitlab/github_import/import_issue_event_worker.rb b/app/workers/gitlab/github_import/import_issue_event_worker.rb index f5e88787a776c506ddc290aff63ff7bd8ce6dd01..375012d576390b7e57d60f75e5293260124e0aa9 100644 --- a/app/workers/gitlab/github_import/import_issue_event_worker.rb +++ b/app/workers/gitlab/github_import/import_issue_event_worker.rb @@ -18,8 +18,7 @@ def object_type end def increment_object_counter(object, project) - counter_type = importer_class::EVENT_COUNTER_MAP[object[:event]] if import_settings.extended_events? - counter_type ||= object_type + counter_type = importer_class::EVENT_COUNTER_MAP[object[:event]] || object_type Gitlab::GithubImport::ObjectCounter.increment(project, counter_type, :imported) 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 38e1fd52889c7b372e0f5ea0bc43550806cba5b2..b78d998db0171ba34401f7f9d71da639ee868aef 100644 --- a/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_collaborators_worker.rb @@ -42,15 +42,9 @@ def skip_to_next_stage(project) def move_to_next_stage(project, waiters = {}) AdvanceStageWorker.perform_async( - project.id, waiters.deep_stringify_keys, next_stage(project) + project.id, waiters.deep_stringify_keys, 'issues_and_diff_notes' ) 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 9618500604ab78273ffb0d1f0a2211fa953dc1be..a91325db31bcfe481c401e10407229326fc49adf 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,36 +15,11 @@ 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 skip_to_next_stage?(project) - importer = ::Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter info(project.id, message: "starting importer", importer: importer.name) waiter = importer.new(project, client).execute - move_to_next_stage(project, { waiter.key => waiter.jobs_remaining }) - end - - 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, next_stage(project)) - end - - def next_stage(project) - return "attachments" if import_settings(project).extended_events? - "notes" + AdvanceStageWorker.perform_async(project.id, { waiter.key => waiter.jobs_remaining }, 'attachments') end end end diff --git a/app/workers/gitlab/github_import/stage/import_notes_worker.rb b/app/workers/gitlab/github_import/stage/import_notes_worker.rb index 8aea27a94d45f77558d6e01e72234803afd4d8ae..3d065e3e8f1ed3e6375cd7df838f7b3596fd1edd 100644 --- a/app/workers/gitlab/github_import/stage/import_notes_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_notes_worker.rb @@ -10,32 +10,7 @@ class ImportNotesWorker # rubocop:disable Scalability/IdempotentWorker include StageMethods - resumes_work_when_interrupted! - - # client - An instance of Gitlab::GithubImport::Client. - # project - An instance of Project. - def import(client, project) - waiters = importers(project).each_with_object({}) do |klass, hash| - info(project.id, message: "starting importer", importer: klass.name) - waiter = klass.new(project, client).execute - hash[waiter.key] = waiter.jobs_remaining - end - - AdvanceStageWorker.perform_async(project.id, waiters.deep_stringify_keys, 'attachments') - end - - def importers(project) - if import_settings(project).enabled?(:single_endpoint_notes_import) - [ - Importer::SingleEndpointMergeRequestNotesImporter, - Importer::SingleEndpointIssueNotesImporter - ] - else - [ - Importer::NotesImporter - ] - end - end + def perform(_project_id); end end end end diff --git a/app/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker.rb b/app/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker.rb index 20b2e5ed6af957b3478ba7148558efbd75f537a0..9567bc311cfa3d6df0991cd4fa66ceda4afd5c93 100644 --- a/app/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker.rb @@ -10,21 +10,7 @@ class ImportPullRequestsMergedByWorker # rubocop:disable Scalability/IdempotentW include StageMethods - resumes_work_when_interrupted! - - # client - An instance of Gitlab::GithubImport::Client. - # project - An instance of Project. - def import(client, project) - waiter = Importer::PullRequests::AllMergedByImporter - .new(project, client) - .execute - - AdvanceStageWorker.perform_async( - project.id, - { waiter.key => waiter.jobs_remaining }, - 'pull_request_review_requests' - ) - end + def perform(_project_id); end end end end diff --git a/app/workers/gitlab/github_import/stage/import_pull_requests_review_requests_worker.rb b/app/workers/gitlab/github_import/stage/import_pull_requests_review_requests_worker.rb index 1262fc23c6c468d96105b8a1b930ba8fd9760a14..8dce1518325e480611c07abdd26da02f2b3532fe 100644 --- a/app/workers/gitlab/github_import/stage/import_pull_requests_review_requests_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_pull_requests_review_requests_worker.rb @@ -10,21 +10,7 @@ class ImportPullRequestsReviewRequestsWorker # rubocop:disable Scalability/Idemp include StageMethods - resumes_work_when_interrupted! - - # client - An instance of Gitlab::GithubImport::Client. - # project - An instance of Project. - def import(client, project) - waiter = Importer::PullRequests::ReviewRequestsImporter - .new(project, client) - .execute - - AdvanceStageWorker.perform_async( - project.id, - { waiter.key => waiter.jobs_remaining }, - 'pull_request_reviews' - ) - end + def perform(project_id); end end end end diff --git a/app/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker.rb b/app/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker.rb index bb4699889dad42040979200122c66442b437672a..742c2475c815d4aa0d820d6cb16b43dbf0ee1930 100644 --- a/app/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker.rb +++ b/app/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker.rb @@ -10,21 +10,7 @@ class ImportPullRequestsReviewsWorker # rubocop:disable Scalability/IdempotentWo include StageMethods - resumes_work_when_interrupted! - - # client - An instance of Gitlab::GithubImport::Client. - # project - An instance of Project. - def import(client, project) - waiter = Importer::PullRequests::ReviewsImporter - .new(project, client) - .execute - - AdvanceStageWorker.perform_async( - project.id, - { waiter.key => waiter.jobs_remaining }, - 'issues_and_diff_notes' - ) - end + def perform(_project_id); end 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 deleted file mode 100644 index fce879f91c5e7b9582c507e20ed33b2431bb9b0a..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/github_import_extended_events.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -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: true diff --git a/doc/api/import.md b/doc/api/import.md index 261340e6924ec6cd3a60f8cc52c57c0f9ead2fe1..62e2825b34dc52995fe36174b55468d7f1ac974e 100644 --- a/doc/api/import.md +++ b/doc/api/import.md @@ -19,6 +19,7 @@ Use the Import API to import repositories from GitHub or Bitbucket Server. > - `collaborators_import` key in `optional_stages` was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/398154) in GitLab 16.0. > - Feature flag `github_import_extended_events` was introduced in GitLab 16.8. Disabled by default. This flag improves the performance of imports but disables the `single_endpoint_issue_events_import` optional stage. > - Feature flag `github_import_extended_events` was [enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/435089) in GitLab 16.9. +> - Improved import performance made [generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/435089) in GitLab 16.11. Feature flag `github_import_extended_events` removed. Import your projects from GitHub to GitLab using the API. @@ -54,7 +55,6 @@ curl --request POST \ "new_name": "NEW-NAME", "github_hostname": "https://github.example.com", "optional_stages": { - "single_endpoint_issue_events_import": true, "single_endpoint_notes_import": true, "attachments_import": true, "collaborators_import": true @@ -64,8 +64,7 @@ curl --request POST \ The following keys are available for `optional_stages`: -- `single_endpoint_issue_events_import`, for issue and pull request events import. If the `github_import_extended_events` feature flag is enabled, this optional stage - is unavailable. +- `single_endpoint_issue_events_import`, for issue and pull request events import. This optional stage was removed in GitLab 16.9. - `single_endpoint_notes_import`, for an alternative and more thorough comments import. - `attachments_import`, for Markdown attachments import. - `collaborators_import`, for importing direct repository collaborators who are not outside collaborators. diff --git a/doc/development/github_importer.md b/doc/development/github_importer.md index 0db36674ab822d451111ddfe30045e04c317f015..7024c48359b901a2032f5f44585c8bda9a876a73 100644 --- a/doc/development/github_importer.md +++ b/doc/development/github_importer.md @@ -95,43 +95,7 @@ For every collaborator, we schedule a job for the `Gitlab::GithubImport::ImportC NOTE: This stage is optional (controlled by `Gitlab::GithubImport::Settings`) and is selected by default. -### 6. Stage::ImportPullRequestsMergedByWorker (deprecated) - -This worker imports the pull requests' _merged-by_ user information. The -[_List pull requests_](https://docs.github.com/en/rest/pulls#list-pull-requests) -API doesn't provide this information. Therefore, this stage must fetch each merged pull request -individually to import this information. A -`Gitlab::GithubImport::PullRequests::ImportMergedByWorker` job is scheduled for each fetched pull -request. - -NOTE: -This stage is skipped when `github_import_extended_events` feature flag is enabled as pull requests merged by information -is imported in the `10. Stage::ImportIssueEventsWorker` stage. This stage will be removed along with the feature flag. - -### 7. Stage::ImportPullRequestsReviewRequestsWorker (deprecated) - -This worker imports assigned reviewers of pull requests. For each pull request, this worker: - -- Fetches all assigned review requests. -- Schedules a `Gitlab::GithubImport::PullRequests::ImportReviewRequestWorker` job for each fetched review request. - -NOTE: -This stage is skipped when `github_import_extended_events` feature flag is enabled as pull requests review requests -information is imported in the `10. Stage::ImportIssueEventsWorker` stage. This stage will be removed along with the -feature flag. - -### 8. Stage::ImportPullRequestsReviewsWorker (deprecated) - -This worker imports reviews of pull requests. For each pull request, this worker: - -- Fetches all the pages of reviews. -- Schedules a `Gitlab::GithubImport::PullRequests::ImportReviewWorker` job for each fetched review. - -NOTE: -This stage is skipped when `github_import_extended_events` feature flag is enabled as pull requests reviews information -is imported in the`10. Stage::ImportIssueEventsWorker` stage. This stage will be removed along with the feature flag. - -### 9. Stage::ImportIssuesAndDiffNotesWorker +### 6. Stage::ImportIssuesAndDiffNotesWorker This worker imports all issues and pull request comments. For every issue, we schedule a job for the `Gitlab::GithubImport::ImportIssueWorker` worker. For @@ -147,7 +111,7 @@ label links in the same worker removes the need for performing a separate crawl through the API data, reducing the number of API calls necessary to import a project. -### 10. Stage::ImportIssueEventsWorker +### 7. Stage::ImportIssueEventsWorker This worker imports all issues and pull request events. For every event, we schedule a job for the `Gitlab::GithubImport::ImportIssueEventWorker` worker. @@ -160,36 +124,12 @@ GitHub are stored in a single table. Therefore, they have globally-unique IDs an Therefore, both issues and pull requests have a common API for most related things. -When the feature flag `github_import_extended_events` is enabled, this stage is used to import -`pull request merged by`, `pull request reviews`, `pull request review requests` and `notes`. This is possible because -[timeline events endpoint](https://docs.github.com/en/rest/issues/timeline?apiVersion=2022-11-28#list-timeline-events-for-an-issue) -also contains such information. - To facilitate the import of `pull request review requests` using the timeline events endpoint, events must be processed sequentially. Given that import workers do not execute in a guaranteed order, the `pull request review requests` events are initially placed in a Redis ordered list. Subsequently, they are consumed in sequence by the `Gitlab::GithubImport::ReplayEventsWorker`. -NOTE: -This stage is mandatory when `github_import_extended_events` feature flag is enabled. Otherwise the stage is optional -and can executed using the [import options](../user/project/import/github.md#select-additional-items-to-import). - -### 11. Stage::ImportNotesWorker (deprecated) - -This worker imports regular comments for both issues and pull requests. For -every comment, we schedule a job for the -`Gitlab::GithubImport::ImportNoteWorker` worker. - -Regular comments have to be imported at the end because the GitHub API used -returns comments for both issues and pull requests. This means we have to wait -for all issues and pull requests to be imported before we can import regular -comments. - -NOTE: -This stage is skipped when `github_import_extended_events` feature flag is enabled as notes are imported in the -`10. Stage::ImportIssueEventsWorker` stage. This stage will be removed along with the feature flag. - -### 12. Stage::ImportAttachmentsWorker +### 8. Stage::ImportAttachmentsWorker This worker imports note attachments that are linked inside Markdown. For each entity with Markdown text in the project, we schedule a job of: @@ -208,7 +148,7 @@ Each job: NOTE: It's an optional stage that could consume significant extra import time (controlled by `Gitlab::GithubImport::Settings`). -### 13. Stage::ImportProtectedBranchesWorker +### 9. Stage::ImportProtectedBranchesWorker This worker imports protected branch rules. For every rule that exists on GitHub, we schedule a job of @@ -217,7 +157,7 @@ For every rule that exists on GitHub, we schedule a job of Each job compares the branch protection rules from GitHub and GitLab and applies the strictest of the rules to the branches in GitLab. -### 14. Stage::FinishImportWorker +### 10. Stage::FinishImportWorker This worker completes the import process by performing some housekeeping (such as flushing any caches) and by marking the import as completed. diff --git a/doc/user/project/import/github.md b/doc/user/project/import/github.md index 788b9143ad860772656f6ffba5531cced76d7984..e3988021b74dc994cc03a3fc567d7b63a9e61ec5 100644 --- a/doc/user/project/import/github.md +++ b/doc/user/project/import/github.md @@ -173,17 +173,16 @@ When the **Organization** tab is selected, you can further narrow down your sear > - Importing collaborators as an additional item was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/398154) in GitLab 16.0. > - Feature flag `github_import_extended_events` was introduced in GitLab 16.8. Disabled by default. This flag improves the performance of imports but removes the **Import issue and pull request events** option. > - Feature flag `github_import_extended_events` was [enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/435089) in GitLab 16.9. +> - Improved import performance made [generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/435089) in GitLab 16.11. Feature flag `github_import_extended_events` removed. To make imports as fast as possible, the following items aren't imported from GitHub by default: -- Issue and pull request events. For example, _opened_ or _closed_, _renamed_, and _labeled_ or _unlabeled_. - More than approximately 30,000 comments because of a [limitation of the GitHub API](#missing-comments). - Markdown attachments from repository comments, release posts, issue descriptions, and pull request descriptions. These can include images, text, or binary attachments. If not imported, links in Markdown to attachments break after you remove the attachments from GitHub. You can choose to import these items, but this could significantly increase import time. To import these items, select the appropriate fields in the UI: -- **Import issue and pull request events**. If the `github_import_extended_events` feature flag is enabled, this option is unavailable. - **Use alternative comments import method**. If importing GitHub projects with more than approximately 30,000 comments across all issues and pull requests, you should enable this method because of a [limitation of the GitHub API](#missing-comments). - **Import Markdown attachments**. diff --git a/lib/gitlab/github_import/importer/events/commented.rb b/lib/gitlab/github_import/importer/events/commented.rb index c9ebc31fa0619d565609307ddf08ad250ab59931..7f06ab99c9c17961d34f730e2b14ec5dc096a5f6 100644 --- a/lib/gitlab/github_import/importer/events/commented.rb +++ b/lib/gitlab/github_import/importer/events/commented.rb @@ -6,8 +6,6 @@ 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, diff --git a/lib/gitlab/github_import/importer/events/merged.rb b/lib/gitlab/github_import/importer/events/merged.rb index 702ea7f1fd53906e4724e3fc49c22d6a702a7712..9a88163d7f93e8dc527d07e2243ea9edad4fbe3c 100644 --- a/lib/gitlab/github_import/importer/events/merged.rb +++ b/lib/gitlab/github_import/importer/events/merged.rb @@ -6,8 +6,7 @@ module Importer module Events class Merged < BaseImporter def execute(issue_event) - create_note(issue_event) if import_settings.extended_events? - + create_note(issue_event) create_event(issue_event) create_state_event(issue_event) end diff --git a/lib/gitlab/github_import/importer/events/reviewed.rb b/lib/gitlab/github_import/importer/events/reviewed.rb index cada0d99686cd11b45ea4abf6323eda4ed2ba47c..6e3972cf95b245d1309d3ad8fc407643b288e26c 100644 --- a/lib/gitlab/github_import/importer/events/reviewed.rb +++ b/lib/gitlab/github_import/importer/events/reviewed.rb @@ -6,8 +6,6 @@ module Importer module Events class Reviewed < BaseImporter def execute(issue_event) - return true unless import_settings.extended_events? - review = Representation::PullRequestReview.from_json_hash( merge_request_iid: issue_event.issuable_id, author: issue_event.actor&.to_hash, diff --git a/lib/gitlab/github_import/importer/issue_event_importer.rb b/lib/gitlab/github_import/importer/issue_event_importer.rb index 9f15e9a25d85ddf183aaa55e1508d30e05a750fd..3cecd17cb0eedf6a3af7ee1dc78fc353b3055c1f 100644 --- a/lib/gitlab/github_import/importer/issue_event_importer.rb +++ b/lib/gitlab/github_import/importer/issue_event_importer.rb @@ -20,9 +20,6 @@ class IssueEventImporter review_requested unassigned unlabeled - ].freeze - - EXTENDED_SUPPORTED_EVENTS = SUPPORTED_EVENTS + %w[ commented reviewed ].freeze diff --git a/lib/gitlab/github_import/importer/issue_events_importer.rb b/lib/gitlab/github_import/importer/issue_events_importer.rb deleted file mode 100644 index a1c706c5d7857fc034fcf9cb66ad6d6f63114374..0000000000000000000000000000000000000000 --- a/lib/gitlab/github_import/importer/issue_events_importer.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module GithubImport - module Importer - class IssueEventsImporter - include ParallelScheduling - - def importer_class - IssueEventImporter - end - - def representation_class - Representation::IssueEvent - end - - def sidekiq_worker_class - ImportIssueEventWorker - end - - def object_type - :issue_event - end - - def collection_method - :repository_issue_events - end - - def id_for_already_imported_cache(event) - event[:id] - end - end - end - end -end diff --git a/lib/gitlab/github_import/importer/pull_requests/all_merged_by_importer.rb b/lib/gitlab/github_import/importer/pull_requests/all_merged_by_importer.rb deleted file mode 100644 index 9aa55fd3eae85a87a5d7bd4f32104d9782e8f457..0000000000000000000000000000000000000000 --- a/lib/gitlab/github_import/importer/pull_requests/all_merged_by_importer.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module GithubImport - module Importer - module PullRequests - class AllMergedByImporter - include ParallelScheduling - - def importer_class - MergedByImporter - end - - def representation_class - Gitlab::GithubImport::Representation::PullRequest - end - - def sidekiq_worker_class - Gitlab::GithubImport::PullRequests::ImportMergedByWorker - end - - def collection_method - :pull_requests_merged_by - end - - def object_type - :pull_request_merged_by - end - - def id_for_already_imported_cache(merge_request) - merge_request.id - end - - def each_object_to_import - merge_requests_to_import.find_each do |merge_request| - Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched) - - pull_request = client.pull_request(project.import_source, merge_request.iid) - yield(pull_request) - - mark_as_imported(merge_request) - end - end - - private - - # Returns only the merge requests that still have merged_by to be imported. - def merge_requests_to_import - project.merge_requests.id_not_in(already_imported_objects).with_state(:merged) - end - - def already_imported_objects - Gitlab::Cache::Import::Caching.values_from_set(already_imported_cache_key) - end - end - end - end - end -end diff --git a/lib/gitlab/github_import/importer/pull_requests/review_requests_importer.rb b/lib/gitlab/github_import/importer/pull_requests/review_requests_importer.rb deleted file mode 100644 index 7f78df615a2d7138c24d5c25fb2054b2e3979943..0000000000000000000000000000000000000000 --- a/lib/gitlab/github_import/importer/pull_requests/review_requests_importer.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module GithubImport - module Importer - module PullRequests - class ReviewRequestsImporter - include ParallelScheduling - - BATCH_SIZE = 100 - - private - - def each_object_to_import(&block) - merge_request_collection.each_batch(of: BATCH_SIZE, column: :iid) do |batch| - batch.each do |merge_request| - repo = project.import_source - - review_requests = client.pull_request_review_requests(repo, merge_request.iid) - review_requests[:merge_request_id] = merge_request.id - review_requests[:merge_request_iid] = merge_request.iid - - Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched) - - yield review_requests - - mark_merge_request_imported(merge_request) - end - end - end - - def importer_class - ReviewRequestImporter - end - - def representation_class - Gitlab::GithubImport::Representation::PullRequests::ReviewRequests - end - - def sidekiq_worker_class - Gitlab::GithubImport::PullRequests::ImportReviewRequestWorker - end - - def collection_method - :pull_request_review_requests - end - - def object_type - :pull_request_review_request - end - - # rubocop:disable CodeReuse/ActiveRecord - def merge_request_collection - project.merge_requests - .where.not(iid: already_imported_merge_requests) - .select(:id, :iid) - end - # rubocop:enable CodeReuse/ActiveRecord - - def merge_request_imported_cache_key - "github-importer/pull_requests/#{collection_method}/already-imported/#{project.id}" - end - - def already_imported_merge_requests - Gitlab::Cache::Import::Caching.values_from_set(merge_request_imported_cache_key) - end - - def mark_merge_request_imported(merge_request) - Gitlab::Cache::Import::Caching.set_add( - merge_request_imported_cache_key, - merge_request.iid - ) - end - end - end - end - end -end diff --git a/lib/gitlab/github_import/importer/pull_requests/reviews_importer.rb b/lib/gitlab/github_import/importer/pull_requests/reviews_importer.rb deleted file mode 100644 index 62c9e6469d74f0bfd09910ec291624d4da285c35..0000000000000000000000000000000000000000 --- a/lib/gitlab/github_import/importer/pull_requests/reviews_importer.rb +++ /dev/null @@ -1,114 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module GithubImport - module Importer - module PullRequests - class ReviewsImporter - include ParallelScheduling - - def initialize(...) - super - - @merge_requests_already_imported_cache_key = - "github-importer/merge_request/already-imported/#{project.id}" - end - - def importer_class - ReviewImporter - end - - def representation_class - Gitlab::GithubImport::Representation::PullRequestReview - end - - def sidekiq_worker_class - Gitlab::GithubImport::PullRequests::ImportReviewWorker - end - - def collection_method - :pull_request_reviews - end - - def object_type - :pull_request_review - end - - def id_for_already_imported_cache(review) - review[:id] - end - - # The worker can be interrupted, by rate limit for instance, - # in different situations. To avoid requesting already imported data, - # if the worker is interrupted: - # - before importing all reviews of a merge request - # The reviews page is cached with the `PageCounter`, by merge request. - # - before importing all merge requests reviews - # Merge requests that had all the reviews imported are cached with - # `mark_merge_request_reviews_imported` - def each_object_to_import(&_block) - each_review_page do |page, merge_request| - page.objects.each do |review| - review = review.to_h - - next if already_imported?(review) - - Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :fetched) - - review[:merge_request_id] = merge_request.id - review[:merge_request_iid] = merge_request.iid - yield(review) - - mark_as_imported(review) - end - end - end - - private - - attr_reader :merge_requests_already_imported_cache_key - - def each_review_page - merge_requests_to_import.find_each do |merge_request| - # The page counter needs to be scoped by merge request to avoid skipping - # pages of reviews from already imported merge requests. - page_counter = Gitlab::Import::PageCounter.new(project, page_counter_id(merge_request)) - repo = project.import_source - options = collection_options.merge(page: page_counter.current) - - client.each_page(collection_method, repo, merge_request.iid, options) do |page| - next unless page_counter.set(page.number) - - yield(page, merge_request) - end - - # Avoid unnecessary Redis cache keys after the work is done. - page_counter.expire! - mark_merge_request_reviews_imported(merge_request) - end - end - - # Returns only the merge requests that still have reviews to be imported. - def merge_requests_to_import - project.merge_requests.id_not_in(already_imported_merge_requests) - end - - def already_imported_merge_requests - Gitlab::Cache::Import::Caching.values_from_set(merge_requests_already_imported_cache_key) - end - - def page_counter_id(merge_request) - "merge_request/#{merge_request.id}/#{collection_method}" - end - - def mark_merge_request_reviews_imported(merge_request) - Gitlab::Cache::Import::Caching.set_add( - merge_requests_already_imported_cache_key, - merge_request.id - ) - end - 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 126a0b8fa4ad0796a5a6745f33b45f399844a78e..a914595ed6e5360e8624eed55c61d3b9fa618a0a 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,7 @@ def each_associated(parent_record, associated) compose_associated_id!(parent_record, associated) - return if already_imported?(associated) || supported_events.exclude?(associated[:event]) + return if already_imported?(associated) || importer_class::SUPPORTED_EVENTS.exclude?(associated[:event]) cache_event(parent_record, associated) @@ -67,8 +67,7 @@ def object_type end def increment_object_counter(event_name) - counter_type = importer_class::EVENT_COUNTER_MAP[event_name] if import_settings.extended_events? - counter_type ||= object_type + counter_type = importer_class::EVENT_COUNTER_MAP[event_name] || object_type Gitlab::GithubImport::ObjectCounter.increment(project, counter_type, :fetched) end @@ -112,8 +111,6 @@ def import_settings end def after_batch_processed(parent) - return unless import_settings.extended_events? - events = events_cache.events(parent) return if events.empty? @@ -124,15 +121,7 @@ def after_batch_processed(parent) 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) diff --git a/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer.rb b/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer.rb deleted file mode 100644 index fe64df45700a0199bb79eb3a276b50cdacdecd92..0000000000000000000000000000000000000000 --- a/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true - -# This importer is used when `github_importer_single_endpoint_notes_import` -# feature flag is on and replaces `IssuesImporter` issue notes import. -# -# It fetches 1 issue's comments at a time using `issue_comments` endpoint, which is -# slower than `NotesImporter` but it makes sure all notes are imported, -# as it can sometimes not be the case for `NotesImporter`, because -# `issues_comments` endpoint it uses can be limited by GitHub API -# to not return all available pages. -module Gitlab - module GithubImport - module Importer - class SingleEndpointIssueNotesImporter - include ParallelScheduling - include SingleEndpointNotesImporting - - def importer_class - NoteImporter - end - - def representation_class - Representation::Note - end - - def sidekiq_worker_class - ImportNoteWorker - end - - def object_type - :note - end - - def collection_method - :issue_comments - end - - private - - def parent_collection - project.issues.where.not(iid: already_imported_parents) # rubocop: disable CodeReuse/ActiveRecord - end - - def page_counter_id(issue) - "issue/#{issue.id}/#{collection_method}" - end - - def parent_imported_cache_key - "github-importer/issue/notes/already-imported/#{project.id}" - end - end - end - end -end diff --git a/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer.rb b/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer.rb deleted file mode 100644 index 3b1991d2b88e590da4ba1ac2c4fc63c6c0609e90..0000000000000000000000000000000000000000 --- a/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true - -# This importer is used when `github_importer_single_endpoint_notes_import` -# feature flag is on and replaces `NotesImporter` MR notes import. -# -# It fetches 1 PR's comments at a time using `issue_comments` endpoint, which is -# slower than `NotesImporter` but it makes sure all notes are imported, -# as it can sometimes not be the case for `NotesImporter`, because -# `issues_comments` endpoint it uses can be limited by GitHub API -# to not return all available pages. -module Gitlab - module GithubImport - module Importer - class SingleEndpointMergeRequestNotesImporter - include ParallelScheduling - include SingleEndpointNotesImporting - - def importer_class - NoteImporter - end - - def representation_class - Representation::Note - end - - def sidekiq_worker_class - ImportNoteWorker - end - - def object_type - :note - end - - def collection_method - :issue_comments - end - - private - - def parent_collection - project.merge_requests.where.not(iid: already_imported_parents) # rubocop: disable CodeReuse/ActiveRecord - end - - def page_counter_id(merge_request) - "merge_request/#{merge_request.id}/#{collection_method}" - end - - def parent_imported_cache_key - "github-importer/merge_request/notes/already-imported/#{project.id}" - end - end - end - end -end diff --git a/lib/gitlab/github_import/settings.rb b/lib/gitlab/github_import/settings.rb index da5833df3a1d13463a544ca08cedac22a2bc46a8..c924b2d8c9516ea6053344f5c72452b79fdcc93e 100644 --- a/lib/gitlab/github_import/settings.rb +++ b/lib/gitlab/github_import/settings.rb @@ -4,14 +4,6 @@ module Gitlab module GithubImport class Settings OPTIONAL_STAGES = { - single_endpoint_issue_events_import: { - label: 'Import issue and pull request events', - selected: false, - details: <<-TEXT.split("\n").map(&:strip).join(' ') - For example, opened or closed, renamed, and labeled or unlabeled. - Time required to import these events depends on how many issues or pull requests your project has. - TEXT - }, single_endpoint_notes_import: { label: 'Use alternative comments import method', selected: false, @@ -38,13 +30,8 @@ class Settings } }.freeze - 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) - + def self.stages_array(_current_user) + OPTIONAL_STAGES.map do |stage_name, data| { name: stage_name.to_s, label: s_(format("GitHubImport|%{text}", text: data[:label])), @@ -66,8 +53,7 @@ def write(user_settings) import_data = project.build_or_assign_import_data( data: { optional_stages: optional_stages, - timeout_strategy: user_settings[:timeout_strategy], - extended_events: user_settings[:extended_events] + timeout_strategy: user_settings[:timeout_strategy] }, credentials: project.import_data&.credentials ) @@ -83,10 +69,6 @@ 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 d4d9bd47e63e560105e22822710842379c5af907..f91f493742add4ddb96f009eff6996d975ad47ee 100644 --- a/lib/gitlab/github_import/single_endpoint_notes_importing.rb +++ b/lib/gitlab/github_import/single_endpoint_notes_importing.rb @@ -2,8 +2,6 @@ # This module is used in: # - SingleEndpointDiffNotesImporter -# - SingleEndpointIssueNotesImporter -# - SingleEndpointMergeRequestNotesImporter # if enabled by Gitlab::GithubImport::Settings # # - SingleEndpointIssueEventsImporter diff --git a/spec/lib/gitlab/github_import/events_cache_spec.rb b/spec/lib/gitlab/github_import/events_cache_spec.rb index 8637f2369773f2cbd0fa80b2cb2849c4e44209dc..ffd7ef66590925861cbc33317df1ed8215d64a26 100644 --- a/spec/lib/gitlab/github_import/events_cache_spec.rb +++ b/spec/lib/gitlab/github_import/events_cache_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::EventsCache, :clean_gitlab_redis_cache, feature_category: :importers do +RSpec.describe Gitlab::GithubImport::EventsCache, :clean_gitlab_redis_shared_state, feature_category: :importers do let(:project) { build_stubbed(:project, id: 1) } let(:issue) { build_stubbed(:issue, iid: 2) } diff --git a/spec/lib/gitlab/github_import/importer/events/commented_spec.rb b/spec/lib/gitlab/github_import/importer/events/commented_spec.rb index bd3bea8768898329269e976c36940d0e35088fa5..dc94df988fb7c9828ce2afa087773813caca385a 100644 --- a/spec/lib/gitlab/github_import/importer/events/commented_spec.rb +++ b/spec/lib/gitlab/github_import/importer/events/commented_spec.rb @@ -23,8 +23,6 @@ ) 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) @@ -32,9 +30,6 @@ 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 @@ -47,14 +42,6 @@ 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 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 30bc8aabe12526b1dfcc581d80694139fbcc8513..ccb323489380a3f4a92bb39c34d68df399044cb7 100644 --- a/spec/lib/gitlab/github_import/importer/events/merged_spec.rb +++ b/spec/lib/gitlab/github_import/importer/events/merged_spec.rb @@ -11,7 +11,6 @@ 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( @@ -33,9 +32,6 @@ 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 @@ -63,6 +59,15 @@ ) end + 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 + context 'when commit ID is present' do let!(:commit) { create(:commit, project: project) } let(:commit_id) { commit.id } @@ -75,27 +80,4 @@ 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 index eb0b4859ae9028ff1ec41d4d76234999321c6738..ae24a7411cabbd784b80a0a535e850feb4a59723 100644 --- a/spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb +++ b/spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb @@ -10,7 +10,6 @@ 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.from_json_hash( @@ -33,9 +32,6 @@ 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 @@ -72,14 +68,4 @@ 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_events_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb deleted file mode 100644 index 6fbb3a89511e7eee5eb10ea4aa377437d93e91a1..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb +++ /dev/null @@ -1,120 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Importer::IssueEventsImporter, feature_category: :importers do - subject(:importer) { described_class.new(project, client, parallel: parallel) } - - let(:project) { build(:project, id: 4, import_source: 'foo/bar') } - let(:client) { instance_double(Gitlab::GithubImport::Client) } - - let(:parallel) { true } - let(:issue_event) do - struct = Struct.new( - :id, :node_id, :url, :actor, :user, :event, :commit_id, :commit_url, :label, :rename, :milestone, :source, - :assignee, :assigner, :review_requester, :requested_reviewer, :issue, :created_at, :performed_via_github_app, - :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 - - describe '#parallel?' do - context 'when running in parallel mode' do - it { expect(importer).to be_parallel } - end - - context 'when running in sequential mode' do - let(:parallel) { false } - - it { expect(importer).not_to be_parallel } - end - end - - describe '#execute' do - context 'when running in parallel mode' do - it 'imports events in parallel' do - expect(importer).to receive(:parallel_import) - - importer.execute - end - end - - context 'when running in sequential mode' do - let(:parallel) { false } - - it 'imports notes in sequence' do - expect(importer).to receive(:sequential_import) - - importer.execute - end - end - end - - describe '#sequential_import' do - let(:parallel) { false } - - it 'imports each event in sequence' do - event_importer = instance_double(Gitlab::GithubImport::Importer::IssueEventImporter) - - allow(importer).to receive(:each_object_to_import).and_yield(issue_event) - - expect(Gitlab::GithubImport::Importer::IssueEventImporter) - .to receive(:new) - .with( - an_instance_of(Gitlab::GithubImport::Representation::IssueEvent), - project, - client - ) - .and_return(event_importer) - - expect(event_importer).to receive(:execute) - - importer.sequential_import - end - end - - describe '#parallel_import', :clean_gitlab_redis_shared_state do - it 'imports each note in parallel' do - allow(importer).to receive(:each_object_to_import).and_yield(issue_event) - - expect(Gitlab::GithubImport::ImportIssueEventWorker).to receive(:perform_in).with( - an_instance_of(Float), project.id, an_instance_of(Hash), an_instance_of(String) - ) - - waiter = importer.parallel_import - - expect(waiter).to be_an_instance_of(Gitlab::JobWaiter) - expect(waiter.jobs_remaining).to eq(1) - end - end - - describe '#importer_class' do - it { expect(importer.importer_class).to eq Gitlab::GithubImport::Importer::IssueEventImporter } - end - - describe '#representation_class' do - it { expect(importer.representation_class).to eq Gitlab::GithubImport::Representation::IssueEvent } - end - - describe '#sidekiq_worker_class' do - it { expect(importer.sidekiq_worker_class).to eq Gitlab::GithubImport::ImportIssueEventWorker } - end - - describe '#object_type' do - it { expect(importer.object_type).to eq :issue_event } - end - - describe '#collection_method' do - it { expect(importer.collection_method).to eq :repository_issue_events } - end - - describe '#id_for_already_imported_cache' do - it 'returns the ID of the given note' do - expect(importer.id_for_already_imported_cache(issue_event)).to eq(issue_event.id) - end - end - - describe '#collection_options' do - it { expect(importer.collection_options).to eq({}) } - end -end diff --git a/spec/lib/gitlab/github_import/importer/pull_requests/all_merged_by_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests/all_merged_by_importer_spec.rb deleted file mode 100644 index 8e13b35eb6bc0294c31871ff06babecbdb60f7f0..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/github_import/importer/pull_requests/all_merged_by_importer_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Importer::PullRequests::AllMergedByImporter, feature_category: :importers do - let(:client) { double } - - let_it_be(:project) { create(:project, import_source: 'http://somegithub.com') } - - subject { described_class.new(project, client) } - - it { is_expected.to include_module(Gitlab::GithubImport::ParallelScheduling) } - - describe '#representation_class' do - it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::PullRequest) } - end - - describe '#importer_class' do - it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequests::MergedByImporter) } - end - - describe '#sidekiq_worker_class' do - it { expect(subject.sidekiq_worker_class).to eq(Gitlab::GithubImport::PullRequests::ImportMergedByWorker) } - end - - describe '#collection_method' do - it { expect(subject.collection_method).to eq(:pull_requests_merged_by) } - end - - describe '#id_for_already_imported_cache' do - it { expect(subject.id_for_already_imported_cache(instance_double(MergeRequest, id: 1))).to eq(1) } - end - - describe '#each_object_to_import', :clean_gitlab_redis_cache do - let!(:merge_request) do - create(:merged_merge_request, iid: 999, source_project: project, target_project: project) - end - - it 'fetches the merged pull requests data' do - pull_request = double - - allow(client) - .to receive(:pull_request) - .exactly(:once) # ensure to be cached on the second call - .with('http://somegithub.com', 999) - .and_return(pull_request) - - expect { |b| subject.each_object_to_import(&b) } - .to yield_with_args(pull_request) - - subject.each_object_to_import - end - - it 'skips cached merge requests' do - Gitlab::Cache::Import::Caching.set_add( - "github-importer/already-imported/#{project.id}/pull_requests_merged_by", - merge_request.id - ) - - expect(client).not_to receive(:pull_request) - - subject.each_object_to_import - end - end -end diff --git a/spec/lib/gitlab/github_import/importer/pull_requests/review_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests/review_requests_importer_spec.rb deleted file mode 100644 index 7ba88b4fa796ef09755c5b5f4ef8aa1276ef4b37..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/github_import/importer/pull_requests/review_requests_importer_spec.rb +++ /dev/null @@ -1,149 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Importer::PullRequests::ReviewRequestsImporter, :clean_gitlab_redis_cache, - feature_category: :importers do - subject(:importer) { described_class.new(project, client) } - - let_it_be(:project) { create(:project, import_source: 'foo') } - - let(:client) { instance_double(Gitlab::GithubImport::Client) } - let(:review_request_struct) { Struct.new(:merge_request_id, :users, keyword_init: true) } - let(:user_struct) { Struct.new(:id, :login, keyword_init: true) } - - shared_context 'when project with merge requests' do - let_it_be(:merge_request_1) { create(:merge_request, source_project: project, target_branch: 'feature1') } - let_it_be(:merge_request_2) { create(:merge_request, source_project: project, target_branch: 'feature2') } - - let(:importer_stub) { instance_double('Gitlab::GithubImport::Importer::NoteAttachmentsImporter') } - let(:importer_attrs) do - [instance_of(Gitlab::GithubImport::Representation::PullRequests::ReviewRequests), project, client] - end - - let(:review_requests_1) do - { - users: [ - { id: 4, login: 'alice' }, - { id: 5, login: 'bob' } - ] - } - end - - let(:review_requests_2) do - { - users: [{ id: 4, login: 'alice' }] - } - end - - before do - allow(client).to receive(:pull_request_review_requests) - .with(project.import_source, merge_request_1.iid).and_return(review_requests_1) - allow(client).to receive(:pull_request_review_requests) - .with(project.import_source, merge_request_2.iid).and_return(review_requests_2) - end - end - - describe '#sequential_import' do - include_context 'when project with merge requests' - - it 'imports each project merge request reviewers' do - expect_next_instances_of( - Gitlab::GithubImport::Importer::PullRequests::ReviewRequestImporter, 2, false, *importer_attrs - ) do |note_attachments_importer| - expect(note_attachments_importer).to receive(:execute) - end - - expect(Gitlab::GithubImport::ObjectCounter) - .to receive(:increment).twice.with(project, :pull_request_review_request, :fetched) - - importer.sequential_import - end - - context 'when merge request is already processed' do - before do - Gitlab::Cache::Import::Caching.set_add( - "github-importer/pull_requests/pull_request_review_requests/already-imported/#{project.id}", - merge_request_1.iid - ) - end - - it "doesn't import this merge request reviewers" do - expect_next_instance_of( - Gitlab::GithubImport::Importer::PullRequests::ReviewRequestImporter, *importer_attrs - ) do |note_attachments_importer| - expect(note_attachments_importer).to receive(:execute) - end - - expect(Gitlab::GithubImport::ObjectCounter) - .to receive(:increment).once.with(project, :pull_request_review_request, :fetched) - - importer.sequential_import - end - end - end - - describe '#parallel_import' do - include_context 'when project with merge requests' - - let(:expected_worker_payload) do - [ - [ - project.id, - { - merge_request_id: merge_request_1.id, - merge_request_iid: merge_request_1.iid, - users: [ - { id: 4, login: 'alice' }, - { id: 5, login: 'bob' } - ] - }.deep_stringify_keys, - instance_of(String) - ], - [ - project.id, - { - merge_request_id: merge_request_2.id, - merge_request_iid: merge_request_2.iid, - users: [ - { id: 4, login: 'alice' } - ] - }.deep_stringify_keys, - instance_of(String) - ] - ] - end - - it 'schedule import for each merge request reviewers' do - expect(Gitlab::GithubImport::PullRequests::ImportReviewRequestWorker) - .to receive(:perform_in).with(an_instance_of(Float), *expected_worker_payload.first) - - expect(Gitlab::GithubImport::PullRequests::ImportReviewRequestWorker) - .to receive(:perform_in).with(an_instance_of(Float), *expected_worker_payload.second) - - expect(Gitlab::GithubImport::ObjectCounter) - .to receive(:increment).twice.with(project, :pull_request_review_request, :fetched) - - importer.parallel_import - end - - context 'when merge request is already processed' do - before do - Gitlab::Cache::Import::Caching.set_add( - "github-importer/pull_requests/pull_request_review_requests/already-imported/#{project.id}", - merge_request_1.iid - ) - end - - it "doesn't schedule import this merge request reviewers" do - expect(Gitlab::GithubImport::PullRequests::ImportReviewRequestWorker) - .to receive(:perform_in).with(an_instance_of(Float), *expected_worker_payload.second) - - expect(Gitlab::GithubImport::ObjectCounter) - .to receive(:increment).once.with(project, :pull_request_review_request, :fetched) - - importer.parallel_import - end - end - end -end diff --git a/spec/lib/gitlab/github_import/importer/pull_requests/reviews_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests/reviews_importer_spec.rb deleted file mode 100644 index c464f9d992ae23e6ca571e3ad2d47fd232bb2bdc..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/github_import/importer/pull_requests/reviews_importer_spec.rb +++ /dev/null @@ -1,95 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Importer::PullRequests::ReviewsImporter, feature_category: :importers do - let(:client) { double } - let(:project) { create(:project, import_source: 'github/repo') } - - subject { described_class.new(project, client) } - - it { is_expected.to include_module(Gitlab::GithubImport::ParallelScheduling) } - - describe '#representation_class' do - it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::PullRequestReview) } - end - - describe '#importer_class' do - it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequests::ReviewImporter) } - end - - describe '#sidekiq_worker_class' do - it { expect(subject.sidekiq_worker_class).to eq(Gitlab::GithubImport::PullRequests::ImportReviewWorker) } - end - - describe '#collection_method' do - it { expect(subject.collection_method).to eq(:pull_request_reviews) } - end - - describe '#object_type' do - it { expect(subject.object_type).to eq(:pull_request_review) } - end - - describe '#id_for_already_imported_cache' do - it { expect(subject.id_for_already_imported_cache({ id: 1 })).to eq(1) } - end - - describe '#each_object_to_import', :clean_gitlab_redis_shared_state do - let(:merge_request) do - create( - :merged_merge_request, - iid: 999, - source_project: project, - target_project: project - ) - end - - let(:review) { { id: 1 } } - - it 'fetches the pull requests reviews data' do - page = Struct.new(:objects, :number).new([review], 1) - - expect(client) - .to receive(:each_page) - .exactly(:once) # ensure to be cached on the second call - .with(:pull_request_reviews, 'github/repo', merge_request.iid, { page: 1 }) - .and_yield(page) - - expect { |b| subject.each_object_to_import(&b) } - .to yield_with_args(review) - - subject.each_object_to_import - - expect(review[:merge_request_id]).to eq(merge_request.id) - expect(review[:merge_request_iid]).to eq(merge_request.iid) - end - - it 'skips cached pages' do - Gitlab::Import::PageCounter - .new(project, "merge_request/#{merge_request.id}/pull_request_reviews") - .set(2) - - expect(review).not_to receive(:merge_request_id=) - - expect(client) - .to receive(:each_page) - .exactly(:once) # ensure to be cached on the second call - .with(:pull_request_reviews, 'github/repo', merge_request.iid, { page: 2 }) - - subject.each_object_to_import - end - - it 'skips cached merge requests' do - Gitlab::Cache::Import::Caching.set_add( - "github-importer/merge_request/already-imported/#{project.id}", - merge_request.id - ) - - expect(review).not_to receive(:merge_request_id=) - - expect(client).not_to receive(:each_page) - - subject.each_object_to_import - 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 552f91ba2c7e8d942361a4da159efaca2b47fc99..4fdf7bd0cac9298c3829ebf9b2728d187edc43cd 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 @@ -104,15 +104,10 @@ let(:page_counter) { instance_double(Gitlab::Import::PageCounter) } - let(:extended_events) { true } - before do 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 @@ -226,16 +221,6 @@ subject.each_object_to_import { |event| event } end - - context 'when extended_events is disabled' do - let(:extended_events) { false } - - it 'increments the issue_event fetched counter' do - expect(Gitlab::GithubImport::ObjectCounter).to receive(:increment).with(project, :issue_event, :fetched) - - subject.each_object_to_import { |event| event } - end - end end end @@ -279,17 +264,6 @@ 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 @@ -319,27 +293,11 @@ 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 } - + describe '#execute', :clean_gitlab_redis_shared_state do 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 }) @@ -362,32 +320,15 @@ .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 + 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 - context 'when extended_events is enabled' do - let(:extended_events) { true } + it 'returns job waiter with the correct remaining jobs count' do + job_waiter = subject.execute - 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 + expect(job_waiter.jobs_remaining).to eq(2) end end end diff --git a/spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb deleted file mode 100644 index f07784cd3b58e4af480a25cd5d41389ea2234516..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Importer::SingleEndpointIssueNotesImporter do - let(:client) { double } - let(:project) { create(:project, import_source: 'github/repo') } - - subject { described_class.new(project, client) } - - it { is_expected.to include_module(Gitlab::GithubImport::ParallelScheduling) } - it { is_expected.to include_module(Gitlab::GithubImport::SingleEndpointNotesImporting) } - it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::Note) } - it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::NoteImporter) } - it { expect(subject.collection_method).to eq(:issue_comments) } - it { expect(subject.object_type).to eq(:note) } - it { expect(subject.id_for_already_imported_cache({ id: 1 })).to eq(1) } - - describe '#each_object_to_import', :clean_gitlab_redis_shared_state do - let(:issue) do - create( - :issue, - iid: 999, - project: project - ) - end - - let(:note) { { id: 1 } } - let(:page) { double(objects: [note], number: 1) } - - it 'fetches data' do - expect(client) - .to receive(:each_page) - .exactly(:once) # ensure to be cached on the second call - .with(:issue_comments, 'github/repo', issue.iid, { page: 1 }) - .and_yield(page) - - expect { |b| subject.each_object_to_import(&b) }.to yield_with_args(note) - - subject.each_object_to_import {} - - expect( - Gitlab::Cache::Import::Caching.set_includes?( - "github-importer/issue/notes/already-imported/#{project.id}", - issue.iid - ) - ).to eq(true) - end - - it 'skips cached pages' do - Gitlab::Import::PageCounter - .new(project, "issue/#{issue.id}/issue_comments") - .set(2) - - expect(client) - .to receive(:each_page) - .exactly(:once) # ensure to be cached on the second call - .with(:issue_comments, 'github/repo', issue.iid, { page: 2 }) - - subject.each_object_to_import {} - end - - it 'skips cached merge requests' do - Gitlab::Cache::Import::Caching.set_add( - "github-importer/issue/notes/already-imported/#{project.id}", - issue.iid - ) - - expect(client).not_to receive(:each_page) - - subject.each_object_to_import {} - end - end -end diff --git a/spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb deleted file mode 100644 index 47bf087e5c00899136f653ff071ddfd15d660f5d..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb +++ /dev/null @@ -1,75 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Importer::SingleEndpointMergeRequestNotesImporter do - let(:client) { double } - let(:project) { create(:project, import_source: 'github/repo') } - - subject { described_class.new(project, client) } - - it { is_expected.to include_module(Gitlab::GithubImport::ParallelScheduling) } - it { is_expected.to include_module(Gitlab::GithubImport::SingleEndpointNotesImporting) } - it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::Note) } - it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::NoteImporter) } - it { expect(subject.collection_method).to eq(:issue_comments) } - it { expect(subject.object_type).to eq(:note) } - it { expect(subject.id_for_already_imported_cache({ id: 1 })).to eq(1) } - - describe '#each_object_to_import', :clean_gitlab_redis_shared_state do - let(:merge_request) do - create( - :merge_request, - iid: 999, - source_project: project, - target_project: project - ) - end - - let(:note) { { id: 1 } } - let(:page) { double(objects: [note], number: 1) } - - it 'fetches data' do - expect(client) - .to receive(:each_page) - .exactly(:once) # ensure to be cached on the second call - .with(:issue_comments, 'github/repo', merge_request.iid, { page: 1 }) - .and_yield(page) - - expect { |b| subject.each_object_to_import(&b) }.to yield_with_args(note) - - subject.each_object_to_import {} - - expect( - Gitlab::Cache::Import::Caching.set_includes?( - "github-importer/merge_request/notes/already-imported/#{project.id}", - merge_request.iid - ) - ).to eq(true) - end - - it 'skips cached pages' do - Gitlab::Import::PageCounter - .new(project, "merge_request/#{merge_request.id}/issue_comments") - .set(2) - - expect(client) - .to receive(:each_page) - .exactly(:once) # ensure to be cached on the second call - .with(:issue_comments, 'github/repo', merge_request.iid, { page: 2 }) - - subject.each_object_to_import {} - end - - it 'skips cached merge requests' do - Gitlab::Cache::Import::Caching.set_add( - "github-importer/merge_request/notes/already-imported/#{project.id}", - merge_request.iid - ) - - expect(client).not_to receive(:each_page) - - subject.each_object_to_import {} - end - end -end diff --git a/spec/lib/gitlab/github_import/settings_spec.rb b/spec/lib/gitlab/github_import/settings_spec.rb index d268f3a865095050ead77b409cfac4903deb55b0..884cb533750a002f76d47b27025eff8e329b03fc 100644 --- a/spec/lib/gitlab/github_import/settings_spec.rb +++ b/spec/lib/gitlab/github_import/settings_spec.rb @@ -9,7 +9,6 @@ let(:optional_stages) do { - single_endpoint_issue_events_import: true, single_endpoint_notes_import: false, attachments_import: false, collaborators_import: false @@ -44,37 +43,12 @@ it 'returns stages list as array' do 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 describe '#write' do let(:data_input) do { optional_stages: { - single_endpoint_issue_events_import: true, single_endpoint_notes_import: 'false', attachments_import: nil, collaborators_import: false, @@ -100,7 +74,6 @@ it 'returns is enabled or not specific optional stage' do project.build_or_assign_import_data(data: { optional_stages: optional_stages }) - expect(settings.enabled?(:single_endpoint_issue_events_import)).to eq true expect(settings.enabled?(:single_endpoint_notes_import)).to eq false expect(settings.enabled?(:attachments_import)).to eq false expect(settings.enabled?(:collaborators_import)).to eq false @@ -111,30 +84,9 @@ it 'returns is disabled or not specific optional stage' do project.build_or_assign_import_data(data: { optional_stages: optional_stages }) - expect(settings.disabled?(:single_endpoint_issue_events_import)).to eq false expect(settings.disabled?(:single_endpoint_notes_import)).to eq true expect(settings.disabled?(:attachments_import)).to eq true 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/lib/gitlab/github_import/user_finder_spec.rb b/spec/lib/gitlab/github_import/user_finder_spec.rb index a843599b1d566465d8f8b72f389d1fd392a3d503..a5e8d428128ef84e3f0423d681ef8be868553fed 100644 --- a/spec/lib/gitlab/github_import/user_finder_spec.rb +++ b/spec/lib/gitlab/github_import/user_finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache, feature_category: :importers do +RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_shared_state, feature_category: :importers do let(:project) do create( :project, diff --git a/spec/lib/gitlab/github_import_spec.rb b/spec/lib/gitlab/github_import_spec.rb index 6324ec0b95cf625e01fa26b5899062d7b577bf0d..831f3eaa46f41053edeb3e490e07360d77991604 100644 --- a/spec/lib/gitlab/github_import_spec.rb +++ b/spec/lib/gitlab/github_import_spec.rb @@ -32,7 +32,7 @@ described_class.new_client_for(project) end - it 'returns the ID of the ghost user', :clean_gitlab_redis_cache do + it 'returns the ID of the ghost user', :clean_gitlab_redis_shared_state do expect(described_class.ghost_user_id).to eq(Users::Internal.ghost.id) end @@ -73,11 +73,11 @@ described_class.new_client_for(project) end - it 'returns the ID of the ghost user', :clean_gitlab_redis_cache do + it 'returns the ID of the ghost user', :clean_gitlab_redis_shared_state do expect(described_class.ghost_user_id).to eq(Users::Internal.ghost.id) end - it 'caches the ghost user ID', :clean_gitlab_redis_cache do + it 'caches the ghost user ID', :clean_gitlab_redis_shared_state do expect(Gitlab::Cache::Import::Caching) .to receive(:write) .once diff --git a/spec/services/import/github_service_spec.rb b/spec/services/import/github_service_spec.rb index 4da9e9dd9174c3d7b4588c7a9f4c9909c4e5f956..9975b9fdb8632dbb5d50028783b492b3c4b438c4 100644 --- a/spec/services/import/github_service_spec.rb +++ b/spec/services/import/github_service_spec.rb @@ -35,7 +35,6 @@ allow(settings) .to receive(:write) .with( - extended_events: true, optional_stages: optional_stages, timeout_strategy: timeout_strategy ) @@ -98,7 +97,6 @@ expect(settings) .to have_received(:write) .with(optional_stages: nil, - extended_events: true, timeout_strategy: timeout_strategy ) expect_snowplow_event( @@ -124,7 +122,6 @@ .to have_received(:write) .with( optional_stages: nil, - extended_events: true, timeout_strategy: timeout_strategy ) expect_snowplow_event( @@ -160,7 +157,6 @@ .to have_received(:write) .with( optional_stages: nil, - extended_events: true, timeout_strategy: timeout_strategy ) expect_snowplow_event( @@ -185,7 +181,6 @@ context 'when optional stages params present' do let(:optional_stages) do { - single_endpoint_issue_events_import: true, single_endpoint_notes_import: 'false', attachments_import: false, collaborators_import: true @@ -199,7 +194,6 @@ .to have_received(:write) .with( optional_stages: optional_stages, - extended_events: true, timeout_strategy: timeout_strategy ) end @@ -215,7 +209,6 @@ .to have_received(:write) .with( optional_stages: optional_stages, - extended_events: true, timeout_strategy: timeout_strategy ) end @@ -229,25 +222,10 @@ .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 using personal access tokens' do diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 858ba89b113fc1a1b80fa3a4779e33db221c719b..2d15999e6b3c29a308da7fa1e17c279630cbb2a3 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -6033,7 +6033,6 @@ - './spec/lib/gitlab/github_import/importer/events/renamed_spec.rb' - './spec/lib/gitlab/github_import/importer/events/reopened_spec.rb' - './spec/lib/gitlab/github_import/importer/issue_and_label_links_importer_spec.rb' -- './spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb' - './spec/lib/gitlab/github_import/importer/issue_importer_spec.rb' - './spec/lib/gitlab/github_import/importer/issues_importer_spec.rb' - './spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb' @@ -6049,8 +6048,6 @@ - './spec/lib/gitlab/github_import/importer/repository_importer_spec.rb' - './spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb' - './spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb' -- './spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb' -- './spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb' - './spec/lib/gitlab/github_import/issuable_finder_spec.rb' - './spec/lib/gitlab/github_import/label_finder_spec.rb' - './spec/lib/gitlab/github_import/logger_spec.rb' @@ -9453,9 +9450,6 @@ - './spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb' - './spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb' - './spec/workers/gitlab/github_import/stage/import_lfs_objects_worker_spec.rb' -- './spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb' -- './spec/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker_spec.rb' -- './spec/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker_spec.rb' - './spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb' - './spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb' - './spec/workers/gitlab/import/stuck_import_job_spec.rb' diff --git a/spec/workers/gitlab/github_import/import_issue_event_worker_spec.rb b/spec/workers/gitlab/github_import/import_issue_event_worker_spec.rb index cbe27934bd54139b82bec515d93a1cb5551a65f6..80f1a800398d35061a0bcd48baad0cc9639ab8c6 100644 --- a/spec/workers/gitlab/github_import/import_issue_event_worker_spec.rb +++ b/spec/workers/gitlab/github_import/import_issue_event_worker_spec.rb @@ -11,7 +11,6 @@ end let(:client) { instance_double(Gitlab::GithubImport::Client) } - let(:extended_events) { true } let(:event_hash) do { 'id' => 6501124486, @@ -26,12 +25,6 @@ } end - before do - allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| - allow(setting).to receive(:extended_events?).and_return(extended_events) - end - end - it 'imports an issue event and increase importer counter' do expect_next_instance_of(Gitlab::GithubImport::Importer::IssueEventImporter, an_instance_of(Gitlab::GithubImport::Representation::IssueEvent), @@ -65,16 +58,6 @@ worker.import(project, client, event_hash) end - - context 'when extended_events is disabled' do - let(:extended_events) { false } - - it 'increments the issue_event importer counter' do - expect(Gitlab::GithubImport::ObjectCounter).to receive(:increment).with(project, :issue_event, :imported) - - worker.import(project, client, event_hash) - end - end end end end diff --git a/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb index 7a085227b367d8072231ac65a38748a737f131ff..b5c2567fd8890c4d58a4959ce9e6de30e63bba78 100644 --- a/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb @@ -35,7 +35,7 @@ expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async) - .with(project.id, { '123' => 2 }, 'pull_requests_merged_by') + .with(project.id, { '123' => 2 }, 'issues_and_diff_notes') worker.import(client, project) end @@ -49,7 +49,7 @@ expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async) - .with(project.id, {}, 'pull_requests_merged_by') + .with(project.id, {}, 'issues_and_diff_notes') worker.import(client, project) end @@ -63,7 +63,7 @@ expect(Gitlab::GithubImport::AdvanceStageWorker) .to receive(:perform_async) - .with(project.id, {}, 'pull_requests_merged_by') + .with(project.id, {}, 'issues_and_diff_notes') worker.import(client, project) 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 6b01f2825e4110c7e13ea3029a501ed9ee7f854c..22df01c8e5fb4d7a1cd959f85f522a5633c520c1 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 @@ -8,62 +8,30 @@ let!(:group) { create(:group, projects: [project]) } let(:settings) { ::Gitlab::GithubImport::Settings.new(project.reload) } let(:stage_enabled) { true } - let(:extended_events) { false } subject(:worker) { described_class.new } - before do - settings.write({ - optional_stages: { single_endpoint_issue_events_import: stage_enabled }, extended_events: extended_events - }) - end - it_behaves_like Gitlab::GithubImport::StageMethods describe '#import' do let(:importer) { instance_double('Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter') } let(:client) { instance_double('Gitlab::GithubImport::Client') } - context 'when stage is enabled' do - it 'imports issue events' do - waiter = Gitlab::JobWaiter.new(2, '123') - - expect(Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter) - .to receive(:new) - .with(project, client) - .and_return(importer) - - expect(importer).to receive(:execute).and_return(waiter) - - expect(Gitlab::GithubImport::AdvanceStageWorker) - .to receive(:perform_async) - .with(project.id, { '123' => 2 }, 'notes') - - worker.import(client, project) - end - end - - context 'when stage is disabled' do - let(:stage_enabled) { false } - - it 'skips issue events import and calls next stage' do - expect(Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter).not_to receive(:new) - expect(Gitlab::GithubImport::AdvanceStageWorker).to receive(:perform_async).with(project.id, {}, 'notes') + it 'imports issue events' do + waiter = Gitlab::JobWaiter.new(2, '123') - worker.import(client, project) - end + expect(Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter) + .to receive(:new) + .with(project, client) + .and_return(importer) - context 'when extended_events is enabled' do - let(:extended_events) { true } + expect(importer).to receive(:execute).and_return(waiter) - 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 + expect(Gitlab::GithubImport::AdvanceStageWorker) + .to receive(:perform_async) + .with(project.id, { '123' => 2 }, 'attachments') - worker.import(client, project) - end - end + worker.import(client, project) end end end diff --git a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb deleted file mode 100644 index a8b40ff43d2a8a7d34d7911733e08b5cc4dff0fa..0000000000000000000000000000000000000000 --- a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb +++ /dev/null @@ -1,65 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Stage::ImportNotesWorker, feature_category: :importers do - let_it_be(:project) { create(:project) } - - let(:settings) { ::Gitlab::GithubImport::Settings.new(project.reload) } - let(:single_endpoint_optional_stage) { true } - - subject(:worker) { described_class.new } - - before do - settings.write({ optional_stages: { single_endpoint_notes_import: single_endpoint_optional_stage } }) - end - - it_behaves_like Gitlab::GithubImport::StageMethods - - describe '#import' do - it 'imports all the notes' do - client = double(:client) - - worker.importers(project).each do |klass| - importer = double(:importer) - waiter = Gitlab::JobWaiter.new(2, '123') - - expect(klass) - .to receive(:new) - .with(project, client) - .and_return(importer) - - expect(importer) - .to receive(:execute) - .and_return(waiter) - end - - expect(Gitlab::GithubImport::AdvanceStageWorker) - .to receive(:perform_async) - .with(project.id, { '123' => 2 }, 'attachments') - - worker.import(client, project) - end - end - - describe '#importers' do - context 'when settings single_endpoint_notes_import is enabled' do - it 'includes single endpoint mr and issue notes importers' do - expect(worker.importers(project)).to contain_exactly( - Gitlab::GithubImport::Importer::SingleEndpointMergeRequestNotesImporter, - Gitlab::GithubImport::Importer::SingleEndpointIssueNotesImporter - ) - end - end - - context 'when settings single_endpoint_notes_import is disabled' do - let(:single_endpoint_optional_stage) { false } - - it 'includes default notes importer' do - expect(worker.importers(project)).to contain_exactly( - Gitlab::GithubImport::Importer::NotesImporter - ) - end - end - end -end diff --git a/spec/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker_spec.rb deleted file mode 100644 index b3cb73c5fa01f9d40b478f8dd6913c548db828d2..0000000000000000000000000000000000000000 --- a/spec/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker_spec.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsMergedByWorker, feature_category: :importers do - let_it_be(:project) { create(:project) } - - subject(:worker) { described_class.new } - - it_behaves_like Gitlab::GithubImport::StageMethods - - describe '#import' do - it 'imports all the pull requests' do - importer = double(:importer) - client = double(:client) - waiter = Gitlab::JobWaiter.new(2, '123') - - expect(Gitlab::GithubImport::Importer::PullRequests::AllMergedByImporter) - .to receive(:new) - .with(project, client) - .and_return(importer) - - expect(importer) - .to receive(:execute) - .and_return(waiter) - - expect(Gitlab::GithubImport::AdvanceStageWorker) - .to receive(:perform_async) - .with(project.id, { '123' => 2 }, 'pull_request_review_requests') - - worker.import(client, project) - end - end -end diff --git a/spec/workers/gitlab/github_import/stage/import_pull_requests_review_requests_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_pull_requests_review_requests_worker_spec.rb deleted file mode 100644 index c7b73357e76090fde399fd813148dd3da8a714ba..0000000000000000000000000000000000000000 --- a/spec/workers/gitlab/github_import/stage/import_pull_requests_review_requests_worker_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsReviewRequestsWorker, feature_category: :importers do - let_it_be(:project) { create(:project) } - - let(:client) { instance_double(Gitlab::GithubImport::Client) } - let(:importer) { instance_double(Gitlab::GithubImport::Importer::PullRequests::ReviewRequestsImporter) } - let(:waiter) { Gitlab::JobWaiter.new(2, '123') } - - subject(:worker) { described_class.new } - - it_behaves_like Gitlab::GithubImport::StageMethods - - describe '#import' do - it 'imports all PR review requests' do - expect(Gitlab::GithubImport::Importer::PullRequests::ReviewRequestsImporter) - .to receive(:new) - .with(project, client) - .and_return(importer) - - expect(importer).to receive(:execute).and_return(waiter) - - expect(Gitlab::GithubImport::AdvanceStageWorker) - .to receive(:perform_async) - .with(project.id, { '123' => 2 }, 'pull_request_reviews') - - worker.import(client, project) - end - end -end diff --git a/spec/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker_spec.rb deleted file mode 100644 index ab3f0b43304b0f6a3d9344b9ad7db2caf0f8b53a..0000000000000000000000000000000000000000 --- a/spec/workers/gitlab/github_import/stage/import_pull_requests_reviews_worker_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsReviewsWorker, feature_category: :importers do - let_it_be(:project) { create(:project) } - - let(:client) { double(:client) } - - subject(:worker) { described_class.new } - - it_behaves_like Gitlab::GithubImport::StageMethods - - describe '#import' do - it 'imports all the pull request reviews' do - importer = double(:importer) - - waiter = Gitlab::JobWaiter.new(2, '123') - - expect(Gitlab::GithubImport::Importer::PullRequests::ReviewsImporter) - .to receive(:new) - .with(project, client) - .and_return(importer) - - expect(importer) - .to receive(:execute) - .and_return(waiter) - - expect(Gitlab::GithubImport::AdvanceStageWorker) - .to receive(:perform_async) - .with(project.id, { '123' => 2 }, 'issues_and_diff_notes') - - worker.import(client, project) - end - end -end