diff options
Diffstat (limited to 'spec/lib/gitlab/github_import')
10 files changed, 543 insertions, 140 deletions
diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index e93d585bc3c..e37b27aeaef 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -600,7 +600,8 @@ RSpec.describe Gitlab::GithubImport::Client, feature_category: :importers do endCursor hasNextPage hasPreviousPage - } + }, + repositoryCount } } TEXT @@ -707,44 +708,30 @@ RSpec.describe Gitlab::GithubImport::Client, feature_category: :importers do end end - describe '#search_repos_by_name' do - let(:expected_query) { 'test in:name is:public,private user:user repo:repo1 repo:repo2 org:org1 org:org2' } - - it 'searches for repositories based on name' do - expect(client.octokit).to receive(:search_repositories).with(expected_query, {}) + describe '#count_repos_by_relation_type_graphql' do + relation_types = { + 'owned' => ' in:name is:public,private user:user', + 'collaborated' => ' in:name is:public,private repo:repo1 repo:repo2', + 'organization' => 'org:org1 org:org2' + } - client.search_repos_by_name('test') - end + relation_types.each do |relation_type, expected_query| + expected_graphql_params = "type: REPOSITORY, query: \"#{expected_query}\"" + expected_graphql = + <<-TEXT + { + search(#{expected_graphql_params}) { + repositoryCount + } + } + TEXT - context 'when pagination options present' do - it 'searches for repositories via expected query' do - expect(client.octokit).to receive(:search_repositories).with( - expected_query, { page: 2, per_page: 25 } + it 'returns count by relation_type' do + expect(client.octokit).to receive(:post).with( + '/graphql', { query: expected_graphql }.to_json ) - client.search_repos_by_name('test', { page: 2, per_page: 25 }) - end - end - - context 'when Faraday error received from octokit', :aggregate_failures do - let(:error_class) { described_class::CLIENT_CONNECTION_ERROR } - let(:info_params) { { 'error.class': error_class } } - - it 'retries on error and succeeds' do - allow_retry(:search_repositories) - - expect(Gitlab::Import::Logger).to receive(:info).with(hash_including(info_params)).once - - expect(client.search_repos_by_name('test')).to eq({}) - end - - it 'retries and does not succeed' do - allow(client.octokit) - .to receive(:search_repositories) - .with(expected_query, {}) - .and_raise(error_class, 'execution expired') - - expect { client.search_repos_by_name('test') }.to raise_error(error_class, 'execution expired') + client.count_repos_by_relation_type_graphql(relation_type: relation_type) end end end diff --git a/spec/lib/gitlab/github_import/clients/proxy_spec.rb b/spec/lib/gitlab/github_import/clients/proxy_spec.rb index 0baff7bafcb..7b2a8fa9d74 100644 --- a/spec/lib/gitlab/github_import/clients/proxy_spec.rb +++ b/spec/lib/gitlab/github_import/clients/proxy_spec.rb @@ -8,6 +8,10 @@ RSpec.describe Gitlab::GithubImport::Clients::Proxy, :manage, feature_category: let(:access_token) { 'test_token' } let(:client_options) { { foo: :bar } } + it { expect(client).to delegate_method(:each_object).to(:client) } + it { expect(client).to delegate_method(:user).to(:client) } + it { expect(client).to delegate_method(:octokit).to(:client) } + describe '#repos' do let(:search_text) { 'search text' } let(:pagination_options) { { limit: 10 } } @@ -15,54 +19,32 @@ RSpec.describe Gitlab::GithubImport::Clients::Proxy, :manage, feature_category: context 'when remove_legacy_github_client FF is enabled' do let(:client_stub) { instance_double(Gitlab::GithubImport::Client) } - context 'with github_client_fetch_repos_via_graphql FF enabled' do - let(:client_response) do - { - data: { - search: { - nodes: [{ name: 'foo' }, { name: 'bar' }], - pageInfo: { startCursor: 'foo', endCursor: 'bar' } - } + let(:client_response) do + { + data: { + search: { + nodes: [{ name: 'foo' }, { name: 'bar' }], + pageInfo: { startCursor: 'foo', endCursor: 'bar' }, + repositoryCount: 2 } } - end - - it 'fetches repos with Gitlab::GithubImport::Client (GraphQL API)' do - expect(Gitlab::GithubImport::Client) - .to receive(:new).with(access_token).and_return(client_stub) - expect(client_stub) - .to receive(:search_repos_by_name_graphql) - .with(search_text, pagination_options).and_return(client_response) - - expect(client.repos(search_text, pagination_options)).to eq( - { - repos: [{ name: 'foo' }, { name: 'bar' }], - page_info: { startCursor: 'foo', endCursor: 'bar' } - } - ) - end + } end - context 'with github_client_fetch_repos_via_graphql FF disabled' do - let(:client_response) do - { items: [{ name: 'foo' }, { name: 'bar' }] } - end - - before do - stub_feature_flags(github_client_fetch_repos_via_graphql: false) - end - - it 'fetches repos with Gitlab::GithubImport::Client (REST API)' do - expect(Gitlab::GithubImport::Client) - .to receive(:new).with(access_token).and_return(client_stub) - expect(client_stub) - .to receive(:search_repos_by_name) - .with(search_text, pagination_options).and_return(client_response) + it 'fetches repos with Gitlab::GithubImport::Client (GraphQL API)' do + expect(Gitlab::GithubImport::Client) + .to receive(:new).with(access_token).and_return(client_stub) + expect(client_stub) + .to receive(:search_repos_by_name_graphql) + .with(search_text, pagination_options).and_return(client_response) - expect(client.repos(search_text, pagination_options)).to eq( - { repos: [{ name: 'foo' }, { name: 'bar' }] } - ) - end + expect(client.repos(search_text, pagination_options)).to eq( + { + repos: [{ name: 'foo' }, { name: 'bar' }], + page_info: { startCursor: 'foo', endCursor: 'bar' }, + count: 2 + } + ) end end @@ -99,4 +81,59 @@ RSpec.describe Gitlab::GithubImport::Clients::Proxy, :manage, feature_category: end end end + + describe '#count_by', :clean_gitlab_redis_cache do + context 'when remove_legacy_github_client FF is enabled' do + let(:client_stub) { instance_double(Gitlab::GithubImport::Client) } + let(:client_response) { { data: { search: { repositoryCount: 1 } } } } + + before do + stub_feature_flags(remove_legacy_github_client: true) + end + + context 'when value is cached' do + before do + Gitlab::Cache::Import::Caching.write('github-importer/provider-repo-count/owned/user_id', 3) + end + + it 'returns repository count from cache' do + expect(Gitlab::GithubImport::Client) + .to receive(:new).with(access_token).and_return(client_stub) + expect(client_stub) + .not_to receive(:count_repos_by_relation_type_graphql) + .with({ relation_type: 'owned' }) + expect(client.count_repos_by('owned', 'user_id')).to eq(3) + end + end + + context 'when value is not cached' do + it 'returns repository count' do + expect(Gitlab::GithubImport::Client) + .to receive(:new).with(access_token).and_return(client_stub) + expect(client_stub) + .to receive(:count_repos_by_relation_type_graphql) + .with({ relation_type: 'owned' }).and_return(client_response) + expect(Gitlab::Cache::Import::Caching) + .to receive(:write) + .with('github-importer/provider-repo-count/owned/user_id', 1, timeout: 5.minutes) + .and_call_original + expect(client.count_repos_by('owned', 'user_id')).to eq(1) + end + end + end + + context 'when remove_legacy_github_client FF is disabled' do + let(:client_stub) { instance_double(Gitlab::LegacyGithubImport::Client) } + + before do + stub_feature_flags(remove_legacy_github_client: false) + end + + it 'returns nil' do + expect(Gitlab::LegacyGithubImport::Client) + .to receive(:new).with(access_token, client_options).and_return(client_stub) + expect(client.count_repos_by('owned', 'user_id')).to be_nil + end + end + end end diff --git a/spec/lib/gitlab/github_import/importer/collaborator_importer_spec.rb b/spec/lib/gitlab/github_import/importer/collaborator_importer_spec.rb new file mode 100644 index 00000000000..07c10fe57f0 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/collaborator_importer_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::CollaboratorImporter, feature_category: :importers do + subject(:importer) { described_class.new(collaborator, project, client) } + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, group: group) } + let_it_be(:user) { create(:user) } + + let(:client) { instance_double(Gitlab::GithubImport::Client) } + let(:github_user_id) { rand(1000) } + let(:collaborator) do + Gitlab::GithubImport::Representation::Collaborator.from_json_hash( + 'id' => github_user_id, + 'login' => user.username, + 'role_name' => github_role_name + ) + end + + let(:basic_member_attrs) do + { + source: project, + user_id: user.id, + member_namespace_id: project.project_namespace_id, + created_by_id: project.creator_id + }.stringify_keys + end + + describe '#execute' do + before do + allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder| + allow(finder).to receive(:find).with(github_user_id, user.username).and_return(user.id) + end + end + + shared_examples 'role mapping' do |collaborator_role, member_access_level| + let(:github_role_name) { collaborator_role } + + it 'creates expected member' do + expect { importer.execute }.to change { project.members.count } + .from(0).to(1) + + expected_member_attrs = basic_member_attrs.merge(access_level: member_access_level) + expect(project.members.last).to have_attributes(expected_member_attrs) + end + end + + it_behaves_like 'role mapping', 'read', Gitlab::Access::GUEST + it_behaves_like 'role mapping', 'triage', Gitlab::Access::REPORTER + it_behaves_like 'role mapping', 'write', Gitlab::Access::DEVELOPER + it_behaves_like 'role mapping', 'maintain', Gitlab::Access::MAINTAINER + it_behaves_like 'role mapping', 'admin', Gitlab::Access::OWNER + + context 'when role name is unknown (custom role)' do + let(:github_role_name) { 'custom_role' } + + it 'raises expected error' do + expect { importer.execute }.to raise_exception( + ::Gitlab::GithubImport::ObjectImporter::NotRetriableError + ).with_message("Unknown GitHub role: #{github_role_name}") + end + end + + context 'when user has lower role in a project group' do + before do + create(:group_member, group: group, user: user, access_level: Gitlab::Access::DEVELOPER) + end + + it_behaves_like 'role mapping', 'maintain', Gitlab::Access::MAINTAINER + end + + context 'when user has higher role in a project group' do + let(:github_role_name) { 'write' } + + before do + create(:group_member, group: group, user: user, access_level: Gitlab::Access::MAINTAINER) + end + + it 'skips creating member for the project' do + expect { importer.execute }.not_to change { project.members.count } + end + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/collaborators_importer_spec.rb b/spec/lib/gitlab/github_import/importer/collaborators_importer_spec.rb new file mode 100644 index 00000000000..e48b562279e --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/collaborators_importer_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::CollaboratorsImporter, feature_category: :importers do + subject(:importer) { described_class.new(project, client, parallel: parallel) } + + let(:parallel) { true } + let(:project) { instance_double(Project, id: 4, import_source: 'foo/bar', import_state: nil) } + let(:client) { instance_double(Gitlab::GithubImport::Client) } + + let(:github_collaborator) do + { + id: 100500, + login: 'bob', + role_name: 'maintainer' + } + end + + describe '#parallel?' do + context 'when parallel option is true' do + it { expect(importer).to be_parallel } + end + + context 'when parallel option is false' 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 collaborators 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 collaborators in sequence' do + expect(importer).to receive(:sequential_import) + importer.execute + end + end + end + + describe '#sequential_import' do + let(:parallel) { false } + + it 'imports each collaborator in sequence' do + collaborator_importer = instance_double(Gitlab::GithubImport::Importer::CollaboratorImporter) + + allow(importer) + .to receive(:each_object_to_import) + .and_yield(github_collaborator) + + expect(Gitlab::GithubImport::Importer::CollaboratorImporter) + .to receive(:new) + .with( + an_instance_of(Gitlab::GithubImport::Representation::Collaborator), + project, + client + ) + .and_return(collaborator_importer) + + expect(collaborator_importer).to receive(:execute) + + importer.sequential_import + end + end + + describe '#parallel_import', :clean_gitlab_redis_cache do + let(:page_struct) { Struct.new(:objects, :number, keyword_init: true) } + + before do + allow(client).to receive(:each_page) + .with(:collaborators, project.import_source, { page: 1 }) + .and_yield(page_struct.new(number: 1, objects: [github_collaborator])) + end + + it 'imports each collaborator in parallel' do + expect(Gitlab::GithubImport::ImportCollaboratorWorker).to receive(:perform_in) + .with(1.second, 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 + + context 'when collaborator is already imported' do + before do + Gitlab::Cache::Import::Caching.set_add( + "github-importer/already-imported/#{project.id}/collaborators", + github_collaborator[:id] + ) + end + + it "doesn't run importer on it" do + expect(Gitlab::GithubImport::ImportCollaboratorWorker).not_to receive(:perform_in) + + waiter = importer.parallel_import + + expect(waiter).to be_an_instance_of(Gitlab::JobWaiter) + expect(waiter.jobs_remaining).to eq(0) + end + end + end + + describe '#id_for_already_imported_cache' do + it 'returns the ID of the given note' do + expect(importer.id_for_already_imported_cache(github_collaborator)) + .to eq(100500) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb index e005d8eda84..16816dfbcea 100644 --- a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb @@ -44,6 +44,10 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do end it 'does not insert label links for non-existing labels' do + expect(importer) + .to receive(:find_target_id) + .and_return(4) + expect(importer.label_finder) .to receive(:id_for) .with('bug') @@ -55,6 +59,20 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do importer.create_labels end + + it 'does not insert label links for non-existing targets' do + expect(importer) + .to receive(:find_target_id) + .and_return(nil) + + expect(importer.label_finder) + .not_to receive(:id_for) + + expect(LabelLink) + .not_to receive(:bulk_insert!) + + importer.create_labels + end end describe '#find_target_id' do diff --git a/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb index 7d4e3c3bcce..450ebe9a719 100644 --- a/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do +RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter, feature_category: :importers do subject(:importer) { described_class.new(note_text, project, client) } - let_it_be(:project) { create(:project) } + let_it_be(:project) { create(:project, import_source: 'nickname/public-test-repo') } let(:note_text) { Gitlab::GithubImport::Representation::NoteText.from_db_record(record) } let(:client) { instance_double('Gitlab::GithubImport::Client') } @@ -13,6 +13,8 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do let(:doc_url) { 'https://github.com/nickname/public-test-repo/files/9020437/git-cheat-sheet.txt' } let(:image_url) { 'https://user-images.githubusercontent.com/6833842/0cf366b61ef2.jpeg' } let(:image_tag_url) { 'https://user-images.githubusercontent.com/6833842/0cf366b61ea5.jpeg' } + let(:project_blob_url) { 'https://github.com/nickname/public-test-repo/blob/main/example.md' } + let(:other_project_blob_url) { 'https://github.com/nickname/other-repo/blob/main/README.md' } let(:text) do <<-TEXT.split("\n").map(&:strip).join("\n") Some text... @@ -20,11 +22,14 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do [special-doc](#{doc_url})  <img width=\"248\" alt=\"tag-image\" src="#{image_tag_url}"> + + [link to project blob file](#{project_blob_url}) + [link to other project blob file](#{other_project_blob_url}) TEXT end shared_examples 'updates record description' do - it do + it 'changes attachment links' do importer.execute record.reload @@ -32,6 +37,22 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do expect(record.description).to include(' expect(record.description).to include('<img width="248" alt="tag-image" src="/uploads') end + + it 'changes link to project blob files' do + importer.execute + + record.reload + expected_blob_link = "[link to project blob file](http://localhost/#{project.full_path}/-/blob/main/example.md)" + expect(record.description).not_to include("[link to project blob file](#{project_blob_url})") + expect(record.description).to include(expected_blob_link) + end + + it "doesn't change links to other projects" do + importer.execute + + record.reload + expect(record.description).to include("[link to other project blob file](#{other_project_blob_url})") + end end describe '#execute' do @@ -72,7 +93,7 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do context 'when importing note attachments' do let(:record) { create(:note, project: project, note: text) } - it 'updates note text with new attachment urls' do + it 'changes note text with new attachment urls' do importer.execute record.reload @@ -80,6 +101,22 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteAttachmentsImporter do expect(record.note).to include(' expect(record.note).to include('<img width="248" alt="tag-image" src="/uploads') end + + it 'changes note links to project blob files' do + importer.execute + + record.reload + expected_blob_link = "[link to project blob file](http://localhost/#{project.full_path}/-/blob/main/example.md)" + expect(record.note).not_to include("[link to project blob file](#{project_blob_url})") + expect(record.note).to include(expected_blob_link) + end + + it "doesn't change note links to other projects" do + importer.execute + + record.reload + expect(record.note).to include("[link to other project blob file](#{other_project_blob_url})") + end end end end diff --git a/spec/lib/gitlab/github_import/markdown/attachment_spec.rb b/spec/lib/gitlab/github_import/markdown/attachment_spec.rb index 588a3076f59..84b0886ebcc 100644 --- a/spec/lib/gitlab/github_import/markdown/attachment_spec.rb +++ b/spec/lib/gitlab/github_import/markdown/attachment_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::Markdown::Attachment do +RSpec.describe Gitlab::GithubImport::Markdown::Attachment, feature_category: :importers do let(:name) { FFaker::Lorem.word } let(:url) { FFaker::Internet.uri('https') } @@ -101,6 +101,62 @@ RSpec.describe Gitlab::GithubImport::Markdown::Attachment do end end + describe '#part_of_project_blob?' do + let(:attachment) { described_class.new('test', url) } + let(:import_source) { 'nickname/public-test-repo' } + + context 'when url is a part of project blob' do + let(:url) { "https://github.com/#{import_source}/blob/main/example.md" } + + it { expect(attachment.part_of_project_blob?(import_source)).to eq true } + end + + context 'when url is not a part of project blob' do + let(:url) { "https://github.com/#{import_source}/files/9020437/git-cheat-sheet.txt" } + + it { expect(attachment.part_of_project_blob?(import_source)).to eq false } + end + end + + describe '#doc_belongs_to_project?' do + let(:attachment) { described_class.new('test', url) } + let(:import_source) { 'nickname/public-test-repo' } + + context 'when url relates to this project' do + let(:url) { "https://github.com/#{import_source}/files/9020437/git-cheat-sheet.txt" } + + it { expect(attachment.doc_belongs_to_project?(import_source)).to eq true } + end + + context 'when url is not related to this project' do + let(:url) { 'https://github.com/nickname/other-repo/files/9020437/git-cheat-sheet.txt' } + + it { expect(attachment.doc_belongs_to_project?(import_source)).to eq false } + end + + context 'when url is a part of project blob' do + let(:url) { "https://github.com/#{import_source}/blob/main/example.md" } + + it { expect(attachment.doc_belongs_to_project?(import_source)).to eq false } + end + end + + describe '#media?' do + let(:attachment) { described_class.new('test', url) } + + context 'when it is a media link' do + let(:url) { 'https://user-images.githubusercontent.com/6833842/0cf366b61ef2.jpeg' } + + it { expect(attachment.media?).to eq true } + end + + context 'when it is not a media link' do + let(:url) { 'https://github.com/nickname/public-test-repo/files/9020437/git-cheat-sheet.txt' } + + it { expect(attachment.media?).to eq false } + end + end + describe '#inspect' do it 'returns attachment basic info' do attachment = described_class.new(name, url) diff --git a/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb index c351ead91eb..9de39a3ff7e 100644 --- a/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb +++ b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb @@ -289,77 +289,52 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling, feature_category: :impo .and_return({ title: 'One' }, { title: 'Two' }, { title: 'Three' }) end - context 'with multiple objects' do - before do - stub_feature_flags(improved_spread_parallel_import: false) - - expect(importer).to receive(:each_object_to_import).and_yield(object).and_yield(object).and_yield(object) - end - - it 'imports data in parallel batches with delays' do - expect(worker_class).to receive(:bulk_perform_in) - .with(1.second, [ - [project.id, { title: 'One' }, an_instance_of(String)], - [project.id, { title: 'Two' }, an_instance_of(String)], - [project.id, { title: 'Three' }, an_instance_of(String)] - ], batch_size: batch_size, batch_delay: batch_delay) - - importer.parallel_import - end + it 'imports data in parallel with delays respecting parallel_import_batch definition and return job waiter' do + allow(::Gitlab::JobWaiter).to receive(:generate_key).and_return('waiter-key') + allow(importer).to receive(:parallel_import_batch).and_return({ size: 2, delay: 1.minute }) + + expect(importer).to receive(:each_object_to_import) + .and_yield(object).and_yield(object).and_yield(object) + expect(worker_class).to receive(:perform_in) + .with(1.second, project.id, { title: 'One' }, 'waiter-key').ordered + expect(worker_class).to receive(:perform_in) + .with(1.second, project.id, { title: 'Two' }, 'waiter-key').ordered + expect(worker_class).to receive(:perform_in) + .with(1.minute + 1.second, project.id, { title: 'Three' }, 'waiter-key').ordered + + job_waiter = importer.parallel_import + + expect(job_waiter.key).to eq('waiter-key') + expect(job_waiter.jobs_remaining).to eq(3) end - context 'when the feature flag `improved_spread_parallel_import` is enabled' do + context 'when job restarts due to API rate limit or Sidekiq interruption' do before do - stub_feature_flags(improved_spread_parallel_import: true) + cache_key = format(described_class::JOB_WAITER_CACHE_KEY, + project: project.id, collection: importer.collection_method) + Gitlab::Cache::Import::Caching.write(cache_key, 'waiter-key') + + cache_key = format(described_class::JOB_WAITER_REMAINING_CACHE_KEY, + project: project.id, collection: importer.collection_method) + Gitlab::Cache::Import::Caching.write(cache_key, 3) end - it 'imports data in parallel with delays respecting parallel_import_batch definition and return job waiter' do - allow(::Gitlab::JobWaiter).to receive(:generate_key).and_return('waiter-key') - allow(importer).to receive(:parallel_import_batch).and_return({ size: 2, delay: 1.minute }) + it "restores job waiter's key and jobs_remaining" do + allow(importer).to receive(:parallel_import_batch).and_return({ size: 1, delay: 1.minute }) + + expect(importer).to receive(:each_object_to_import).and_yield(object).and_yield(object).and_yield(object) - expect(importer).to receive(:each_object_to_import) - .and_yield(object).and_yield(object).and_yield(object) expect(worker_class).to receive(:perform_in) .with(1.second, project.id, { title: 'One' }, 'waiter-key').ordered expect(worker_class).to receive(:perform_in) - .with(1.second, project.id, { title: 'Two' }, 'waiter-key').ordered + .with(1.minute + 1.second, project.id, { title: 'Two' }, 'waiter-key').ordered expect(worker_class).to receive(:perform_in) - .with(1.minute + 1.second, project.id, { title: 'Three' }, 'waiter-key').ordered + .with(2.minutes + 1.second, project.id, { title: 'Three' }, 'waiter-key').ordered job_waiter = importer.parallel_import expect(job_waiter.key).to eq('waiter-key') - expect(job_waiter.jobs_remaining).to eq(3) - end - - context 'when job restarts due to API rate limit or Sidekiq interruption' do - before do - cache_key = format(described_class::JOB_WAITER_CACHE_KEY, - project: project.id, collection: importer.collection_method) - Gitlab::Cache::Import::Caching.write(cache_key, 'waiter-key') - - cache_key = format(described_class::JOB_WAITER_REMAINING_CACHE_KEY, - project: project.id, collection: importer.collection_method) - Gitlab::Cache::Import::Caching.write(cache_key, 3) - end - - it "restores job waiter's key and jobs_remaining" do - allow(importer).to receive(:parallel_import_batch).and_return({ size: 1, delay: 1.minute }) - - expect(importer).to receive(:each_object_to_import).and_yield(object).and_yield(object).and_yield(object) - - expect(worker_class).to receive(:perform_in) - .with(1.second, project.id, { title: 'One' }, 'waiter-key').ordered - expect(worker_class).to receive(:perform_in) - .with(1.minute + 1.second, project.id, { title: 'Two' }, 'waiter-key').ordered - expect(worker_class).to receive(:perform_in) - .with(2.minutes + 1.second, project.id, { title: 'Three' }, 'waiter-key').ordered - - job_waiter = importer.parallel_import - - expect(job_waiter.key).to eq('waiter-key') - expect(job_waiter.jobs_remaining).to eq(6) - end + expect(job_waiter.jobs_remaining).to eq(6) end end end diff --git a/spec/lib/gitlab/github_import/project_relation_type_spec.rb b/spec/lib/gitlab/github_import/project_relation_type_spec.rb new file mode 100644 index 00000000000..419cb6de121 --- /dev/null +++ b/spec/lib/gitlab/github_import/project_relation_type_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::ProjectRelationType, :manage, feature_category: :importers do + subject(:project_relation_type) { described_class.new(client) } + + let(:octokit) { instance_double(Octokit::Client) } + let(:client) do + instance_double(Gitlab::GithubImport::Clients::Proxy, octokit: octokit, user: { login: 'nickname' }) + end + + describe '#for', :use_clean_rails_redis_caching do + before do + allow(client).to receive(:each_object).with(:organizations).and_yield({ login: 'great-org' }) + allow(octokit).to receive(:access_token).and_return('stub') + end + + context "when it's user owned repo" do + let(:import_source) { 'nickname/repo_name' } + + it { expect(project_relation_type.for(import_source)).to eq 'owned' } + end + + context "when it's organization repo" do + let(:import_source) { 'great-org/repo_name' } + + it { expect(project_relation_type.for(import_source)).to eq 'organization' } + end + + context "when it's user collaborated repo" do + let(:import_source) { 'some-another-namespace/repo_name' } + + it { expect(project_relation_type.for(import_source)).to eq 'collaborated' } + end + + context 'with cache' do + let(:import_source) { 'some-another-namespace/repo_name' } + + it 'calls client only once during 5 minutes timeframe', :request_store do + expect(project_relation_type.for(import_source)).to eq 'collaborated' + expect(project_relation_type.for('another/repo')).to eq 'collaborated' + + expect(client).to have_received(:each_object).once + expect(client).to have_received(:user).once + end + end + end +end diff --git a/spec/lib/gitlab/github_import/representation/collaborator_spec.rb b/spec/lib/gitlab/github_import/representation/collaborator_spec.rb new file mode 100644 index 00000000000..d5952f9459b --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/collaborator_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::GithubImport::Representation::Collaborator, feature_category: :importers do + shared_examples 'a Collaborator' do + it 'returns an instance of Collaborator' do + expect(collaborator).to be_an_instance_of(described_class) + end + + context 'with Collaborator' do + it 'includes the user ID' do + expect(collaborator.id).to eq(42) + end + + it 'includes the username' do + expect(collaborator.login).to eq('alice') + end + + it 'includes the role' do + expect(collaborator.role_name).to eq('maintainer') + end + end + end + + describe '.from_api_response' do + it_behaves_like 'a Collaborator' do + let(:response) { { id: 42, login: 'alice', role_name: 'maintainer' } } + let(:collaborator) { described_class.from_api_response(response) } + end + end + + describe '.from_json_hash' do + it_behaves_like 'a Collaborator' do + let(:hash) { { 'id' => 42, 'login' => 'alice', role_name: 'maintainer' } } + let(:collaborator) { described_class.from_json_hash(hash) } + end + end +end |