diff options
Diffstat (limited to 'spec/lib/gitlab/github_import')
9 files changed, 310 insertions, 6 deletions
diff --git a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb index 0af840d2c10..3dc15c7c059 100644 --- a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb @@ -20,6 +20,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter do noteable_type: 'MergeRequest', noteable_id: 1, commit_id: '123abc', + original_commit_id: 'original123abc', file_path: 'README.md', diff_hunk: hunk, author: Gitlab::GithubImport::Representation::User @@ -64,13 +65,14 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter do LegacyDiffNote.table_name, [ { + discussion_id: anything, noteable_type: 'MergeRequest', noteable_id: merge_request.id, project_id: project.id, author_id: user.id, note: 'Hello', system: false, - commit_id: '123abc', + commit_id: 'original123abc', line_code: note.line_code, type: 'LegacyDiffNote', created_at: created_at, @@ -95,13 +97,14 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter do LegacyDiffNote.table_name, [ { + discussion_id: anything, noteable_type: 'MergeRequest', noteable_id: merge_request.id, project_id: project.id, author_id: project.creator_id, note: "*Created by: #{user.username}*\n\nHello", system: false, - commit_id: '123abc', + commit_id: 'original123abc', line_code: note.line_code, type: 'LegacyDiffNote', created_at: created_at, diff --git a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb index 7750e508713..46b9959ff64 100644 --- a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNotesImporter do html_url: 'https://github.com/foo/bar/pull/42', path: 'README.md', commit_id: '123abc', + original_commit_id: 'original123abc', diff_hunk: "@@ -1 +1 @@\n-Hello\n+Hello world", user: double(:user, id: 4, login: 'alice'), body: 'Hello world', 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 new file mode 100644 index 00000000000..8c71d7d0ed7 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/single_endpoint_diff_notes_importer_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::SingleEndpointDiffNotesImporter 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::DiffNote) } + it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::DiffNoteImporter) } + it { expect(subject.collection_method).to eq(:pull_request_comments) } + it { expect(subject.object_type).to eq(:diff_note) } + it { expect(subject.id_for_already_imported_cache(double(id: 1))).to eq(1) } + + 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 + + let(:note) { double(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(:pull_request_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/diff_notes/already-imported/#{project.id}", + merge_request.iid + ) + ).to eq(true) + end + + it 'skips cached pages' do + Gitlab::GithubImport::PageCounter + .new(project, "merge_request/#{merge_request.id}/pull_request_comments") + .set(2) + + expect(client) + .to receive(:each_page) + .exactly(:once) # ensure to be cached on the second call + .with(:pull_request_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/diff_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/importer/single_endpoint_issue_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb new file mode 100644 index 00000000000..8d8f2730880 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/single_endpoint_issue_notes_importer_spec.rb @@ -0,0 +1,74 @@ +# 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(double(id: 1))).to eq(1) } + + describe '#each_object_to_import', :clean_gitlab_redis_cache do + let(:issue) do + create( + :issue, + iid: 999, + project: project + ) + end + + let(:note) { double(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::GithubImport::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 new file mode 100644 index 00000000000..b8282212a90 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/single_endpoint_merge_request_notes_importer_spec.rb @@ -0,0 +1,75 @@ +# 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(double(id: 1))).to eq(1) } + + describe '#each_object_to_import', :clean_gitlab_redis_cache do + let(:merge_request) do + create( + :merge_request, + iid: 999, + source_project: project, + target_project: project + ) + end + + let(:note) { double(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::GithubImport::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/issuable_finder_spec.rb b/spec/lib/gitlab/github_import/issuable_finder_spec.rb index f009b61ad89..3afd006109b 100644 --- a/spec/lib/gitlab/github_import/issuable_finder_spec.rb +++ b/spec/lib/gitlab/github_import/issuable_finder_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::IssuableFinder, :clean_gitlab_redis_cache do - let(:project) { double(:project, id: 4) } + let(:project) { double(:project, id: 4, group: nil) } let(:issue) do double(:issue, issuable_type: MergeRequest, iid: 1) end @@ -26,15 +26,77 @@ RSpec.describe Gitlab::GithubImport::IssuableFinder, :clean_gitlab_redis_cache d expect { finder.database_id }.to raise_error(TypeError) end + + context 'when group is present' do + context 'when github_importer_single_endpoint_notes_import feature flag is enabled' do + it 'reads cache value with longer timeout' do + project = create(:project, import_url: 'http://t0ken@github.com/user/repo.git') + group = create(:group, projects: [project]) + + stub_feature_flags(github_importer_single_endpoint_notes_import: group) + + expect(Gitlab::Cache::Import::Caching) + .to receive(:read) + .with(anything, timeout: Gitlab::Cache::Import::Caching::LONGER_TIMEOUT) + + described_class.new(project, issue).database_id + end + end + + context 'when github_importer_single_endpoint_notes_import feature flag is disabled' do + it 'reads cache value with default timeout' do + project = double(:project, id: 4, group: create(:group)) + + stub_feature_flags(github_importer_single_endpoint_notes_import: false) + + expect(Gitlab::Cache::Import::Caching) + .to receive(:read) + .with(anything, timeout: Gitlab::Cache::Import::Caching::TIMEOUT) + + described_class.new(project, issue).database_id + end + end + end end describe '#cache_database_id' do it 'caches the ID of a database row' do expect(Gitlab::Cache::Import::Caching) .to receive(:write) - .with('github-import/issuable-finder/4/MergeRequest/1', 10) + .with('github-import/issuable-finder/4/MergeRequest/1', 10, timeout: 86400) finder.cache_database_id(10) end + + context 'when group is present' do + context 'when github_importer_single_endpoint_notes_import feature flag is enabled' do + it 'caches value with longer timeout' do + project = create(:project, import_url: 'http://t0ken@github.com/user/repo.git') + group = create(:group, projects: [project]) + + stub_feature_flags(github_importer_single_endpoint_notes_import: group) + + expect(Gitlab::Cache::Import::Caching) + .to receive(:write) + .with(anything, anything, timeout: Gitlab::Cache::Import::Caching::LONGER_TIMEOUT) + + described_class.new(project, issue).cache_database_id(10) + end + end + + context 'when github_importer_single_endpoint_notes_import feature flag is disabled' do + it 'caches value with default timeout' do + project = double(:project, id: 4, group: create(:group)) + + stub_feature_flags(github_importer_single_endpoint_notes_import: false) + + expect(Gitlab::Cache::Import::Caching) + .to receive(:write) + .with(anything, anything, timeout: Gitlab::Cache::Import::Caching::TIMEOUT) + + described_class.new(project, issue).cache_database_id(10) + end + end + end end end diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb index 7e540674258..7c24cd0a5db 100644 --- a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb +++ b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb @@ -67,6 +67,7 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do html_url: 'https://github.com/foo/bar/pull/42', path: 'README.md', commit_id: '123abc', + original_commit_id: 'original123abc', diff_hunk: hunk, user: double(:user, id: 4, login: 'alice'), body: 'Hello world', @@ -99,6 +100,7 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do 'noteable_id' => 42, 'file_path' => 'README.md', 'commit_id' => '123abc', + 'original_commit_id' => 'original123abc', 'diff_hunk' => hunk, 'author' => { 'id' => 4, 'login' => 'alice' }, 'note' => 'Hello world', @@ -117,6 +119,7 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do 'noteable_id' => 42, 'file_path' => 'README.md', 'commit_id' => '123abc', + 'original_commit_id' => 'original123abc', 'diff_hunk' => hunk, 'note' => 'Hello world', 'created_at' => created_at.to_s, @@ -145,6 +148,7 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do 'noteable_id' => 42, 'file_path' => 'README.md', 'commit_id' => '123abc', + 'original_commit_id' => 'original123abc', 'diff_hunk' => hunk, 'author' => { 'id' => 4, 'login' => 'alice' }, 'note' => 'Hello world', diff --git a/spec/lib/gitlab/github_import/sequential_importer_spec.rb b/spec/lib/gitlab/github_import/sequential_importer_spec.rb index a5e89049ed9..3c3f8ff59d0 100644 --- a/spec/lib/gitlab/github_import/sequential_importer_spec.rb +++ b/spec/lib/gitlab/github_import/sequential_importer_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Gitlab::GithubImport::SequentialImporter do describe '#execute' do it 'imports a project in sequence' do repository = double(:repository) - project = double(:project, id: 1, repository: repository, import_url: 'http://t0ken@github.another-domain.com/repo-org/repo.git') + project = double(:project, id: 1, repository: repository, import_url: 'http://t0ken@github.another-domain.com/repo-org/repo.git', group: nil) importer = described_class.new(project, token: 'foo') expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance| diff --git a/spec/lib/gitlab/github_import/user_finder_spec.rb b/spec/lib/gitlab/github_import/user_finder_spec.rb index f81fa3b1e2e..8eb6eedd72d 100644 --- a/spec/lib/gitlab/github_import/user_finder_spec.rb +++ b/spec/lib/gitlab/github_import/user_finder_spec.rb @@ -195,7 +195,7 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do expect(Gitlab::Cache::Import::Caching) .to receive(:write) - .with(an_instance_of(String), email) + .with(an_instance_of(String), email, timeout: Gitlab::Cache::Import::Caching::TIMEOUT) finder.email_for_github_username('kittens') end @@ -211,6 +211,16 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do expect(finder.email_for_github_username('kittens')).to be_nil end + + it 'shortens the timeout for Email address in cache when an Email address is private/nil from GitHub' do + user = double(:user, email: nil) + expect(client).to receive(:user).with('kittens').and_return(user) + + expect(Gitlab::Cache::Import::Caching) + .to receive(:write).with(an_instance_of(String), nil, timeout: Gitlab::Cache::Import::Caching::SHORTER_TIMEOUT) + + expect(finder.email_for_github_username('kittens')).to be_nil + end end end |