diff options
Diffstat (limited to 'spec/services')
5 files changed, 337 insertions, 15 deletions
diff --git a/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb b/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb new file mode 100644 index 00000000000..eb9324fd24b --- /dev/null +++ b/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb @@ -0,0 +1,169 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ClickHouse::SyncStrategies::BaseSyncStrategy, feature_category: :value_stream_management do + let(:strategy) { described_class.new } + + describe '#execute' do + subject(:execute) { strategy.execute } + + context 'when clickhouse configuration database is available', :click_house do + before do + allow(strategy).to receive(:model_class).and_return(::Event) + allow(strategy).to receive(:projections).and_return([:id]) + allow(strategy).to receive(:csv_mapping).and_return({ id: :id }) + allow(strategy).to receive(:insert_query).and_return("INSERT INTO events (id) SETTINGS async_insert=1, + wait_for_async_insert=1 FORMAT CSV") + end + + context 'when there is nothing to sync' do + it 'adds metadata for the worker' do + expect(execute).to eq({ status: :processed, records_inserted: 0, reached_end_of_table: true }) + + events = ClickHouse::Client.select('SELECT * FROM events', :main) + expect(events).to be_empty + end + end + + context 'when syncing records' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:project_event2) { create(:event, :closed, project: project, target: issue) } + let_it_be(:event_without_parent) { create(:event, :joined, project: nil, group: nil) } + let_it_be(:group_event) { create(:event, :created, group: group, project: nil) } + let_it_be(:project_event1) { create(:event, :created, project: project, target: issue) } + + it 'inserts all records' do + expect(execute).to eq({ status: :processed, records_inserted: 4, reached_end_of_table: true }) + + expected_records = [ + hash_including('id' => project_event2.id), + hash_including('id' => event_without_parent.id), + hash_including('id' => group_event.id), + hash_including('id' => project_event1.id) + ] + + events = ClickHouse::Client.select('SELECT * FROM events ORDER BY id', :main) + + expect(events).to match(expected_records) + + last_processed_id = ClickHouse::SyncCursor.cursor_for(:events) + expect(last_processed_id).to eq(project_event1.id) + end + + context 'when multiple batches are needed' do + before do + stub_const("#{described_class}::BATCH_SIZE", 1) + stub_const("#{described_class}::INSERT_BATCH_SIZE", 1) + end + + it 'inserts all records' do + expect(execute).to eq({ status: :processed, records_inserted: 4, reached_end_of_table: true }) + + events = ClickHouse::Client.select('SELECT * FROM events', :main) + expect(events.size).to eq(4) + end + + context 'when new records are inserted while processing' do + it 'does not process new records created during the iteration' do + # Simulating the case when there is an insert during the iteration + call_count = 0 + allow(strategy).to receive(:next_batch).and_wrap_original do |method| + call_count += 1 + create(:event) if call_count == 3 + method.call + end + + expect(execute).to eq({ status: :processed, records_inserted: 4, reached_end_of_table: true }) + end + end + end + + context 'when time limit is reached' do + before do + stub_const("#{described_class}::BATCH_SIZE", 1) + end + + it 'stops the processing' do + allow_next_instance_of(Analytics::CycleAnalytics::RuntimeLimiter) do |runtime_limiter| + allow(runtime_limiter).to receive(:over_time?).and_return(false, true) + end + + expect(execute).to eq({ status: :processed, records_inserted: 2, reached_end_of_table: false }) + + last_processed_id = ClickHouse::SyncCursor.cursor_for(:events) + expect(last_processed_id).to eq(event_without_parent.id) + end + end + + context 'when syncing from a certain point' do + before do + ClickHouse::SyncCursor.update_cursor_for(:events, project_event2.id) + end + + it 'syncs records after the cursor' do + expect(execute).to eq({ status: :processed, records_inserted: 3, reached_end_of_table: true }) + + events = ClickHouse::Client.select('SELECT id FROM events ORDER BY id', :main) + + expect(events).to eq([{ 'id' => event_without_parent.id }, { 'id' => group_event.id }, + { 'id' => project_event1.id }]) + end + + context 'when there is nothing to sync' do + it 'does nothing' do + ClickHouse::SyncCursor.update_cursor_for(:events, project_event1.id) + + expect(execute).to eq({ status: :processed, records_inserted: 0, reached_end_of_table: true }) + + events = ClickHouse::Client.select('SELECT id FROM events ORDER BY id', :main) + expect(events).to be_empty + end + end + end + end + end + + context 'when clickhouse is not configured' do + before do + allow(ClickHouse::Client.configuration).to receive(:databases).and_return({}) + end + + it 'skips execution' do + expect(execute).to eq({ status: :disabled }) + end + end + + context 'when exclusive lease error happens' do + it 'skips execution' do + allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db }) + + expect(strategy).to receive(:in_lock).and_raise(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError) + expect(execute).to eq({ status: :skipped }) + end + end + end + + describe '#projections' do + it 'raises a NotImplementedError' do + expect { strategy.send(:projections) }.to raise_error(NotImplementedError, + "Subclasses must implement `projections`") + end + end + + describe '#csv_mapping' do + it 'raises a NotImplementedError' do + expect { strategy.send(:csv_mapping) }.to raise_error(NotImplementedError, + "Subclasses must implement `csv_mapping`") + end + end + + describe '#insert_query' do + it 'raises a NotImplementedError' do + expect { strategy.send(:insert_query) }.to raise_error(NotImplementedError, + "Subclasses must implement `insert_query`") + end + end +end diff --git a/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb b/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb new file mode 100644 index 00000000000..05fcf6ddeb3 --- /dev/null +++ b/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ClickHouse::SyncStrategies::EventSyncStrategy, feature_category: :value_stream_management do + let(:strategy) { described_class.new } + + describe '#execute' do + subject(:execute) { strategy.execute } + + context 'when syncing records', :click_house do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:project_event2) { create(:event, :closed, project: project, target: issue) } + let_it_be(:event_without_parent) { create(:event, :joined, project: nil, group: nil) } + let_it_be(:group_event) { create(:event, :created, group: group, project: nil) } + let_it_be(:project_event1) { create(:event, :created, project: project, target: issue) } + # looks invalid but we have some records like this on PRD + + it 'correctly inserts all records' do + expect(execute).to eq({ status: :processed, records_inserted: 4, reached_end_of_table: true }) + + expected_records = [ + hash_including('id' => project_event2.id, 'path' => "#{group.id}/#{project.project_namespace.id}/", + 'target_type' => 'Issue'), + hash_including('id' => event_without_parent.id, 'path' => '', 'target_type' => ''), + hash_including('id' => group_event.id, 'path' => "#{group.id}/", 'target_type' => ''), + hash_including('id' => project_event1.id, 'path' => "#{group.id}/#{project.project_namespace.id}/", + 'target_type' => 'Issue') + ] + + events = ClickHouse::Client.select('SELECT * FROM events ORDER BY id', :main) + + expect(events).to match(expected_records) + + last_processed_id = ClickHouse::SyncCursor.cursor_for(:events) + expect(last_processed_id).to eq(project_event1.id) + end + end + end + + describe '#projections' do + it 'returns correct projections' do + expect(strategy.send(:projections)).to match_array([ + :id, + described_class::PATH_COLUMN, + :author_id, + :target_id, + :target_type, + 'action AS raw_action', + 'EXTRACT(epoch FROM created_at) AS casted_created_at', + 'EXTRACT(epoch FROM updated_at) AS casted_updated_at' + ]) + end + end + + describe '#csv_mapping' do + it 'returns correct csv mapping' do + expect(strategy.send(:csv_mapping)).to eq({ + id: :id, + path: :path, + author_id: :author_id, + target_id: :target_id, + target_type: :target_type, + action: :raw_action, + created_at: :casted_created_at, + updated_at: :casted_updated_at + }) + end + end + + describe '#insert_query' do + let(:expected_query) do + <<~SQL.squish + INSERT INTO events (id, path, author_id, + target_id, target_type, + action, created_at, updated_at) + SETTINGS async_insert=1, + wait_for_async_insert=1 FORMAT CSV + SQL + end + + it 'returns correct insert query' do + expect(strategy.send(:insert_query)).to eq(expected_query) + end + end + + describe '#model_class' do + it 'returns the correct model class' do + expect(strategy.send(:model_class)).to eq(::Event) + end + end + + describe '#enabled?' do + context 'when the clickhouse database is configured the feature flag is enabled' do + before do + allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db }) + stub_feature_flags(event_sync_worker_for_click_house: true) + end + + it 'returns true' do + expect(strategy.send(:enabled?)).to be_truthy + end + end + + context 'when the clickhouse database is not configured' do + before do + allow(ClickHouse::Client.configuration).to receive(:databases).and_return({}) + end + + it 'returns false' do + expect(strategy.send(:enabled?)).to be_falsey + end + end + + context 'when the feature flag is disabled' do + before do + allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db }) + stub_feature_flags(event_sync_worker_for_click_house: false) + end + + it 'returns false' do + expect(strategy.send(:enabled?)).to be_falsey + end + end + end +end diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb index 3860543a85e..b23f5856575 100644 --- a/spec/services/members/update_service_spec.rb +++ b/spec/services/members/update_service_spec.rb @@ -263,7 +263,7 @@ RSpec.describe Members::UpdateService, feature_category: :groups_and_projects do it 'emails the users that their group membership expiry has changed' do members.each do |member| - expect(notification_service).to receive(:updated_group_member_expiration).with(member) + expect(notification_service).to receive(:updated_member_expiration).with(member) end subject diff --git a/spec/services/ml/create_model_service_spec.rb b/spec/services/ml/create_model_service_spec.rb index 74c1dd5fec7..88e7c00d1f9 100644 --- a/spec/services/ml/create_model_service_spec.rb +++ b/spec/services/ml/create_model_service_spec.rb @@ -50,9 +50,10 @@ RSpec.describe ::Ml::CreateModelService, feature_category: :mlops do let(:name) { existing_model.name } let(:project) { existing_model.project } - it 'raises an error', :aggregate_failures do - expect { create_model }.to raise_error(ActiveRecord::RecordInvalid) + it 'returns a model with errors', :aggregate_failures do + expect(create_model).not_to be_persisted expect(Gitlab::InternalEvents).not_to have_received(:track_event) + expect(create_model.errors.full_messages).to eq(["Name has already been taken"]) end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 0012cc112c1..910e1d85a6b 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -3319,18 +3319,6 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do let(:notification_trigger) { group.add_guest(added_user) } end end - - describe '#updated_group_member_expiration' do - let_it_be(:group_member) { create(:group_member) } - - it 'emails the user that their group membership expiry has changed' do - expect_next_instance_of(NotificationService) do |notification| - allow(notification).to receive(:updated_group_member_expiration).with(group_member) - end - - group_member.update!(expires_at: 5.days.from_now) - end - end end describe 'ProjectMember', :deliver_mails_inline do @@ -3509,6 +3497,42 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do end end + describe '#updated_member_expiration' do + subject(:updated_member_expiration) { notification.updated_member_expiration(member) } + + context 'for group member' do + let_it_be(:member) { create(:group_member) } + + it 'triggers a notification about the expiration change' do + updated_member_expiration + + expect_delivery_jobs_count(1) + expect_enqueud_email('Group', member.id, mail: 'member_expiration_date_updated_email') + end + end + + context 'for project member' do + let_it_be(:member) { create(:project_member) } + + it 'does not trigger a notification' do + updated_member_expiration + + expect_delivery_jobs_count(0) + end + end + end + + describe '#updated_member_access_level' do + let_it_be(:member) { create(:group_member) } + + it 'triggers a notification about the access_level change' do + notification.updated_member_access_level(member) + + expect_delivery_jobs_count(1) + expect_enqueud_email('Group', member.id, mail: 'member_access_granted_email') + end + end + context 'guest user in private project', :deliver_mails_inline do let(:private_project) { create(:project, :private) } let(:guest) { create(:user) } |