diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-12-21 06:15:00 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-12-21 06:15:00 +0300 |
commit | e3ecb7dc093db47b9491e20d9f20de02b4ac2b6d (patch) | |
tree | e87e5dd85e9c33644b3eddcd47a6214e5537d2c9 /spec/lib | |
parent | 4aa6fba6d825b88d23ff37668e78c851bec102b0 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec/lib')
14 files changed, 717 insertions, 18 deletions
diff --git a/spec/lib/gitlab/cache/import/caching_spec.rb b/spec/lib/gitlab/cache/import/caching_spec.rb index 8f1c552e0b7..6cde51b668a 100644 --- a/spec/lib/gitlab/cache/import/caching_spec.rb +++ b/spec/lib/gitlab/cache/import/caching_spec.rb @@ -224,4 +224,56 @@ RSpec.describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache, :clean subject { described_class.write_if_greater('foo', value) } end end + + describe '.list_add' do + it 'adds a value to a list' do + described_class.list_add('foo', 10) + described_class.list_add('foo', 20) + + key = described_class.cache_key_for('foo') + values = Gitlab::Redis::Cache.with { |r| r.lrange(key, 0, -1) } + + expect(values).to eq(%w[10 20]) + end + + context 'when a limit is provided' do + it 'limits the size of the list to the number of items defined by the limit' do + described_class.list_add('foo', 10, limit: 3) + described_class.list_add('foo', 20, limit: 3) + described_class.list_add('foo', 30, limit: 3) + described_class.list_add('foo', 40, limit: 3) + + key = described_class.cache_key_for('foo') + values = Gitlab::Redis::Cache.with { |r| r.lrange(key, 0, -1) } + + expect(values).to eq(%w[20 30 40]) + end + end + + it_behaves_like 'validated redis value' do + subject { described_class.list_add('foo', value) } + end + end + + describe '.values_from_list' do + it 'returns empty hash when the list is empty' do + expect(described_class.values_from_list('foo')).to eq([]) + end + + it 'returns the items stored in the list in order' do + described_class.list_add('foo', 10) + described_class.list_add('foo', 20) + described_class.list_add('foo', 10) + + expect(described_class.values_from_list('foo')).to eq(%w[10 20 10]) + end + end + + describe '.del' do + it 'deletes the key' do + described_class.write('foo', 'value') + + expect { described_class.del('foo') }.to change { described_class.read('foo') }.from('value').to(nil) + end + end end diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index b47f5ea5df0..a43c7ba008e 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -21,6 +21,15 @@ RSpec.describe 'new tables missing sharding_key', feature_category: :cell do ] end + # Some reasons to exempt a table: + # 1. It has no foreign key for performance reasons + # 2. It does not yet have a foreign key as the index is still being backfilled + let(:allowed_to_be_missing_foreign_key) do + [ + 'p_catalog_resource_sync_events.project_id' + ] + end + let(:starting_from_milestone) { 16.6 } let(:allowed_sharding_key_referenced_tables) { %w[projects namespaces organizations] } @@ -38,6 +47,19 @@ RSpec.describe 'new tables missing sharding_key', feature_category: :cell do expect(column_exists?(table_name, column_name)).to eq(true), "Could not find sharding key column #{table_name}.#{column_name}" expect(referenced_table_name).to be_in(allowed_sharding_key_referenced_tables) + + if allowed_to_be_missing_foreign_key.include?("#{table_name}.#{column_name}") + expect(has_foreign_key?(table_name, column_name)).to eq(false), + "The column `#{table_name}.#{column_name}` has a foreign key so cannot be " \ + "allowed_to_be_missing_foreign_key. " \ + "If this is a foreign key referencing the specified table #{referenced_table_name} " \ + "then you must remove it from allowed_to_be_missing_foreign_key" + else + expect(has_foreign_key?(table_name, column_name, to_table_name: referenced_table_name)).to eq(true), + "Missing a foreign key constraint for `#{table_name}.#{column_name}` " \ + "referencing #{referenced_table_name}. " \ + "All sharding keys must have a foreign key constraint" + end end end end @@ -137,6 +159,25 @@ RSpec.describe 'new tables missing sharding_key', feature_category: :cell do result.count > 0 end + def has_foreign_key?(from_table_name, column_name, to_table_name: nil) + where_clause = { + constrained_table_name: from_table_name, + constrained_columns: [column_name] + } + + where_clause[:referenced_table_name] = to_table_name if to_table_name + + fk = ::Gitlab::Database::PostgresForeignKey.where(where_clause).first + + lfk = ::Gitlab::Database::LooseForeignKeys.definitions.find do |d| + d.from_table == from_table_name && + (to_table_name.nil? || d.to_table == to_table_name) && + d.options[:column] == column_name + end + + fk.present? || lfk.present? + end + def column_exists?(table_name, column_name) sql = <<~SQL SELECT 1 diff --git a/spec/lib/gitlab/github_import/events_cache_spec.rb b/spec/lib/gitlab/github_import/events_cache_spec.rb new file mode 100644 index 00000000000..8637f236977 --- /dev/null +++ b/spec/lib/gitlab/github_import/events_cache_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::EventsCache, :clean_gitlab_redis_cache, feature_category: :importers do + let(:project) { build_stubbed(:project, id: 1) } + let(:issue) { build_stubbed(:issue, iid: 2) } + + let(:event_cache) { described_class.new(project) } + + def build_event(event) + Gitlab::GithubImport::Representation::IssueEvent.from_json_hash(event) + end + + describe '#add' do + it 'adds event to cache' do + expect(Gitlab::Cache::Import::Caching).to receive(:list_add).with( + 'github-importer/events/1/Issue/2', + an_instance_of(String), + limit: described_class::MAX_NUMBER_OF_EVENTS + ) + + event_cache.add(issue, build_event({ event: 'closed' })) + end + + context 'when events is too large to cache' do + before do + stub_const("#{described_class}::MAX_EVENT_SIZE", 1.byte) + end + + it 'does not add event to cache' do + expect(Gitlab::Cache::Import::Caching).not_to receive(:list_add) + expect(Gitlab::GithubImport::Logger).to receive(:warn).with( + message: 'Event too large to cache', + project_id: project.id, + github_identifiers: { + event: 'closed', + id: '99', + issuable_iid: '2' + } + ) + + event_cache.add(issue, build_event({ event: 'closed', id: '99', issue: { number: '2' } })) + end + end + end + + describe '#events' do + it 'retrieves the list of events from the cache in the correct order' do + key = 'github-importer/events/1/Issue/2' + + Gitlab::Cache::Import::Caching.list_add(key, { event: 'merged', created_at: '2023-01-02T00:00:00Z' }.to_json) + Gitlab::Cache::Import::Caching.list_add(key, { event: 'closed', created_at: '2023-01-03T00:00:00Z' }.to_json) + Gitlab::Cache::Import::Caching.list_add(key, { event: 'commented', created_at: '2023-01-01T00:00:00Z' }.to_json) + + events = event_cache.events(issue).map(&:to_hash) + + expect(events).to match([ + a_hash_including(event: 'commented', created_at: '2023-01-01 00:00:00 UTC'), + a_hash_including(event: 'merged', created_at: '2023-01-02 00:00:00 UTC'), + a_hash_including(event: 'closed', created_at: '2023-01-03 00:00:00 UTC') + ]) + end + + context 'when no event was added' do + it 'returns an empty array' do + expect(event_cache.events(issue)).to eq([]) + end + end + end + + describe '#delete' do + it 'deletes the list' do + expect(Gitlab::Cache::Import::Caching).to receive(:del).with('github-importer/events/1/Issue/2') + + event_cache.delete(issue) + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/events/commented_spec.rb b/spec/lib/gitlab/github_import/importer/events/commented_spec.rb new file mode 100644 index 00000000000..bd3bea87688 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/events/commented_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::Events::Commented, feature_category: :importers do + subject(:importer) { described_class.new(project, client) } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + + let(:client) { instance_double('Gitlab::GithubImport::Client') } + let(:issuable) { create(:issue, project: project) } + + let(:issue_event) do + Gitlab::GithubImport::Representation::IssueEvent.new( + id: 1196850910, + actor: { id: user.id, login: user.username }, + event: 'commented', + created_at: '2022-07-27T14:41:11Z', + updated_at: '2022-07-27T14:41:11Z', + body: 'This is my note', + issue: { number: issuable.iid, pull_request: issuable.is_a?(MergeRequest) } + ) + end + + let(:extended_events) { true } + + before do + allow_next_instance_of(Gitlab::GithubImport::IssuableFinder) do |finder| + allow(finder).to receive(:database_id).and_return(issuable.id) + end + allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder| + allow(finder).to receive(:find).with(user.id, user.username).and_return(user.id) + end + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end + end + + shared_examples 'new note' do + it 'creates a note' do + expect { importer.execute(issue_event) }.to change { Note.count }.by(1) + + expect(issuable.notes.last).to have_attributes( + note: 'This is my note', + author_id: user.id, + noteable_type: issuable.class.name.to_s + ) + end + + context 'when extended_events is disabled' do + let(:extended_events) { false } + + it 'does not create a note' do + expect { importer.execute(issue_event) }.not_to change { Note.count } + end + end + end + + context 'with Issue' do + it_behaves_like 'new note' + end + + context 'with MergeRequest' do + let(:issuable) { create(:merge_request, source_project: project, target_project: project) } + + it_behaves_like 'new note' + end +end diff --git a/spec/lib/gitlab/github_import/importer/events/merged_spec.rb b/spec/lib/gitlab/github_import/importer/events/merged_spec.rb index 4ea62557dd6..30bc8aabe12 100644 --- a/spec/lib/gitlab/github_import/importer/events/merged_spec.rb +++ b/spec/lib/gitlab/github_import/importer/events/merged_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Gitlab::GithubImport::Importer::Events::Merged, feature_category: let(:client) { instance_double('Gitlab::GithubImport::Client') } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:commit_id) { nil } + let(:extended_events) { false } let(:issue_event) do Gitlab::GithubImport::Representation::IssueEvent.from_json_hash( @@ -32,6 +33,9 @@ RSpec.describe Gitlab::GithubImport::Importer::Events::Merged, feature_category: allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder| allow(finder).to receive(:find).with(user.id, user.username).and_return(user.id) end + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end end it 'creates expected event and state event' do @@ -71,4 +75,27 @@ RSpec.describe Gitlab::GithubImport::Importer::Events::Merged, feature_category: expect(state_event.source_commit).to eq commit_id[0..40] end end + + describe 'extended events' do + context 'when using extended events' do + let(:extended_events) { true } + + it 'creates a merged by note' do + expect { importer.execute(issue_event) }.to change { Note.count }.by(1) + + last_note = merge_request.notes.last + expect(last_note.created_at).to eq(issue_event.created_at) + expect(last_note.author).to eq(project.owner) + expect(last_note.note).to eq("*Merged by: #{user.username} at #{issue_event.created_at}*") + end + end + + context 'when not using extended events' do + let(:extended_events) { false } + + it 'does not create a merged by note' do + expect { importer.execute(issue_event) }.not_to change { Note.count } + end + end + end end diff --git a/spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb b/spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb new file mode 100644 index 00000000000..f60a9d65269 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/events/reviewed_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::Events::Reviewed, feature_category: :importers do + subject(:importer) { described_class.new(project, client) } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + + let(:client) { instance_double('Gitlab::GithubImport::Client') } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:extended_events) { true } + + let(:issue_event) do + Gitlab::GithubImport::Representation::IssueEvent.new( + id: 1196850910, + actor: { id: user.id, login: user.username }, + event: 'reviewed', + submitted_at: '2022-07-27T14:41:11Z', + body: 'This is my review', + state: state, + issue: { number: merge_request.iid, pull_request: true } + ) + end + + let(:state) { 'commented' } + + before do + allow_next_instance_of(Gitlab::GithubImport::IssuableFinder) do |finder| + allow(finder).to receive(:database_id).and_return(merge_request.id) + end + allow_next_instance_of(Gitlab::GithubImport::UserFinder) do |finder| + allow(finder).to receive(:find).with(user.id, user.username).and_return(user.id) + end + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end + end + + it 'creates a review note', :aggregate_failures do + expect { importer.execute(issue_event) }.to change { Note.count }.by(1) + + last_note = merge_request.notes.last + expect(last_note.note).to include("This is my review") + expect(last_note.author).to eq(user) + expect(last_note.created_at).to eq(issue_event.submitted_at) + end + + it 'does not create a reviewer for the Merge Request', :aggregate_failures do + expect { importer.execute(issue_event) }.not_to change { MergeRequestReviewer.count } + end + + context 'when stage is approved' do + let(:state) { 'approved' } + + it 'creates an approval for the Merge Request', :aggregate_failures do + expect { importer.execute(issue_event) }.to change { Approval.count }.by(1).and change { Note.count }.by(2) + + expect(merge_request.approved_by_users.reload).to include(user) + expect(merge_request.approvals.last.created_at).to eq(issue_event.submitted_at) + + note = merge_request.notes.where(system: false).last + expect(note.note).to include("This is my review") + expect(note.author).to eq(user) + expect(note.created_at).to eq(issue_event.submitted_at) + + system_note = merge_request.notes.where(system: true).last + expect(system_note.note).to eq('approved this merge request') + expect(system_note.author).to eq(user) + expect(system_note.created_at).to eq(issue_event.submitted_at) + expect(system_note.system_note_metadata.action).to eq('approved') + end + end + + context 'when extended events is false' do + let(:extended_events) { false } + + it 'does nothing' do + expect { importer.execute(issue_event) } + .to not_change { Note.count } + .and not_change { Approval.count } + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/issue_event_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_event_importer_spec.rb index 2389489e867..ffe6c237506 100644 --- a/spec/lib/gitlab/github_import/importer/issue_event_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/issue_event_importer_spec.rb @@ -115,6 +115,18 @@ RSpec.describe Gitlab::GithubImport::Importer::IssueEventImporter, :clean_gitlab it_behaves_like 'triggers specific event importer', Gitlab::GithubImport::Importer::Events::Merged end + context "when it's commented issue event" do + let(:event_name) { 'commented' } + + it_behaves_like 'triggers specific event importer', Gitlab::GithubImport::Importer::Events::Commented + end + + context "when it's reviewed issue event" do + let(:event_name) { 'reviewed' } + + it_behaves_like 'triggers specific event importer', Gitlab::GithubImport::Importer::Events::Reviewed + end + context "when it's unknown issue event" do let(:event_name) { 'fake' } diff --git a/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb index f7ee6fee6dc..174cc720717 100644 --- a/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/issue_events_importer_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Gitlab::GithubImport::Importer::IssueEventsImporter, feature_cate struct = Struct.new( :id, :node_id, :url, :actor, :event, :commit_id, :commit_url, :label, :rename, :milestone, :source, :assignee, :assigner, :review_requester, :requested_reviewer, :issue, :created_at, :performed_via_github_app, - keyword_init: true + :body, :updated_at, :submitted_at, :state, keyword_init: true ) struct.new(id: rand(10), event: 'closed', created_at: '2022-04-26 18:30:53 UTC') end diff --git a/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb index 6846c99fb63..1651774b5ce 100644 --- a/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_requests/review_importer_spec.rb @@ -30,6 +30,12 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequests::ReviewImporter, expect(merge_request.reviewers).to contain_exactly(author) end + context 'when add_reviewer option is false' do + it 'does not change Merge Request reviewers' do + expect { subject.execute(add_reviewer: false) }.not_to change { MergeRequestReviewer.count } + end + end + context 'when reviewer already exists' do before do create( @@ -309,6 +315,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequests::ReviewImporter, extra.reverse_merge( author: { id: 999, login: 'author' }, merge_request_id: merge_request.id, + merge_request_iid: merge_request.iid, review_type: type, note: 'note', submitted_at: submitted_at.to_s diff --git a/spec/lib/gitlab/github_import/importer/replay_events_importer_spec.rb b/spec/lib/gitlab/github_import/importer/replay_events_importer_spec.rb new file mode 100644 index 00000000000..a2233ffe8cc --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/replay_events_importer_spec.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::ReplayEventsImporter, feature_category: :importers do + let_it_be(:association) { create(:merged_merge_request) } + let_it_be(:project) { association.project } + let(:user1) { build(:user1) } + let(:user2) { build(:user2) } + let(:user3) { build(:user3) } + let(:client) { instance_double(Gitlab::GithubImport::Client) } + + let(:representation) do + Gitlab::GithubImport::Representation::ReplayEvent.new( + issuable_type: association.class.name.to_s, issuable_iid: association.iid + ) + end + + let(:importer) { described_class.new(representation, project, client) } + + describe '#execute' do + before do + events = [ + { + requested_reviewer: { id: 1, login: 'user1' }, + event: 'review_requested' + }, + { + requested_reviewer: { id: 1, login: 'user1' }, + event: 'review_request_removed' + }, + { + requested_reviewer: { id: 2, login: 'user2' }, + event: 'review_requested' + }, + { + requested_reviewer: { id: 2, login: 'user2' }, + event: 'review_request_removed' + }, + { + requested_reviewer: { id: 2, login: 'user2' }, + event: 'review_requested' + }, + { + requested_reviewer: { id: 3, login: 'user3' }, + event: 'review_requested' + } + ] + + representations = events.map { |e| Gitlab::GithubImport::Representation::IssueEvent.from_json_hash(e) } + + allow_next_instance_of(Gitlab::GithubImport::EventsCache) do |events_cache| + allow(events_cache).to receive(:events).with(association).and_return(representations) + end + end + + context 'when association is a MergeRequest' do + it 'imports reviewers' do + representation = instance_double(Gitlab::GithubImport::Representation::PullRequests::ReviewRequests) + + expect(Gitlab::GithubImport::Representation::PullRequests::ReviewRequests).to receive(:from_json_hash).with( + merge_request_id: association.id, + merge_request_iid: association.iid, + users: [ + { id: 2, login: 'user2' }, + { id: 3, login: 'user3' } + ] + ).and_return(representation) + + expect_next_instance_of( + Gitlab::GithubImport::Importer::PullRequests::ReviewRequestImporter, anything, project, client + ) do |review_impoter| + expect(review_impoter).to receive(:execute) + end + + importer.execute + end + end + + context 'when association is not found' do + let(:representation) do + Gitlab::GithubImport::Representation::ReplayEvent.new( + issuable_type: association.class.name.to_s, issuable_iid: -1 + ) + end + + it 'does not read events' do + expect(Gitlab::GithubImport::EventsCache).not_to receive(:new) + + importer.execute + end + end + + context 'when issueable type is not supported' do + let(:representation) do + Gitlab::GithubImport::Representation::ReplayEvent.new( + issuable_type: 'Issue', issuable_iid: association.iid + ) + end + + it 'does not read events' do + expect(Gitlab::GithubImport::EventsCache).not_to receive(:new) + + importer.execute + end + end + end +end diff --git a/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb b/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb index 91f89f0779c..ab51ecf7d61 100644 --- a/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/single_endpoint_issue_events_importer_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter, feature_category: :importers do - let(:client) { double } + let(:client) { Gitlab::GithubImport::Client.new('token') } - let_it_be(:project) { create(:project, :import_started, import_source: 'http://somegithub.com') } + let_it_be(:project) { create(:project, :import_started, import_source: 'foo/bar') } let!(:issuable) { create(:issue, project: project) } @@ -88,23 +88,32 @@ RSpec.describe Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter describe '#each_object_to_import', :clean_gitlab_redis_cache do let(:issue_event) do struct = Struct.new(:id, :event, :created_at, :issue, keyword_init: true) - struct.new(id: 1, event: 'closed', created_at: '2022-04-26 18:30:53 UTC') + struct.new(id: 1, event: event_name, created_at: '2022-04-26 18:30:53 UTC') end + let(:event_name) { 'closed' } + + let(:page_events) { [issue_event] } + let(:page) do instance_double( Gitlab::GithubImport::Client::Page, - number: 1, objects: [issue_event] + number: 1, objects: page_events ) end let(:page_counter) { instance_double(Gitlab::GithubImport::PageCounter) } + let(:extended_events) { true } + before do allow(Gitlab::Redis::SharedState).to receive(:with).and_return('OK') allow(client).to receive(:each_page).once.with(:issue_timeline, project.import_source, issuable.iid, { state: 'all', sort: 'created', direction: 'asc', page: 1 } ).and_yield(page) + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end end context 'with issues' do @@ -190,10 +199,7 @@ RSpec.describe Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter end context 'when event is not supported' do - let(:issue_event) do - struct = Struct.new(:id, :event, :created_at, :issue, keyword_init: true) - struct.new(id: 1, event: 'not_supported_event', created_at: '2022-04-26 18:30:53 UTC') - end + let(:event_name) { 'not_supported_event' } it "doesn't process this event" do counter = 0 @@ -201,5 +207,156 @@ RSpec.describe Gitlab::GithubImport::Importer::SingleEndpointIssueEventsImporter expect(counter).to eq 0 end end + + describe 'save events' do + shared_examples 'saves event' do + it 'saves event' do + expect(Gitlab::GithubImport::Representation::IssueEvent).to receive(:from_api_response).with(issue_event.to_h) + .and_call_original + + expect_next_instance_of(Gitlab::GithubImport::EventsCache) do |events_cache| + expect(events_cache).to receive(:add).with( + issuable, + an_instance_of(Gitlab::GithubImport::Representation::IssueEvent) + ) + end + + subject.each_object_to_import { |event| event } + end + end + + context 'when event is review_requested' do + let(:event_name) { 'review_requested' } + + it_behaves_like 'saves event' + end + + context 'when event is review_request_removed' do + let(:event_name) { 'review_request_removed' } + + it_behaves_like 'saves event' + end + + context 'when event is closed' do + let(:event_name) { 'closed' } + + it 'does not save event' do + expect_next_instance_of(Gitlab::GithubImport::EventsCache) do |events_cache| + expect(events_cache).not_to receive(:add) + end + + subject.each_object_to_import { |event| event } + end + end + + context 'when extended_events is disabled' do + let(:event_name) { 'review_requested' } + let(:extended_events) { false } + + it 'does not save event' do + expect(Gitlab::GithubImport::EventsCache).not_to receive(:new) + + subject.each_object_to_import { |event| event } + end + end + end + + describe 'after batch processed' do + context 'when events should be replayed' do + let(:event_name) { 'review_requested' } + + it 'enqueues worker to replay events' do + allow(Gitlab::JobWaiter).to receive(:generate_key).and_return('job_waiter_key') + + expect(Gitlab::GithubImport::ReplayEventsWorker).to receive(:perform_async) + .with( + project.id, + { 'issuable_type' => issuable.class.name.to_s, 'issuable_iid' => issuable.iid }, + 'job_waiter_key' + ) + + subject.each_object_to_import { |event| event } + end + end + + context 'when events are not relevant' do + let(:event_name) { 'closed' } + + it 'does not replay events' do + expect(Gitlab::GithubImport::ReplayEventsWorker).not_to receive(:perform_async) + + subject.each_object_to_import { |event| event } + end + end + + context 'when extended_events is disabled' do + let(:extended_events) { false } + + it 'does not replay events' do + expect(Gitlab::GithubImport::ReplayEventsWorker).not_to receive(:perform_async) + + subject.each_object_to_import { |event| event } + end + end + end + end + + describe '#execute', :clean_gitlab_redis_cache do + let(:extended_events) { false } + + before do + allow_next_instance_of(Gitlab::GithubImport::Settings) do |setting| + allow(setting).to receive(:extended_events?).and_return(extended_events) + end + + stub_request(:get, 'https://api.github.com/rate_limit') + .to_return(status: 200, headers: { 'X-RateLimit-Limit' => 5000, 'X-RateLimit-Remaining' => 5000 }) + + events = [ + { + id: 1, + event: 'review_requested', + created_at: '2022-04-26 18:30:53 UTC', + issue: { + number: issuable.iid, + pull_request: true + } + } + ] + + endpoint = 'https://api.github.com/repos/foo/bar/issues/1/timeline' \ + '?direction=asc&page=1&per_page=100&sort=created&state=all' + + stub_request(:get, endpoint) + .to_return(status: 200, body: events.to_json, headers: { 'Content-Type' => 'application/json' }) + end + + context 'when extended_events is disabled' do + it 'enqueues importer worker' do + expect { subject.execute }.to change { Gitlab::GithubImport::ReplayEventsWorker.jobs.size }.by(0) + .and change { Gitlab::GithubImport::ImportIssueEventWorker.jobs.size }.by(1) + end + + it 'returns job waiter with the correct remaining jobs count' do + job_waiter = subject.execute + + expect(job_waiter.jobs_remaining).to eq(1) + end + end + + context 'when extended_events is enabled' do + let(:extended_events) { true } + + it 'enqueues importer worker and replay worker' do + expect { subject.execute }.to change { Gitlab::GithubImport::ReplayEventsWorker.jobs.size }.by(1) + .and change { Gitlab::GithubImport::ImportIssueEventWorker.jobs.size }.by(1) + end + + it 'returns job waiter with the correct remaining jobs count' do + job_waiter = subject.execute + + expect(job_waiter.jobs_remaining).to eq(2) + end + end end end diff --git a/spec/lib/gitlab/github_import/representation/issue_event_spec.rb b/spec/lib/gitlab/github_import/representation/issue_event_spec.rb index 6620dee0fd0..de0509c3e5e 100644 --- a/spec/lib/gitlab/github_import/representation/issue_event_spec.rb +++ b/spec/lib/gitlab/github_import/representation/issue_event_spec.rb @@ -168,8 +168,8 @@ RSpec.describe Gitlab::GithubImport::Representation::IssueEvent do describe '.from_api_response' do let(:response) do event_resource = Struct.new( - :id, :node_id, :url, :actor, :event, :commit_id, :commit_url, :label, :rename, :milestone, - :source, :assignee, :requested_reviewer, :review_requester, :issue, :created_at, + :id, :node_id, :url, :actor, :event, :commit_id, :commit_url, :label, :rename, :milestone, :state, :body, + :source, :assignee, :requested_reviewer, :review_requester, :issue, :created_at, :updated_at, :submitted_at, :performed_via_github_app, keyword_init: true ) diff --git a/spec/lib/gitlab/github_import/representation/replay_event_spec.rb b/spec/lib/gitlab/github_import/representation/replay_event_spec.rb new file mode 100644 index 00000000000..1afefb76c6a --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/replay_event_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::GithubImport::Representation::ReplayEvent, feature_category: :importers do + describe '.from_json_hash' do + it 'returns an instance of ReplayEvent' do + representation = described_class.from_json_hash(issuable_iid: 1, issuable_type: 'MergeRequest') + + expect(representation).to be_an_instance_of(described_class) + end + end + + describe '#github_identifiers' do + it 'returns a hash with needed identifiers' do + representation = described_class.new(issuable_type: 'MergeRequest', issuable_iid: 1) + + expect(representation.github_identifiers).to eq({ + issuable_type: 'MergeRequest', + issuable_iid: 1 + }) + end + end +end diff --git a/spec/lib/gitlab/github_import/settings_spec.rb b/spec/lib/gitlab/github_import/settings_spec.rb index ea1526ca25f..d268f3a8650 100644 --- a/spec/lib/gitlab/github_import/settings_spec.rb +++ b/spec/lib/gitlab/github_import/settings_spec.rb @@ -21,12 +21,6 @@ RSpec.describe Gitlab::GithubImport::Settings, feature_category: :importers do stages = described_class::OPTIONAL_STAGES [ { - name: 'single_endpoint_issue_events_import', - label: stages[:single_endpoint_issue_events_import][:label], - selected: false, - details: stages[:single_endpoint_issue_events_import][:details] - }, - { name: 'single_endpoint_notes_import', label: stages[:single_endpoint_notes_import][:label], selected: false, @@ -48,7 +42,31 @@ RSpec.describe Gitlab::GithubImport::Settings, feature_category: :importers do end it 'returns stages list as array' do - expect(described_class.stages_array).to match_array(expected_list) + expect(described_class.stages_array(project.owner)).to match_array(expected_list) + end + + context 'when `github_import_extended_events` feature flag is disabled' do + let(:expected_list_with_deprecated_options) do + stages = described_class::OPTIONAL_STAGES + + expected_list.concat( + [ + { + name: 'single_endpoint_issue_events_import', + label: stages[:single_endpoint_issue_events_import][:label], + selected: false, + details: stages[:single_endpoint_issue_events_import][:details] + } + ]) + end + + before do + stub_feature_flags(github_import_extended_events: false) + end + + it 'returns stages list as array' do + expect(described_class.stages_array(project.owner)).to match_array(expected_list_with_deprecated_options) + end end end @@ -99,4 +117,24 @@ RSpec.describe Gitlab::GithubImport::Settings, feature_category: :importers do 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 |