diff --git a/.rubocop_todo/rspec/verified_doubles.yml b/.rubocop_todo/rspec/verified_doubles.yml index ffc0b2a3824be7aba5a59c5b144f87502fcf98ba..4824f2007d3200381b9630d28e0ad6f8755ee28a 100644 --- a/.rubocop_todo/rspec/verified_doubles.yml +++ b/.rubocop_todo/rspec/verified_doubles.yml @@ -536,7 +536,6 @@ RSpec/VerifiedDoubles: - 'spec/lib/gitlab/github_import/markdown_text_spec.rb' - 'spec/lib/gitlab/github_import/milestone_finder_spec.rb' - 'spec/lib/gitlab/github_import/object_counter_spec.rb' - - 'spec/lib/gitlab/github_import/page_counter_spec.rb' - 'spec/lib/gitlab/github_import/parallel_importer_spec.rb' - 'spec/lib/gitlab/github_import/parallel_scheduling_spec.rb' - 'spec/lib/gitlab/github_import/representation/diff_note_spec.rb' diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 63cc855e4bd9add82e72e9eff408c782e2fd7976..a07e05f5d8e76a8f8baeb6044e30c3a704bfa3b9 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -2584,7 +2584,7 @@ :urgency: :low :resource_boundary: :unknown :weight: 1 - :idempotent: false + :idempotent: true :tags: [] - :name: bulk_import :worker_name: BulkImportWorker diff --git a/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb b/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb index dd18139fc9e3dc9fdd85dcff15045ef144784337..4c323a117556870818d9c393d9a60c94c15dd43b 100644 --- a/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb +++ b/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb @@ -3,9 +3,11 @@ module Gitlab module BitbucketServerImport module Stage - class ImportUsersWorker # rubocop:disable Scalability/IdempotentWorker -- ImportPullRequestsWorker is not idempotent + class ImportUsersWorker include StageMethods + idempotent! + private def import(project) diff --git a/app/workers/gitlab/github_gists_import/start_import_worker.rb b/app/workers/gitlab/github_gists_import/start_import_worker.rb index f7d3eb1d759f91c1359b1c35d6c59319136840d7..d6c637f6d4937591f223eb735151c56022d1c4e4 100644 --- a/app/workers/gitlab/github_gists_import/start_import_worker.rb +++ b/app/workers/gitlab/github_gists_import/start_import_worker.rb @@ -17,7 +17,7 @@ class StartImportWorker # rubocop:disable Scalability/IdempotentWorker Gitlab::GithubGistsImport::Status.new(msg['args'][0]).fail! user = User.find(msg['args'][0]) - Gitlab::GithubImport::PageCounter.new(user, :gists, 'github-gists-importer').expire! + Gitlab::Import::PageCounter.new(user, :gists, 'github-gists-importer').expire! end def perform(user_id, encrypted_token) diff --git a/lib/gitlab/bitbucket_server_import/importers/users_importer.rb b/lib/gitlab/bitbucket_server_import/importers/users_importer.rb index f8d0521afb26118a6749f9136f68373c76f88321..fc96f894a998bda4c70231f3403d8afedc0c9ab8 100644 --- a/lib/gitlab/bitbucket_server_import/importers/users_importer.rb +++ b/lib/gitlab/bitbucket_server_import/importers/users_importer.rb @@ -19,23 +19,26 @@ def initialize(project) def execute log_info(import_stage: 'import_users', message: 'starting') - page = 1 + current = page_counter.current loop do log_info( import_stage: 'import_users', - message: "importing page #{page} using batch size #{BATCH_SIZE}" + message: "importing page #{current} using batch size #{BATCH_SIZE}" ) - users = client.users(project_key, page_offset: page, limit: BATCH_SIZE).to_a + users = client.users(project_key, page_offset: current, limit: BATCH_SIZE).to_a break if users.empty? cache_users(users) - page += 1 + current += 1 + page_counter.set(current) end + page_counter.expire! + log_info(import_stage: 'import_users', message: 'finished') end @@ -57,6 +60,10 @@ def client def project_key project.import_data.data['project_key'] end + + def page_counter + @page_counter ||= Gitlab::Import::PageCounter.new(project, :users, 'bitbucket-server-importer') + end end end end diff --git a/lib/gitlab/github_gists_import/importer/gists_importer.rb b/lib/gitlab/github_gists_import/importer/gists_importer.rb index 08744dbaf5f626d9f60790bfc8ba95458375dac0..4ed6b2db5f8faee314f703b14af16a2c2eb129be 100644 --- a/lib/gitlab/github_gists_import/importer/gists_importer.rb +++ b/lib/gitlab/github_gists_import/importer/gists_importer.rb @@ -39,7 +39,7 @@ def spread_parallel_import end def fetch_gists_to_import - page_counter = Gitlab::GithubImport::PageCounter.new(user, :gists, 'github-gists-importer') + page_counter = Gitlab::Import::PageCounter.new(user, :gists, 'github-gists-importer') collection = [] client.each_page(:gists, nil, page: page_counter.current) do |page| diff --git a/lib/gitlab/github_import/importer/pull_requests/reviews_importer.rb b/lib/gitlab/github_import/importer/pull_requests/reviews_importer.rb index 347423b0e21c5497cff86d04d174043a97dfd43c..62c9e6469d74f0bfd09910ec291624d4da285c35 100644 --- a/lib/gitlab/github_import/importer/pull_requests/reviews_importer.rb +++ b/lib/gitlab/github_import/importer/pull_requests/reviews_importer.rb @@ -72,7 +72,7 @@ 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 = PageCounter.new(project, page_counter_id(merge_request)) + page_counter = Gitlab::Import::PageCounter.new(project, page_counter_id(merge_request)) repo = project.import_source options = collection_options.merge(page: page_counter.current) diff --git a/lib/gitlab/github_import/parallel_scheduling.rb b/lib/gitlab/github_import/parallel_scheduling.rb index ce93b5203df6b3c888136d55499ab204fbcf50f7..da685d41140b5ffa219a9de62c000587130b1d71 100644 --- a/lib/gitlab/github_import/parallel_scheduling.rb +++ b/lib/gitlab/github_import/parallel_scheduling.rb @@ -25,7 +25,7 @@ def initialize(project, client, parallel: true) @project = project @client = client @parallel = parallel - @page_counter = PageCounter.new(project, collection_method) + @page_counter = Gitlab::Import::PageCounter.new(project, collection_method) @already_imported_cache_key = format(ALREADY_IMPORTED_CACHE_KEY, project: project.id, collection: collection_method) @job_waiter_cache_key = format(JOB_WAITER_CACHE_KEY, project: project.id, collection: collection_method) diff --git a/lib/gitlab/github_import/single_endpoint_notes_importing.rb b/lib/gitlab/github_import/single_endpoint_notes_importing.rb index 3584288da573106b1a69a5acb6b74c4d1687d772..e2eb2671636ac3d6d4938375e3cf1b6acfa2057e 100644 --- a/lib/gitlab/github_import/single_endpoint_notes_importing.rb +++ b/lib/gitlab/github_import/single_endpoint_notes_importing.rb @@ -75,7 +75,7 @@ def process_batch(batch) batch.each do |parent_record| # The page counter needs to be scoped by parent_record to avoid skipping # pages of notes from already imported parent_record. - page_counter = PageCounter.new(project, page_counter_id(parent_record)) + page_counter = Gitlab::Import::PageCounter.new(project, page_counter_id(parent_record)) repo = project.import_source options = collection_options.merge(page: page_counter.current) diff --git a/lib/gitlab/github_import/page_counter.rb b/lib/gitlab/import/page_counter.rb similarity index 88% rename from lib/gitlab/github_import/page_counter.rb rename to lib/gitlab/import/page_counter.rb index c238ccb893214a65a18bbaea4801a526a5a6ff4b..44ab9c256d796d56705cc8ef115448928abe310b 100644 --- a/lib/gitlab/github_import/page_counter.rb +++ b/lib/gitlab/import/page_counter.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Gitlab - module GithubImport + module Import # PageCounter can be used to keep track of the last imported page of a # collection, allowing workers to resume where they left off in the event of # an error. @@ -12,7 +12,7 @@ class PageCounter CACHE_KEY = '%{import_type}/page-counter/%{object}/%{collection}' def initialize(object, collection, import_type = 'github-importer') - @cache_key = CACHE_KEY % { import_type: import_type, object: object.id, collection: collection } + @cache_key = format(CACHE_KEY, import_type: import_type, object: object.id, collection: collection) end # Sets the page number to the given value. diff --git a/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb index 33d6ab9451364050f4ac283869599d66c298a675..79010390628b673fdee3259772c42e7c7adefdfc 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb @@ -52,6 +52,13 @@ expect(logger).to receive(:info).with(hash_including(message: 'importing page 3 using batch size 2')) expect(logger).to receive(:info).with(hash_including(message: 'finished')) + expect_next_instance_of(Gitlab::Import::PageCounter) do |page_counter| + expect(page_counter).to receive(:current).and_call_original.once + expect(page_counter).to receive(:set).with(2).and_call_original.once + expect(page_counter).to receive(:set).with(3).and_call_original.once + expect(page_counter).to receive(:expire!).and_call_original.once + end + expect(Gitlab::Cache::Import::Caching).to receive(:write_multiple).and_call_original.twice importer.execute diff --git a/spec/lib/gitlab/github_gists_import/importer/gists_importer_spec.rb b/spec/lib/gitlab/github_gists_import/importer/gists_importer_spec.rb index d555a847ea54a31ad06bb8a61dfcc77f993db569..b6a57ef8b57ca07888e366abe706ce67b1c5fff3 100644 --- a/spec/lib/gitlab/github_gists_import/importer/gists_importer_spec.rb +++ b/spec/lib/gitlab/github_gists_import/importer/gists_importer_spec.rb @@ -8,7 +8,7 @@ let_it_be(:user) { create(:user) } let(:client) { instance_double('Gitlab::GithubImport::Client', rate_limit_resets_in: 5) } let(:token) { 'token' } - let(:page_counter) { instance_double('Gitlab::GithubImport::PageCounter', current: 1, set: true, expire!: true) } + let(:page_counter) { instance_double('Gitlab::Import::PageCounter', current: 1, set: true, expire!: true) } let(:page) { instance_double('Gitlab::GithubImport::Client::Page', objects: [gist], number: 1) } let(:url) { 'https://gist.github.com/foo/bar.git' } let(:waiter) { Gitlab::JobWaiter.new(0, 'some-job-key') } @@ -62,7 +62,7 @@ .with(token, parallel: true) .and_return(client) - allow(Gitlab::GithubImport::PageCounter) + allow(Gitlab::Import::PageCounter) .to receive(:new) .with(user, :gists, 'github-gists-importer') .and_return(page_counter) diff --git a/spec/lib/gitlab/github_import/importer/protected_branches_importer_spec.rb b/spec/lib/gitlab/github_import/importer/protected_branches_importer_spec.rb index b0892767fb3385f40577c4da9a93a07b92ed7246..6420f20efb6eaf87b65d08c21c36105a675a6d65 100644 --- a/spec/lib/gitlab/github_import/importer/protected_branches_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/protected_branches_importer_spec.rb @@ -166,7 +166,7 @@ # when user has no admin rights on repo let(:unknown_protection_branch) { branch_struct.new(name: 'development', protection: nil) } - let(:page_counter) { instance_double(Gitlab::GithubImport::PageCounter) } + let(:page_counter) { instance_double(Gitlab::Import::PageCounter) } before do allow(client).to receive(:branches).with(project.import_source) 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 index f5779f300b8d49ab8d168a201598ff28c7e89c21..94248f60a0be97b99c546fe3b816c2ed0eea28dc 100644 --- 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 @@ -69,7 +69,7 @@ end it 'skips cached pages' do - Gitlab::GithubImport::PageCounter + Gitlab::Import::PageCounter .new(project, "merge_request/#{merge_request.id}/pull_request_reviews") .set(2) diff --git a/spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb index 6fe0494d7cd17a0553e4f4ab48e8f74d99317837..d2e63eba954620433061e7077f7d4a30dd17d0cc 100644 --- a/spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb @@ -53,7 +53,7 @@ end it 'skips cached pages' do - Gitlab::GithubImport::PageCounter + Gitlab::Import::PageCounter .new(project, "merge_request/#{merge_request.id}/pull_request_comments") .set(2) diff --git a/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb b/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb index 91f89f0779c0e11697bda9c439367a032e7b1835..3d33a1db1d78002c3629add825e756c6b4dcb7c4 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 @@ -98,7 +98,7 @@ ) end - let(:page_counter) { instance_double(Gitlab::GithubImport::PageCounter) } + let(:page_counter) { instance_double(Gitlab::Import::PageCounter) } before do allow(Gitlab::Redis::SharedState).to receive(:with).and_return('OK') @@ -152,7 +152,7 @@ end it 'triggers page number increment' do - expect(Gitlab::GithubImport::PageCounter) + expect(Gitlab::Import::PageCounter) .to receive(:new).with(project, 'issues/1/issue_timeline') .and_return(page_counter) expect(page_counter).to receive(:current).and_return(1) @@ -166,7 +166,7 @@ context 'when page is already processed' do before do - page_counter = Gitlab::GithubImport::PageCounter.new( + page_counter = Gitlab::Import::PageCounter.new( project, subject.page_counter_id(issuable) ) page_counter.set(page.number) 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 index 88613244c8b16f9d224c2682d7e69b0413cabdfb..c0f0d86d625eb1352884400a55b25b5c37b264e3 100644 --- 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 @@ -52,7 +52,7 @@ end it 'skips cached pages' do - Gitlab::GithubImport::PageCounter + Gitlab::Import::PageCounter .new(project, "issue/#{issue.id}/issue_comments") .set(2) 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 index 601cd7a8f151b653c4b60891a8d163e28799b3d9..2d981a3d14f6341fa53b22515a381d98c42f0fb9 100644 --- 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 @@ -53,7 +53,7 @@ end it 'skips cached pages' do - Gitlab::GithubImport::PageCounter + Gitlab::Import::PageCounter .new(project, "merge_request/#{merge_request.id}/issue_comments") .set(2) diff --git a/spec/lib/gitlab/github_import/page_counter_spec.rb b/spec/lib/gitlab/import/page_counter_spec.rb similarity index 90% rename from spec/lib/gitlab/github_import/page_counter_spec.rb rename to spec/lib/gitlab/import/page_counter_spec.rb index ddb62cc8fad7c00f44cd340c57cdebe0f467f4cc..a7a4e301aa30c8710b69037cd935d39e03e52984 100644 --- a/spec/lib/gitlab/github_import/page_counter_spec.rb +++ b/spec/lib/gitlab/import/page_counter_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::PageCounter, :clean_gitlab_redis_cache, feature_category: :importers do - let(:project) { double(:project, id: 1) } +RSpec.describe Gitlab::Import::PageCounter, :clean_gitlab_redis_cache, feature_category: :importers do + let(:project) { instance_double(Project, id: 1) } let(:counter) { described_class.new(project, :issues) } describe '#initialize' do diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 592d0cadaf552bacb01cba5ff9142fadfaf3b4a1..e7af4156459b8165cfd07bb33dfcc402a01f3f36 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -6118,7 +6118,6 @@ - './spec/lib/gitlab/github_import/markdown_text_spec.rb' - './spec/lib/gitlab/github_import/milestone_finder_spec.rb' - './spec/lib/gitlab/github_import/object_counter_spec.rb' -- './spec/lib/gitlab/github_import/page_counter_spec.rb' - './spec/lib/gitlab/github_import/parallel_importer_spec.rb' - './spec/lib/gitlab/github_import/parallel_scheduling_spec.rb' - './spec/lib/gitlab/github_import/representation/diff_note_spec.rb' @@ -6246,6 +6245,7 @@ - './spec/lib/gitlab/i18n/translation_entry_spec.rb' - './spec/lib/gitlab/identifier_spec.rb' - './spec/lib/gitlab/import/database_helpers_spec.rb' +- './spec/lib/gitlab/import/page_counter_spec.rb' - './spec/lib/gitlab/import_export/after_export_strategies/base_after_export_strategy_spec.rb' - './spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb' - './spec/lib/gitlab/import_export/after_export_strategy_builder_spec.rb' diff --git a/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb b/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb index d4cd1b823495291cdeb598297c01716e46e1091a..1141d08729d356d844f4f70ef83e6d6325967330 100644 --- a/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb +++ b/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb @@ -3,7 +3,14 @@ require 'spec_helper' RSpec.describe Gitlab::BitbucketServerImport::Stage::ImportUsersWorker, feature_category: :importers do - let_it_be(:project) { create(:project, :import_started) } + let_it_be(:project) do + create(:project, :import_started, + import_data_attributes: { + data: { 'project_key' => 'key', 'repo_slug' => 'slug' }, + credentials: { 'base_uri' => 'http://bitbucket.org/', 'user' => 'bitbucket', 'password' => 'password' } + } + ) + end let(:worker) { described_class.new } @@ -15,6 +22,12 @@ allow_next_instance_of(Gitlab::BitbucketServerImport::Importers::UsersImporter) do |importer| allow(importer).to receive(:execute) end + + allow(Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker).to receive(:perform_async).and_return(nil) + end + + it_behaves_like 'an idempotent worker' do + let(:job_args) { [project.id] } end it 'schedules the next stage' do